Remove ChromeActivity ref from CustomTabCompositorContentInitializer

CustomTabCompositorContentInitializer relies on ChromeActivity to
initialize compositor content. This method is part of ChromeActivity's
interface, so removing the dependency is difficult.

Moving the #initializeCompositorContent behind an interface and
injecting that into CustomTabCompositorContentInitializer to decouple
the two.

This change is meant to be an example on how to handle this case.

Bug: 1123209
Change-Id: I8ba93cd80074c8935bd07428327f74ca2973eb19
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2429870Reviewed-by: Peter Conn <peconn@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Commit-Queue: Brandon Wylie <wylieb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#812820}

chrome/android/java/src/org/chromium/chrome/browser/DEPS

@@ -128,9 +128,6 @@ specific_include_rules = {
   "CustomTabBottomBarDelegate\.java": [
     "+chrome/android/java/src/org/chromium/chrome/browser/app/ChromeActivity.java",
   ],
-  "CustomTabCompositorContentInitializer\.java": [
-    "+chrome/android/java/src/org/chromium/chrome/browser/app/ChromeActivity.java",
-  ],
   "CustomTabDelegateFactory\.java": [
     "+chrome/android/java/src/org/chromium/chrome/browser/app/ChromeActivity.java",
   ],

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

@@ -220,7 +220,7 @@ public abstract class ChromeActivity<C extends ChromeActivityComponent>
         implements TabCreatorManager, PolicyChangeListener, ContextualSearchTabPromotionDelegate,
                    SnackbarManageable, SceneChangeObserver,
                    StatusBarColorController.StatusBarColorProvider, AppMenuDelegate, AppMenuBlocker,
-                   MenuOrKeyboardActionController {
+                   MenuOrKeyboardActionController, CompositorViewHolder.Initializer {
     /**
      * No control container to inflate during initialization.
      */
@@ -411,7 +411,8 @@ public abstract class ChromeActivity<C extends ChromeActivityComponent>
                         getWindowAndroid(), this::getCompositorViewHolder, this,
                         this::getCurrentTabCreator, this::isCustomTab,
                         getStatusBarColorController(), ScreenOrientationProvider.getInstance(),
-                        this::getNotificationManagerProxy)
+                        this::getNotificationManagerProxy, getTabContentManagerSupplier(),
+                        /* CompositorViewHolder.Initializer */ this)
                 : overridenCommonsFactory.create(this, mRootUiCoordinator::getBottomSheetController,
                         mTabModelSelectorSupplier, getBrowserControlsManager(),
                         getBrowserControlsManager(), getBrowserControlsManager(),
@@ -420,7 +421,8 @@ public abstract class ChromeActivity<C extends ChromeActivityComponent>
                         getTabContentManager(), getWindowAndroid(), this::getCompositorViewHolder,
                         this, this::getCurrentTabCreator, this::isCustomTab,
                         getStatusBarColorController(), ScreenOrientationProvider.getInstance(),
-                        this::getNotificationManagerProxy);
+                        this::getNotificationManagerProxy, getTabContentManagerSupplier(),
+                        /* CompositorViewHolder.Initializer */ this);
 
         return createComponent(commonsModule);
     }
@@ -1718,18 +1720,7 @@ public abstract class ChromeActivity<C extends ChromeActivityComponent>
         return false;
     }
 
