Commit 669d8132 authored by Alex Ilin's avatar Alex Ilin Committed by Commit Bot

[ChromeColors] Revert theme changes when user navigates away from webUI

This CL calls ChromeColorsService::RevertThemeChanges() from
~ChromeCustomizeThemesHandler() to not apply unconfirmed changes when
the user navigates away from a webUI page with the color picker.

Screencast: https://drive.google.com/file/d/1APJjUVJv3WL3m2Nr71ukAG9o1ZRMnMMc/view?usp=sharing

Bug: 1095530, 1115301
Change-Id: Ic2a7a886b1e9ad8328687621a5dd9e44d1563280
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2414175
Commit-Queue: Alex Ilin <alexilin@chromium.org>
Reviewed-by: default avatarTibor Goldschwendt <tiborg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#807893}
parent 5a885b23
...@@ -43,7 +43,10 @@ ChromeCustomizeThemesHandler::ChromeCustomizeThemesHandler( ...@@ -43,7 +43,10 @@ ChromeCustomizeThemesHandler::ChromeCustomizeThemesHandler(
content::NotificationService::AllSources()); content::NotificationService::AllSources());
} }
ChromeCustomizeThemesHandler::~ChromeCustomizeThemesHandler() = default; ChromeCustomizeThemesHandler::~ChromeCustomizeThemesHandler() {
// Revert any unconfirmed theme changes.
chrome_colors_service_->RevertThemeChangesForTab(web_contents_);
}
void ChromeCustomizeThemesHandler::ApplyAutogeneratedTheme( void ChromeCustomizeThemesHandler::ApplyAutogeneratedTheme(
const SkColor& frame_color) { const SkColor& frame_color) {
......
...@@ -142,6 +142,15 @@ class ChromeCustomizeThemesHandlerTest : public testing::Test { ...@@ -142,6 +142,15 @@ class ChromeCustomizeThemesHandlerTest : public testing::Test {
web_contents_, profile()); web_contents_, profile());
} }
void TearDown() override {
if (handler_) {
// Confirm all pending changes to not trigger a revert on destruction.
handler_->ConfirmThemeChanges();
}
}
void ResetHandler() { handler_.reset(); }
extensions::TestExtensionEnvironment* env() { extensions::TestExtensionEnvironment* env() {
return &extension_environment_; return &extension_environment_;
} }
...@@ -272,3 +281,19 @@ TEST_F(ChromeCustomizeThemesHandlerTest, ConfirmThemeChanges) { ...@@ -272,3 +281,19 @@ TEST_F(ChromeCustomizeThemesHandlerTest, ConfirmThemeChanges) {
handler()->RevertThemeChanges(); handler()->RevertThemeChanges();
EXPECT_TRUE(theme_service()->UsingDefaultTheme()); EXPECT_TRUE(theme_service()->UsingDefaultTheme());
} }
TEST_F(ChromeCustomizeThemesHandlerTest, ResetHandler) {
constexpr SkColor kAutogeneratedThemeColor = SK_ColorBLUE;
theme_service()->BuildAutogeneratedThemeFromColor(kAutogeneratedThemeColor);
handler()->ApplyDefaultTheme();
EXPECT_TRUE(theme_service()->UsingDefaultTheme());
// Cleans all pending SetTheme() calls.
base::RunLoop().RunUntilIdle();
// Handler should revert theme changes when it's destructed.
ResetHandler();
EXPECT_TRUE(theme_service()->UsingAutogeneratedTheme());
EXPECT_EQ(kAutogeneratedThemeColor,
theme_service()->GetAutogeneratedThemeColor());
}
...@@ -78,8 +78,9 @@ export class CustomizeThemesElement extends mixinBehaviors ...@@ -78,8 +78,9 @@ export class CustomizeThemesElement extends mixinBehaviors
/** @override */ /** @override */
disconnectedCallback() { disconnectedCallback() {
super.disconnectedCallback(); this.revertThemeChanges();
this.callbackRouter_.removeListener(assert(this.setThemeListenerId_)); this.callbackRouter_.removeListener(assert(this.setThemeListenerId_));
super.disconnectedCallback();
} }
confirmThemeChanges() { confirmThemeChanges() {
......
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