[Chrome Colors] Disable previous extension when autogenerated themes are installed.

In order to be able to revert to extension after autogenerated theme
is installed the extensions should be disabled.

Change-Id: I63e2bcd4b82c804039e9868e107a936c155dfd64
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1623502Reviewed-by: Gayane Petrosyan <gayane@chromium.org>
Reviewed-by: Evan Stade <estade@chromium.org>
Commit-Queue: Gayane Petrosyan <gayane@chromium.org>
Cr-Commit-Position: refs/heads/master@{#669125}

chrome/browser/themes/theme_service.cc

@@ -546,12 +546,18 @@ const ui::ThemeProvider& ThemeService::GetDefaultThemeProviderForProfile(
 }
 
 void ThemeService::BuildFromColor(SkColor color) {
+  base::Optional<std::string> previous_theme_id;
+  if (UsingExtensionTheme())
+    previous_theme_id = GetThemeID();
+
   scoped_refptr<BrowserThemePack> pack(
       new BrowserThemePack(CustomThemeSupplier::ThemeType::AUTOGENERATED));
   BrowserThemePack::BuildFromColor(color, pack.get());
   SwapThemeSupplier(std::move(pack));
   if (theme_supplier_) {
     SetThemePrefsForColor(color);
+    if (previous_theme_id.has_value())
+      DisableExtension(previous_theme_id.value());
     NotifyThemeChanged();
   }
 }
@@ -997,31 +1003,24 @@ void ThemeService::OnThemeBuiltFromExtension(
       FROM_HERE, base::BindOnce(&WritePackToDiskCallback,
                                 base::RetainedRef(pack), extension->path()));
   base::OnceClosure callback = ThemeService::GetRevertThemeCallback();
-  const std::string previous_theme_id = GetThemeID();
+  base::Optional<std::string> previous_theme_id;
+  if (UsingExtensionTheme())
+    previous_theme_id = GetThemeID();
+
   SwapThemeSupplier(std::move(pack));
   SetThemePrefsForExtension(extension);
   NotifyThemeChanged();
 
   // Same old theme, but the theme has changed (migrated) or auto-updated.
-  if (previous_theme_id == extension->id())
+  if (previous_theme_id.has_value() &&
+      previous_theme_id.value() == extension->id())
     return;
 
   base::RecordAction(UserMetricsAction("Themes_Installed"));
 
-  bool can_revert_theme = previous_theme_id == kDefaultThemeID;
-  if (previous_theme_id != kDefaultThemeID &&
-      service->GetInstalledExtension(previous_theme_id)) {
-    // Do not disable the previous theme if it is already uninstalled. Sending
-    // NOTIFICATION_BROWSER_THEME_CHANGED causes the previous theme to be
-    // uninstalled when the notification causes the remaining infobar to close
-    // and does not open any new infobars. See crbug.com/468280.
-
-    // Disable the old theme.
-    service->DisableExtension(previous_theme_id,
-                              extensions::disable_reason::DISABLE_USER_ACTION);
-
-    can_revert_theme = true;
-  }
+  bool can_revert_theme = true;
+  if (previous_theme_id.has_value())
+    can_revert_theme = DisableExtension(previous_theme_id.value());
 
   // Offer to revert to the old theme.
   if (can_revert_theme && !suppress_infobar && extension->is_theme()) {
@@ -1066,3 +1065,19 @@ void ThemeService::SetThemePrefsForColor(SkColor color) {
   profile_->GetPrefs()->SetString(prefs::kCurrentThemeID,
                                   kAutogeneratedThemeID);
 }
+
+bool ThemeService::DisableExtension(const std::string& extension_id) {
+  extensions::ExtensionService* service =
+      extensions::ExtensionSystem::Get(profile_)->extension_service();
+
+  if (service && service->GetInstalledExtension(extension_id)) {
+    // Do not disable the previous theme if it is already uninstalled. Sending
+    // NOTIFICATION_BROWSER_THEME_CHANGED causes the previous theme to be
+    // uninstalled when the notification causes the remaining infobar to close
+    // and does not open any new infobars. See crbug.com/468280.
+    service->DisableExtension(extension_id,
+                              extensions::disable_reason::DISABLE_USER_ACTION);
+    return true;
+  }
+  return false;
+}

chrome/browser/themes/theme_service.h

@@ -303,6 +303,8 @@ class ThemeService : public content::NotificationObserver,
   void SetThemePrefsForExtension(const extensions::Extension* extension);
   void SetThemePrefsForColor(SkColor color);
 
+  bool DisableExtension(const std::string& extension_id);
+
   ui::ResourceBundle& rb_;
   Profile* profile_;
 

chrome/browser/themes/theme_service_unittest.cc

@@ -508,4 +508,20 @@ TEST_F(ThemeServiceTest, BuildFromColorTest) {
   EXPECT_TRUE(path.empty());
 }
 
+TEST_F(ThemeServiceTest, BuildFromColor_DisableExtensionTest) {
+  ThemeService* theme_service =
+      ThemeServiceFactory::GetForProfile(profile_.get());
+  base::ScopedTempDir temp_dir1;
+  ASSERT_TRUE(temp_dir1.CreateUniqueTempDir());
+  const std::string& extension1_id =
+      LoadUnpackedMinimalThemeAt(temp_dir1.GetPath());
+  EXPECT_EQ(extension1_id, theme_service->GetThemeID());
+  EXPECT_TRUE(service_->IsExtensionEnabled(extension1_id));
+
+  // Setting autogenerated theme should disable previous theme.
+  theme_service->BuildFromColor(SkColorSetRGB(100, 100, 100));
+  EXPECT_TRUE(theme_service->UsingAutogenerated());
+  EXPECT_FALSE(service_->IsExtensionEnabled(extension1_id));
+}
+
 }  // namespace theme_service_internal