Commit 7d76b17d authored by Patrick Noland's avatar Patrick Noland Committed by Commit Bot

[ToolbarMVC] Create dedicated ThemeColorProvider for CustomTabToolbar

CustomTabToolbar follows different coloring logic that isn't fully
captured by either AppThemeColorProvider or TabThemeColorProvider, since
it also tracks the color set by CustomTabToolbarColorController due to,
e.g. the possibility of customization by the host app. Unfortunately
CustomTabToolbarColorController isn't a ThemeColorProvider, but we can
instead create a dumb ThemeColorProvider that just blindly tracks it.

Bug: 1128337
Change-Id: I9d910ee049a7a96d485053214b6dab6c0110f28d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2430228Reviewed-by: default avatarPeter Conn <peconn@chromium.org>
Reviewed-by: default avatarMatthew Jones <mdjones@chromium.org>
Commit-Queue: Patrick Noland <pnoland@chromium.org>
Cr-Commit-Position: refs/heads/master@{#811755}
parent c30fa0aa
......@@ -1548,6 +1548,7 @@ chrome_java_sources = [
"java/src/org/chromium/chrome/browser/toolbar/LocationBarModel.java",
"java/src/org/chromium/chrome/browser/toolbar/NewTabButton.java",
"java/src/org/chromium/chrome/browser/toolbar/ProgressAnimationSmooth.java",
"java/src/org/chromium/chrome/browser/toolbar/SettableThemeColorProvider.java",
"java/src/org/chromium/chrome/browser/toolbar/TabCountProvider.java",
"java/src/org/chromium/chrome/browser/toolbar/TabSwitcherButtonCoordinator.java",
"java/src/org/chromium/chrome/browser/toolbar/TabSwitcherButtonProperties.java",
......
......@@ -445,8 +445,8 @@ 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.
mUseDarkColors =
!ColorUtils.shouldUseLightForegroundOnBackground(background.getColor());
int backgroundColor = background.getColor();
mUseDarkColors = !ColorUtils.shouldUseLightForegroundOnBackground(backgroundColor);
mLocationBar.updateVisualsForState();
}
});
......
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
package org.chromium.chrome.browser.toolbar;
import android.content.Context;
import org.chromium.chrome.browser.ThemeColorProvider;
/**
* {@link ThemeColorProvider} that blindly tracks whatever primary color it's set to.
* It contains no actual tracking logic; to function properly, setPrimaryColor must be called each
* time the color changes.
*/
@Deprecated
class SettableThemeColorProvider extends ThemeColorProvider {
/**
* @param context The {@link Context} that is used to retrieve color related resources.
*/
public SettableThemeColorProvider(Context context) {
super(context);
}
/**
* Sets the primary color to the specified value.
*/
public void setPrimaryColor(int color, boolean shouldAnimate) {
updatePrimaryColor(color, shouldAnimate);
}
}
......@@ -121,6 +121,7 @@ public class ToolbarManager implements UrlFocusChangeListener, ThemeColorObserve
private final TabCountProvider mTabCountProvider;
private final ThemeColorProvider mTabThemeColorProvider;
private AppThemeColorProvider mAppThemeColorProvider;
private SettableThemeColorProvider mCustomTabThemeColorProvider;
private final TopToolbarCoordinator mToolbar;
private final ToolbarControlContainer mControlContainer;
private final BrowserControlsStateProvider.Observer mBrowserControlsObserver;
......@@ -286,6 +287,7 @@ public class ToolbarManager implements UrlFocusChangeListener, ThemeColorObserve
mAppThemeColorProvider = new AppThemeColorProvider(mActivity);
// Observe tint changes to update sub-components that rely on the tint (crbug.com/1077684).
mAppThemeColorProvider.addTintObserver(this);
mCustomTabThemeColorProvider = new SettableThemeColorProvider(mActivity);
mActivityTabProvider = tabProvider;
mToolbarTabController = new ToolbarTabControllerImpl(mLocationBarModel::getTab,
......@@ -309,7 +311,8 @@ public class ToolbarManager implements UrlFocusChangeListener, ThemeColorObserve
-> setUrlBarFocus(focus, type),
mActivity.getCompositorViewHolder()::requestFocus, shouldShowUpdateBadge,
mActivity::isInOverviewMode,
mActivity.isCustomTab() ? mAppThemeColorProvider : browsingModeThemeColorProvider,
mActivity.isCustomTab() ? mCustomTabThemeColorProvider
: browsingModeThemeColorProvider,
R.id.menu_button_wrapper);
MenuButtonCoordinator startSurfaceMenuButtonCoordinator = new MenuButtonCoordinator(
appMenuCoordinatorSupplier, mControlsVisibilityDelegate, mActivity,
......@@ -1038,6 +1041,11 @@ public class ToolbarManager implements UrlFocusChangeListener, ThemeColorObserve
mCurrentThemeColor = color;
mLocationBarModel.setPrimaryColor(color);
mToolbar.onPrimaryColorChanged(shouldAnimate);
// TODO(https://crbug.com/865801, pnoland): Rationalize theme color logic
// into a set of documented, self-contained providers that we can inject to the appropriate
// sub-components. That will let us have every component handle its own coloring, and remove
// onThemeColorChanged from ToolbarManager.
mCustomTabThemeColorProvider.setPrimaryColor(color, shouldAnimate);
}
@Override
......
......@@ -274,6 +274,11 @@ public class MenuButton extends FrameLayout implements TintObserver {
drawable.draw(canvas);
}
@VisibleForTesting
public boolean getUseLightDrawablesForTesting() {
return mUseLightDrawables;
}
@Override
public void onTintChanged(ColorStateList tintList, boolean useLight) {
ApiCompatibilityUtils.setImageTintList(mMenuImageButton, tintList);
......
......@@ -113,6 +113,7 @@ import org.chromium.chrome.browser.tab.TabTestUtils;
import org.chromium.chrome.browser.tabmodel.TabModelObserver;
import org.chromium.chrome.browser.tabmodel.TabModelSelector;
import org.chromium.chrome.browser.test.ScreenShooter;
import org.chromium.chrome.browser.toolbar.menu_button.MenuButton;
import org.chromium.chrome.browser.ui.appmenu.AppMenuCoordinator;
import org.chromium.chrome.browser.ui.appmenu.AppMenuHandler;
import org.chromium.chrome.browser.ui.appmenu.AppMenuTestSupport;
......@@ -808,6 +809,10 @@ public class CustomTabActivityTest {
assertEquals(ColorUtils.getDarkenedColorForStatusBar(expectedColor),
mCustomTabActivityTestRule.getActivity().getWindow().getStatusBarColor());
}
MenuButton menuButtonView = toolbarView.findViewById(R.id.menu_button_wrapper);
assertEquals(menuButtonView.getUseLightDrawablesForTesting(),
ColorUtils.shouldUseLightForegroundOnBackground(expectedColor));
}
/**
......@@ -2143,6 +2148,9 @@ public class CustomTabActivityTest {
Assert.assertTrue("Night mode should be enabled on K+ with dark color scheme set.",
cctActivity.getNightModeStateProvider().isInNightMode());
MenuButton menuButtonView = cctActivity.findViewById(R.id.menu_button_wrapper);
assertTrue(menuButtonView.getUseLightDrawablesForTesting());
}
@Test
......@@ -2157,6 +2165,9 @@ public class CustomTabActivityTest {
Assert.assertNotNull(cctActivity.getNightModeStateProvider());
Assert.assertFalse(cctActivity.getNightModeStateProvider().isInNightMode());
MenuButton menuButtonView = cctActivity.findViewById(R.id.menu_button_wrapper);
assertFalse(menuButtonView.getUseLightDrawablesForTesting());
}
private void addColorSchemeToIntent(Intent intent, int colorScheme) {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment