weblayer: adds TabListCallback

This informs observers of events related to the set of tabs.

BUG=none
TEST=TabListCallbackTest*

Change-Id: I6e1975f7d59ffcf1a92dce072fc7d8f0194f10ce
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1902130
Commit-Queue: Scott Violet <sky@chromium.org>
Reviewed-by: Bo <boliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#713542}

weblayer/browser/java/BUILD.gn

@@ -141,6 +141,7 @@ android_aidl("aidl") {
   import_include = [ "org/chromium/weblayer_private/interfaces" ]
   sources = [
     "org/chromium/weblayer_private/interfaces/IBrowser.aidl",
+    "org/chromium/weblayer_private/interfaces/IBrowserClient.aidl",
     "org/chromium/weblayer_private/interfaces/IBrowserFragment.aidl",
     "org/chromium/weblayer_private/interfaces/IChildProcessService.aidl",
     "org/chromium/weblayer_private/interfaces/IClientNavigation.aidl",

weblayer/browser/java/org/chromium/weblayer_private/BrowserImpl.java

@@ -7,12 +7,15 @@ package org.chromium.weblayer_private;
 import android.content.Context;
 import android.content.Intent;
 import android.os.Bundle;
+import android.os.RemoteException;
 import android.view.View;
 import android.view.ViewGroup;
 import android.webkit.ValueCallback;
 
 import org.chromium.ui.base.WindowAndroid;
+import org.chromium.weblayer_private.interfaces.APICallException;
 import org.chromium.weblayer_private.interfaces.IBrowser;
+import org.chromium.weblayer_private.interfaces.IBrowserClient;
 import org.chromium.weblayer_private.interfaces.IObjectWrapper;
 import org.chromium.weblayer_private.interfaces.IProfile;
 import org.chromium.weblayer_private.interfaces.ITab;
@@ -29,6 +32,7 @@ public class BrowserImpl extends IBrowser.Stub {
     private BrowserViewController mViewController;
     private FragmentWindowAndroid mWindowAndroid;
     private ArrayList<TabImpl> mTabs = new ArrayList<TabImpl>();
+    private IBrowserClient mClient;
 
     public BrowserImpl(ProfileImpl profile, Bundle savedInstanceState) {
         mProfile = profile;
@@ -47,7 +51,7 @@ public class BrowserImpl extends IBrowser.Stub {
         mWindowAndroid = windowAndroid;
         mViewController = new BrowserViewController(context, windowAndroid);
         TabImpl tab = new TabImpl(mProfile, windowAndroid);
-        attachTab(tab);
+        addTab(tab);
         boolean set_active_result = setActiveTab(tab);
         assert set_active_result;
     }
@@ -93,19 +97,25 @@ public class BrowserImpl extends IBrowser.Stub {
         return mProfile;
     }
 
-    public void attachTab(ITab iTab) {
+    @Override
+    public void addTab(ITab iTab) {
         TabImpl tab = (TabImpl) iTab;
         if (tab.getBrowser() == this) return;
-        attachTabImpl(tab);
+        addTabImpl(tab);
     }
 
-    private void attachTabImpl(TabImpl tab) {
+    private void addTabImpl(TabImpl tab) {
         // Null case is only during initial creation.
         if (tab.getBrowser() != this && tab.getBrowser() != null) {
             tab.getBrowser().detachTab(tab);
         }
         mTabs.add(tab);
         tab.attachToBrowser(this);
+        try {
+            if (mClient != null) mClient.onTabAdded(tab);
+        } catch (RemoteException e) {
+            throw new APICallException(e);
+        }
     }
 
     public void detachTab(ITab iTab) {
@@ -113,6 +123,11 @@ public class BrowserImpl extends IBrowser.Stub {
         if (tab.getBrowser() != this) return;
         if (getActiveTab() == tab) setActiveTab(null);
         mTabs.remove(tab);
+        try {
+            if (mClient != null) mClient.onTabRemoved(tab.getId());
+        } catch (RemoteException e) {
+            throw new APICallException(e);
+        }
         // This doesn't reset state on TabImpl as |browser| is either about to be
         // destroyed, or switching to a different fragment.
     }
@@ -120,8 +135,15 @@ public class BrowserImpl extends IBrowser.Stub {
     @Override
     public boolean setActiveTab(ITab controller) {
         TabImpl tab = (TabImpl) controller;
-        if (tab.getBrowser() != this) return false;
+        if (tab != null && tab.getBrowser() != this) return false;
         mViewController.setActiveTab(tab);
+        try {
+            if (mClient != null) {
+                mClient.onActiveTabChanged(tab != null ? tab.getId() : 0);
+            }
+        } catch (RemoteException e) {
+            throw new APICallException(e);
+        }
         return true;
     }
 
@@ -139,6 +161,17 @@ public class BrowserImpl extends IBrowser.Stub {
         return getActiveTab() != null ? getActiveTab().getId() : 0;
     }
 
+    @Override
+    public void setClient(IBrowserClient client) {
+        mClient = client;
+    }
+
+    @Override
+    public void destroyTab(ITab iTab) {
+        detachTab(iTab);
+        ((TabImpl) iTab).destroy();
+    }
+
     public View getFragmentView() {
         return getViewController().getView();
     }

weblayer/browser/java/org/chromium/weblayer_private/ContentView.java

@@ -245,8 +245,10 @@ public class ContentView extends FrameLayout
         try {
             TraceEvent.begin("ContentView.onFocusChanged");
             super.onFocusChanged(gainFocus, direction, previouslyFocusedRect);
-            getViewEventSink().setHideKeyboardOnBlur(true);
-            getViewEventSink().onViewFocusChanged(gainFocus);
+            if (mWebContents != null) {
+                getViewEventSink().setHideKeyboardOnBlur(true);
+                getViewEventSink().onViewFocusChanged(gainFocus);
+            }
         } finally {
             TraceEvent.end("ContentView.onFocusChanged");
         }
@@ -255,7 +257,9 @@ public class ContentView extends FrameLayout
     @Override
     public void onWindowFocusChanged(boolean hasWindowFocus) {
         super.onWindowFocusChanged(hasWindowFocus);
-        getViewEventSink().onWindowFocusChanged(hasWindowFocus);
+        if (mWebContents != null) {
+            getViewEventSink().onWindowFocusChanged(hasWindowFocus);
+        }
     }
 
     @Override
@@ -335,7 +339,9 @@ public class ContentView extends FrameLayout
 
     @Override
     protected void onConfigurationChanged(Configuration newConfig) {
-        getViewEventSink().onConfigurationChanged(newConfig);
+        if (mWebContents != null) {
+            getViewEventSink().onConfigurationChanged(newConfig);
+        }
         super.onConfigurationChanged(newConfig);
     }
 
@@ -409,13 +415,17 @@ public class ContentView extends FrameLayout
     @Override
     protected void onAttachedToWindow() {
         super.onAttachedToWindow();
-        getViewEventSink().onAttachedToWindow();
+        if (mWebContents != null) {
+            getViewEventSink().onAttachedToWindow();
+        }
     }
 
     @Override
     protected void onDetachedFromWindow() {
         super.onDetachedFromWindow();
-        getViewEventSink().onDetachedFromWindow();
+        if (mWebContents != null) {
+            getViewEventSink().onDetachedFromWindow();
+        }
     }
 
     // Implements SmartClipProvider

weblayer/browser/java/org/chromium/weblayer_private/NewTabCallbackProxy.java

@@ -36,8 +36,8 @@ public final class NewTabCallbackProxy {
         assert mTab.getBrowser() != null;
         TabImpl tab =
                 new TabImpl(mTab.getProfile(), mTab.getBrowser().getWindowAndroid(), nativeTab);
-        mTab.getBrowser().attachTab(tab);
-        mTab.getClient().onNewTab(tab, mode);
+        mTab.getBrowser().addTab(tab);
+        mTab.getClient().onNewTab(tab.getId(), mode);
     }
 
     @NativeMethods

weblayer/browser/java/org/chromium/weblayer_private/interfaces/IBrowser.aidl

@@ -4,6 +4,7 @@
 
 package org.chromium.weblayer_private.interfaces;
 
+import org.chromium.weblayer_private.interfaces.IBrowserClient;
 import org.chromium.weblayer_private.interfaces.IObjectWrapper;
 import org.chromium.weblayer_private.interfaces.ITab;
 
@@ -23,4 +24,9 @@ interface IBrowser {
 
   int getActiveTabId() = 4;
   List getTabs() = 5;
+
+  void setClient(in IBrowserClient client) = 6;
+
+  void addTab(in ITab tab) = 7;
+  void destroyTab(in ITab tab) = 8;
 }

weblayer/browser/java/org/chromium/weblayer_private/interfaces/IBrowserClient.aidl

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.weblayer_private.interfaces;
+
+import org.chromium.weblayer_private.interfaces.ITab;
+
+interface IBrowserClient {
+  void onActiveTabChanged(in int activeTabId) = 0;
+  void onTabAdded(in ITab tab) = 1;
+  void onTabRemoved(in int tabId) = 2;
+}

weblayer/browser/java/org/chromium/weblayer_private/interfaces/ITabClient.aidl

@@ -11,5 +11,5 @@ package org.chromium.weblayer_private.interfaces;
 interface ITabClient {
   void visibleUrlChanged(in String url) = 0;
 
-  void onNewTab(in ITab tab, in int mode) = 1;
+  void onNewTab(in int tabId, in int mode) = 1;
 }

weblayer/browser/java/org/chromium/weblayer_private/interfaces/WebLayerVersion.java

@@ -9,4 +9,4 @@ package org.chromium.weblayer_private.interfaces;
  *
  * Whenever any AIDL file is changed, sVersionNumber must be incremented.
  * */
-public final class WebLayerVersion { public static final int sVersionNumber = 10; }
+public final class WebLayerVersion { public static final int sVersionNumber = 11; }

weblayer/public/java/BUILD.gn

@@ -39,6 +39,7 @@ android_library("java") {
     "org/chromium/weblayer/Profile.java",
     "org/chromium/weblayer/Tab.java",
     "org/chromium/weblayer/TabCallback.java",
+    "org/chromium/weblayer/TabListCallback.java",
     "org/chromium/weblayer/ThreadCheck.java",
     "org/chromium/weblayer/UnsupportedVersionException.java",
     "org/chromium/weblayer/WebLayer.java",

weblayer/public/java/org/chromium/weblayer/Browser.java

@@ -14,13 +14,11 @@ import androidx.annotation.Nullable;
 
 import org.chromium.weblayer_private.interfaces.APICallException;
 import org.chromium.weblayer_private.interfaces.IBrowser;
+import org.chromium.weblayer_private.interfaces.IBrowserClient;
 import org.chromium.weblayer_private.interfaces.ITab;
 import org.chromium.weblayer_private.interfaces.ObjectWrapper;
 
-import java.util.ArrayList;
-import java.util.HashMap;
 import java.util.List;
-import java.util.Map;
 
 /**
  * Browser contains any number of Tabs, with one active Tab. The active Tab is visible to the user,
@@ -30,12 +28,16 @@ import java.util.Map;
  */
 public final class Browser {
     private final IBrowser mImpl;
-    // Maps from id (as returned from ITab.getId() to Tab).
-    private final Map<Integer, Tab> mTabMap;
+    private final ObserverList<TabListCallback> mTabListCallbacks;
 
     Browser(IBrowser impl) {
         mImpl = impl;
-        mTabMap = new HashMap<Integer, Tab>();
+        mTabListCallbacks = new ObserverList<TabListCallback>();
+        try {
+            mImpl.setClient(new BrowserClientImpl());
+        } catch (RemoteException e) {
+            throw new APICallException(e);
+        }
         try {
             for (Object tab : impl.getTabs()) {
                 // getTabs() returns List<TabImpl>, which isn't accessible from the client library.
@@ -48,17 +50,6 @@ public final class Browser {
         }
     }
 
-    // This is called when a new Tab is created.
-    void registerTab(Tab tab) {
-        try {
-            int id = tab.getITab().getId();
-            assert !mTabMap.containsKey(id);
-            mTabMap.put(id, tab);
-        } catch (RemoteException e) {
-            throw new APICallException(e);
-        }
-    }
-
     /**
      * Returns the Browser for the supplied Fragment; null if
      * {@link fragment} was not created by WebLayer.
@@ -71,6 +62,13 @@ public final class Browser {
                                                    : null;
     }
 
+    // Called prior to notifying IBrowser of destroy().
+    void prepareForDestroy() {
+        for (Tab tab : getTabs()) {
+            Tab.unregisterTab(tab);
+        }
+    }
+
     /**
      * Sets the active (visible) Tab. Only one Tab is visible at a time.
      *
@@ -84,7 +82,7 @@ public final class Browser {
     public void setActiveTab(@NonNull Tab tab) {
         ThreadCheck.ensureOnUiThread();
         try {
-            if (!mImpl.setActiveTab(tab.getITab())) {
+            if (getActiveTab() != tab && !mImpl.setActiveTab(tab.getITab())) {
                 throw new IllegalStateException("attachTab() must be called before "
                         + "setActiveTab");
             }
@@ -93,6 +91,23 @@ public final class Browser {
         }
     }
 
+    /**
+     * Adds a tab to this Browser. If {link tab} is the active Tab of another Browser, then the
+     * other Browser's active tab is set to null. This does nothing if {@link tab} is already
+     * contained in this Browser.
+     *
+     * @param tab The Tab to add.
+     */
+    public void addTab(@NonNull Tab tab) {
+        ThreadCheck.ensureOnUiThread();
+        if (tab.getBrowser() == this) return;
+        try {
+            mImpl.addTab(tab.getITab());
+        } catch (RemoteException e) {
+            throw new APICallException(e);
+        }
+    }
+
     /**
      * Returns the active (visible) Tab associated with this
      * Browser.
@@ -103,7 +118,9 @@ public final class Browser {
     public Tab getActiveTab() {
         ThreadCheck.ensureOnUiThread();
         try {
-            return mTabMap.get(mImpl.getActiveTabId());
+            Tab tab = Tab.getTabById(mImpl.getActiveTabId());
+            assert tab == null || tab.getBrowser() == this;
+            return tab;
         } catch (RemoteException e) {
             throw new APICallException(e);
         }
@@ -117,21 +134,47 @@ public final class Browser {
     @NonNull
     public List<Tab> getTabs() {
         ThreadCheck.ensureOnUiThread();
-        return new ArrayList<Tab>(mTabMap.values());
+        return Tab.getTabsInBrowser(this);
     }
 
     /**
-     * Disposes a Tab. If {@link tabl} is the active Tab,
-     * no Tab is made active. After this call {@link tabl} should not be
-     * used.
+     * Disposes a Tab. If {@link tab} is the active Tab, no Tab is made active. After this call
+     *  {@link tab} should not be used.
      *
      * @param tab The Tab to dispose.
      *
      * @throws IllegalStateException is {@link tab} is not in this Browser.
      */
-    public void disposeTab(Tab tab) {
+    public void destroyTab(@NonNull Tab tab) {
         ThreadCheck.ensureOnUiThread();
-        // TODO(sky): implement this.
+        if (tab.getBrowser() != this) {
+            throw new IllegalStateException("destroyTab() must be called on a Tab in the Browser");
+        }
+        try {
+            mImpl.destroyTab(tab.getITab());
+        } catch (RemoteException e) {
+            throw new APICallException(e);
+        }
+    }
+
+    /**
+     * Adds a TabListCallback.
+     *
+     * @param callback The TabListCallback.
+     */
+    public void addTabListCallback(@NonNull TabListCallback callback) {
+        ThreadCheck.ensureOnUiThread();
+        mTabListCallbacks.addObserver(callback);
+    }
+
+    /**
+     * Removes a TabListCallback.
+     *
+     * @param callback The TabListCallback.
+     */
+    public void removeTabListCallback(@NonNull TabListCallback callback) {
+        ThreadCheck.ensureOnUiThread();
+        mTabListCallbacks.removeObserver(callback);
     }
 
     /**
@@ -188,4 +231,47 @@ public final class Browser {
             throw new APICallException(e);
         }
     }
+
+    private final class BrowserClientImpl extends IBrowserClient.Stub {
+        @Override
+        public void onActiveTabChanged(int activeTabId) {
+            Tab tab = Tab.getTabById(activeTabId);
+            for (TabListCallback callback : mTabListCallbacks) {
+                callback.onActiveTabChanged(tab);
+            }
+        }
+
+        @Override
+        public void onTabAdded(ITab iTab) {
+            int id = 0;
+            try {
+                id = iTab.getId();
+            } catch (RemoteException e) {
+                throw new APICallException(e);
+            }
+            Tab tab = Tab.getTabById(id);
+            if (tab == null) {
+                tab = new Tab(iTab, Browser.this);
+            } else {
+                tab.setBrowser(Browser.this);
+            }
+            for (TabListCallback callback : mTabListCallbacks) {
+                callback.onTabAdded(tab);
+            }
+        }
+
+        @Override
+        public void onTabRemoved(int tabId) {
+            Tab tab = Tab.getTabById(tabId);
+            // This should only be called with a previously created tab.
+            assert tab != null;
+            // And this should only be called for tabs attached to this browser.
+            assert tab.getBrowser() == Browser.this;
+
+            tab.setBrowser(null);
+            for (TabListCallback callback : mTabListCallbacks) {
+                callback.onTabRemoved(tab);
+            }
+        }
+    }
 }

weblayer/public/java/org/chromium/weblayer/BrowserFragment.java

@@ -340,6 +340,7 @@ public final class BrowserFragment extends Fragment {
     @Override
     public void onDestroy() {
         ThreadCheck.ensureOnUiThread();
+        mBrowser.prepareForDestroy();
         try {
             mRemoteFragment.handleOnDestroy();
             // The other side does the clean up automatically in handleOnDestroy()

weblayer/public/java/org/chromium/weblayer/Tab.java

@@ -22,6 +22,11 @@ import org.chromium.weblayer_private.interfaces.ITab;
 import org.chromium.weblayer_private.interfaces.ITabClient;
 import org.chromium.weblayer_private.interfaces.ObjectWrapper;
 
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
 /**
  * Represents a single tab in a browser. More specifically, owns a NavigationController, and allows
  * configuring state of the tab, such as delegates and callbacks.
@@ -30,6 +35,9 @@ public final class Tab {
     /** The top level key of the JSON object returned by executeScript(). */
     public static final String SCRIPT_RESULT_KEY = "result";
 
+    // Maps from id (as returned from ITab.getId()) to Tab.
+    private static final Map<Integer, Tab> sTabMap = new HashMap<Integer, Tab>();
+
     private final ITab mImpl;
     private FullscreenCallbackClientImpl mFullscreenCallbackClient;
     private final NavigationController mNavigationController;
@@ -37,11 +45,14 @@ public final class Tab {
     private Browser mBrowser;
     private DownloadCallbackClientImpl mDownloadCallbackClient;
     private NewTabCallback mNewTabCallback;
+    // Id from the remote side.
+    private final int mId;
 
     Tab(ITab impl, Browser browser) {
         mImpl = impl;
         mBrowser = browser;
         try {
+            mId = impl.getId();
             mImpl.setClient(new TabClientImpl());
         } catch (RemoteException e) {
             throw new APICallException(e);
@@ -49,7 +60,37 @@ public final class Tab {
 
         mCallbacks = new ObserverList<TabCallback>();
         mNavigationController = NavigationController.create(mImpl);
-        mBrowser.registerTab(this);
+        registerTab(this);
+    }
+
+    static void registerTab(Tab tab) {
+        assert getTabById(tab.getId()) == null;
+        sTabMap.put(tab.getId(), tab);
+    }
+
+    static void unregisterTab(Tab tab) {
+        assert getTabById(tab.getId()) != null;
+        sTabMap.remove(tab.getId());
+    }
+
+    static Tab getTabById(int id) {
+        return sTabMap.get(id);
+    }
+
+    static List<Tab> getTabsInBrowser(Browser browser) {
+        List<Tab> tabs = new ArrayList<Tab>();
+        for (Tab tab : sTabMap.values()) {
+            if (tab.getBrowser() == browser) tabs.add(tab);
+        }
+        return tabs;
+    }
+
+    int getId() {
+        return mId;
+    }
+
+    void setBrowser(Browser browser) {
+        mBrowser = browser;
     }
 
     public Browser getBrowser() {
@@ -171,11 +212,14 @@ public final class Tab {
         }
 
         @Override
-        public void onNewTab(ITab iTab, int mode) {
+        public void onNewTab(int tabId, int mode) {
             // This should only be hit if setNewTabCallback() has been called with a non-null
             // value.
             assert mNewTabCallback != null;
-            Tab tab = new Tab(iTab, mBrowser);
+            Tab tab = getTabById(tabId);
+            // Tab should have already been created by way of BrowserClient.
+            assert tab != null;
+            assert tab.getBrowser() == getBrowser();
             mNewTabCallback.onNewTab(tab, mode);
         }
     }

weblayer/public/java/org/chromium/weblayer/TabListCallback.java

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.weblayer;
+
+import androidx.annotation.NonNull;
+import androidx.annotation.Nullable;
+
+/**
+ * An interface for observing changes to the set of tabs in a browser.
+ */
+public abstract class TabListCallback {
+    /**
+     * The active tab has changed.
+     *
+     * @param activeTab The newly active tab.
+     */
+    public void onActiveTabChanged(@Nullable Tab activeTab) {}
+
+    /**
+     * A tab was added to the Browser.
+     *
+     * @param tab The tab that was added.
+     */
+    public void onTabAdded(@NonNull Tab tab) {}
+
+    /**
+     * A tab was removed from the Browser.
+     *
+     * @param tab The tab that was removed.
+     */
+    public void onTabRemoved(@NonNull Tab tab) {}
+}

weblayer/shell/android/BUILD.gn

@@ -204,12 +204,14 @@ instrumentation_test_apk("weblayer_instrumentation_test_apk") {
     "javatests/src/org/chromium/weblayer/test/InputTypesTest.java",
     "javatests/src/org/chromium/weblayer/test/InstrumentationActivityTestRule.java",
     "javatests/src/org/chromium/weblayer/test/NavigationTest.java",
+    "javatests/src/org/chromium/weblayer/test/NewTabCallbackImpl.java",
     "javatests/src/org/chromium/weblayer/test/NewTabCallbackTest.java",
     "javatests/src/org/chromium/weblayer/test/RenderingTest.java",
     "javatests/src/org/chromium/weblayer/test/ProfileTest.java",
     "javatests/src/org/chromium/weblayer/test/SmokeTest.java",
     "javatests/src/org/chromium/weblayer/test/TabCallbackTest.java",
     "javatests/src/org/chromium/weblayer/test/TopControlsTest.java",
+    "javatests/src/org/chromium/weblayer/test/TabListCallbackTest.java",
   ]
   additional_apks = [
     "//weblayer/shell/android:weblayer_support_apk",

weblayer/shell/android/javatests/src/org/chromium/weblayer/test/NewTabCallbackImpl.java

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.weblayer.test;
+
+import org.chromium.base.test.util.CallbackHelper;
+import org.chromium.weblayer.NewTabCallback;
+import org.chromium.weblayer.Tab;
+
+/**
+ * NewTabCallback test helper. Primarily used to wait for a new tab to be created.
+ */
+public class NewTabCallbackImpl extends NewTabCallback {
+    private final CallbackHelper mCallbackHelper = new CallbackHelper();
+
+    @Override
+    public void onNewTab(Tab tab, int mode) {
+        mCallbackHelper.notifyCalled();
+        tab.getBrowser().setActiveTab(tab);
+    }
+
+    public void waitForNewTab() {
+        try {
+            // waitForFirst() only handles a single call. If you need more convert from
+            // waitForFirst().
+            mCallbackHelper.waitForFirst();
+        } catch (Exception e) {
+            throw new RuntimeException(e);
+        }
+    }
+}

weblayer/shell/android/javatests/src/org/chromium/weblayer/test/NewTabCallbackTest.java

@@ -15,10 +15,8 @@ import org.junit.Test;
 import org.junit.runner.RunWith;
 
 import org.chromium.base.test.BaseJUnit4ClassRunner;
-import org.chromium.base.test.util.CallbackHelper;
 import org.chromium.content_public.browser.test.util.TestThreadUtils;
 import org.chromium.net.test.EmbeddedTestServer;
-import org.chromium.weblayer.NewTabCallback;
 import org.chromium.weblayer.Tab;
 import org.chromium.weblayer.shell.InstrumentationActivity;
 
@@ -34,24 +32,6 @@ public class NewTabCallbackTest {
     private EmbeddedTestServer mTestServer;
     private InstrumentationActivity mActivity;
 
-    private static class NewTabCallbackImpl extends NewTabCallback {
-        private CallbackHelper mCallbackHelper = new CallbackHelper();
-
-        @Override
-        public void onNewTab(Tab tab, int mode) {
-            mCallbackHelper.notifyCalled();
-            tab.getBrowser().setActiveTab(tab);
-        }
-
-        public void waitForNewTab() {
-            try {
-                mCallbackHelper.waitForCallback(0);
-            } catch (Exception e) {
-                throw new RuntimeException(e);
-            }
-        }
-    }
-
     @Before
     public void setUp() {
         mTestServer = new EmbeddedTestServer();

weblayer/shell/android/javatests/src/org/chromium/weblayer/test/TabListCallbackTest.java

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.weblayer.test;
+
+import android.os.Bundle;
+import android.support.test.InstrumentationRegistry;
+import android.support.test.filters.SmallTest;
+
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import org.chromium.base.test.BaseJUnit4ClassRunner;
+import org.chromium.content_public.browser.test.util.TestThreadUtils;
+import org.chromium.net.test.EmbeddedTestServer;
+import org.chromium.weblayer.Browser;
+import org.chromium.weblayer.Tab;
+import org.chromium.weblayer.TabListCallback;
+import org.chromium.weblayer.shell.InstrumentationActivity;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+
+/**
+ * Tests that NewTabCallback methods are invoked as expected.
+ */
+@RunWith(BaseJUnit4ClassRunner.class)
+public class TabListCallbackTest {
+    @Rule
+    public InstrumentationActivityTestRule mActivityTestRule =
+            new InstrumentationActivityTestRule();
+
+    private EmbeddedTestServer mTestServer;
+    private InstrumentationActivity mActivity;
+    private Tab mFirstTab;
+    private Tab mSecondTab;
+
+    private static class TabListCallbackImpl extends TabListCallback {
+        public static final String ADDED = "added";
+        public static final String ACTIVE = "active";
+        public static final String REMOVED = "removed";
+
+        private List<String> mObservedValues =
+                Collections.synchronizedList(new ArrayList<String>());
+
+        @Override
+        public void onActiveTabChanged(Tab activeTab) {
+            recordValue(ACTIVE);
+        }
+
+        @Override
+        public void onTabAdded(Tab tab) {
+            recordValue(ADDED);
+        }
+
+        @Override
+        public void onTabRemoved(Tab tab) {
+            recordValue(REMOVED);
+        }
+
+        private void recordValue(String parameter) {
+            mObservedValues.add(parameter);
+        }
+
+        public List<String> getObservedValues() {
+            return mObservedValues;
+        }
+    }
+
+    @Before
+    public void setUp() {
+        mTestServer = new EmbeddedTestServer();
+        mTestServer.initializeNative(InstrumentationRegistry.getInstrumentation().getContext(),
+                EmbeddedTestServer.ServerHTTPSSetting.USE_HTTP);
+        mTestServer.addDefaultHandlers("weblayer/test/data");
+        Assert.assertTrue(mTestServer.start(0));
+
+        String url = mTestServer.getURL("/new_browser.html");
+        mActivity = mActivityTestRule.launchShellWithUrl(url);
+        Assert.assertNotNull(mActivity);
+        NewTabCallbackImpl callback = new NewTabCallbackImpl();
+        mFirstTab = TestThreadUtils.runOnUiThreadBlockingNoException(() -> {
+            Tab tab = mActivity.getBrowser().getActiveTab();
+            tab.setNewTabCallback(callback);
+            return tab;
+        });
+
+        EventUtils.simulateTouchCenterOfView(mActivity.getWindow().getDecorView());
+        callback.waitForNewTab();
+        TestThreadUtils.runOnUiThreadBlocking(() -> {
+            Assert.assertEquals(2, mActivity.getBrowser().getTabs().size());
+            mSecondTab = mActivity.getBrowser().getActiveTab();
+            Assert.assertNotSame(mFirstTab, mSecondTab);
+        });
+    }
+
+    @After
+    public void tearDown() {
+        mTestServer.stopAndDestroyServer();
+    }
+
+    @Test
+    @SmallTest
+    public void testActiveTabChanged() {
+        TestThreadUtils.runOnUiThreadBlocking(() -> {
+            TabListCallbackImpl callback = new TabListCallbackImpl();
+            mActivity.getBrowser().addTabListCallback(callback);
+            mActivity.getBrowser().setActiveTab(mFirstTab);
+            Assert.assertTrue(callback.getObservedValues().contains(TabListCallbackImpl.ACTIVE));
+        });
+    }
+
+    @Test
+    @SmallTest
+    public void testMoveToDifferentFragment() {
+        TestThreadUtils.runOnUiThreadBlocking(() -> {
+            Browser browser2 =
+                    Browser.fromFragment(mActivity.createBrowserFragment(0, new Bundle()));
+            Browser browser1 = mActivity.getBrowser();
+            TabListCallbackImpl callback1 = new TabListCallbackImpl();
+            browser1.addTabListCallback(callback1);
+
+            TabListCallbackImpl callback2 = new TabListCallbackImpl();
+            browser2.addTabListCallback(callback2);
+
+            // Move the active tab from browser1 to browser2.
+            Tab tabToMove = browser1.getActiveTab();
+            browser2.addTab(tabToMove);
+            // This should notify callback1 the active tab changed and a tab was removed.
+            int browser1ActiveIndex =
+                    callback1.getObservedValues().indexOf(TabListCallbackImpl.ACTIVE);
+            Assert.assertNotSame(-1, browser1ActiveIndex);
+            int browser1RemoveIndex =
+                    callback1.getObservedValues().indexOf(TabListCallbackImpl.REMOVED);
+            Assert.assertNotSame(-1, browser1RemoveIndex);
+            Assert.assertTrue(browser1ActiveIndex < browser1RemoveIndex);
+            Assert.assertSame(null, browser1.getActiveTab());
+            Assert.assertEquals(1, browser1.getTabs().size());
+            Assert.assertFalse(browser1.getTabs().contains(tabToMove));
+
+            // callback2 should be notified of the insert.
+            Assert.assertTrue(callback2.getObservedValues().contains(TabListCallbackImpl.ADDED));
+            Assert.assertEquals(2, browser2.getTabs().size());
+            Assert.assertTrue(browser2.getTabs().contains(tabToMove));
+        });
+    }
+
+    @Test
+    @SmallTest
+    public void testDispose() {
+        TestThreadUtils.runOnUiThreadBlocking(() -> {
+            TabListCallbackImpl callback = new TabListCallbackImpl();
+            Browser browser = mActivity.getBrowser();
+            browser.addTabListCallback(callback);
+            browser.destroyTab(mActivity.getBrowser().getActiveTab());
+            Assert.assertTrue(callback.getObservedValues().contains(TabListCallbackImpl.ACTIVE));
+            Assert.assertEquals(1, browser.getTabs().size());
+        });
+    }
+}

weblayer/shell/android/shell_apk/src/org/chromium/weblayer/shell/InstrumentationActivity.java

@@ -153,7 +153,11 @@ public class InstrumentationActivity extends FragmentActivity {
                 return fragments.get(0);
             }
         }
+        return createBrowserFragment(mMainViewId, savedInstanceState);
+    }
 
+    public Fragment createBrowserFragment(int viewId, Bundle savedInstanceState) {
+        FragmentManager fragmentManager = getSupportFragmentManager();
         String profileName = getIntent().hasExtra(EXTRA_PROFILE_NAME)
                 ? getIntent().getStringExtra(EXTRA_PROFILE_NAME)
                 : "DefaultProfile";
@@ -164,7 +168,7 @@ public class InstrumentationActivity extends FragmentActivity {
 
         Fragment fragment = WebLayer.createBrowserFragment(profilePath);
         FragmentTransaction transaction = fragmentManager.beginTransaction();
-        transaction.add(mMainViewId, fragment);
+        transaction.add(viewId, fragment);
 
         // Note the commitNow() instead of commit(). We want the fragment to get attached to
         // activity synchronously, so we can use all the functionality immediately. Otherwise we'd