[ToolbarMVC] Set a profileSupplier on LocationBar

Rather than setting the profile each each time directly, we inject a supplier that can be observed. This reduces the complexity of the interface and will let us constructor inject the supplier more easily in the future.

Change-Id: Ibd587e1a5496e5bca3d27eb7120aacbedb2ff138
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2419635
Commit-Queue: Patrick Noland <pnoland@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Reviewed-by: Brandon Wylie <wylieb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#810011}

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

@@ -11,6 +11,7 @@ import android.view.Window;
 
 import androidx.annotation.IntDef;
 
+import org.chromium.base.supplier.ObservableSupplier;
 import org.chromium.base.supplier.Supplier;
 import org.chromium.chrome.browser.ActivityTabProvider;
 import org.chromium.chrome.browser.WindowDelegate;
@@ -83,17 +84,6 @@ public interface LocationBar extends UrlBarDelegate, FakeboxDelegate {
      */
     void onTabLoadingNTP(NewTabPage ntp);
 
-    /**
-     * Called to set the autocomplete profile to a new profile.
-     */
-    void setAutocompleteProfile(Profile profile);
-
-    /**
-     * Specify whether location bar should present icons when focused.
-     * @param showIcon True if we should show the icons when the url is focused.
-     */
-    void setShowIconsWhenUrlFocused(boolean showIcon);
-
     /**
      * Call to force the UI to update the state of various buttons based on whether or not the
      * current tab is incognito.
@@ -225,4 +215,11 @@ public interface LocationBar extends UrlBarDelegate, FakeboxDelegate {
      */
     void updateSearchEngineStatusIcon(boolean shouldShowSearchEngineLogo,
             boolean isSearchEngineGoogle, String searchEngineUrl);
+
+    /**
+     * 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/LocationBarLayout.java

@@ -30,10 +30,13 @@ import androidx.core.view.MarginLayoutParamsCompat;
 import androidx.core.view.ViewCompat;
 
 import org.chromium.base.ApiCompatibilityUtils;
+import org.chromium.base.Callback;
+import org.chromium.base.CallbackController;
 import org.chromium.base.CommandLine;
 import org.chromium.base.ObserverList;
 import org.chromium.base.metrics.RecordHistogram;
 import org.chromium.base.metrics.RecordUserAction;
+import org.chromium.base.supplier.ObservableSupplier;
 import org.chromium.base.supplier.Supplier;
 import org.chromium.chrome.R;
 import org.chromium.chrome.browser.ActivityTabProvider;
@@ -136,6 +139,9 @@ public class LocationBarLayout extends FrameLayout
     private Runnable mKeyboardResizeModeTask;
     private Runnable mKeyboardHideTask;
     private boolean mKeyboardShouldShow;
+    private ObservableSupplier<Profile> mProfileSupplier;
+    private Callback<Profile> mProfileSupplierObserver;
+    private CallbackController mCallbackController = new CallbackController();
 
     /**
      * Class to handle input from a hardware keyboard when the focus is on the URL bar. In
@@ -242,6 +248,17 @@ public class LocationBarLayout extends FrameLayout
             mAssistantVoiceSearchService.destroy();
             mAssistantVoiceSearchService = null;
         }
+
+        if (mCallbackController != null) {
+            mCallbackController.destroy();
+            mCallbackController = null;
+        }
+
+        if (mProfileSupplier != null) {
+            mProfileSupplier.removeObserver(mProfileSupplierObserver);
+            mProfileSupplier = null;
+            mProfileSupplierObserver = null;
+        }
     }
 
     @Override
@@ -362,6 +379,7 @@ public class LocationBarLayout extends FrameLayout
                         TemplateUrlServiceFactory.get(), GSAState.getInstance(getContext()), this);
         mVoiceRecognitionHandler.setAssistantVoiceSearchService(mAssistantVoiceSearchService);
         onAssistantVoiceSearchServiceChanged();
+        setProfile(mProfileSupplier.get());
     }
 
     /**
@@ -382,22 +400,28 @@ public class LocationBarLayout extends FrameLayout
         mStatusCoordinator.setShouldAnimateIconChanges(shouldAnimate);
     }
 
+    @Override
+    public void setProfileSupplier(ObservableSupplier<Profile> profileSupplier) {
+        assert profileSupplier != null;
+        assert mProfileSupplier == null;
+        mProfileSupplier = profileSupplier;
+        mProfileSupplierObserver = mCallbackController.makeCancelable(this::setProfile);
+        mProfileSupplier.addObserver(mProfileSupplierObserver);
+    }
+
     /**
-     * Updates the profile used for generating autocomplete suggestions.
+     * Updates the profile used by this LocationBar, for, e.g. determining incognito status or
+     * generating autocomplete suggestions..
      * @param profile The profile to be used.
      */
-    @Override
-    public void setAutocompleteProfile(Profile profile) {
-        // This will only be called once at least one tab exists, and the tab model is told to
-        // update its state. During Chrome initialization the tab model update happens after the
-        // call to onNativeLibraryReady, so this assert will not fire.
-        assert mNativeInitialized : "Setting Autocomplete Profile before native side initialized";
+    private void setProfile(Profile profile) {
+        if (profile == null || !mNativeInitialized) return;
         mAutocompleteCoordinator.setAutocompleteProfile(profile);
         mOmniboxPrerender.initializeForProfile(profile);
-    }
 
-    @Override
-    public void setShowIconsWhenUrlFocused(boolean showIcon) {}
+        setShowIconsWhenUrlFocused(
+                SearchEngineLogoUtils.shouldShowSearchEngineLogo(profile.isOffTheRecord()));
+    }
 
     /** Focuses the current page. */
     private void focusCurrentTab() {
@@ -441,6 +465,12 @@ public class LocationBarLayout extends FrameLayout
         return mUrlFocusChangeInProgress;
     }
 
+    /**
+     * Specify whether location bar should present icons when focused.
+     * @param showIcon True if we should show the icons when the url is focused.
+     */
+    protected void setShowIconsWhenUrlFocused(boolean showIcon) {}
+
     /**
      * @param inProgress Whether a URL focus change is taking place.
      */

chrome/android/java/src/org/chromium/chrome/browser/searchwidget/SearchActivity.java

@@ -22,6 +22,7 @@ import org.chromium.base.Callback;
 import org.chromium.base.IntentUtils;
 import org.chromium.base.Log;
 import org.chromium.base.metrics.RecordUserAction;
+import org.chromium.base.supplier.ObservableSupplierImpl;
 import org.chromium.chrome.R;
 import org.chromium.chrome.browser.IntentHandler;
 import org.chromium.chrome.browser.WebContentsFactory;
@@ -32,6 +33,7 @@ import org.chromium.chrome.browser.document.ChromeLauncherActivity;
 import org.chromium.chrome.browser.init.AsyncInitializationActivity;
 import org.chromium.chrome.browser.init.SingleWindowKeyboardVisibilityDelegate;
 import org.chromium.chrome.browser.locale.LocaleManager;
+import org.chromium.chrome.browser.profiles.Profile;
 import org.chromium.chrome.browser.tab.Tab;
 import org.chromium.chrome.browser.tab.TabBuilder;
 import org.chromium.chrome.browser.tab.TabDelegateFactory;
@@ -112,6 +114,7 @@ public class SearchActivity extends AsyncInitializationActivity
     private SnackbarManager mSnackbarManager;
     private SearchBoxDataProvider mSearchBoxDataProvider;
     private Tab mTab;
+    private ObservableSupplierImpl<Profile> mProfileSupplier = new ObservableSupplierImpl<>();
 
     @Override
     protected boolean isStartedUpCorrectly(Intent intent) {
@@ -164,6 +167,7 @@ public class SearchActivity extends AsyncInitializationActivity
         mSearchBox.setToolbarDataProvider(mSearchBoxDataProvider);
         mSearchBox.initializeControls(
                 new WindowDelegate(getWindow()), getWindowAndroid(), null, null, null, null);
+        mSearchBox.setProfileSupplier(mProfileSupplier);
 
         // Kick off everything needed for the user to type into the box.
         beginQuery();
@@ -237,16 +241,19 @@ public class SearchActivity extends AsyncInitializationActivity
                 return null;
             }
         };
