Toolbar: Fix wrong location bar background color (2)

This CL fixes wrong theme color bugs still left in the toolbar.

1) There are situations when WebContents theme color hasn't changed
   but we still need to check if the overall top UI theme can change.
   For this, TabThemeColorHelper invokes TabObserver#onDidChangeThemeColor
   with either the current tab theme or the one from WebContents accordingly.

2) When in tab switcher, location bar model's |mPrimaryColor| should be
   updated when going back and forth between normal/incognito tab model.
   Added a new line doing this, since this is not automatically done
   the after the toolbar theme refactoring.

Bug: 1157417, 1157433
Change-Id: Ib648d4ce10f6589eae54742631fc083edf0119f2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2599678
Commit-Queue: Jinsuk Kim <jinsukkim@chromium.org>
Reviewed-by: Theresa  <twellington@chromium.org>
Cr-Commit-Position: refs/heads/master@{#838965}

chrome/android/java/src/org/chromium/chrome/browser/tab/TabImpl.java

@@ -249,8 +249,7 @@ public class TabImpl implements Tab, TabObscuringHandler.Observer {
             }
         };
         mTabViewManager = new TabViewManagerImpl(this);
-        mThemeColorHelper = new TabThemeColorHelper(
-                this, () -> updateThemeColor(getWebContents().getThemeColor()));
+        mThemeColorHelper = new TabThemeColorHelper(this, this::updateThemeColor);
         mThemeColor = TabState.UNSPECIFIED_THEME_COLOR;
     }
 
