Android: Move BrowserControlsManager to RootUiCoordinator

This CL lets RootUiCoordinator create and provide BrowserControlsManager
to other classes. Chrome*Activity still has #getBrowserControlsManager
which now just defers the call to RootUiCoordinator.

This makes |createComponent| and |createRootUiCoordinator| in
ChromeActivity#performPreInflationStartup form dependencies on each other
for CCT/Webapp. This was broken by creating RootUiCoordinator first and
having some components be passed to it as a supplier instead.

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

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

@@ -251,9 +251,6 @@ public abstract class ChromeActivity<C extends ChromeActivityComponent>
     // Observes when sync becomes ready to create the mContextReporter.
     private ProfileSyncService.SyncStateChangedListener mSyncStateChangedListener;
 
-    @Nullable
-    private BrowserControlsManager mBrowserControlsManager;
-
     // The PictureInPictureController is initialized lazily https://crbug.com/729738.
     private PictureInPictureController mPictureInPictureController;
 
@@ -325,6 +322,10 @@ public abstract class ChromeActivity<C extends ChromeActivityComponent>
 
     @Override
     public void performPreInflationStartup() {
+        // Make sure the root coordinator is created prior to calling super to ensure all the
+        // activity lifecycle events are called.
+        mRootUiCoordinator = createRootUiCoordinator();
+
         // Create component before calling super to give its members a chance to catch
         // onPreInflationStartup event.
         mComponent = createComponent();
@@ -336,9 +337,6 @@ public abstract class ChromeActivity<C extends ChromeActivityComponent>
             if (oldBridge != null) oldBridge.destroy();
             mBookmarkBridgeSupplier.set(profile == null ? null : new BookmarkBridge(profile));
         });
-        // Make sure the root coordinator is created prior to calling super to ensure all the
-        // activity lifecycle events are called.
-        mRootUiCoordinator = createRootUiCoordinator();
 
         super.performPreInflationStartup();
 
@@ -1261,11 +1259,6 @@ public abstract class ChromeActivity<C extends ChromeActivityComponent>
             mActivityTabStartupMetricsTracker = null;
         }
 
