[ToolbarMVC] Instantiate LocationBarCoordinator in ToolbarManager

This lets us easily wire necessary dependencies to its constructor,
which then lets us remove several LocationBar interface methods.
ToolbarPhone/ToolbarTablet now have their coordinator set rather than
instantiating it themselves.

Change-Id: I8983b9c898c5676c1bdc7034de9cde2bb5de351a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2472467
Commit-Queue: Patrick Noland <pnoland@chromium.org>
Reviewed-by: who/bttk <bttk@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Reviewed-by: Peter Conn <peconn@chromium.org>
Reviewed-by: Brandon Wylie <wylieb@chromium.org>
Reviewed-by: Filip Gorski <fgorski@chromium.org>
Cr-Commit-Position: refs/heads/master@{#819016}

chrome/android/java/src/org/chromium/chrome/browser/customtabs/features/toolbar/CustomTabToolbar.java

@@ -42,12 +42,8 @@ import org.chromium.base.ApiCompatibilityUtils;
 import org.chromium.base.ThreadUtils;
 import org.chromium.base.library_loader.LibraryLoader;
 import org.chromium.base.metrics.RecordUserAction;
-import org.chromium.base.supplier.ObservableSupplier;
-import org.chromium.base.supplier.Supplier;
 import org.chromium.base.task.PostTask;
 import org.chromium.chrome.R;
-import org.chromium.chrome.browser.ActivityTabProvider;
-import org.chromium.chrome.browser.WindowDelegate;
 import org.chromium.chrome.browser.app.ChromeActivity;
 import org.chromium.chrome.browser.native_page.NativePageFactory;
 import org.chromium.chrome.browser.offlinepages.OfflinePageUtils;
@@ -60,11 +56,8 @@ import org.chromium.chrome.browser.omnibox.UrlBarData;
 import org.chromium.chrome.browser.omnibox.voice.VoiceRecognitionHandler;
 import org.chromium.chrome.browser.page_info.ChromePageInfoControllerDelegate;
 import org.chromium.chrome.browser.page_info.ChromePermissionParamsListBuilderDelegate;
-import org.chromium.chrome.browser.profiles.Profile;
-import org.chromium.chrome.browser.share.ShareDelegate;
 import org.chromium.chrome.browser.tab.Tab;
 import org.chromium.chrome.browser.tab.TrustedCdn;
-import org.chromium.chrome.browser.tabmodel.IncognitoStateProvider;
 import org.chromium.chrome.browser.toolbar.ToolbarColors;
 import org.chromium.chrome.browser.toolbar.ToolbarDataProvider;
 import org.chromium.chrome.browser.toolbar.ToolbarTabController;
@@ -83,9 +76,7 @@ import org.chromium.content_public.common.ContentUrlConstants;
 import org.chromium.net.GURLUtils;
 import org.chromium.ui.base.Clipboard;
 import org.chromium.ui.base.DeviceFormFactor;
-import org.chromium.ui.base.WindowAndroid;
 import org.chromium.ui.interpolators.BakedBezierInterpolator;
-import org.chromium.ui.modaldialog.ModalDialogManager;
 import org.chromium.ui.text.SpanApplier;
 import org.chromium.ui.text.SpanApplier.SpanInfo;
 import org.chromium.ui.util.ColorUtils;
@@ -170,7 +161,7 @@ public class CustomTabToolbar extends ToolbarLayout implements View.OnLongClickL
     private String mFirstUrl;
 
     protected ToolbarDataProvider mToolbarDataProvider;
-    private LocationBar mLocationBar;
+    private CustomTabLocationBar mLocationBar;
 
     private Runnable mTitleAnimationStarter = new Runnable() {
         @Override
@@ -201,7 +192,7 @@ public class CustomTabToolbar extends ToolbarLayout implements View.OnLongClickL
         mLiteStatusView = findViewById(R.id.url_bar_lite_status);
         mLiteStatusSeparatorView = findViewById(R.id.url_bar_lite_status_separator);
         mUrlCoordinator = new UrlBarCoordinator((UrlBar) mUrlBar);
-        mLocationBar = new LocationBarImpl();
+        mLocationBar = new CustomTabLocationBar();
         mUrlCoordinator.setDelegate(mLocationBar);
         mUrlCoordinator.setAllowFocus(false);
         mTitleBar = findViewById(R.id.title_bar);
@@ -623,7 +614,10 @@ public class CustomTabToolbar extends ToolbarLayout implements View.OnLongClickL
         mCustomActionButtons.setLayoutParams(p);
     }
 
-    private class LocationBarImpl implements LocationBar {
+    /**
+     * Custom tab-specific implementation of the LocationBar interface.
+     */
+    public class CustomTabLocationBar implements LocationBar {
         @Override
         public void onNativeLibraryReady() {
             mSecurityButton.setOnClickListener(v -> {
@@ -763,12 +757,12 @@ public class CustomTabToolbar extends ToolbarLayout implements View.OnLongClickL
             updateStatusIcon();
         }
 
-        @Override
+        /** Sets the {@link ToolbarDataProvider} to be used for accessing {@link Toolbar} state. */
         public void setToolbarDataProvider(ToolbarDataProvider model) {
             mToolbarDataProvider = model;
         }
 
-        @Override
+        /** Gets the {@link ToolbarDataProvider} to be used for accessing {@link Toolbar} state. */
         public ToolbarDataProvider getToolbarDataProvider() {
             return mToolbarDataProvider;
         }
@@ -799,13 +793,6 @@ public class CustomTabToolbar extends ToolbarLayout implements View.OnLongClickL
             }
         }
 
-        @Override
-        public void initializeControls(WindowDelegate windowDelegate, WindowAndroid windowAndroid,
-                ActivityTabProvider activityTabProvider,
-                Supplier<ModalDialogManager> modalDialogManager,
-                Supplier<ShareDelegate> shareDelegateSupplier,
-                IncognitoStateProvider incognitoStateProvider) {}
-
         @Override
         public void updateStatusIcon() {
             if (mState == STATE_TITLE_ONLY) return;
@@ -838,7 +825,6 @@ public class CustomTabToolbar extends ToolbarLayout implements View.OnLongClickL
             return mSecurityButton;
         }
 
-        @Override
         public void setDefaultTextEditActionModeCallback(ToolbarActionModeCallback callback) {
             mUrlCoordinator.setActionModeCallback(callback);
         }
@@ -868,9 +854,6 @@ public class CustomTabToolbar extends ToolbarLayout implements View.OnLongClickL
         @Override
         public void updateMicButtonState() {}
 
-        @Override
-        public void setProfileSupplier(ObservableSupplier<Profile> profileSupplier) {}
-
         // Implements FakeBoxDelegate.
         @Override
         public boolean isUrlBarFocused() {

chrome/android/java/src/org/chromium/chrome/browser/omnibox/LocationBar.java

@@ -4,28 +4,14 @@
 
 package org.chromium.chrome.browser.omnibox;
 
-import android.app.Activity;
 import android.view.View;
 import android.view.ViewGroup;
-import android.view.Window;
 
-import org.chromium.base.supplier.ObservableSupplier;
-import org.chromium.base.supplier.Supplier;
-import org.chromium.chrome.browser.ActivityTabProvider;
-import org.chromium.chrome.browser.WindowDelegate;
 import org.chromium.chrome.browser.lifecycle.Destroyable;
 import org.chromium.chrome.browser.ntp.FakeboxDelegate;
 import org.chromium.chrome.browser.ntp.NewTabPage;
 import org.chromium.chrome.browser.omnibox.UrlBar.UrlBarDelegate;
-import org.chromium.chrome.browser.profiles.Profile;
-import org.chromium.chrome.browser.share.ShareDelegate;
 import org.chromium.chrome.browser.tab.Tab;
-import org.chromium.chrome.browser.tabmodel.IncognitoStateProvider;
-import org.chromium.chrome.browser.toolbar.ToolbarDataProvider;
-import org.chromium.chrome.browser.toolbar.top.Toolbar;
-import org.chromium.chrome.browser.toolbar.top.ToolbarActionModeCallback;
-import org.chromium.ui.base.WindowAndroid;
-import org.chromium.ui.modaldialog.ModalDialogManager;
 
 /**
  * Container that holds the {@link UrlBar} and SSL state related with the current {@link Tab}.
@@ -73,30 +59,6 @@ public interface LocationBar extends UrlBarDelegate, FakeboxDelegate, Destroyabl
      */
     void updateLoadingState(boolean updateUrl);
 
-    /** Sets the {@link ToolbarDataProvider} to be used for accessing {@link Toolbar} state. */
-    void setToolbarDataProvider(ToolbarDataProvider model);
-
-    /** Gets the {@link ToolbarDataProvider} to be used for accessing {@link Toolbar} state. */
-    ToolbarDataProvider getToolbarDataProvider();
-
-    /**
-     * Initialize controls that will act as hooks to various functions.
-     *
-     * @param windowDelegate {@link WindowDelegate} that will provide {@link Window} related info.
-     * @param windowAndroid {@link WindowAndroid} that is used by the owning {@link Activity}.
-     * @param activityTabProvider An {@link ActivityTabProvider} to access the activity's current
-     *         tab.
-     * @param modalDialogManagerSupplier A supplier for {@link ModalDialogManager} object.
-     * @param shareDelegateSupplier A supplier for {@link ShareDelegate} object.
-     * @param incognitoStateProvider An {@link IncognitoStateProvider} to access the current
-     *         incognito state.
-     */
-    void initializeControls(WindowDelegate windowDelegate, WindowAndroid windowAndroid,
-            ActivityTabProvider activityTabProvider,
-            Supplier<ModalDialogManager> modalDialogManagerSupplier,
-            Supplier<ShareDelegate> shareDelegateSupplier,
-            IncognitoStateProvider incognitoStateProvider);
-
     /**
      * Triggers the cursor to be visible in the UrlBar without triggering any of the focus animation
      * logic.
@@ -131,14 +93,4 @@ public interface LocationBar extends UrlBarDelegate, FakeboxDelegate, Destroyabl
     /** Updates the state of the mic button if there is one. */
     void updateMicButtonState();
 
-    /** Sets the callback to be used by default for text editing action bar. */
-    void setDefaultTextEditActionModeCallback(ToolbarActionModeCallback callback);
-
-    /**
-     * Sets the (observable) supplier of the active profile. This supplier will notify observers of
-     * changes to the active profile, e.g. when selecting an incognito tab model.
-     *
-     * @param profileSupplier The supplier of the active profile.
-     */
-    void setProfileSupplier(ObservableSupplier<Profile> profileSupplier);
 }

chrome/android/java/src/org/chromium/chrome/browser/omnibox/LocationBarCoordinator.java

@@ -55,10 +55,29 @@ public final class LocationBarCoordinator implements LocationBar {
      *
      * @param locationBarLayout Inflated {@link LocationBarPhone} or {@link LocationBarTablet}.
      *         {@code LocationBarCoordinator} takes ownership and will destroy this object.
+     * @param profileObservableSupplier The supplier of the active profile.
+     * @param toolbarDataProvider {@link ToolbarDataProvider} to be used for accessing Toolbar
+     *         state.
+     * @param actionModeCallback The default callback for text editing action bar to use.
+     * @param windowDelegate {@link WindowDelegate} that will provide {@link Window} related info.
+     * @param windowAndroid {@link WindowAndroid} that is used by the owning {@link Activity}.
+     * @param activityTabProvider An {@link ActivityTabProvider} to access the activity's current
+     *         tab.
+     * @param modalDialogManagerSupplier A supplier for {@link ModalDialogManager} object.
+     * @param shareDelegateSupplier A supplier for {@link ShareDelegate} object.
+     * @param incognitoStateProvider An {@link IncognitoStateProvider} to access the current
+     *         incognito state.
      * @throws IllegalArgumentException if the view is neither {@link LocationBarPhone} nor {@link
      *         LocationBarTablet}.
      */
-    public LocationBarCoordinator(View locationBarLayout) {
+    public LocationBarCoordinator(View locationBarLayout,
+            ObservableSupplier<Profile> profileObservableSupplier,
+            ToolbarDataProvider toolbarDataProvider, ToolbarActionModeCallback actionModeCallback,
+            WindowDelegate windowDelegate, WindowAndroid windowAndroid,
+            ActivityTabProvider activityTabProvider,
+            Supplier<ModalDialogManager> modalDialogManagerSupplier,
+            Supplier<ShareDelegate> shareDelegateSupplier,
+            IncognitoStateProvider incognitoStateProvider) {
         mLocationBarLayout = (LocationBarLayout) locationBarLayout;
 
         if (locationBarLayout instanceof LocationBarPhone) {
@@ -71,6 +90,12 @@ public final class LocationBarCoordinator implements LocationBar {
                            + locationBarLayout.getClass();
             throw new IllegalArgumentException(locationBarLayout.getClass().toString());
         }
+
+        mLocationBarLayout.setToolbarDataProvider(toolbarDataProvider);
+        mLocationBarLayout.setProfileSupplier(profileObservableSupplier);
+        mLocationBarLayout.setDefaultTextEditActionModeCallback(actionModeCallback);
+        mLocationBarLayout.initializeControls(windowDelegate, windowAndroid, activityTabProvider,
+                modalDialogManagerSupplier, shareDelegateSupplier, incognitoStateProvider);
     }
 
     @Override
@@ -125,26 +150,10 @@ public final class LocationBarCoordinator implements LocationBar {
         mLocationBarLayout.updateLoadingState(updateUrl);
     }
 
-    @Override
-    public void setToolbarDataProvider(ToolbarDataProvider dataProvider) {
-        mLocationBarLayout.setToolbarDataProvider(dataProvider);
-    }
-
-    @Override
     public ToolbarDataProvider getToolbarDataProvider() {
         return mLocationBarLayout.getToolbarDataProvider();
     }
 
-    @Override
-    public void initializeControls(WindowDelegate windowDelegate, WindowAndroid windowAndroid,
-            ActivityTabProvider activityTabProvider,
-            Supplier<ModalDialogManager> modalDialogManagerSupplier,
-            Supplier<ShareDelegate> shareDelegateSupplier,
-            IncognitoStateProvider incognitoStateProvider) {
-        mLocationBarLayout.initializeControls(windowDelegate, windowAndroid, activityTabProvider,
-                modalDialogManagerSupplier, shareDelegateSupplier, incognitoStateProvider);
-    }
-
     @Override
     public void showUrlBarCursorWithoutFocusAnimations() {
         mLocationBarLayout.showUrlBarCursorWithoutFocusAnimations();
@@ -180,16 +189,6 @@ public final class LocationBarCoordinator implements LocationBar {
         mLocationBarLayout.updateMicButtonState();
     }
 
-    @Override
-    public void setDefaultTextEditActionModeCallback(ToolbarActionModeCallback callback) {
-        mLocationBarLayout.setDefaultTextEditActionModeCallback(callback);
-    }
-
-    @Override
-    public void setProfileSupplier(ObservableSupplier<Profile> profileSupplier) {
-        mLocationBarLayout.setProfileSupplier(profileSupplier);
-    }
-
     @Nullable
     @Override
     public View getViewForUrlBackFocus() {

chrome/android/java/src/org/chromium/chrome/browser/toolbar/ToolbarManager.java

@@ -43,6 +43,8 @@ import org.chromium.chrome.browser.compositor.layouts.LayoutManager;
 import org.chromium.chrome.browser.compositor.layouts.OverviewModeBehavior;
 import org.chromium.chrome.browser.compositor.layouts.OverviewModeBehavior.OverviewModeObserver;
 import org.chromium.chrome.browser.compositor.layouts.SceneChangeObserver;
+import org.chromium.chrome.browser.customtabs.features.toolbar.CustomTabToolbar;
+import org.chromium.chrome.browser.customtabs.features.toolbar.CustomTabToolbar.CustomTabLocationBar;
 import org.chromium.chrome.browser.feature_engagement.TrackerFactory;
 import org.chromium.chrome.browser.findinpage.FindToolbarManager;
 import org.chromium.chrome.browser.findinpage.FindToolbarObserver;
@@ -55,6 +57,7 @@ import org.chromium.chrome.browser.ntp.IncognitoNewTabPage;
 import org.chromium.chrome.browser.ntp.NewTabPage;
 import org.chromium.chrome.browser.omnibox.LocationBar;
 import org.chromium.chrome.browser.omnibox.OmniboxFocusReason;
+import org.chromium.chrome.browser.omnibox.LocationBarCoordinator;
 import org.chromium.chrome.browser.omnibox.UrlFocusChangeListener;
 import org.chromium.chrome.browser.previews.Previews;
 import org.chromium.chrome.browser.previews.PreviewsAndroidBridge;
@@ -334,17 +337,29 @@ public class ToolbarManager implements UrlFocusChangeListener, ThemeColorObserve
                 new ActionModeController(mActivity, mActionBarDelegate, toolbarActionModeCallback);
 
         mActionModeController.setTabStripHeight(mToolbar.getTabStripHeight());
-        mLocationBar = mToolbar.getLocationBar();
-        mLocationBar.setProfileSupplier(profileSupplier);
-        mLocationBar.setToolbarDataProvider(mLocationBarModel);
+
+        if (toolbarLayout instanceof CustomTabToolbar) {
+            CustomTabLocationBar customTabLocationBar =
+                    (CustomTabLocationBar) mToolbar.getLocationBar();
+            // TODO(https://crbug.com/1140188): Use a factory to wire these dependencies.
+            customTabLocationBar.setToolbarDataProvider(mLocationBarModel);
+            customTabLocationBar.setDefaultTextEditActionModeCallback(
+                    mActionModeController.getActionModeCallback());
+            mLocationBar = customTabLocationBar;
+        } else {
+            LocationBarCoordinator locationBarCoordinator = new LocationBarCoordinator(
+                    mActivity.findViewById(R.id.location_bar), profileSupplier, mLocationBarModel,
+                    mActionModeController.getActionModeCallback(),
+                    new WindowDelegate(mActivity.getWindow()), mActivity.getWindowAndroid(),
+                    mActivityTabProvider, mActivity::getModalDialogManager,
+                    mActivity.getShareDelegateSupplier(), mIncognitoStateProvider);
+            toolbarLayout.setLocationBarCoordinator(locationBarCoordinator);
+            mLocationBar = locationBarCoordinator;
+        }
+
         mLocationBar.addUrlFocusChangeListener(this);
-        mLocationBar.setDefaultTextEditActionModeCallback(
-                mActionModeController.getActionModeCallback());
-        mLocationBar.initializeControls(new WindowDelegate(mActivity.getWindow()),
-                mActivity.getWindowAndroid(), mActivityTabProvider,
-                mActivity::getModalDialogManager, mActivity.getShareDelegateSupplier(),
-                mIncognitoStateProvider);
-        Runnable clickDelegate = () -> setUrlBarFocus(false, OmniboxFocusReason.UNFOCUS);
+        Runnable clickDelegate =
+                () -> setUrlBarFocus(false, OmniboxFocusReason.UNFOCUS);
         View scrimTarget = mActivity.getCompositorViewHolder();
         mLocationBarFocusHandler = new LocationBarFocusScrimHandler(scrimCoordinator,
                 tabObscuringHandler, activity, activity.getNightModeStateProvider(),

chrome/android/java/src/org/chromium/chrome/browser/toolbar/top/ToolbarLayout.java

@@ -33,6 +33,7 @@ import org.chromium.chrome.browser.findinpage.FindToolbar;
 import org.chromium.chrome.browser.ntp.NewTabPage;
 import org.chromium.chrome.browser.omnibox.LocationBar;
 import org.chromium.chrome.browser.omnibox.OmniboxFocusReason;
+import org.chromium.chrome.browser.omnibox.LocationBarCoordinator;
 import org.chromium.chrome.browser.omnibox.UrlBarData;
 import org.chromium.chrome.browser.profiles.Profile;
 import org.chromium.chrome.browser.tab.Tab;
@@ -145,6 +146,10 @@ public abstract class ToolbarLayout
         mAppMenuButtonHelper = appMenuButtonHelper;
     }
 
+    // TODO(pnoland, https://crbug.com/865801): Move this from ToolbarLayout to forthcoming
+    // BrowsingModeToolbarCoordinator.
+    public void setLocationBarCoordinator(LocationBarCoordinator locationBarCoordinator) {}
+
     /**
      * Cleans up any code as necessary.
      */

chrome/android/java/src/org/chromium/chrome/browser/toolbar/top/ToolbarPhone.java

@@ -338,7 +338,6 @@ public class ToolbarPhone extends ToolbarLayout
     public void onFinishInflate() {
         try (TraceEvent te = TraceEvent.scoped("ToolbarPhone.onFinishInflate")) {
             super.onFinishInflate();
-            mLocationBar = new LocationBarCoordinator(findViewById(R.id.location_bar));
             mToolbarButtonsContainer = (ViewGroup) findViewById(R.id.toolbar_buttons);
             mHomeButton = findViewById(R.id.home_button);
             mUrlBar = (TextView) findViewById(R.id.url_bar);
@@ -346,8 +345,6 @@ public class ToolbarPhone extends ToolbarLayout
             mToolbarBackground =
                     new ColorDrawable(getToolbarColorForVisualState(VisualState.NORMAL));
 
-            initLocationBarBackground();
-
             setLayoutTransition(null);
 
             if (getMenuButtonCoordinator() != null) {
@@ -362,6 +359,12 @@ public class ToolbarPhone extends ToolbarLayout
         }
     }
 
+    @Override
+    public void setLocationBarCoordinator(LocationBarCoordinator locationBarCoordinator) {
+        mLocationBar = locationBarCoordinator;
+        initLocationBarBackground();
+    }
+
     @Override
     void destroy() {
         if (mHomeButton != null) {

chrome/android/java/src/org/chromium/chrome/browser/toolbar/top/ToolbarTablet.java

@@ -110,7 +110,6 @@ public class ToolbarTablet extends ToolbarLayout
     @Override
     public void onFinishInflate() {
         super.onFinishInflate();
-        mLocationBar = new LocationBarCoordinator(findViewById(R.id.location_bar));
         mHomeButton = findViewById(R.id.home_button);
         mBackButton = findViewById(R.id.back_button);
         mForwardButton = findViewById(R.id.forward_button);
@@ -147,6 +146,11 @@ public class ToolbarTablet extends ToolbarLayout
         mToolbarButtons = new ImageButton[] {mBackButton, mForwardButton, mReloadButton};
     }
 
+    @Override
+    public void setLocationBarCoordinator(LocationBarCoordinator locationBarCoordinator) {
+        mLocationBar = locationBarCoordinator;
+    }
+
     @Override
     void destroy() {
         if (mHomeButton != null) {