Skip to content

Commit dbc7548

Browse files
gastonrodChromium LUCI CQ
authored andcommitted
Make browser_preferred_color_scheme light when custom theme is on.
Issue: When chrome has a browser theme on that changes the browser's color and the user preferences are set to dark mode, UsedColorSchemeRootScrollbars makes root scrollbars be dark in pages where no color scheme is specified. This becomes an issue when the browser's theme is a light color, which contrasts too starkly with the dark scrollbars. To repro this issue, please see the attached bug. The `browser_preferred_color_scheme` flag is used to send the preferred color scheme from the browser process to blink in the renderer process to help with the selection of a color scheme for scrollbars. Fix: - This CL changes the `browser_preferred_color_scheme` browser setting to be set to the default value (kLight color scheme) when a custom theme is detected in the browser. With this change, UsedColorSchemeRootScrollbars doesn't override the root scrollbar by changing it to dark mode. - The `browser_preferred_color_scheme` setting is also being renamed to better represent what it pertains to, and is now called `preferred_root_scrollbar_color_scheme`. All the relevant references and functions have been renamed to reflect this. Bug: 337904215 Change-Id: I2517747f2a4cef061187daf4191c0fc8660d7fcd Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5502831 Reviewed-by: Yaroslav Shalivskyy <[email protected]> Reviewed-by: Philip Rogers <[email protected]> Commit-Queue: Gaston Rodriguez <[email protected]> Reviewed-by: danakj <[email protected]> Cr-Commit-Position: refs/heads/main@{#1298816}
1 parent 70ca252 commit dbc7548

File tree

15 files changed

+148
-43
lines changed

15 files changed

+148
-43
lines changed

chrome/browser/chrome_content_browser_client.cc

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,8 @@
164164
#include "chrome/browser/supervised_user/supervised_user_google_auth_navigation_throttle.h"
165165
#include "chrome/browser/supervised_user/supervised_user_navigation_throttle.h"
166166
#include "chrome/browser/task_manager/sampling/task_manager_impl.h"
167+
#include "chrome/browser/themes/theme_service.h"
168+
#include "chrome/browser/themes/theme_service_factory.h"
167169
#include "chrome/browser/tracing/chrome_tracing_delegate.h"
168170
#include "chrome/browser/translate/translate_service.h"
169171
#include "chrome/browser/ui/blocked_content/blocked_window_params.h"
@@ -3701,18 +3703,30 @@ bool UpdatePreferredColorScheme(WebPreferences* web_prefs,
37013703
delegate->IsNightModeEnabled()
37023704
? blink::mojom::PreferredColorScheme::kDark
37033705
: blink::mojom::PreferredColorScheme::kLight;
3704-
web_prefs->browser_preferred_color_scheme =
3706+
web_prefs->preferred_root_scrollbar_color_scheme =
37053707
web_prefs->preferred_color_scheme;
37063708
}
37073709
#else
37083710
// Update based on native theme scheme.
37093711
web_prefs->preferred_color_scheme =
37103712
ToBlinkPreferredColorScheme(native_theme->GetPreferredColorScheme());
37113713

3714+
bool using_different_colored_frame = false;
3715+
if (Profile* profile =
3716+
Profile::FromBrowserContext(web_contents->GetBrowserContext())) {
3717+
if (ThemeService* theme_service =
3718+
ThemeServiceFactory::GetForProfile(profile)) {
3719+
using_different_colored_frame = !theme_service->UsingDefaultTheme() ||
3720+
theme_service->GetUserColor().has_value();
3721+
}
3722+
}
3723+
37123724
// Update based on the ColorProvider associated with `web_contents`. Depends
3713-
// on the browser color mode settings.
3714-
web_prefs->browser_preferred_color_scheme =
3715-
web_contents->GetColorMode() == ui::ColorProviderKey::ColorMode::kLight
3725+
// on the browser color mode settings and whether the user profile has set a
3726+
// custom coloring for the browser ui.
3727+
web_prefs->preferred_root_scrollbar_color_scheme =
3728+
web_contents->GetColorMode() == ui::ColorProviderKey::ColorMode::kLight ||
3729+
using_different_colored_frame
37163730
? blink::mojom::PreferredColorScheme::kLight
37173731
: blink::mojom::PreferredColorScheme::kDark;
37183732
#endif // BUILDFLAG(IS_ANDROID)