-        if (mBrowserControlsManager != null) {
-            mBrowserControlsManager.destroy();
-            mBrowserControlsManager = null;
-        }
-
         if (mTabModelsInitialized) {
             TabModelSelector selector = getTabModelSelector();
             if (selector != null) selector.destroy();
@@ -1635,16 +1628,7 @@ public abstract class ChromeActivity<C extends ChromeActivityComponent>
      */
     @NonNull
     public BrowserControlsManager getBrowserControlsManager() {
-        if (mBrowserControlsManager == null) {
-            // When finish()ing, getBrowserControlsManager() is required to perform cleanup logic.
-            // It should never be called when it results in creating a new manager though.
-            if (isActivityFinishingOrDestroyed()) {
-                throw new IllegalStateException();
-            }
-            mBrowserControlsManager = createBrowserControlsManager();
-            assert mBrowserControlsManager != null;
-        }
-        return mBrowserControlsManager;
+        return mRootUiCoordinator.getBrowserControlsManager();
     }
 
     /**

chrome/android/java/src/org/chromium/chrome/browser/ChromeTabbedActivity.java

@@ -81,7 +81,6 @@ import org.chromium.chrome.browser.firstrun.FirstRunSignInProcessor;
 import org.chromium.chrome.browser.flags.ActivityType;
 import org.chromium.chrome.browser.flags.ChromeFeatureList;
 import org.chromium.chrome.browser.flags.ChromeSwitches;
-import org.chromium.chrome.browser.fullscreen.BrowserControlsManager;
 import org.chromium.chrome.browser.gesturenav.NavigationSheet;
 import org.chromium.chrome.browser.homepage.HomepageManager;
 import org.chromium.chrome.browser.incognito.IncognitoNotificationManager;
@@ -243,7 +242,6 @@ public class ChromeTabbedActivity extends ChromeActivity<ChromeActivityComponent
     private TabModelSelectorTabObserver mTabModelSelectorTabObserver;
     private TabModelSelectorTabModelObserver mTabModelObserver;
 
-    private ComposedBrowserControlsVisibilityDelegate mAppBrowserControlsVisibilityDelegate;
     private BrowserControlsVisibilityDelegate mVrBrowserControlsVisibilityDelegate;
     private TabModalLifetimeHandler mTabModalHandler;
 
@@ -2207,25 +2205,16 @@ public class ChromeTabbedActivity extends ChromeActivity<ChromeActivityComponent
     }
 
     private ComposedBrowserControlsVisibilityDelegate getAppBrowserControlsVisibilityDelegate() {
-        if (mAppBrowserControlsVisibilityDelegate == null) {
-            mAppBrowserControlsVisibilityDelegate = new ComposedBrowserControlsVisibilityDelegate();
-        }
-        return mAppBrowserControlsVisibilityDelegate;
-    }
-
-    @Override
-    protected BrowserControlsManager createBrowserControlsManager() {
-        BrowserControlsManager manager = super.createBrowserControlsManager();
-        getAppBrowserControlsVisibilityDelegate().addDelegate(
-                manager.getBrowserVisibilityDelegate());
-        return manager;
+        // TODO(jinsukkim): Move this to RootUiCoordinator.
+        return ((TabbedRootUiCoordinator) mRootUiCoordinator)
+                .getAppBrowserControlsVisibilityDelegate();
     }
 
     @Override
     protected ModalDialogManager createModalDialogManager() {
         ModalDialogManager manager = super.createModalDialogManager();
         mTabModalHandler = new TabModalLifetimeHandler(this, manager,
-                getAppBrowserControlsVisibilityDelegate(), this::getTabObscuringHandler);
+                this::getAppBrowserControlsVisibilityDelegate, this::getTabObscuringHandler);
         return manager;
     }
 
@@ -2276,7 +2265,8 @@ public class ChromeTabbedActivity extends ChromeActivity<ChromeActivityComponent
         if (mVrBrowserControlsVisibilityDelegate == null) {
             mVrBrowserControlsVisibilityDelegate =
                     new BrowserControlsVisibilityDelegate(BrowserControlsState.BOTH);
-            mAppBrowserControlsVisibilityDelegate.addDelegate(mVrBrowserControlsVisibilityDelegate);
+            getAppBrowserControlsVisibilityDelegate().addDelegate(
+                    mVrBrowserControlsVisibilityDelegate);
         }
         mVrBrowserControlsVisibilityDelegate.set(BrowserControlsState.HIDDEN);
     }

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

@@ -150,7 +150,7 @@ public abstract class BaseCustomTabActivity extends ChromeActivity<BaseCustomTab
     @Override
     protected RootUiCoordinator createRootUiCoordinator() {
         return new BaseCustomTabRootUiCoordinator(this, getShareDelegateSupplier(),
-                mToolbarCoordinator, mNavigationController, getActivityTabProvider(),
+                () -> mToolbarCoordinator, () -> mNavigationController, getActivityTabProvider(),
                 mTabModelProfileSupplier, mBookmarkBridgeSupplier, mOverviewModeBehaviorSupplier,
                 this::getContextualSearchManager);
     }

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

@@ -28,13 +28,13 @@ import org.chromium.components.feature_engagement.Tracker;
  */
 public class BaseCustomTabRootUiCoordinator
         extends RootUiCoordinator implements NativeInitObserver {
-    private final CustomTabToolbarCoordinator mToolbarCoordinator;
-    private final CustomTabActivityNavigationController mNavigationController;
+    private final Supplier<CustomTabToolbarCoordinator> mToolbarCoordinator;
+    private final Supplier<CustomTabActivityNavigationController> mNavigationController;
 
     public BaseCustomTabRootUiCoordinator(ChromeActivity activity,
             ObservableSupplier<ShareDelegate> shareDelegateSupplier,
-            CustomTabToolbarCoordinator customTabToolbarCoordinator,
-            CustomTabActivityNavigationController customTabNavigationController,
+            Supplier<CustomTabToolbarCoordinator> customTabToolbarCoordinator,
+            Supplier<CustomTabActivityNavigationController> customTabNavigationController,
             ActivityTabProvider tabProvider, ObservableSupplier<Profile> profileSupplier,
             ObservableSupplier<BookmarkBridge> bookmarkBridgeSupplier,
             ObservableSupplier<OverviewModeBehavior> overviewModeBehaviorSupplier,
@@ -50,8 +50,8 @@ public class BaseCustomTabRootUiCoordinator
     protected void initializeToolbar() {
         super.initializeToolbar();
 
-        mToolbarCoordinator.onToolbarInitialized(mToolbarManager);
-        mNavigationController.onToolbarInitialized(mToolbarManager);
+        mToolbarCoordinator.get().onToolbarInitialized(mToolbarManager);
+        mNavigationController.get().onToolbarInitialized(mToolbarManager);
     }
 
     @Override

chrome/android/java/src/org/chromium/chrome/browser/modaldialog/TabModalLifetimeHandler.java

@@ -52,7 +52,7 @@ public class TabModalLifetimeHandler implements NativeInitObserver, Destroyable
 
     private final ChromeActivity mActivity;
     private final ModalDialogManager mManager;
-    private final ComposedBrowserControlsVisibilityDelegate mAppVisibilityDelegate;
+    private final Supplier<ComposedBrowserControlsVisibilityDelegate> mAppVisibilityDelegate;
     private final Supplier<TabObscuringHandler> mTabObscuringHandlerSupplier;
     private ChromeTabModalPresenter mPresenter;
     private TabModelSelectorTabModelObserver mTabModelObserver;
@@ -67,7 +67,7 @@ public class TabModalLifetimeHandler implements NativeInitObserver, Destroyable
      * @param tabObscuringHandler {@link TabObscuringHandler} object.
      */
     public TabModalLifetimeHandler(ChromeActivity activity, ModalDialogManager manager,
-            ComposedBrowserControlsVisibilityDelegate appVisibilityDelegate,
+            Supplier<ComposedBrowserControlsVisibilityDelegate> appVisibilityDelegate,
             Supplier<TabObscuringHandler> tabObscuringHandler) {
         mActivity = activity;
         mManager = manager;
@@ -99,7 +99,7 @@ public class TabModalLifetimeHandler implements NativeInitObserver, Destroyable
     @Override
     public void onFinishNativeInitialization() {
         mPresenter = new ChromeTabModalPresenter(mActivity, mTabObscuringHandlerSupplier);
-        mAppVisibilityDelegate.addDelegate(mPresenter.getBrowserControlsVisibilityDelegate());
+        mAppVisibilityDelegate.get().addDelegate(mPresenter.getBrowserControlsVisibilityDelegate());
         mManager.registerPresenter(mPresenter, ModalDialogType.TAB);
 
         handleTabChanged(mActivity.getActivityTab());

chrome/android/java/src/org/chromium/chrome/browser/tabbed_mode/TabbedRootUiCoordinator.java

@@ -24,6 +24,7 @@ import org.chromium.chrome.browser.contextualsearch.ContextualSearchManager;
 import org.chromium.chrome.browser.datareduction.DataReductionPromoScreen;
 import org.chromium.chrome.browser.firstrun.FirstRunStatus;
 import org.chromium.chrome.browser.flags.ChromeFeatureList;
+import org.chromium.chrome.browser.fullscreen.BrowserControlsManager;
 import org.chromium.chrome.browser.gesturenav.HistoryNavigationCoordinator;
 import org.chromium.chrome.browser.gesturenav.NavigationSheet;
 import org.chromium.chrome.browser.gesturenav.TabbedSheetDelegate;
@@ -50,6 +51,7 @@ import org.chromium.chrome.browser.ui.default_browser_promo.DefaultBrowserPromoU
 import org.chromium.chrome.browser.ui.tablet.emptybackground.EmptyBackgroundViewWrapper;
 import org.chromium.chrome.browser.vr.VrModuleProvider;
 import org.chromium.components.browser_ui.bottomsheet.EmptyBottomSheetObserver;
+import org.chromium.components.browser_ui.util.ComposedBrowserControlsVisibilityDelegate;
 import org.chromium.ui.base.DeviceFormFactor;
 
 /**
@@ -71,6 +73,7 @@ public class TabbedRootUiCoordinator extends RootUiCoordinator implements Native
     private Callback<Boolean> mIntentWithEffectObserver;
     private HistoryNavigationCoordinator mHistoryNavigationCoordinator;
     private NavigationSheet mNavigationSheet;
+    private ComposedBrowserControlsVisibilityDelegate mAppBrowserControlsVisibilityDelegate;
 
     /**
      * Construct a new TabbedRootUiCoordinator.
@@ -324,6 +327,24 @@ public class TabbedRootUiCoordinator extends RootUiCoordinator implements Native
         mToolbarManager.getFakeboxDelegate().addUrlFocusChangeListener(mUrlFocusChangeListener);
     }
 
+    @Override
+    protected BrowserControlsManager createBrowserControlsManager() {
+        BrowserControlsManager manager = super.createBrowserControlsManager();
+        getAppBrowserControlsVisibilityDelegate().addDelegate(
+                manager.getBrowserVisibilityDelegate());
+        return manager;
+    }
+
+    /**
+     * @return {@link ComposedBrowserControlsVisibilityDelegate} object for tabbed activity.
+     */
+    public ComposedBrowserControlsVisibilityDelegate getAppBrowserControlsVisibilityDelegate() {
+        if (mAppBrowserControlsVisibilityDelegate == null) {
+            mAppBrowserControlsVisibilityDelegate = new ComposedBrowserControlsVisibilityDelegate();
+        }
+        return mAppBrowserControlsVisibilityDelegate;
+    }
+
     @VisibleForTesting
     public StatusIndicatorCoordinator getStatusIndicatorCoordinatorForTesting() {
         return mStatusIndicatorCoordinator;

chrome/android/java/src/org/chromium/chrome/browser/ui/RootUiCoordinator.java

@@ -10,6 +10,7 @@ import android.text.TextUtils;
 import android.view.View;
 import android.view.ViewGroup;
 
+import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
 import androidx.annotation.VisibleForTesting;
 
@@ -39,6 +40,7 @@ import org.chromium.chrome.browser.findinpage.FindToolbar;
 import org.chromium.chrome.browser.findinpage.FindToolbarManager;
 import org.chromium.chrome.browser.findinpage.FindToolbarObserver;
 import org.chromium.chrome.browser.flags.ActivityType;
+import org.chromium.chrome.browser.fullscreen.BrowserControlsManager;
 import org.chromium.chrome.browser.identity_disc.IdentityDiscController;
 import org.chromium.chrome.browser.lifecycle.Destroyable;
 import org.chromium.chrome.browser.lifecycle.InflationObserver;
@@ -145,6 +147,8 @@ public class RootUiCoordinator
     private BottomSheetObserver mContextualSearchSuppressor;
     private final Supplier<ContextualSearchManager> mContextualSearchManagerSupplier;
     protected final CallbackController mCallbackController;
+    @Nullable
+    private BrowserControlsManager mBrowserControlsManager;
 
     /**
      * Create a new {@link RootUiCoordinator} for the given activity.
@@ -254,6 +258,11 @@ public class RootUiCoordinator
             mBottomSheetController.destroy();
         }
 
+        if (mBrowserControlsManager != null) {
+            mBrowserControlsManager.destroy();
+            mBrowserControlsManager = null;
+        }
+
         if (mButtonDataProviders != null) {
             for (ButtonDataProvider provider : mButtonDataProviders) {
                 provider.destroy();
@@ -661,6 +670,33 @@ public class RootUiCoordinator
         return mBottomSheetController;
     }
 
+    /**
+     * Gets the browser controls manager, creates it unless already created.
+     */
+    @NonNull
+    public BrowserControlsManager getBrowserControlsManager() {
+        if (mBrowserControlsManager == null) {
+            // When finish()ing, getBrowserControlsManager() is required to perform cleanup logic.
+            // It should never be called when it results in creating a new manager though.
+            if (mActivity.isActivityFinishingOrDestroyed()) {
+                throw new IllegalStateException();
+            }
+            mBrowserControlsManager = createBrowserControlsManager();
+            assert mBrowserControlsManager != null;
+        }
+        return mBrowserControlsManager;
+    }
+
+    /**
+     * Create a browser controls manager to be used by ChromeActivity.
+     * Note: This may be called before native code is initialized.
+     * @return A {@link BrowserControlsManager} instance that's been created.
+     */
+    @NonNull
+    protected BrowserControlsManager createBrowserControlsManager() {
+        return new BrowserControlsManager(mActivity, BrowserControlsManager.ControlsPosition.TOP);
+    }
+
     /** @return The {@link ScrimCoordinator} to control activity's primary scrim. */
     // TODO(crbug.com/1064140): This method is used to pass ScrimCoordinator to StartSurface. We
     // should be able to create StartSurface in this class so that we don't need to expose this