Use ActivityTabProvider for TopToolbarOverlay

This patch replaces the custom tab supplier used in the LayoutManager
with the standard ActivityTabProvider. The fact that the top toolbar
overlay now uses a 'push' model to update instead of a passive 'pull'
means the tab sends the correct information immediately when ready
instead of the guessing that needed to happen before. This simplifies
the logic and reduces the number of tab sources.

Bug: 1100332
Change-Id: I49667aebed4de36fd34be957a5fbf421e510713d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2354996Reviewed-by: Theresa  <twellington@chromium.org>
Reviewed-by: Jinsuk Kim <jinsukkim@chromium.org>
Commit-Queue: Matthew Jones <mdjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#805082}

chrome/android/java/src/org/chromium/chrome/browser/app/ChromeActivity.java

@@ -1692,8 +1692,8 @@ public abstract class ChromeActivity<C extends ChromeActivityComponent>
         mCompositorViewHolder.setBrowserControlsManager(getBrowserControlsManager());
         mCompositorViewHolder.setUrlBar(urlBar);
         mCompositorViewHolder.setInsetObserverView(getInsetObserverView());
-        mCompositorViewHolder.onFinishNativeInitialization(
-                getTabModelSelector(), this, getTabContentManager(), mContextualSearchManager);
+        mCompositorViewHolder.onFinishNativeInitialization(getTabModelSelector(), this,
+                getTabContentManager(), mContextualSearchManager, mActivityTabProvider);
 
         if (controlContainer != null && DeviceClassManager.enableToolbarSwipe()
                 && getCompositorViewHolder().getLayoutManager().getToolbarSwipeHandler() != null) {

chrome/android/java/src/org/chromium/chrome/browser/compositor/CompositorViewHolder.java

@@ -43,6 +43,7 @@ import org.chromium.base.compat.ApiHelperForO;
 import org.chromium.base.supplier.ObservableSupplier;
 import org.chromium.base.supplier.ObservableSupplierImpl;
 import org.chromium.chrome.R;
+import org.chromium.chrome.browser.ActivityTabProvider;
 import org.chromium.chrome.browser.browser_controls.BrowserControlsStateProvider;
 import org.chromium.chrome.browser.browser_controls.BrowserControlsUtils;
 import org.chromium.chrome.browser.compositor.Invalidator.Client;
@@ -1222,14 +1223,16 @@ public class CompositorViewHolder extends FrameLayout
      * @param tabCreatorManager       The {@link TabCreatorManager} for this view.
      * @param tabContentManager       The {@link TabContentManager} for the tabs.
      * @param contextualSearchManager A {@link ContextualSearchManagementDelegate} instance.
+     * @param tabProvider             A means of acquiring the active tab.
      */
     public void onFinishNativeInitialization(TabModelSelector tabModelSelector,
             TabCreatorManager tabCreatorManager, TabContentManager tabContentManager,
-            ContextualSearchManagementDelegate contextualSearchManager) {
+            ContextualSearchManagementDelegate contextualSearchManager,
+            ActivityTabProvider tabProvider) {
         assert mLayoutManager != null && mControlContainer != null;
         mLayoutManager.init(tabModelSelector, tabCreatorManager, tabContentManager,
                 mControlContainer, contextualSearchManager,
-                mCompositorView.getResourceManager().getDynamicResourceLoader());
+                mCompositorView.getResourceManager().getDynamicResourceLoader(), tabProvider);
 
         mTabModelSelector = tabModelSelector;
         tabModelSelector.addObserver(new EmptyTabModelSelectorObserver() {

chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/LayoutManager.java

@@ -19,6 +19,7 @@ import org.chromium.base.ObserverList;
 import org.chromium.base.TraceEvent;
 import org.chromium.base.supplier.ObservableSupplierImpl;
 import org.chromium.base.supplier.Supplier;
+import org.chromium.chrome.browser.ActivityTabProvider;
 import org.chromium.chrome.browser.browser_controls.BrowserControlsStateProvider;
 import org.chromium.chrome.browser.browser_controls.BrowserControlsUtils;
 import org.chromium.chrome.browser.browser_controls.BrowserControlsVisibilityManager;
@@ -152,14 +153,8 @@ public class LayoutManager implements LayoutUpdateHost, LayoutProvider,
     /** The animation handler responsible for updating all the browser compositor's animations. */
     private final CompositorAnimationHandler mAnimationHandler;
 
-    /**
-     * Current tab to provide Toolbar overlay with. Unlike TabModelSelector#getCurrentTab
-     * which returns null right after a tab is closed, this keeps the reference until
-     * TabModelObserver#didCloseTab is triggered for Toolbar overlay to have a chance
-     * to retrieve the right textbox color from it.
-     */
-    private ObservableSupplierImpl<Tab> mCurrentTabSupplier;
-
+    private final ObservableSupplierImpl<TabModelSelector> mTabModelSelectorSupplier =
+            new ObservableSupplierImpl<>();
     private final ObservableSupplierImpl<TabContentManager> mTabContentManagerSupplier =
             new ObservableSupplierImpl<>();
     private final ObservableSupplierImpl<BrowserControlsStateProvider>
@@ -177,7 +172,6 @@ public class LayoutManager implements LayoutUpdateHost, LayoutProvider,
             } else if (tab.getId() != lastId) {
                 tabSelected(tab.getId(), lastId, tab.isIncognito());
             }
-            mCurrentTabSupplier.set(tab);
         }
 
         @Override
@@ -220,8 +214,6 @@ public class LayoutManager implements LayoutUpdateHost, LayoutProvider,
         @Override
         public void didCloseTab(int tabId, boolean incognito) {
             tabClosed(tabId, incognito, false);
-            mCurrentTabSupplier.set(
-                    getTabModelSelector() != null ? getTabModelSelector().getCurrentTab() : null);
         }
 
         @Override
@@ -250,7 +242,6 @@ public class LayoutManager implements LayoutUpdateHost, LayoutProvider,
         mPxToDp = 1.f / mHost.getContext().getResources().getDisplayMetrics().density;
         mAndroidViewShownSupplier = new ObservableSupplierImpl<>();
         mAndroidViewShownSupplier.set(true);
-        mCurrentTabSupplier = new ObservableSupplierImpl<>();
 
         mContext = host.getContext();
         LayoutRenderHost renderHost = host.getLayoutRenderHost();
@@ -410,9 +401,8 @@ public class LayoutManager implements LayoutUpdateHost, LayoutProvider,
     public void init(TabModelSelector selector, TabCreatorManager creator,
             TabContentManager content, ControlContainer controlContainer,
             ContextualSearchManagementDelegate contextualSearchDelegate,
-            DynamicResourceLoader dynamicResourceLoader) {
+            DynamicResourceLoader dynamicResourceLoader, ActivityTabProvider tabProvider) {
         LayoutRenderHost renderHost = mHost.getLayoutRenderHost();
-        mCurrentTabSupplier.set(selector.getCurrentTab());
 
         // Build Layouts
         mStaticLayout = new StaticLayout(mContext, this, renderHost, mHost, mFrameRequestSupplier,
@@ -430,7 +420,7 @@ public class LayoutManager implements LayoutUpdateHost, LayoutProvider,
                     -> getActiveLayout() != null ? getActiveLayout().getViewportMode()
                                                  : Layout.ViewportMode.ALWAYS_FULLSCREEN;
             mToolbarOverlay = new TopToolbarOverlayCoordinator(mContext, mFrameRequestSupplier,
-                    this, controlContainer, mCurrentTabSupplier, getBrowserControlsManager(),
+                    this, controlContainer, tabProvider, getBrowserControlsManager(),
                     viewportModeSupplier, mAndroidViewShownSupplier,
                     () -> renderHost.getResourceManager());
         }
@@ -473,7 +463,7 @@ public class LayoutManager implements LayoutUpdateHost, LayoutProvider,
     // mTabModelSelector should only be set once.
     public void setTabModelSelector(TabModelSelector selector) {
         mTabModelSelector = selector;
-        mCurrentTabSupplier.set(selector.getCurrentTab());
+        mTabModelSelectorSupplier.set(selector);
         mTabModelSelectorTabObserver = new TabModelSelectorTabObserver(mTabModelSelector) {
             @Override
             public void onShown(Tab tab, @TabSelectionType int type) {
@@ -534,7 +524,6 @@ public class LayoutManager implements LayoutUpdateHost, LayoutProvider,
             getTabModelSelector().getTabModelFilterProvider().removeTabModelFilterObserver(
                     mTabModelFilterObserver);
         }
-        mCurrentTabSupplier.set(null);
     }
 
     /**

chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/LayoutManagerChrome.java

@@ -13,6 +13,7 @@ import androidx.annotation.VisibleForTesting;
 import org.chromium.base.ObserverList;
 import org.chromium.base.metrics.RecordUserAction;
 import org.chromium.base.supplier.ObservableSupplier;
+import org.chromium.chrome.browser.ActivityTabProvider;
 import org.chromium.chrome.browser.accessibility_tab_switcher.OverviewListLayout;
 import org.chromium.chrome.browser.browser_controls.BrowserControlsStateProvider;
 import org.chromium.chrome.browser.compositor.TitleCache;
@@ -147,7 +148,7 @@ public class LayoutManagerChrome
     public void init(TabModelSelector selector, TabCreatorManager creator,
             TabContentManager content, ControlContainer controlContainer,
             ContextualSearchManagementDelegate contextualSearchDelegate,
-            DynamicResourceLoader dynamicResourceLoader) {
+            DynamicResourceLoader dynamicResourceLoader, ActivityTabProvider tabProvider) {
         Context context = mHost.getContext();
         LayoutRenderHost renderHost = mHost.getLayoutRenderHost();
         BrowserControlsStateProvider browserControlsStateProvider =
@@ -166,7 +167,7 @@ public class LayoutManagerChrome
         }
 
         super.init(selector, creator, content, controlContainer, contextualSearchDelegate,
-                dynamicResourceLoader);
+                dynamicResourceLoader, tabProvider);
 
         // TODO: TitleCache should be a part of the ResourceManager.
         mTitleCache = mHost.getTitleCache();

chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/LayoutManagerChromePhone.java

@@ -7,6 +7,7 @@ package org.chromium.chrome.browser.compositor.layouts;
 import android.content.Context;
 import android.view.ViewGroup;
 
+import org.chromium.chrome.browser.ActivityTabProvider;
 import org.chromium.chrome.browser.compositor.layouts.content.TabContentManager;
 import org.chromium.chrome.browser.compositor.layouts.phone.SimpleAnimationLayout;
 import org.chromium.chrome.browser.compositor.overlays.SceneOverlay;
@@ -45,7 +46,7 @@ public class LayoutManagerChromePhone extends LayoutManagerChrome {
     public void init(TabModelSelector selector, TabCreatorManager creator,
             TabContentManager content, ControlContainer controlContainer,
             ContextualSearchManagementDelegate contextualSearchDelegate,
-            DynamicResourceLoader dynamicResourceLoader) {
+            DynamicResourceLoader dynamicResourceLoader, ActivityTabProvider tabProvider) {
         Context context = mHost.getContext();
         LayoutRenderHost renderHost = mHost.getLayoutRenderHost();
 
@@ -53,7 +54,7 @@ public class LayoutManagerChromePhone extends LayoutManagerChrome {
         mSimpleAnimationLayout = new SimpleAnimationLayout(context, this, renderHost);
 
         super.init(selector, creator, content, controlContainer, contextualSearchDelegate,
-                dynamicResourceLoader);
+                dynamicResourceLoader, tabProvider);
 
         // Set up layout parameters
         mStaticLayout.setLayoutHandlesTabLifecycles(false);

chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/LayoutManagerChromeTablet.java

@@ -7,6 +7,7 @@ package org.chromium.chrome.browser.compositor.layouts;
 import android.view.ViewGroup;
 
 import org.chromium.chrome.R;
+import org.chromium.chrome.browser.ActivityTabProvider;
 import org.chromium.chrome.browser.compositor.layouts.content.TabContentManager;
 import org.chromium.chrome.browser.compositor.overlays.strip.StripLayoutHelperManager;
 import org.chromium.chrome.browser.contextualsearch.ContextualSearchManagementDelegate;
@@ -84,13 +85,13 @@ public class LayoutManagerChromeTablet extends LayoutManagerChrome {
     public void init(TabModelSelector selector, TabCreatorManager creator,
             TabContentManager content, ControlContainer controlContainer,
             ContextualSearchManagementDelegate contextualSearchDelegate,
-            DynamicResourceLoader dynamicResourceLoader) {
+            DynamicResourceLoader dynamicResourceLoader, ActivityTabProvider tabProvider) {
         if (mTabStripLayoutHelperManager != null) {
             mTabStripLayoutHelperManager.setTabModelSelector(selector, creator);
         }
 
         super.init(selector, creator, content, controlContainer, contextualSearchDelegate,
-                dynamicResourceLoader);
+                dynamicResourceLoader, tabProvider);
 
         // Make sure any tabs already restored get loaded into the title cache.
         List<TabModel> models = selector.getModels();

chrome/android/java/src/org/chromium/chrome/browser/compositor/overlays/toolbar/TopToolbarOverlayCoordinator.java

@@ -10,6 +10,7 @@ import android.graphics.RectF;
 import org.chromium.base.supplier.ObservableSupplier;
 import org.chromium.base.supplier.Supplier;
 import org.chromium.chrome.R;
+import org.chromium.chrome.browser.ActivityTabProvider;
 import org.chromium.chrome.browser.browser_controls.BrowserControlsStateProvider;
 import org.chromium.chrome.browser.compositor.LayerTitleCache;
 import org.chromium.chrome.browser.compositor.layouts.CompositorModelChangeProcessor;
@@ -18,7 +19,6 @@ import org.chromium.chrome.browser.compositor.layouts.components.VirtualView;
 import org.chromium.chrome.browser.compositor.layouts.eventfilter.EventFilter;
 import org.chromium.chrome.browser.compositor.overlays.SceneOverlay;
 import org.chromium.chrome.browser.compositor.scene_layer.SceneOverlayLayer;
-import org.chromium.chrome.browser.tab.Tab;
 import org.chromium.chrome.browser.toolbar.ControlContainer;
 import org.chromium.ui.modelutil.PropertyModel;
 import org.chromium.ui.resources.ResourceManager;
@@ -42,7 +42,7 @@ public class TopToolbarOverlayCoordinator implements SceneOverlay {
     public TopToolbarOverlayCoordinator(Context context,
             CompositorModelChangeProcessor.FrameRequestSupplier frameRequestSupplier,
             LayoutManager layoutManager, ControlContainer controlContainer,
-            ObservableSupplier<Tab> tabSupplier,
+            ActivityTabProvider tabSupplier,
             BrowserControlsStateProvider browserControlsStateProvider,
             Supplier<Integer> viewportModeSupplier,
             ObservableSupplier<Boolean> androidViewShownSupplier,

chrome/android/java/src/org/chromium/chrome/browser/compositor/overlays/toolbar/TopToolbarOverlayMediator.java

@@ -12,6 +12,7 @@ import androidx.annotation.VisibleForTesting;
 import org.chromium.base.Callback;
 import org.chromium.base.supplier.ObservableSupplier;
 import org.chromium.base.supplier.Supplier;
+import org.chromium.chrome.browser.ActivityTabProvider;
 import org.chromium.chrome.browser.browser_controls.BrowserControlsStateProvider;
 import org.chromium.chrome.browser.browser_controls.BrowserControlsUtils;
 import org.chromium.chrome.browser.compositor.layouts.Layout;
@@ -46,10 +47,10 @@ public class TopToolbarOverlayMediator {
     private final ControlContainer mToolbarContainer;
 
     /** Provides current tab. */
-    private final ObservableSupplier<Tab> mTabSupplier;
+    private final ActivityTabProvider mTabSupplier;
 
     /** An observer that watches for changes in the active tab. */
-    private final Callback<Tab> mTabSupplierObserver;
+    private final ActivityTabProvider.ActivityTabObserver mTabSupplierObserver;
 
     /** Access to the current state of the browser controls. */
     private final BrowserControlsStateProvider mBrowserControlsStateProvider;
@@ -73,7 +74,7 @@ public class TopToolbarOverlayMediator {
     private Tab mLastActiveTab;
 
     TopToolbarOverlayMediator(PropertyModel model, Context context, LayoutManager layoutManager,
-            ControlContainer controlContainer, ObservableSupplier<Tab> tabSupplier,
+            ControlContainer controlContainer, ActivityTabProvider tabSupplier,
             BrowserControlsStateProvider browserControlsStateProvider,
             Supplier<Integer> viewportModeSupplier,
             ObservableSupplier<Boolean> androidViewShownSupplier) {
@@ -117,7 +118,7 @@ public class TopToolbarOverlayMediator {
 
         // Keep an observer attached to the visible tab (and only the visible tab) to update
         // properties including theme color.
-        mTabSupplierObserver = (tab) -> {
+        mTabSupplierObserver = (tab, hint) -> {
             if (mLastActiveTab != null) mLastActiveTab.removeObserver(currentTabObserver);
             if (tab == null) return;
 
@@ -127,7 +128,7 @@ public class TopToolbarOverlayMediator {
             updateThemeColor(mLastActiveTab);
             updateProgress();
         };
-        mTabSupplier.addObserver(mTabSupplierObserver);
+        mTabSupplier.addObserverAndTrigger(mTabSupplierObserver);
 
         mAndroidViewShownObserver = (shown) -> updateShadowState();
         mAndroidViewShownSupplier.addObserver(mAndroidViewShownObserver);
@@ -225,7 +226,7 @@ public class TopToolbarOverlayMediator {
     /** Clean up any state and observers. */
     void destroy() {
         mTabSupplier.removeObserver(mTabSupplierObserver);
-        mTabSupplierObserver.onResult(null);
+        mTabSupplierObserver.onActivityTabChanged(null, false);
         mLastActiveTab = null;
 
         mLayoutManager.removeSceneChangeObserver(mSceneChangeObserver);

chrome/android/javatests/src/org/chromium/chrome/browser/compositor/layouts/LayoutManagerTest.java

@@ -25,6 +25,8 @@ import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.MockitoAnnotations;
 
 import org.chromium.base.MathUtils;
 import org.chromium.base.test.UiThreadTest;
@@ -32,6 +34,7 @@ import org.chromium.base.test.util.CommandLineFlags;
 import org.chromium.base.test.util.DisabledTest;
 import org.chromium.base.test.util.Feature;
 import org.chromium.base.test.util.Restriction;
+import org.chromium.chrome.browser.ActivityTabProvider;
 import org.chromium.chrome.browser.accessibility_tab_switcher.OverviewListLayout;
 import org.chromium.chrome.browser.compositor.animation.CompositorAnimationHandler;
 import org.chromium.chrome.browser.compositor.layouts.components.LayoutTab;
@@ -73,6 +76,9 @@ public class LayoutManagerTest implements MockTabModelDelegate {
     @Rule
     public ChromeTabbedActivityTestRule mActivityTestRule = new ChromeTabbedActivityTestRule();
 
+    @Mock
+    private ActivityTabProvider mTabSupplier;
+
     private long mLastDownTime;
 
     private TabModelSelector mTabModelSelector;
@@ -160,7 +166,7 @@ public class LayoutManagerTest implements MockTabModelDelegate {
         mManagerPhone = new LayoutManagerChromePhone(layoutManagerHost, container, null);
         mManager = mManagerPhone;
         CompositorAnimationHandler.setTestingMode(true);
-        mManager.init(mTabModelSelector, null, tabContentManager, null, null, null);
+        mManager.init(mTabModelSelector, null, tabContentManager, null, null, null, mTabSupplier);
         initializeMotionEvent();
     }
 
@@ -626,6 +632,8 @@ public class LayoutManagerTest implements MockTabModelDelegate {
 
     @Before
     public void setUp() {
+        MockitoAnnotations.initMocks(this);
+
         // Load the browser process.
         TestThreadUtils.runOnUiThreadBlocking(
                 () -> { ChromeBrowserInitializer.getInstance().handleSynchronousStartup(); });

chrome/android/junit/src/org/chromium/chrome/browser/compositor/overlays/toolbar/TopToolbarOverlayMediatorTest.java

@@ -23,6 +23,7 @@ import org.mockito.MockitoAnnotations;
 import org.chromium.base.supplier.ObservableSupplier;
 import org.chromium.base.supplier.ObservableSupplierImpl;
 import org.chromium.base.test.BaseRobolectricTestRunner;
+import org.chromium.chrome.browser.ActivityTabProvider;
 import org.chromium.chrome.browser.browser_controls.BrowserControlsStateProvider;
 import org.chromium.chrome.browser.compositor.layouts.Layout.ViewportMode;
 import org.chromium.chrome.browser.compositor.layouts.LayoutManager;
@@ -65,16 +66,18 @@ public class TopToolbarOverlayMediatorTest {
     @Captor
     private ArgumentCaptor<BrowserControlsStateProvider.Observer> mBrowserControlsObserverCaptor;
 
-    private ObservableSupplierImpl<Tab> mTabSupplier;
+    @Mock
+    private ActivityTabProvider mTabSupplier;
+
+    @Captor
+    private ArgumentCaptor<ActivityTabProvider.ActivityTabObserver> mActivityTabObserverCaptor;
+
     private ObservableSupplierImpl<Boolean> mAndroidViewShownSupplier;
 
     @Before
     public void beforeTest() {
         MockitoAnnotations.initMocks(this);
 
-        mTabSupplier = new ObservableSupplierImpl<>();
-        mTabSupplier.set(mTab);
-
         mAndroidViewShownSupplier = new ObservableSupplierImpl<>();
         mAndroidViewShownSupplier.set(true);
 
@@ -100,11 +103,20 @@ public class TopToolbarOverlayMediatorTest {
                 mAndroidViewShownSupplier);
 
         // Ensure the observer is added to the initial tab.
+        verify(mTabSupplier).addObserverAndTrigger(mActivityTabObserverCaptor.capture());
+        setTabSupplierTab(mTab);
+
         verify(mTab).addObserver(mTabObserverCaptor.capture());
 
         verify(mBrowserControlsProvider).addObserver(mBrowserControlsObserverCaptor.capture());
     }
 
+    /** Set the tab that will be returned by the supplier and trigger the observer event. */
+    private void setTabSupplierTab(Tab tab) {
+        when(mTabSupplier.get()).thenReturn(tab);
+        mActivityTabObserverCaptor.getValue().onActivityTabChanged(tab, false);
+    }
+
     @After
     public void afterTest() {
         // Unset any testing state the tests may have set.
@@ -113,7 +125,7 @@ public class TopToolbarOverlayMediatorTest {
 
     @Test
     public void testTabObserverAfterTabSwitch() {
-        mTabSupplier.set(mTab2);
+        setTabSupplierTab(mTab2);
 
         // Make sure the tab observer for this overlay is only observing the "current" tab.
         verify(mTab).removeObserver(mTabObserverCaptor.getValue());
@@ -159,7 +171,7 @@ public class TopToolbarOverlayMediatorTest {
 
         // Ensure the progress is correct on tab switch.
         mTabObserverCaptor.getValue().onLoadProgressChanged(mTab, 0.f);
-        mTabSupplier.set(mTab2);
+        setTabSupplierTab(mTab2);
     }
 
     @Test