Commit 546c62fc authored by Joel Hockey's avatar Joel Hockey Committed by Commit Bot

Separate browser theme NTP and active tab colors

The related bug shows that infobar is using the wrong color for
non-tabbed PWAs. Infobar takes its color from the active tab color,
and non-tabbed PWAs are currently setting active tab color to the
theme color (rather than alt color) in order to get the desired NTP
page during page load.

The fix here is to roll back some of crrev.com/c/2094484 and set
active tab color back to the alt color, and then to separate NTP
and active tab colors so that NTP can be kept with the desired
theme color.

All other clients of BrowserThemePack are unchanged since ntp_color
is taking active tab color in GetAutogeneratedThemeColors.  Only
AppBrowserController is changing for non-tabbed PWAs to set alt color
for active_tab (fixes bug relating to info bar), but can now keep
the theme color for NTP so as not to regress crbug.com/1059696.

For reference the previous changes for AppBrowserController:
crrev.com/c/2045278
crrev.com/c/2064164
crrev.com/c/2094484

Screenshots: crbug.com/1096027#c1

Bug: 1096027
Change-Id: I7db5a2836fef381116b4082b1fde219ba8bdec95
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2248391Reviewed-by: default avatarAlan Cutter <alancutter@chromium.org>
Reviewed-by: default avatarPeter Kasting <pkasting@chromium.org>
Commit-Queue: Joel Hockey <joelhockey@chromium.org>
Cr-Commit-Position: refs/heads/master@{#779686}
parent f8c9a794
...@@ -809,22 +809,27 @@ void BrowserThemePack::BuildFromColors(AutogeneratedThemeColors colors, ...@@ -809,22 +809,27 @@ void BrowserThemePack::BuildFromColors(AutogeneratedThemeColors colors,
// NOTE! If you make any changes here, please update kThemePackVersion. // NOTE! If you make any changes here, please update kThemePackVersion.
// Frame.
pack->SetColor(TP::COLOR_FRAME_ACTIVE, colors.frame_color); pack->SetColor(TP::COLOR_FRAME_ACTIVE, colors.frame_color);
// Inactive tab uses frame color.
pack->SetColor(TP::COLOR_TAB_BACKGROUND_INACTIVE_FRAME_ACTIVE, pack->SetColor(TP::COLOR_TAB_BACKGROUND_INACTIVE_FRAME_ACTIVE,
colors.frame_color); colors.frame_color);
pack->SetColor(TP::COLOR_TAB_FOREGROUND_INACTIVE_FRAME_ACTIVE, pack->SetColor(TP::COLOR_TAB_FOREGROUND_INACTIVE_FRAME_ACTIVE,
colors.frame_text_color); colors.frame_text_color);
// Toolbar and active tab (set in SetFrameAndToolbarRelatedColors) use active
// tab color.
pack->SetColor(TP::COLOR_TOOLBAR, colors.active_tab_color); pack->SetColor(TP::COLOR_TOOLBAR, colors.active_tab_color);
pack->SetColor(TP::COLOR_TAB_FOREGROUND_ACTIVE_FRAME_ACTIVE, pack->SetColor(TP::COLOR_TAB_FOREGROUND_ACTIVE_FRAME_ACTIVE,
colors.active_tab_text_color); colors.active_tab_text_color);
pack->SetColor(TP::COLOR_TOOLBAR_BUTTON_ICON, colors.active_tab_text_color); pack->SetColor(TP::COLOR_TOOLBAR_BUTTON_ICON, colors.active_tab_text_color);
pack->SetColor(TP::COLOR_BOOKMARK_TEXT, colors.active_tab_text_color); pack->SetColor(TP::COLOR_BOOKMARK_TEXT, colors.active_tab_text_color);
pack->SetColor(TP::COLOR_NTP_BACKGROUND, colors.active_tab_color); // NTP.
pack->SetColor(TP::COLOR_NTP_BACKGROUND, colors.ntp_color);
pack->SetColor(TP::COLOR_NTP_TEXT, pack->SetColor(TP::COLOR_NTP_TEXT,
color_utils::GetColorWithMaxContrast(colors.active_tab_color)); color_utils::GetColorWithMaxContrast(colors.ntp_color));
// Always use alternate logo (not colorful one) for all backgrounds except // Always use alternate logo (not colorful one) for all backgrounds except
// white. // white.
......
...@@ -482,31 +482,33 @@ void AppBrowserController::UpdateThemePack() { ...@@ -482,31 +482,33 @@ void AppBrowserController::UpdateThemePack() {
} }
theme_pack_ = base::MakeRefCounted<BrowserThemePack>( theme_pack_ = base::MakeRefCounted<BrowserThemePack>(
CustomThemeSupplier::AUTOGENERATED); CustomThemeSupplier::AUTOGENERATED);
// AutogeneratedThemeColors will generally make the frame color match the
// theme color, but often adjusts it.
// We will use a similar approach to generate a main color and alt color
// with a 1.6 contrast, but no adjustment to the main color.
AutogeneratedThemeColors colors; AutogeneratedThemeColors colors;
SkColor theme_text_color = color_utils::GetColorWithMaxContrast(*theme_color); SkColor theme_text_color = color_utils::GetColorWithMaxContrast(*theme_color);
SkColor alt_color = color_utils::BlendForMinContrast(
*theme_color, *theme_color, base::nullopt,
kAutogeneratedThemeActiveTabPreferredContrast)
.color;
SkColor alt_text_color = color_utils::GetColorWithMaxContrast(alt_color);
if (has_tab_strip_) { if (has_tab_strip_) {
// AutogeneratedThemeColors will generally make the frame color match the // When we have tabs, active tab gets theme color.
// theme color, but often adjusts it. colors.frame_color = alt_color;
// We will use a similar approach to generate a main color and alt color colors.frame_text_color = alt_text_color;
// with a 1.6 contrast ensuring that the active tab is exactly the theme.
colors.active_tab_color = *theme_color; colors.active_tab_color = *theme_color;
colors.active_tab_text_color = theme_text_color; colors.active_tab_text_color = theme_text_color;
colors.frame_color = color_utils::BlendForMinContrast( colors.ntp_color = *theme_color;
*theme_color, *theme_color, base::nullopt,
kAutogeneratedThemeActiveTabPreferredContrast)
.color;
colors.frame_text_color =
color_utils::GetColorWithMaxContrast(colors.frame_color);
} else { } else {
// Set frame and active_tab to the same color when there are no tabs. // With no tabs, frame gets theme color.
// Tab colors are used for tooltips and NTP background (bg shown until page
// loads).
// TODO(crbug.com/1053823): Add tests for theme properties being set in this // TODO(crbug.com/1053823): Add tests for theme properties being set in this
// branch. // branch.
colors.frame_color = *theme_color; colors.frame_color = *theme_color;
colors.frame_text_color = theme_text_color; colors.frame_text_color = theme_text_color;
colors.active_tab_color = *theme_color; colors.active_tab_color = alt_color;
colors.active_tab_text_color = theme_text_color; colors.active_tab_text_color = alt_text_color;
colors.ntp_color = *theme_color;
} }
BrowserThemePack::BuildFromColors(colors, theme_pack_.get()); BrowserThemePack::BuildFromColors(colors, theme_pack_.get());
} }
......
...@@ -132,5 +132,5 @@ AutogeneratedThemeColors GetAutogeneratedThemeColors(SkColor color) { ...@@ -132,5 +132,5 @@ AutogeneratedThemeColors GetAutogeneratedThemeColors(SkColor color) {
frame_color = DarkenColor(frame_color, kDarkenStep); frame_color = DarkenColor(frame_color, kDarkenStep);
} }
return {frame_color, frame_text_color, active_tab_color, return {frame_color, frame_text_color, active_tab_color,
active_tab_text_color}; active_tab_text_color, active_tab_color};
} }
...@@ -22,6 +22,7 @@ struct AutogeneratedThemeColors { ...@@ -22,6 +22,7 @@ struct AutogeneratedThemeColors {
SkColor frame_text_color; SkColor frame_text_color;
SkColor active_tab_color; SkColor active_tab_color;
SkColor active_tab_text_color; SkColor active_tab_text_color;
SkColor ntp_color;
}; };
// Generates theme colors for the given |color|. // Generates theme colors for the given |color|.
......
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