+
+        WebContents webContents = WebContentsFactory.createWebContents(false, false);
         mTab = new TabBuilder()
                        .setWindow(getWindowAndroid())
                        .setLaunchType(TabLaunchType.FROM_EXTERNAL_APP)
-                       .setWebContents(WebContentsFactory.createWebContents(false, false))
+                       .setWebContents(webContents)
                        .setDelegateFactory(factory)
                        .build();
         mTab.loadUrl(new LoadUrlParams(ContentUrlConstants.ABOUT_BLANK_DISPLAY_URL));
 
         mSearchBoxDataProvider.onNativeLibraryReady(mTab);
         mSearchBox.onNativeLibraryReady();
+        mProfileSupplier.set(Profile.fromWebContents(webContents));
 
         // Force the user to choose a search engine if they have to.
         final Callback<Boolean> onSearchEngineFinalizedCallback = new Callback<Boolean>() {

chrome/android/java/src/org/chromium/chrome/browser/searchwidget/SearchActivityLocationBarLayout.java

@@ -9,10 +9,8 @@ import android.os.Handler;
 import android.text.TextUtils;
 import android.util.AttributeSet;
 import android.view.View;
-
 import androidx.annotation.Nullable;
 import androidx.annotation.VisibleForTesting;
-
 import org.chromium.chrome.R;
 import org.chromium.chrome.browser.locale.LocaleManager;
 import org.chromium.chrome.browser.omnibox.LocationBarLayout;
@@ -20,7 +18,6 @@ import org.chromium.chrome.browser.omnibox.UrlBar;
 import org.chromium.chrome.browser.omnibox.UrlBarCoordinator.SelectionState;
 import org.chromium.chrome.browser.omnibox.UrlBarData;
 import org.chromium.chrome.browser.omnibox.voice.VoiceRecognitionHandler;
-import org.chromium.chrome.browser.profiles.Profile;
 import org.chromium.chrome.browser.toolbar.top.ToolbarPhone;
 
 /** Implementation of the {@link LocationBarLayout} that is displayed for widget searches. */
@@ -79,8 +76,6 @@ public class SearchActivityLocationBarLayout extends LocationBarLayout {
         super.onNativeLibraryReady();
         mNativeLibraryReady = true;
 
-        setAutocompleteProfile(Profile.getLastUsedRegularProfile());
-
         mPendingSearchPromoDecision = LocaleManager.getInstance().needToCheckForSearchEnginePromo();
         getAutocompleteCoordinator().setShouldPreventOmniboxAutocomplete(
                 mPendingSearchPromoDecision);

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

@@ -130,9 +130,7 @@ public class ToolbarManager implements UrlFocusChangeListener, ThemeColorObserve
     private final ActivityTabProvider mActivityTabProvider;
     private final LocationBarModel mLocationBarModel;
     private ObservableSupplier<BookmarkBridge> mBookmarkBridgeSupplier;
-    private ObservableSupplier<Profile> mProfileSupplier;
     private final Callback<BookmarkBridge> mBookmarkBridgeSupplierObserver;
-    private final Callback<Profile> mProfileSupplierObserver;
     private TemplateUrlServiceObserver mTemplateUrlObserver;
     private LocationBar mLocationBar;
     private FindToolbarManager mFindToolbarManager;
@@ -250,9 +248,6 @@ public class ToolbarManager implements UrlFocusChangeListener, ThemeColorObserve
         // reference the same object.
         mBookmarkBridgeSupplierObserver = this::setBookmarkBridge;
         mBookmarkBridgeSupplier.addObserver(mBookmarkBridgeSupplierObserver);
-        mProfileSupplier = profileSupplier;
-        mProfileSupplierObserver = this::setCurrentProfile;
-        mProfileSupplier.addObserver(mProfileSupplierObserver);
 
         mOverviewModeBehaviorSupplier = overviewModeBehaviorSupplier;
         mOverviewModeBehaviorSupplier.onAvailable(
@@ -286,7 +281,7 @@ public class ToolbarManager implements UrlFocusChangeListener, ThemeColorObserve
         mToolbarTabController = new ToolbarTabControllerImpl(mLocationBarModel::getTab,
                 // clang-format off
                 () -> mShowStartSurfaceSupplier != null && mShowStartSurfaceSupplier.get(),
-                mProfileSupplier, () -> mBottomControlsCoordinator, this::updateButtonStatus);
+                profileSupplier, () -> mBottomControlsCoordinator, this::updateButtonStatus);
         // clang-format on
 
         BrowserStateBrowserControlsVisibilityDelegate controlsVisibilityDelegate =
@@ -327,6 +322,7 @@ public class ToolbarManager implements UrlFocusChangeListener, ThemeColorObserve
         mToolbar.setPaintInvalidator(invalidator);
         mActionModeController.setTabStripHeight(mToolbar.getTabStripHeight());
         mLocationBar = mToolbar.getLocationBar();
+        mLocationBar.setProfileSupplier(profileSupplier);
         mLocationBar.setToolbarDataProvider(mLocationBarModel);
         mLocationBar.addUrlFocusChangeListener(this);
         mLocationBar.setDefaultTextEditActionModeCallback(
@@ -760,7 +756,6 @@ public class ToolbarManager implements UrlFocusChangeListener, ThemeColorObserve
             mControlContainer.setReadyForBitmapCapture(true);
         }
 
-        setCurrentProfile(mProfileSupplier.get());
         TraceEvent.end("ToolbarManager.initializeWithNative");
     }
 
@@ -895,11 +890,6 @@ public class ToolbarManager implements UrlFocusChangeListener, ThemeColorObserve
             mFindToolbarManager = null;
         }
 
-        if (mProfileSupplier != null) {
-            mProfileSupplier.removeObserver(mProfileSupplierObserver);
-            mProfileSupplier = null;
-        }
-
         if (mMenuButtonCoordinator != null) {
             mMenuButtonCoordinator.destroy();
             mMenuButtonCoordinator = null;
@@ -1236,17 +1226,6 @@ public class ToolbarManager implements UrlFocusChangeListener, ThemeColorObserve
         updateButtonStatus();
     }
 
-    // TODO(https://crbug.com/865801): Inject a profile supplier directly to mLocationBar and remove
-    // setCurrentProfile.
-    private void setCurrentProfile(Profile profile) {
-        if (profile != null && mInitializedWithNative) {
-            mLocationBar.setAutocompleteProfile(profile);
-            mLocationBar.setShowIconsWhenUrlFocused(
-                    SearchEngineLogoUtils.shouldShowSearchEngineLogo(
-                            mLocationBarModel.isIncognito()));
-        }
-    }
-
     private void setBookmarkBridge(BookmarkBridge bookmarkBridge) {
         if (bookmarkBridge == null) return;
         bookmarkBridge.addObserver(mBookmarksObserver);

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

@@ -41,6 +41,7 @@ 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;
@@ -817,10 +818,7 @@ public class CustomTabToolbar extends ToolbarLayout implements View.OnLongClickL
         public void onTabLoadingNTP(NewTabPage ntp) {}
 
         @Override
-        public void setAutocompleteProfile(Profile profile) {}
-
-        @Override
-        public void setShowIconsWhenUrlFocused(boolean showIcon) {}
+        public void setProfileSupplier(ObservableSupplier<Profile> profileSupplier) {}
 
         @Override
         public void setUnfocusedWidth(int unfocusedWidth) {}