[ToolbarMVC] Add onPrimaryColorChanged to observed location bar data

This change dissolves method LocationBarLayout#updateVisualsForState().
Method LocationBarCoordinator#updateVisualsForState() remains, and is
used in Toolbar.

Bug: 1142883
Change-Id: If8b7b46bd6634adc514d607baab406205852263b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2545192Reviewed-by: Filip Gorski <fgorski@chromium.org>
Reviewed-by: Patrick Noland <pnoland@chromium.org>
Commit-Queue: bttk <bttk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#833362}

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

@@ -61,6 +61,7 @@ import org.chromium.chrome.browser.tab.Tab;
 import org.chromium.chrome.browser.tab.TrustedCdn;
 import org.chromium.chrome.browser.toolbar.LocationBarModel;
 import org.chromium.chrome.browser.toolbar.ToolbarColors;
+import org.chromium.chrome.browser.toolbar.ToolbarProgressBar;
 import org.chromium.chrome.browser.toolbar.top.ToolbarLayout;
 import org.chromium.chrome.browser.toolbar.top.ToolbarPhone;
 import org.chromium.components.browser_ui.styles.ChromeColors;
@@ -249,7 +250,6 @@ public class CustomTabToolbar extends ToolbarLayout implements View.OnLongClickL
         mLocationBarModel = locationBarModel;
         mLocationBar =
                 new CustomTabLocationBar(locationBarModel, actionModeCallback, (UrlBar) mUrlBar);
-        mLocationBar.updateVisualsForState();
         return mLocationBar;
     }
 
@@ -495,6 +495,7 @@ public class CustomTabToolbar extends ToolbarLayout implements View.OnLongClickL
         super.onConfigurationChanged(newConfig);
         mLocationBarModel.notifyTitleChanged();
         mLocationBarModel.notifyUrlChanged();
+        mLocationBarModel.notifyPrimaryColorChanged();
     }
 
     @Override
@@ -540,9 +541,7 @@ public class CustomTabToolbar extends ToolbarLayout implements View.OnLongClickL
                 // Using the current background color instead of the final color in case this
                 // animation was cancelled.  This ensures the assets are updated to the visible
                 // color.
-                int backgroundColor = background.getColor();
-                mUseDarkColors = !ColorUtils.shouldUseLightForegroundOnBackground(backgroundColor);
-                mLocationBar.updateVisualsForState();
+                updateUseDarkColors(background.getColor());
             }
         });
         mBrandColorTransitionAnimation.start();
@@ -550,7 +549,12 @@ public class CustomTabToolbar extends ToolbarLayout implements View.OnLongClickL
         if (!shouldAnimate) mBrandColorTransitionAnimation.end();
     }
 