chrome/browser/chrome_content_browser_client_browsertest.cc

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@
2222
#include "chrome/browser/search/instant_service.h"
2323
#include "chrome/browser/search/instant_service_factory.h"
2424
#include "chrome/browser/search/search.h"
25+
#include "chrome/browser/themes/theme_service.h"
26+
#include "chrome/browser/themes/theme_service_factory.h"
2527
#include "chrome/browser/ui/browser.h"
2628
#include "chrome/browser/ui/search/instant_test_base.h"
2729
#include "chrome/browser/ui/tabs/tab_strip_model.h"
@@ -65,6 +67,7 @@
6567
#include "net/test/embedded_test_server/embedded_test_server.h"
6668
#include "net/test/embedded_test_server/http_request.h"
6769
#include "net/test/embedded_test_server/http_response.h"
70+
#include "third_party/blink/public/mojom/webpreferences/web_preferences.mojom.h"
6871
#include "ui/base/data_transfer_policy/data_transfer_endpoint.h"
6972
#include "ui/color/color_provider.h"
7073
#include "ui/color/color_provider_key.h"
@@ -691,6 +694,80 @@ INSTANTIATE_TEST_SUITE_P(All,
691694
PrefersColorSchemeTest,
692695
testing::Combine(testing::Bool(), testing::Bool()));
693696