-    /**
-     * Initializes the {@link CompositorViewHolder} with the relevant content it needs to properly
-     * show content on the screen.
-     * @param layoutManager             A {@link LayoutManager} instance.  This class is
-     *                                  responsible for driving all high level screen content and
-     *                                  determines which {@link Layout} is shown when.
-     * @param urlBar                    The {@link View} representing the URL bar (must be
-     *                                  focusable) or {@code null} if none exists.
-     * @param contentContainer          A {@link ViewGroup} that can have content attached by
-     *                                  {@link Layout}s.
-     * @param controlContainer          A {@link ControlContainer} instance to draw.
-     */
+    @Override
     public void initializeCompositorContent(LayoutManager layoutManager, View urlBar,
             ViewGroup contentContainer, ControlContainer controlContainer) {
         if (mContextualSearchManager != null) {

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

@@ -102,6 +102,27 @@ public class CompositorViewHolder extends FrameLayout
                    ViewGroup.OnHierarchyChangeListener {
     private static final long SYSTEM_UI_VIEWPORT_UPDATE_DELAY_MS = 500;
 
+    /**
+     * Initializer interface used to decouple initialization from the class that owns
+     * the CompositorViewHolder.
+     */
+    public interface Initializer {
+        /**
+         * Initializes the {@link CompositorViewHolder} with the relevant content it needs to
+         * properly show content on the screen.
+         * @param layoutManager             A {@link LayoutManager} instance.  This class is
+         *                                  responsible for driving all high level screen content
+         * and determines which {@link Layout} is shown when.
+         * @param urlBar                    The {@link View} representing the URL bar (must be
+         *                                  focusable) or {@code null} if none exists.
+         * @param contentContainer          A {@link ViewGroup} that can have content attached by
+         *                                  {@link Layout}s.
+         * @param controlContainer          A {@link ControlContainer} instance to draw.
+         */
+        void initializeCompositorContent(LayoutManager layoutManager, View urlBar,
+                ViewGroup contentContainer, ControlContainer controlContainer);
+    }
+
     /**
      * Observer interface for any object that needs to process touch events.
      */

chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabCompositorContentInitializer.java

@@ -4,12 +4,14 @@
 
 package org.chromium.chrome.browser.customtabs;
 
+import android.app.Activity;
 import android.view.ViewGroup;
 
 import org.chromium.base.Callback;
-import org.chromium.chrome.browser.app.ChromeActivity;
+import org.chromium.base.supplier.ObservableSupplier;
 import org.chromium.chrome.browser.compositor.CompositorViewHolder;
 import org.chromium.chrome.browser.compositor.layouts.LayoutManager;
+import org.chromium.chrome.browser.compositor.layouts.content.TabContentManager;
 import org.chromium.chrome.browser.dependency_injection.ActivityScope;
 import org.chromium.chrome.browser.lifecycle.ActivityLifecycleDispatcher;
 import org.chromium.chrome.browser.lifecycle.NativeInitObserver;
@@ -26,20 +28,26 @@ import dagger.Lazy;
  */
 @ActivityScope
 public class CustomTabCompositorContentInitializer implements NativeInitObserver {
-    private final ActivityLifecycleDispatcher mLifecycleDispatcher;
+    private final List<Callback<LayoutManager>> mListeners = new ArrayList<>();
 
-    private final ChromeActivity<?> mActivity;
+    private final ActivityLifecycleDispatcher mLifecycleDispatcher;
+    private final Activity mActivity;
     private final Lazy<CompositorViewHolder> mCompositorViewHolder;
+    private final ObservableSupplier<TabContentManager> mTabContentManagerSupplier;
+    private final CompositorViewHolder.Initializer mCompositorViewHolderInitializer;
 
-    private final List<Callback<LayoutManager>> mListeners = new ArrayList<>();
     private boolean mInitialized;
 
     @Inject
     public CustomTabCompositorContentInitializer(ActivityLifecycleDispatcher lifecycleDispatcher,
-            ChromeActivity<?> activity, Lazy<CompositorViewHolder> compositorViewHolder) {
+            Activity activity, Lazy<CompositorViewHolder> compositorViewHolder,
+            ObservableSupplier<TabContentManager> tabContentManagerSupplier,
+            CompositorViewHolder.Initializer compositorViewHolderInitializer) {
         mLifecycleDispatcher = lifecycleDispatcher;
         mActivity = activity;
         mCompositorViewHolder = compositorViewHolder;
+        mTabContentManagerSupplier = tabContentManagerSupplier;
+        mCompositorViewHolderInitializer = compositorViewHolderInitializer;
 
         lifecycleDispatcher.register(this);
     }
@@ -49,7 +57,6 @@ public class CustomTabCompositorContentInitializer implements NativeInitObserver
      * initialized, or immediately (synchronously) if it is already initialized.
      */
     public void addCallback(Callback<LayoutManager> callback) {
-
         if (mInitialized) {
             callback.onResult(mCompositorViewHolder.get().getLayoutManager());
         } else {
@@ -60,10 +67,10 @@ public class CustomTabCompositorContentInitializer implements NativeInitObserver
     @Override
     public void onFinishNativeInitialization() {
         ViewGroup contentContainer = mActivity.findViewById(android.R.id.content);
-        LayoutManager layoutDriver = new LayoutManager(mCompositorViewHolder.get(),
-                contentContainer, mActivity.getTabContentManagerSupplier());
+        LayoutManager layoutDriver = new LayoutManager(
+                mCompositorViewHolder.get(), contentContainer, mTabContentManagerSupplier);
 
-        mActivity.initializeCompositorContent(layoutDriver,
+        mCompositorViewHolderInitializer.initializeCompositorContent(layoutDriver,
                 mActivity.findViewById(org.chromium.chrome.R.id.url_bar), contentContainer,
                 mActivity.findViewById(org.chromium.chrome.R.id.control_container));
 

chrome/android/java/src/org/chromium/chrome/browser/dependency_injection/ChromeActivityCommonsModule.java

@@ -13,6 +13,7 @@ import android.content.Context;
 import android.content.res.Resources;
 import android.view.View;
 
+import org.chromium.base.supplier.ObservableSupplier;
 import org.chromium.base.supplier.Supplier;
 import org.chromium.chrome.browser.ActivityTabProvider;
 import org.chromium.chrome.browser.app.ChromeActivity;
@@ -65,6 +66,8 @@ public class ChromeActivityCommonsModule {
     private final StatusBarColorController mStatusBarColorController;
     private final ScreenOrientationProvider mScreenOrientationProvider;
     private final Supplier<NotificationManagerProxy> mNotificationManagerProxySupplier;
+    private final ObservableSupplier<TabContentManager> mTabContentManagerSupplier;
+    private final CompositorViewHolder.Initializer mCompositorViewHolderInitializer;
 
     /** See {@link ModuleFactoryOverrides} */
     public interface Factory {
@@ -84,7 +87,9 @@ public class ChromeActivityCommonsModule {
                 Supplier<Boolean> isPromotableToTabSupplier,
                 StatusBarColorController statusBarColorController,
                 ScreenOrientationProvider screenOrientationProvider,
-                Supplier<NotificationManagerProxy> notificationManagerProxySupplier);
+                Supplier<NotificationManagerProxy> notificationManagerProxySupplier,
+                ObservableSupplier<TabContentManager> tabContentManagerSupplier,
+                CompositorViewHolder.Initializer compositorViewHolderInitializer);
     }
 
     public ChromeActivityCommonsModule(ChromeActivity activity,
@@ -103,7 +108,9 @@ public class ChromeActivityCommonsModule {
             Supplier<Boolean> isPromotableToTabSupplier,
             StatusBarColorController statusBarColorController,
             ScreenOrientationProvider screenOrientationProvider,
-            Supplier<NotificationManagerProxy> notificationManagerProxySupplier) {
+            Supplier<NotificationManagerProxy> notificationManagerProxySupplier,
+            ObservableSupplier<TabContentManager> tabContentManagerSupplier,
+            CompositorViewHolder.Initializer compositorViewHolderInitializer) {
         mActivity = activity;
         mBottomSheetControllerSupplier = bottomSheetControllerSupplier;
         mTabModelSelectorSupplier = tabModelSelectorSupplier;
@@ -124,6 +131,8 @@ public class ChromeActivityCommonsModule {
         mStatusBarColorController = statusBarColorController;
         mScreenOrientationProvider = screenOrientationProvider;
         mNotificationManagerProxySupplier = notificationManagerProxySupplier;
+        mTabContentManagerSupplier = tabContentManagerSupplier;
+        mCompositorViewHolderInitializer = compositorViewHolderInitializer;
     }
 
     @Provides
@@ -255,4 +264,14 @@ public class ChromeActivityCommonsModule {
     public NotificationManagerProxy provideNotificationManagerProxy() {
         return mNotificationManagerProxySupplier.get();
     }
+
+    @Provides
+    public ObservableSupplier<TabContentManager> provideTabContentManagerSupplier() {
+        return mTabContentManagerSupplier;
+    }
+
+    @Provides
+    public CompositorViewHolder.Initializer provideCompositorViewHolderInitializer() {
+        return mCompositorViewHolderInitializer;
+    }
 }

chrome/android/javatests/src/org/chromium/chrome/browser/browserservices/RunningInChromeTest.java

@@ -79,21 +79,25 @@ public class RunningInChromeTest {
 
     private final TestRule mModuleOverridesRule = new ModuleOverridesRule().setOverride(
             ChromeActivityCommonsModule.Factory.class,
-            (activity, bottomSheetControllerSupplier, tabModelSelectorSupplier,
-                    browserControlsManager, browserControlsVisibilityManager, browserControlsSizer,
-                    fullscreenManager, layoutManagerSupplier, lifecycleDispatcher,
-                    snackbarManagerSupplier, activityTabProvider, tabContentManager,
-                    activityWindowAndroid, compositorViewHolderSupplier, tabCreatorManager,
-                    tabCreatorSupplier, isPromotableToTabSupplier, statusBarColorController,
-                    screenOrientationProvider, notificationManagerProxySupplier) -> {
-                return new ChromeActivityCommonsModule(activity, bottomSheetControllerSupplier,
+            (activity, bottomSheetController, tabModelSelectorSupplier, browserControlsManager,
+                    browserControlsVisibilityManager, browserControlsSizer, fullscreenManager,
+                    layoutManagerSupplier, lifecycleDispatcher, snackbarManagerSupplier,
+                    activityTabProvider, tabContentManager, activityWindowAndroid,
+                    compositorViewHolderSupplier, tabCreatorManager, tabCreatorSupplier,
+                    isPromotableToTabSupplier, statusBarColorController, screenOrientationProvider,
+                    notificationManagerProxySupplier, tabContentManagerSupplier,
+                    compositorViewHolderInitializer) -> {
+                return new ChromeActivityCommonsModule(activity, bottomSheetController,
                         tabModelSelectorSupplier, browserControlsManager,
                         browserControlsVisibilityManager, browserControlsSizer, fullscreenManager,
                         layoutManagerSupplier, lifecycleDispatcher, snackbarManagerSupplier,
                         activityTabProvider, tabContentManager, activityWindowAndroid,
                         compositorViewHolderSupplier, tabCreatorManager, tabCreatorSupplier,
                         isPromotableToTabSupplier, statusBarColorController,
-                        screenOrientationProvider, () -> mMockNotificationManager);
+                        screenOrientationProvider,
+                        ()
+                                -> mMockNotificationManager,
+                        tabContentManagerSupplier, compositorViewHolderInitializer);
             });
 
     @Rule

chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebApkInitializationTest.java

@@ -92,7 +92,8 @@ public class WebApkInitializationTest {
                     snackbarManagerSupplier, activityTabProvider, tabContentManager,
                     activityWindowAndroid, compositorViewHolderSupplier, tabCreatorManager,
                     tabCreatorSupplier, isPromotableToTabSupplier, statusBarColorController,
-                    screenOrientationProvider, notificationManagerProxySupplier) -> {
+                    screenOrientationProvider, notificationManagerProxySupplier,
+                    tabContentManagerSupplier, compositorViewHolderInitializer) -> {
                 mTrackingActivityLifecycleDispatcher.init(lifecycleDispatcher);
                 return new ChromeActivityCommonsModule(activity, bottomSheetControllerSupplier,
                         tabModelSelectorSupplier, browserControlsManager,
@@ -101,7 +102,8 @@ public class WebApkInitializationTest {
                         snackbarManagerSupplier, activityTabProvider, tabContentManager,
                         activityWindowAndroid, compositorViewHolderSupplier, tabCreatorManager,
                         tabCreatorSupplier, isPromotableToTabSupplier, statusBarColorController,
-                        screenOrientationProvider, notificationManagerProxySupplier);
+                        screenOrientationProvider, notificationManagerProxySupplier,
+                        tabContentManagerSupplier, compositorViewHolderInitializer);
             });
 
     private final WebApkActivityTestRule mActivityRule = new WebApkActivityTestRule();