@@ -1158,7 +1157,6 @@ public class TabImpl implements Tab, TabObscuringHandler.Observer {
     }
 
     void updateThemeColor(int themeColor) {
-        if (mThemeColor == themeColor) return;
         mThemeColor = themeColor;
         RewindableIterator<TabObserver> observers = getTabObservers();
         while (observers.hasNext()) observers.next().onDidChangeThemeColor(this, themeColor);

chrome/android/java/src/org/chromium/chrome/browser/tab/TabThemeColorHelper.java

@@ -4,6 +4,7 @@
 
 package org.chromium.chrome.browser.tab;
 
+import org.chromium.base.Callback;
 import org.chromium.content_public.browser.NavigationHandle;
 import org.chromium.net.NetError;
 import org.chromium.url.GURL;
@@ -13,41 +14,43 @@ import org.chromium.url.GURL;
  */
 public class TabThemeColorHelper extends EmptyTabObserver {
     private final Tab mTab;
-    private final Runnable mUpdateRunnable;
+    private final Callback mUpdateCallback;
 
-    TabThemeColorHelper(Tab tab, Runnable updateRunnable) {
+    TabThemeColorHelper(Tab tab, Callback<Integer> updateCallback) {
         mTab = tab;
-        mUpdateRunnable = updateRunnable;
+        mUpdateCallback = updateCallback;
         tab.addObserver(this);
     }
 
     /**
      * Notifies the listeners of the tab theme color change.
      */
-    void updateIfNeeded() {
-        mUpdateRunnable.run();
+    private void updateIfNeeded(Tab tab, boolean didWebContentsThemeColorChange) {
+        int themeColor = tab.getThemeColor();
+        if (didWebContentsThemeColorChange) themeColor = tab.getWebContents().getThemeColor();
+        mUpdateCallback.onResult(themeColor);
     }
 
     // TabObserver
 
     @Override
     public void onSSLStateUpdated(Tab tab) {
-        updateIfNeeded();
+        updateIfNeeded(tab, false);
     }
 
     @Override
     public void onUrlUpdated(Tab tab) {
-        updateIfNeeded();
+        updateIfNeeded(tab, false);
     }
 
     @Override
     public void onDidFailLoad(Tab tab, boolean isMainFrame, int errorCode, GURL failingUrl) {
-        updateIfNeeded();
+        updateIfNeeded(tab, true);
     }
 
     @Override
     public void onDidFinishNavigation(Tab tab, NavigationHandle navigation) {
-        if (navigation.errorCode() != NetError.OK) updateIfNeeded();
+        if (navigation.errorCode() != NetError.OK) updateIfNeeded(tab, true);
     }
 
     @Override

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

@@ -1582,8 +1582,10 @@ public class ToolbarManager implements UrlFocusChangeListener, ThemeColorObserve
         if (previousTab != tab || wasIncognito != isIncognito) {
             int defaultPrimaryColor =
                     ChromeColors.getDefaultThemeColor(mActivity.getResources(), isIncognito);
-            int primaryColor =
-                    tab != null ? mTopUiThemeColorProvider.getThemeColor() : defaultPrimaryColor;
+            int primaryColor = tab != null
+                    ? mTopUiThemeColorProvider.calculateColor(tab, tab.getThemeColor())
+                    : defaultPrimaryColor;
+            // TODO(jinsukkim): Let TopUiThemeColorProvider handle this by updating the theme color.
             onThemeColorChanged(primaryColor, false);
 
             onTabOrModelChanged();

chrome/android/javatests/src/org/chromium/chrome/browser/TabThemeTest.java

@@ -19,9 +19,8 @@ import org.chromium.base.test.util.CommandLineFlags;
 import org.chromium.base.test.util.Feature;
 import org.chromium.base.test.util.Restriction;
 import org.chromium.chrome.browser.flags.ChromeSwitches;
-import org.chromium.chrome.browser.tab.EmptyTabObserver;
 import org.chromium.chrome.browser.tab.Tab;
-import org.chromium.chrome.browser.tab.TabState;
+import org.chromium.chrome.browser.theme.ThemeColorProvider.ThemeColorObserver;
 import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
 import org.chromium.chrome.test.ChromeTabbedActivityTestRule;
 import org.chromium.components.browser_ui.styles.ChromeColors;
@@ -51,7 +50,7 @@ public class TabThemeTest {
     /**
      * A WebContentsObserver for watching changes in the theme color.
      */
-    private static class ThemeColorWebContentsObserver extends EmptyTabObserver {
+    private static class ThemeColorWebContentsObserver implements ThemeColorObserver {
         private CallbackHelper mCallbackHelper;
         private int mColor;
 
@@ -60,7 +59,7 @@ public class TabThemeTest {
         }
 
         @Override
-        public void onDidChangeThemeColor(Tab tab, int color) {
+        public void onThemeColorChanged(int color, boolean shouldAnimate) {
             mColor = color;
             mCallbackHelper.notifyCalled();
         }
@@ -115,7 +114,10 @@ public class TabThemeTest {
 
         ThemeColorWebContentsObserver colorObserver = new ThemeColorWebContentsObserver();
         CallbackHelper themeColorHelper = colorObserver.getCallbackHelper();
-        tab.addObserver(colorObserver);
+        mActivityTestRule.getActivity()
+                .getRootUiCoordinatorForTesting()
+                .getTopUiThemeColorProvider()
+                .addThemeColorObserver(colorObserver);
 
         // Navigate to a themed page.
         int curCallCount = themeColorHelper.getCallCount();
@@ -139,7 +141,7 @@ public class TabThemeTest {
         curCallCount = themeColorHelper.getCallCount();
         mActivityTestRule.loadUrl(testServer.getURL(TEST_PAGE));
         themeColorHelper.waitForCallback(curCallCount, 1);
-        assertColorsEqual(TabState.UNSPECIFIED_THEME_COLOR, colorObserver.getColor());
+        assertColorsEqual(getDefaultThemeColor(tab), colorObserver.getColor());
         assertColorsEqual(getDefaultThemeColor(tab), getThemeColor());
 
         // Navigate to a themed page from a non-native page.

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

@@ -35,6 +35,7 @@ import org.chromium.chrome.browser.tab.EmptyTabObserver;
 import org.chromium.chrome.browser.tab.Tab;
 import org.chromium.chrome.browser.tab.TabObserver;
 import org.chromium.chrome.browser.tabmodel.TabModelSelectorTabObserver;
+import org.chromium.chrome.browser.theme.ThemeColorProvider.ThemeColorObserver;
 import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
 import org.chromium.chrome.test.ChromeTabbedActivityTestRule;
 import org.chromium.chrome.test.util.ChromeTabUtils;
@@ -260,18 +261,23 @@ public class OmniboxTest {
                 ServerCertificate.CERT_OK);
         CallbackHelper didThemeColorChangedCallbackHelper = new CallbackHelper();
         CallbackHelper onSSLStateUpdatedCallbackHelper = new CallbackHelper();
-        ThreadUtils.runOnUiThreadBlocking(
-                ()
-                        -> new TabModelSelectorTabObserver(
-                                mActivityTestRule.getActivity().getTabModelSelector()) {
-                    @Override
-                    public void onDidChangeThemeColor(Tab tab, int color) {
-                        didThemeColorChangedCallbackHelper.notifyCalled();
-                    }
+        ThreadUtils.runOnUiThreadBlocking(() -> {
+            new TabModelSelectorTabObserver(mActivityTestRule.getActivity().getTabModelSelector()) {
                 @Override
                 public void onSSLStateUpdated(Tab tab) {
                     onSSLStateUpdatedCallbackHelper.notifyCalled();
                 }
+            };
+
+            mActivityTestRule.getActivity()
+                    .getRootUiCoordinatorForTesting()
+                    .getTopUiThemeColorProvider()
+                    .addThemeColorObserver(new ThemeColorObserver() {
+                        @Override
+                        public void onThemeColorChanged(int color, boolean shouldAnimate) {
+                            didThemeColorChangedCallbackHelper.notifyCalled();
+                        }
+                    });
         });
 
         try {