697+
class PreferredRootScrollbarColorSchemeChromeClientTest
698+
: public testing::WithParamInterface<std::tuple<bool, bool>>,
699+
public InProcessBrowserTest {
700+
protected:
701+
PreferredRootScrollbarColorSchemeChromeClientTest()
702+
: dark_mode_(std::get<0>(GetParam())),
703+
uses_custom_theme_(std::get<1>(GetParam())),
704+
theme_client_(&test_theme_) {
705+
test_theme_.SetDarkMode(dark_mode_);
706+
}
707+
708+
void SetUpOnMainThread() override {
709+
InProcessBrowserTest::SetUpOnMainThread();
710+
original_client_ = SetBrowserClientForTesting(&theme_client_);
711+
test_theme_.SetDarkMode(dark_mode_);
712+
ui::NativeTheme::GetInstanceForNativeUi()->set_use_dark_colors(dark_mode_);
713+
ThemeService* theme_service =
714+
ThemeServiceFactory::GetForProfile(browser()->profile());
715+
if (uses_custom_theme_) {
716+
theme_service->BuildAutogeneratedThemeFromColor(SK_ColorRED);
717+
} else {
718+
theme_service->UseDefaultTheme();
719+
}
720+
}
721+
722+
~PreferredRootScrollbarColorSchemeChromeClientTest() override {
723+
CHECK_EQ(&theme_client_, SetBrowserClientForTesting(original_client_));
724+
}
725+
726+
blink::mojom::PreferredColorScheme ExpectedColorScheme() const {
727+
return dark_mode_ && !uses_custom_theme_
728+
? blink::mojom::PreferredColorScheme::kDark
729+
: blink::mojom::PreferredColorScheme::kLight;
730+
}
731+
732+
private:
733+
class ChromeContentBrowserClientWithWebTheme
734+
: public ChromeContentBrowserClient {
735+
public:
736+
explicit ChromeContentBrowserClientWithWebTheme(
737+
const ui::NativeTheme* theme)
738+
: theme_(theme) {}
739+
740+
protected:
741+
const ui::NativeTheme* GetWebTheme() const override { return theme_; }
742+
743+
private:
744+
const raw_ptr<const ui::NativeTheme> theme_;
745+
};
746+
747+
const bool dark_mode_ = false;
748+
const bool uses_custom_theme_ = false;
749+
raw_ptr<content::ContentBrowserClient> original_client_ = nullptr;
750+
ui::TestNativeTheme test_theme_;
751+
ChromeContentBrowserClientWithWebTheme theme_client_;
752+
};
753+
754+
// This test verifies that the preferred color scheme for root scrollbars is set
755+
// appropriately following the web content's color scheme and the presence of
756+
// a custom theme.
757+
IN_PROC_BROWSER_TEST_P(PreferredRootScrollbarColorSchemeChromeClientTest,
758+
ScrollbarFollowsPreferredColorScheme) {
759+
EXPECT_EQ(browser()
760+
->tab_strip_model()
761+
->GetActiveWebContents()
762+
->GetOrCreateWebPreferences()
763+
.preferred_root_scrollbar_color_scheme,
764+
ExpectedColorScheme());
765+
}
766+
767+
INSTANTIATE_TEST_SUITE_P(All,
768+
PreferredRootScrollbarColorSchemeChromeClientTest,
769+
testing::Combine(testing::Bool(), testing::Bool()));
770+
694771
class PrefersContrastTest
695772
: public testing::WithParamInterface<ui::NativeTheme::PreferredContrast>,
696773
public InProcessBrowserTest {

third_party/blink/common/web_preferences/web_preferences_mojom_traits.cc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,8 @@ bool StructTraits<blink::mojom::WebPreferencesDataView,
219219
out->require_transient_activation_for_html_fullscreen =
220220
data.require_transient_activation_for_html_fullscreen();
221221
out->in_forced_colors = data.in_forced_colors();
222-
out->browser_preferred_color_scheme = data.browser_preferred_color_scheme();
222+
out->preferred_root_scrollbar_color_scheme =
223+
data.preferred_root_scrollbar_color_scheme();
223224
out->preferred_color_scheme = data.preferred_color_scheme();
224225
out->preferred_contrast = data.preferred_contrast();
225226
out->picture_in_picture_enabled = data.picture_in_picture_enabled();

third_party/blink/public/common/web_preferences/web_preferences.h

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -355,10 +355,12 @@ struct BLINK_COMMON_EXPORT WebPreferences {
355355
// when to apply system color overrides to author specified styles.
356356
bool in_forced_colors = false;
357357

358-
// The preferred color scheme set by the user's browser settings. The scheme
359-
// is used to evaluate the used color scheme. Currently, only used for the
360-
// scrollbars' used color scheme.
361-
blink::mojom::PreferredColorScheme browser_preferred_color_scheme =
358+
// The preferred color scheme set by the user's browser settings. The variable
359+
// follows the browser's color mode setting unless a browser theme (custom or
360+
// not) is defined, in which case the color scheme is set to the default
361+
// value. This value is used to evaluate the used color scheme in non overlay
362+
// root scrollbars.
363+
blink::mojom::PreferredColorScheme preferred_root_scrollbar_color_scheme =
362364
blink::mojom::PreferredColorScheme::kLight;
363365

364366
// The preferred color scheme for the web content. The scheme is used to

third_party/blink/public/common/web_preferences/web_preferences_mojom_traits.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -713,9 +713,10 @@ struct BLINK_COMMON_EXPORT StructTraits<blink::mojom::WebPreferencesDataView,
713713
return r.in_forced_colors;
714714
}
715715

716-
static blink::mojom::PreferredColorScheme browser_preferred_color_scheme(
716+
static blink::mojom::PreferredColorScheme
717+
preferred_root_scrollbar_color_scheme(
717718
const blink::web_pref::WebPreferences& r) {
718-
return r.browser_preferred_color_scheme;
719+
return r.preferred_root_scrollbar_color_scheme;
719720
}
720721

721722
static blink::mojom::PreferredColorScheme preferred_color_scheme(

third_party/blink/public/mojom/webpreferences/web_preferences.mojom

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -419,10 +419,12 @@ struct WebPreferences {
419419
// when to apply system color overrides to author specified styles.
420420
bool in_forced_colors;
421421

422-
// The preferred color scheme set by the user's browser settings. The scheme
423-
// is used to evaluate the used color scheme. Currently, only used for the
424-
// scrollbars' used color scheme.
425-
PreferredColorScheme browser_preferred_color_scheme;
422+
// The preferred color scheme set by the user's browser settings. The variable
423+
// follows the browser's color mode setting unless a browser theme (custom or
424+
// not) is defined, in which case the color scheme is set to the default
425+
// value. This value is used to evaluate the used color scheme in non overlay
426+
// root scrollbars.
427+
PreferredColorScheme preferred_root_scrollbar_color_scheme;
426428

427429
// The preferred color scheme for the web content. The scheme is used to
428430
// evaluate the prefers-color-scheme media query and resolve UA color scheme

third_party/blink/public/web/web_settings.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -269,7 +269,7 @@ class WebSettings {
269269
virtual void SetLazyLoadingImageMarginPx4G(int) = 0;
270270
virtual void SetForceDarkModeEnabled(bool) = 0;
271271
virtual void SetInForcedColors(bool) = 0;
272-
virtual void SetBrowserPreferredColorScheme(
272+
virtual void SetPreferredRootScrollbarColorScheme(
273273
blink::mojom::PreferredColorScheme) = 0;
274274
virtual void SetPreferredColorScheme(blink::mojom::PreferredColorScheme) = 0;
275275
virtual void SetPreferredContrast(mojom::PreferredContrast) = 0;

third_party/blink/renderer/core/exported/web_settings_impl.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -757,9 +757,9 @@ void WebSettingsImpl::SetInForcedColors(bool in_forced_colors) {
757757
settings_->SetInForcedColors(in_forced_colors);
758758
}
759759

760-
void WebSettingsImpl::SetBrowserPreferredColorScheme(
760+
void WebSettingsImpl::SetPreferredRootScrollbarColorScheme(
761761
mojom::blink::PreferredColorScheme color_scheme) {
762-
settings_->SetBrowserPreferredColorScheme(color_scheme);
762+
settings_->SetPreferredRootScrollbarColorScheme(color_scheme);
763763
}
764764

765765
void WebSettingsImpl::SetPreferredColorScheme(

third_party/blink/renderer/core/exported/web_settings_impl.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -222,7 +222,7 @@ class CORE_EXPORT WebSettingsImpl final : public WebSettings {
222222

223223
void SetForceDarkModeEnabled(bool) override;
224224
void SetInForcedColors(bool) override;
225-
void SetBrowserPreferredColorScheme(
225+
void SetPreferredRootScrollbarColorScheme(
226226
mojom::blink::PreferredColorScheme) override;
227227
void SetPreferredColorScheme(mojom::blink::PreferredColorScheme) override;
228228
void SetPreferredContrast(mojom::blink::PreferredContrast) override;

third_party/blink/renderer/core/exported/web_view_impl.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1792,8 +1792,8 @@ void WebView::ApplyWebPreferences(const web_pref::WebPreferences& prefs,
17921792

17931793
settings->SetLazyLoadEnabled(prefs.lazy_load_enabled);
17941794
settings->SetInForcedColors(prefs.in_forced_colors);
1795-
settings->SetBrowserPreferredColorScheme(
1796-
prefs.browser_preferred_color_scheme);
1795+
settings->SetPreferredRootScrollbarColorScheme(
1796+
prefs.preferred_root_scrollbar_color_scheme);
17971797
settings->SetPreferredColorScheme(prefs.preferred_color_scheme);
17981798
settings->SetPreferredContrast(prefs.preferred_contrast);
17991799

0 commit comments

Comments
 (0)