Skip to content

Commit

Permalink
Speculative fix for crash when emulating forced colors via DevTools
Browse files Browse the repository at this point in the history
This CL attempts to fix a crash that occurs when the DevTools feature
is used to emulate forced colors. The crash is caused by an
inappropriate change of the key from the renderer, which leads to a
call to a Windows OS method, GetSysColor(). The fix removes the key
change and uses the custom creation function that was already
implemented for the emulation. The crash could not be reproduced
locally and will be followed up once this lands in canary to confirm
that the fix works.

(cherry picked from commit 0beb9ed)

Bug: 1500381
Change-Id: I5ba9c29baad7d76bf79afe91eaecf8f9ebbed49d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5013943
Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Reviewed-by: Alison Maher <almaher@microsoft.com>
Commit-Queue: Sam Davis Omekara <samomekarajr@microsoft.com>
Cr-Original-Commit-Position: refs/heads/main@{#1221806}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5055192
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Owners-Override: Prudhvikumar Bommana <pbommana@google.com>
Cr-Commit-Position: refs/branch-heads/6045@{#1468}
Cr-Branched-From: 905e8bd-refs/heads/main@{#1204232}
  • Loading branch information
Sam Davis Omekara (from Dev Box) authored and Daniel Yip committed Nov 24, 2023
1 parent 9aaf65b commit 08d6575
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 43 deletions.
3 changes: 0 additions & 3 deletions third_party/blink/public/platform/web_theme_engine.h
Original file line number Diff line number Diff line change
Expand Up @@ -309,9 +309,6 @@ class WebThemeEngine {
const ui::RendererColorMap& forced_colors_map) {
return false;
}
virtual void AdjustForcedColorsProvider(
ui::ColorProviderKey::ForcedColors forced_colors_state,
ui::ColorProviderKey::ColorMode color_mode) {}
};

} // namespace blink
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -341,10 +341,6 @@ void WebThemeEngineDefault::OverrideForcedColorsTheme(bool is_dark_theme) {
{ui::NativeTheme::SystemThemeColor::kWindow, 0xFFFFFFFF},
{ui::NativeTheme::SystemThemeColor::kWindowText, 0xFF000000},
};
AdjustForcedColorsProvider(ui::ColorProviderKey::ForcedColors::kEmulated,
is_dark_theme
? ui::ColorProviderKey::ColorMode::kDark
: ui::ColorProviderKey::ColorMode::kLight);
EmulateForcedColors(is_dark_theme, /*is_web_test=*/false);
ui::NativeTheme::GetInstanceForWeb()->UpdateSystemColorInfo(
false, true, is_dark_theme ? dark_theme : light_theme);
Expand All @@ -367,15 +363,6 @@ void WebThemeEngineDefault::ResetToSystemColors(
SystemColorInfoState system_color_info_state) {
base::flat_map<ui::NativeTheme::SystemThemeColor, uint32_t> colors;

ui::ColorProviderKey::ForcedColors initial_forced_colors_state =
system_color_info_state.forced_colors
? ui::ColorProviderKey::ForcedColors::kActive
: ui::ColorProviderKey::ForcedColors::kNone;
ui::ColorProviderKey::ColorMode initial_color_mode =
system_color_info_state.is_dark_mode
? ui::ColorProviderKey::ColorMode::kDark
: ui::ColorProviderKey::ColorMode::kLight;
AdjustForcedColorsProvider(initial_forced_colors_state, initial_color_mode);
for (const auto& color : system_color_info_state.colors) {
colors.insert({NativeSystemThemeColor(color.first), color.second});
}
Expand Down Expand Up @@ -440,25 +427,6 @@ bool WebThemeEngineDefault::UpdateColorProviders(
return did_color_provider_update;
}

void WebThemeEngineDefault::AdjustForcedColorsProvider(
ui::ColorProviderKey::ForcedColors forced_colors_state,
ui::ColorProviderKey::ColorMode color_mode) {
auto key = ui::NativeTheme::GetInstanceForWeb()->GetColorProviderKey(
/*custom_theme=*/nullptr);
key.forced_colors = forced_colors_state;
key.color_mode = color_mode;
ui::ColorProvider* color_provider =
ui::ColorProviderManager::Get().GetColorProviderFor(key);
CHECK(color_provider);

const ui::RendererColorMap& emulated_forced_colors_map =
ui::CreateRendererColorMap(*color_provider);
if (!IsRendererColorMappingEquivalent(forced_colors_provider_,
emulated_forced_colors_map)) {
forced_colors_provider_ = std::move(*color_provider);
}
}

bool WebThemeEngineDefault::ShouldPartBeAffectedByAccentColor(
WebThemeEngine::Part part,
WebThemeEngine::State state,
Expand Down Expand Up @@ -544,8 +512,11 @@ mojom::ColorScheme WebThemeEngineDefault::CalculateColorSchemeForAccentColor(

const ui::ColorProvider* WebThemeEngineDefault::GetColorProviderForPainting(
mojom::ColorScheme color_scheme) const {
if (emulate_forced_colors_ && GetForcedColors() == ForcedColors::kActive) {
return &emulated_forced_colors_provider_;
if (GetForcedColors() == ForcedColors::kActive) {
if (emulate_forced_colors_) {
return &emulated_forced_colors_provider_;
}
return &forced_colors_provider_;
}
return color_scheme == mojom::ColorScheme::kLight ? &light_color_provider_
: &dark_color_provider_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,6 @@ class WebThemeEngineDefault : public WebThemeEngine {
const ui::RendererColorMap& light_colors,
const ui::RendererColorMap& dark_colors,
const ui::RendererColorMap& forced_colors_map) override;
void AdjustForcedColorsProvider(
ui::ColorProviderKey::ForcedColors forced_colors_state,
ui::ColorProviderKey::ColorMode color_mode) override;
void EmulateForcedColors(bool is_dark_theme, bool is_web_test) override;
bool IsFluentOverlayScrollbarEnabled() const override;
int GetPaintedScrollbarTrackInset() const override;
Expand Down Expand Up @@ -98,9 +95,9 @@ class WebThemeEngineDefault : public WebThemeEngine {
ui::ColorProvider forced_colors_provider_;

// This provider is used when forced color emulation is enabled, overriding
// the light or dark color providers.
// TODO(samomekarajr): Remove this provider when we use the forced colors
// provider for painting.
// the light, dark or forced colors color providers.
// TODO(samomekarajr): Remove this provider when we figure out how to change
// the ForcedColors key from the renderer.
ui::ColorProvider emulated_forced_colors_provider_;
};

Expand Down

0 comments on commit 08d6575

Please sign in to comment.