-
+    private void updateUseDarkColors(int backgroundColor) {
+        boolean useDarkColors = !ColorUtils.shouldUseLightForegroundOnBackground(backgroundColor);
+        if (mUseDarkColors == useDarkColors) return;
+        mUseDarkColors = useDarkColors;
+        mLocationBar.onUseDarkColorsChanged();
+    }
 
     @Override
     protected void onMeasure(int widthMeasureSpec, int heightMeasureSpec) {
@@ -613,8 +617,9 @@ public class CustomTabToolbar extends ToolbarLayout implements View.OnLongClickL
                 ActionMode.Callback actionModeCallback, UrlBar urlBar) {
             mLocationBarDataProvider = locationBarDataProvider;
             mLocationBarDataProvider.addObserver(this);
-            mUrlCoordinator = new UrlBarCoordinator(
-                    urlBar, null, actionModeCallback, /*focusChangeCallback=*/(unused) -> {}, this);
+            mUrlCoordinator = new UrlBarCoordinator(urlBar, /*windowDelegate=*/null,
+                    actionModeCallback, /*focusChangeCallback=*/(unused) -> {}, this);
+            onPrimaryColorChanged();
         }
 
         public void onNativeLibraryReady() {
@@ -752,34 +757,48 @@ public class CustomTabToolbar extends ToolbarLayout implements View.OnLongClickL
         public void onNtpStartedLoading() {}
 
         @Override
-        public void updateLoadingState(boolean updateUrl) {
-            if (updateUrl) onUrlChanged();
-            updateStatusIcon();
+        public void onPrimaryColorChanged() {
+            onUseDarkColorsChanged();
+            updateProgressBarColors();
         }
 
-        @Override
-        public void updateVisualsForState() {
-            Resources resources = getResources();
-            updateStatusIcon();
+        private void onUseDarkColorsChanged() {
             updateButtonsTint();
             if (mUrlCoordinator.setUseDarkTextColors(mUseDarkColors)) {
+                // Update the URL to make it use the new color scheme.
                 onUrlChanged();
             }
 
-            mTitleBar.setTextColor(ApiCompatibilityUtils.getColor(resources,
+            mTitleBar.setTextColor(ApiCompatibilityUtils.getColor(getResources(),
                     mUseDarkColors ? R.color.default_text_color_dark
                                    : R.color.default_text_color_light));
+        }
 
-            if (getProgressBar() != null) {
-                if (!ToolbarColors.isUsingDefaultToolbarColor(
-                            getResources(), false, getBackground().getColor())) {
-                    getProgressBar().setThemeColor(getBackground().getColor(), false);
-                } else {
-                    getProgressBar().setBackgroundColor(ApiCompatibilityUtils.getColor(
-                            resources, R.color.progress_bar_background));
-                    getProgressBar().setForegroundColor(ApiCompatibilityUtils.getColor(
-                            resources, R.color.progress_bar_foreground));
-                }
+        @Override
+        public void updateLoadingState(boolean updateUrl) {
+            if (updateUrl) onUrlChanged();
+            updateStatusIcon();
+        }
+
+        @Override
+        public void updateVisualsForState() {
+            updateStatusIcon();
+            updateProgressBarColors();
+        }
+
+        private void updateProgressBarColors() {
+            final ToolbarProgressBar progressBar = getProgressBar();
+            if (progressBar == null) return;
+            final Resources resources = getResources();
+            final int backgroundColor = getBackground().getColor();
+            if (ToolbarColors.isUsingDefaultToolbarColor(
+                        resources, /*isIncognito=*/false, backgroundColor)) {
+                progressBar.setBackgroundColor(
+                        ApiCompatibilityUtils.getColor(resources, R.color.progress_bar_background));
+                progressBar.setForegroundColor(
+                        ApiCompatibilityUtils.getColor(resources, R.color.progress_bar_foreground));
+            } else {
+                progressBar.setThemeColor(backgroundColor, /*isIncognito=*/false);
             }
         }
 

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

@@ -30,10 +30,16 @@ public interface LocationBarDataProvider {
      * consumer will query the data it cares about.
      */
     interface Observer {
-        void onTitleChanged();
-        void onUrlChanged();
         void onIncognitoStateChanged();
         void onNtpStartedLoading();
+
+        /**
+         * Notifies about a possible change of the value of {@link #getPrimaryColor()}, or {@link
+         * #isUsingBrandColor()}.
+         */
+        void onPrimaryColorChanged();
+        void onTitleChanged();
+        void onUrlChanged();
         // TODO(https://crbug.com/1139481): Add methods for other LocationBarDataProvider
         // data, e.g. security state.
     }

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

@@ -325,7 +325,7 @@ public class LocationBarLayout extends FrameLayout implements OnClickListener {
         }
         mDeferredNativeRunnables.clear();
 
-        updateVisualsForState();
+        onPrimaryColorChanged();
 
         updateMicButtonVisibility();
 
@@ -592,48 +592,48 @@ public class LocationBarLayout extends FrameLayout implements OnClickListener {
         if (visibility == View.VISIBLE) updateMicButtonState();
     }
 
+    /** Updates visuals after the primary color has changed. */
+    @CallSuper
+    public void onPrimaryColorChanged() {
+        updateAssistantVoiceSearchColors();
+        updateUseDarkColors();
+    }
+
     /**
-     * Call to force the UI to update the state of various buttons based on whether or not the
-     * current tab is incognito.
+     * Update visuals to use a correct light or dark color scheme depending on the primary color.
      */
-    public void updateVisualsForState() {
-        // If the location bar is focused, the toolbar background color would be the default color
-        // regardless of whether it is branded or not.
-        final int defaultPrimaryColor = ChromeColors.getDefaultThemeColor(
-                getResources(), mLocationBarDataProvider.isIncognito());
-        final int primaryColor =
-                mUrlHasFocus ? defaultPrimaryColor : mLocationBarDataProvider.getPrimaryColor();
-
-        // This will be called between inflation and initialization. For those calls, using a null
-        // ColorStateList should have no visible impact to the user.
-        AssistantVoiceSearchService assistantVoiceSearchService =
-                mAssistantVoiceSearchServiceSupplier.get();
-        ColorStateList micColorStateList = assistantVoiceSearchService == null
-                ? null
-                : assistantVoiceSearchService.getMicButtonColorStateList(
-                        primaryColor, getContext());
-        ApiCompatibilityUtils.setImageTintList(mMicButton, micColorStateList);
-
+    private void updateUseDarkColors() {
+        // TODO(crbug.com/1114183): Unify light and dark color logic in chrome and make it clear
+        // whether the foreground or background color is dark.
         final boolean useDarkColors =
-                !ColorUtils.shouldUseLightForegroundOnBackground(primaryColor);
-        ColorStateList colorStateList =
-                ChromeColors.getPrimaryIconTint(getContext(), !useDarkColors);
-        ApiCompatibilityUtils.setImageTintList(mDeleteButton, colorStateList);
+                !ColorUtils.shouldUseLightForegroundOnBackground(getPrimaryBackgroundColor());
 
+        ApiCompatibilityUtils.setImageTintList(
+                mDeleteButton, ChromeColors.getPrimaryIconTint(getContext(), !useDarkColors));
         // If the URL changed colors and is not focused, update the URL to account for the new
         // color scheme.
         if (mUrlCoordinator.setUseDarkTextColors(useDarkColors) && !mUrlBar.hasFocus()) {
             setUrl(mLocationBarDataProvider.getCurrentUrl());
         }
-
         mStatusCoordinator.setUseDarkColors(useDarkColors);
-
         if (mAutocompleteCoordinator != null) {
             mAutocompleteCoordinator.updateVisualsForState(
                     useDarkColors, mLocationBarDataProvider.isIncognito());
         }
     }
 
+    /** Returns the primary color based on the url focus, and incognito state. */
+    private int getPrimaryBackgroundColor() {
+        // If the url bar is focused, the toolbar background color is the default color regardless
+        // of whether it is branded or not.
+        if (mUrlHasFocus) {
+            return ChromeColors.getDefaultThemeColor(
+                    getResources(), mLocationBarDataProvider.isIncognito());
+        } else {
+            return mLocationBarDataProvider.getPrimaryColor();
+        }
+    }
+
     protected void onNtpStartedLoading() {}
 
     public View getContainerView() {
@@ -644,8 +644,6 @@ public class LocationBarLayout extends FrameLayout implements OnClickListener {
         return mStatusCoordinator.getSecurityIconView();
     }
 
-    public void setShowTitle(boolean showTitle) {}
-
     protected WindowAndroid getWindowAndroid() {
         return mWindowAndroid;
     }
@@ -656,13 +654,21 @@ public class LocationBarLayout extends FrameLayout implements OnClickListener {
         assert assistantVoiceSearchService != null;
         Drawable drawable = assistantVoiceSearchService.getCurrentMicDrawable();
         mMicButton.setImageDrawable(drawable);
+        updateAssistantVoiceSearchColors();
+    }
 
-        final int defaultPrimaryColor = ChromeColors.getDefaultThemeColor(
-                getResources(), mLocationBarDataProvider.isIncognito());
-        final int primaryColor =
-                mUrlHasFocus ? defaultPrimaryColor : mLocationBarDataProvider.getPrimaryColor();
-        ColorStateList colorStateList =
-                assistantVoiceSearchService.getMicButtonColorStateList(primaryColor, getContext());
+    private void updateAssistantVoiceSearchColors() {
+        AssistantVoiceSearchService assistantVoiceSearchService =
+                mAssistantVoiceSearchServiceSupplier.get();
+        ColorStateList colorStateList;
+        // This will be called between inflation and initialization. For those calls, using a null
+        // ColorStateList should have no visible impact to the user.
+        if (assistantVoiceSearchService == null) {
+            colorStateList = null;
+        } else {
+            colorStateList = assistantVoiceSearchService.getMicButtonColorStateList(
+                    getPrimaryBackgroundColor(), getContext());
+        }
         ApiCompatibilityUtils.setImageTintList(mMicButton, colorStateList);
     }
 
@@ -923,6 +929,7 @@ public class LocationBarLayout extends FrameLayout implements OnClickListener {
         mUrlHasFocus = hasFocus;
         updateButtonVisibility();
         updateShouldAnimateIconChanges();
+        onPrimaryColorChanged();
 
         if (mUrlHasFocus) {
             if (mNativeInitialized) RecordUserAction.record("FocusLocation");
@@ -959,8 +966,6 @@ public class LocationBarLayout extends FrameLayout implements OnClickListener {
             if (imm.isActive(mUrlBar)) imm.hideSoftInputFromWindow(getWindowToken(), 0, null);
         }
 
-        if (mLocationBarDataProvider.isUsingBrandColor()) updateVisualsForState();
-
         mStatusCoordinator.onUrlFocusChange(mUrlHasFocus);
 
         if (!mUrlFocusedWithoutAnimations) handleUrlFocusAnimation(mUrlHasFocus);

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

@@ -90,27 +90,30 @@ class LocationBarMediator implements LocationBar, LocationBarDataProvider.Observ
     }
 
     // LocationBarData.Observer implementation
-
     @Override
-    public void onTitleChanged() {}
+    public void onIncognitoStateChanged() {
+        mLocationBarLayout.updateMicButtonState();
+    }
 
     @Override
-    public void onUrlChanged() {
-        mLocationBarLayout.setUrl(mLocationBarDataProvider.getCurrentUrl());
+    public void onNtpStartedLoading() {
+        mLocationBarLayout.onNtpStartedLoading();
     }
 
     @Override
-    public void onIncognitoStateChanged() {
-        mLocationBarLayout.updateMicButtonState();
+    public void onPrimaryColorChanged() {
+        mLocationBarLayout.onPrimaryColorChanged();
     }
 
     @Override
-    public void onNtpStartedLoading() {
-        mLocationBarLayout.onNtpStartedLoading();
+    public void onTitleChanged() {}
+
+    @Override
+    public void onUrlChanged() {
+        mLocationBarLayout.setUrl(mLocationBarDataProvider.getCurrentUrl());
     }
 
     // LocationBar implementation.
-
     @Override
     public void destroy() {
         mLocationBarLayout = null;
@@ -126,12 +129,12 @@ class LocationBarMediator implements LocationBar, LocationBarDataProvider.Observ
 
     @Override
     public void updateVisualsForState() {
-        mLocationBarLayout.updateVisualsForState();
+        mLocationBarLayout.onPrimaryColorChanged();
     }
 
     @Override
     public void setShowTitle(boolean showTitle) {
-        mLocationBarLayout.setShowTitle(showTitle);
+        // This method is only used in CustomTabToolbar.
     }
 
     @Override

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

@@ -178,8 +178,8 @@ class LocationBarPhone extends LocationBarLayout {
     }
 
     @Override
-    public void updateVisualsForState() {
-        super.updateVisualsForState();
+    public void onPrimaryColorChanged() {
+        super.onPrimaryColorChanged();
         boolean isIncognito = mLocationBarDataProvider.isIncognito();
         setShowIconsWhenUrlFocused(SearchEngineLogoUtils.shouldShowSearchEngineLogo(isIncognito));
         updateStatusVisibility();

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

@@ -121,6 +121,7 @@ public class LocationBarModel implements ToolbarDataProvider, LocationBarDataPro
         updateUsingBrandColor();
         notifyTitleChanged();
         notifyUrlChanged();
+        notifyPrimaryColorChanged();
     }
 
     @Override
@@ -346,7 +347,10 @@ public class LocationBarModel implements ToolbarDataProvider, LocationBarDataPro
     }
 
     public void setShouldShowOmniboxInOverviewMode(boolean shouldShowOmniboxInOverviewMode) {
-        mShouldShowOmniboxInOverviewMode = shouldShowOmniboxInOverviewMode;
+        if (mShouldShowOmniboxInOverviewMode != shouldShowOmniboxInOverviewMode) {
+            mShouldShowOmniboxInOverviewMode = shouldShowOmniboxInOverviewMode;
+            notifyPrimaryColorChanged();
+        }
     }
 
     /**
@@ -356,6 +360,7 @@ public class LocationBarModel implements ToolbarDataProvider, LocationBarDataPro
     public void setPrimaryColor(int color) {
         mPrimaryColor = color;
         updateUsingBrandColor();
+        notifyPrimaryColorChanged();
     }
 
     private void updateUsingBrandColor() {
@@ -379,6 +384,12 @@ public class LocationBarModel implements ToolbarDataProvider, LocationBarDataPro
         return isInOverviewAndShowingOmnibox() || mIsUsingBrandColor;
     }
 
+    public void notifyPrimaryColorChanged() {
+        for (LocationBarDataProvider.Observer observer : mLocationBarDataObservers) {
+            observer.onPrimaryColorChanged();
+        }
+    }
+
     @Override
     public boolean isOfflinePage() {
         return hasTab() && OfflinePageUtils.isOfflinePage(mTab);

chrome/android/javatests/src/org/chromium/chrome/browser/omnibox/LocationBarLayoutTest.java

@@ -677,7 +677,7 @@ public class LocationBarLayoutTest {
         Tab tab = mActivityTestRule.loadUrlInNewTab(url, incognito);
         setupModelsForCurrentTab();
         setUrlToPageUrl(locationBar);
-        TestThreadUtils.runOnUiThreadBlocking(() -> { locationBar.updateVisualsForState(); });
+        TestThreadUtils.runOnUiThreadBlocking(() -> { locationBar.onPrimaryColorChanged(); });
         return tab;
     }