Skip to content

Commit

Permalink
[WebEngine] Create one TabRegistry per WebEngine
Browse files Browse the repository at this point in the history
There was previously one TabRegistry for all WebEngines but it makes
more sense to store tabs per WebEngine, they get also cleaned up again
when a WebEngine is destroyed.

Change-Id: I176db1b11b548d0a3ff19432281fce70daf90a81
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4100292
Reviewed-by: Rayan Kanso <rayankans@chromium.org>
Commit-Queue: Susanne Westphal <swestphal@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1085414}
  • Loading branch information
s-westphal authored and Chromium LUCI CQ committed Dec 20, 2022
1 parent f885311 commit b0e3d9c
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,14 @@
class TabListObserverDelegate extends ITabListObserverDelegate.Stub {
private final Handler mHandler = new Handler(Looper.getMainLooper());

private TabRegistry mTabRegistry;
private ObserverList<TabListObserver> mTabListObservers = new ObserverList<TabListObserver>();

public TabListObserverDelegate() {
public TabListObserverDelegate(TabRegistry tabRegistry) {
// Assert on UI thread as ObserverList can only be accessed from one thread.
ThreadCheck.ensureOnUiThread();

mTabRegistry = tabRegistry;
}

/**
Expand Down Expand Up @@ -52,7 +55,7 @@ public void notifyActiveTabChanged(@Nullable ITabParams tabParams) {
mHandler.post(() -> {
Tab tab = null;
if (tabParams != null) {
tab = TabRegistry.getInstance().getOrCreateTab(tabParams);
tab = mTabRegistry.getOrCreateTab(tabParams);
}
for (TabListObserver observer : mTabListObservers) {
observer.onActiveTabChanged(tab);
Expand All @@ -63,7 +66,7 @@ public void notifyActiveTabChanged(@Nullable ITabParams tabParams) {
@Override
public void notifyTabAdded(@NonNull ITabParams tabParams) {
mHandler.post(() -> {
Tab tab = TabRegistry.getInstance().getOrCreateTab(tabParams);
Tab tab = mTabRegistry.getOrCreateTab(tabParams);
for (TabListObserver observer : mTabListObservers) {
observer.onTabAdded(tab);
}
Expand All @@ -73,11 +76,11 @@ public void notifyTabAdded(@NonNull ITabParams tabParams) {
@Override
public void notifyTabRemoved(@NonNull ITabParams tabParams) {
mHandler.post(() -> {
Tab tab = TabRegistry.getInstance().getOrCreateTab(tabParams);
Tab tab = mTabRegistry.getOrCreateTab(tabParams);
for (TabListObserver observer : mTabListObservers) {
observer.onTabRemoved(tab);
}
TabRegistry.getInstance().removeTab(tab);
mTabRegistry.removeTab(tab);
});
}

Expand Down
8 changes: 5 additions & 3 deletions weblayer/public/java/org/chromium/webengine/TabManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@
public class TabManager {
private ITabManagerDelegate mDelegate;

private final TabListObserverDelegate mTabListObserverDelegate = new TabListObserverDelegate();
private TabRegistry mTabRegistry = new TabRegistry();
private final TabListObserverDelegate mTabListObserverDelegate;

private final class RequestNavigationCallback extends IBooleanCallback.Stub {
private CallbackToFutureAdapter.Completer<Boolean> mCompleter;
Expand Down Expand Up @@ -60,7 +61,7 @@ private final class TabCallback extends ITabCallback.Stub {
public void onResult(@Nullable ITabParams tabParams) {
if (tabParams != null) {
new Handler(Looper.getMainLooper()).post(() -> {
mCompleter.set(TabRegistry.getInstance().getOrCreateTab(tabParams));
mCompleter.set(mTabRegistry.getOrCreateTab(tabParams));
});
return;
}
Expand All @@ -70,6 +71,7 @@ public void onResult(@Nullable ITabParams tabParams) {

TabManager(ITabManagerDelegate delegate) {
mDelegate = delegate;
mTabListObserverDelegate = new TabListObserverDelegate(mTabRegistry);
try {
mDelegate.setTabListObserverDelegate(mTabListObserverDelegate);
} catch (RemoteException e) {
Expand Down Expand Up @@ -170,6 +172,6 @@ public ListenableFuture<Boolean> tryNavigateBack() {

void invalidate() {
mDelegate = null;
TabRegistry.getInstance().invalidate();
mTabRegistry.invalidate();
}
}
13 changes: 2 additions & 11 deletions weblayer/public/java/org/chromium/webengine/TabRegistry.java
Original file line number Diff line number Diff line change
Expand Up @@ -10,23 +10,14 @@
import java.util.Map;

/**
* Tab registry for storing open {@link Tab}s on the webengine side.
* Tab registry for storing open {@link Tab}s per WebEngine on the webengine side.
*
* For internal use only.
*/
class TabRegistry {
private Map<String, Tab> mGuidToTab = new HashMap<String, Tab>();

private static TabRegistry sInstance;

private TabRegistry() {}

static TabRegistry getInstance() {
if (sInstance == null) {
sInstance = new TabRegistry();
}
return sInstance;
}
TabRegistry() {}

Tab getOrCreateTab(ITabParams tabParams) {
Tab tab = mGuidToTab.get(tabParams.tabGuid);
Expand Down

0 comments on commit b0e3d9c

Please sign in to comment.