[DRAFT] Allow a user to revert to their previous theme without closing infobar

chrome/browser/themes/theme_service.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/themes/theme_service.h"
 
 #include "base/bind.h"
 #include "base/memory/ref_counted_memory.h"
 #include "base/prefs/pref_service.h"
 #include "base/sequenced_task_runner.h"
@@ skipping 37 matching lines @@
 const char* ThemeService::kDefaultThemeID = "";
 
 namespace {
 
 // The default theme if we've gone to the theme gallery and installed the
 // "Default" theme. We have to detect this case specifically. (By the time we
 // realize we've installed the default theme, we already have an extension
 // unpacked on the filesystem.)
 const char* kDefaultThemeGalleryID = "hkacjpbfdknhflllbcmjibkdeoafencn";
 
+// Wait this many seconds after startup to garbage collect unused themes.

[Yoyo Zhou, 2013/07/24 21:55:03]

Why wait?

+const int kRemoveUnusedThemesStartupDelay = 30;
+
 SkColor TintForUnderline(SkColor input) {
   return SkColorSetA(input, SkColorGetA(input) / 3);
 }
 
 SkColor IncreaseLightness(SkColor color, double percent) {
   color_utils::HSL result;
   color_utils::SkColorToHSL(color, &result);
   result.l += (1 - result.l) * percent;
   return color_utils::HSLToSkColor(result, SkColorGetA(color));
 }
 
 // Writes the theme pack to disk on a separate thread.
 void WritePackToDiskCallback(BrowserThemePack* pack,
                              const base::FilePath& path) {
   if (!pack->WriteToDisk(path))
     NOTREACHED() << "Could not write theme pack to disk";
 }
 
 }  // namespace
 
 ThemeService::ThemeService()
     : rb_(ResourceBundle::GetSharedInstance()),
       profile_(NULL),
       ready_(false),
-      number_of_infobars_(0) {
+      number_of_infobars_(0),
+      installed_pending_load_id_(kDefaultThemeID),
+      weak_ptr_factory_(this) {
 }
 
 ThemeService::~ThemeService() {
   FreePlatformCaches();
 }
 
 void ThemeService::Init(Profile* profile) {
   DCHECK(CalledOnValidThread());
   profile_ = profile;
 
   LoadThemePrefs();
 
   if (!ready_) {
     registrar_.Add(this,
                    chrome::NOTIFICATION_EXTENSIONS_READY,
                    content::Source<Profile>(profile_));
   }
 
   theme_syncable_service_.reset(new ThemeSyncableService(profile_, this));
+
+  registrar_.Add(this,
+                 chrome::NOTIFICATION_EXTENSION_INSTALLED,
+                 content::Source<Profile>(profile_));
+  registrar_.Add(this,
+                 chrome::NOTIFICATION_EXTENSION_LOADED,
+                 content::Source<Profile>(profile_));
+  registrar_.Add(this,
+                 chrome::NOTIFICATION_EXTENSION_ENABLED,
+                 content::Source<Profile>(profile_));
+  registrar_.Add(this,
+                 chrome::NOTIFICATION_EXTENSION_UNLOADED,
+                 content::Source<Profile>(profile_));
+
+  base::MessageLoop::current()->PostDelayedTask(FROM_HERE,
+      base::Bind(&ThemeService::RemoveUnusedThemes,
+                 weak_ptr_factory_.GetWeakPtr(),
+                 false),
+      base::TimeDelta::FromSeconds(kRemoveUnusedThemesStartupDelay));
 }
 
 gfx::Image ThemeService::GetImageNamed(int id) const {
   DCHECK(CalledOnValidThread());
 
   // For a managed user, use the special frame instead of the default one.
   // TODO(akuegel): Remove this once we have the default managed user theme.
   if (IsManagedUser()) {
     if (id == IDR_THEME_FRAME)
       id = IDR_MANAGED_USER_THEME_FRAME;
@@ skipping 110 matching lines @@
     data = theme_pack_->GetRawData(id, scale_factor);
   if (!data)
     data = rb_.LoadDataResourceBytesForScale(id, ui::SCALE_FACTOR_100P);
 
   return data;
 }
 
 void ThemeService::Observe(int type,
                            const content::NotificationSource& source,
                            const content::NotificationDetails& details) {
-  DCHECK(type == chrome::NOTIFICATION_EXTENSIONS_READY);
-  registrar_.Remove(this, chrome::NOTIFICATION_EXTENSIONS_READY,
-      content::Source<Profile>(profile_));
+  using content::Details;
+  switch (type) {
+    case chrome::NOTIFICATION_EXTENSIONS_READY:
+      registrar_.Remove(this, chrome::NOTIFICATION_EXTENSIONS_READY,
+          content::Source<Profile>(profile_));
 
-  MigrateTheme();
-  set_ready();
+      MigrateTheme();
+      set_ready();
 
-  // Send notification in case anyone requested data and cached it when the
-  // theme service was not ready yet.
-  NotifyThemeChanged();
+      // Send notification in case anyone requested data and cached it when the
+      // theme service was not ready yet.
+      NotifyThemeChanged();
+      break;
+    case chrome::NOTIFICATION_EXTENSION_INSTALLED:
+    {
+      Details<const extensions::InstalledExtensionInfo> installed_details(
+          details);
+      // The theme may be initially disabled. Wait till it is loaded (if ever)

[Yoyo Zhou, 2013/07/24 21:55:03]

I don't see why we need to listen for extension installs or enables
specifically. See comment below.

[pkotwicz, 2013/07/25 01:25:29]

You are right. I should be able to move registering for the
NOTIFICATION_EXTENSION_LOADED notification and I can avoid listening to
NOTIFICATION_EXTENSION_INSTALLED and NOTIFICATION_EXTENSION_ENABLED as a result.

As background the ThemeService is created before the ExtensionService is inited
but after ExtensionSystem is created.
It can also be used before the ExtensionService has finished initing which makes
things ugly.

+      // to change the current theme.
+      if (installed_details->extension->is_theme())
+        installed_pending_load_id_ = installed_details->extension->id();
+      break;
+    }
+    case chrome::NOTIFICATION_EXTENSION_LOADED:
+    {
+      const Extension* extension = Details<const Extension>(details).ptr();
+      if (extension->is_theme() &&
+          installed_pending_load_id_ != kDefaultThemeID &&
+          installed_pending_load_id_ == extension->id()) {

[Yoyo Zhou, 2013/07/24 21:55:03]

What's the scenario where we load a theme and don't expect to set it? (When we
enable an extension, it also gets loaded.) Whenever this would happen, the theme
would be enabled and yet not active, which seems to be what you're trying to
fix.

+        SetTheme(extension);
+      }
+      installed_pending_load_id_ = kDefaultThemeID;
+    }
+    case chrome::NOTIFICATION_EXTENSION_ENABLED:
+    {
+      const Extension* extension = Details<const Extension>(details).ptr();
+      if (extension->is_theme())
+        SetTheme(extension);
+      break;
+    }
+    case chrome::NOTIFICATION_EXTENSION_UNLOADED:
+    {
+      Details<const extensions::UnloadedExtensionInfo> unloaded_details(
+          details);
+      if (unloaded_details->reason != extension_misc::UNLOAD_REASON_UPDATE &&
+          unloaded_details->extension->is_theme() &&
+          unloaded_details->extension->id() == GetThemeID()) {
+        UseDefaultTheme();
+      }
+      break;
+    }
+  }
 }
 
 void ThemeService::SetTheme(const Extension* extension) {
+  DCHECK(extension->is_theme());
+  ExtensionService* service =
+      extensions::ExtensionSystem::Get(profile_)->extension_service();
+  if (!service->IsExtensionEnabled(extension->id())) {
+    // |extension| is disabled when reverting to the previous theme via an
+    // infobar.
+    service->EnableExtension(extension->id());
+    // Enabling the extension will call back to SetTheme().
+    return;
+  }
+
+  std::string previous_theme_id = GetThemeID();
+
   // Clear our image cache.
   FreePlatformCaches();
 
-  DCHECK(extension);
-  DCHECK(extension->is_theme());
-  if (DCHECK_IS_ON()) {
-    ExtensionService* service =
-        extensions::ExtensionSystem::Get(profile_)->extension_service();
-    DCHECK(service);
-    DCHECK(service->GetExtensionById(extension->id(), false));
-  }
-
   BuildFromExtension(extension);
   SaveThemeID(extension->id());
 
   NotifyThemeChanged();
   content::RecordAction(UserMetricsAction("Themes_Installed"));
+
+  if (previous_theme_id != kDefaultThemeID &&
+      previous_theme_id != extension->id()) {
+    // Disable the old theme.
+    service->DisableExtension(previous_theme_id,
+                              extensions::Extension::DISABLE_USER_ACTION);
+  }
 }
 
-void ThemeService::RemoveUnusedThemes() {
+void ThemeService::RemoveUnusedThemes(bool ignore_infobars) {
+  if (!ignore_infobars && number_of_infobars_ != 0)
+    return;
   if (!profile_)
     return;
   ExtensionService* service = profile_->GetExtensionService();
   if (!service)
     return;
   std::string current_theme = GetThemeID();
   std::vector<std::string> remove_list;
   const ExtensionSet* extensions = service->extensions();
   for (ExtensionSet::const_iterator it = extensions->begin();
        it != extensions->end(); ++it) {
     if ((*it)->is_theme() && (*it)->id() != current_theme) {
       remove_list.push_back((*it)->id());
     }
   }
+  extensions = service->disabled_extensions();
+  for (ExtensionSet::const_iterator it = extensions->begin();
+       it != extensions->end(); ++it) {
+    if ((*it)->is_theme() && (*it)->id() != current_theme) {
+      remove_list.push_back((*it)->id());
+    }
+  }
   for (size_t i = 0; i < remove_list.size(); ++i)
     service->UninstallExtension(remove_list[i], false, NULL);
 }
 
 void ThemeService::UseDefaultTheme() {
   ClearAllThemeData();
   NotifyThemeChanged();
   content::RecordAction(UserMetricsAction("Themes_Reset"));
 }
 
@@ skipping 26 matching lines @@
 }
 
 void ThemeService::ClearAllThemeData() {
   // Clear our image cache.
   FreePlatformCaches();
   theme_pack_ = NULL;
 
   profile_->GetPrefs()->ClearPref(prefs::kCurrentThemePackFilename);
   SaveThemeID(kDefaultThemeID);
 
-  RemoveUnusedThemes();
+  // There should be no more infobars. This may not be the case because of
+  // http://crbug.com/62154
+  base::MessageLoop::current()->PostTask(FROM_HERE,
+      base::Bind(&ThemeService::RemoveUnusedThemes,
+                 weak_ptr_factory_.GetWeakPtr(),
+                 true));
 }
 
 void ThemeService::LoadThemePrefs() {
   PrefService* prefs = profile_->GetPrefs();
 
   std::string current_id = GetThemeID();
   if (current_id == kDefaultThemeID) {
     set_ready();
     return;
   }
@@ skipping 105 matching lines @@
 }
 
 void ThemeService::OnInfobarDisplayed() {
   number_of_infobars_++;
 }
 
 void ThemeService::OnInfobarDestroyed() {
   number_of_infobars_--;
 
   if (number_of_infobars_ == 0)
-    RemoveUnusedThemes();
+    RemoveUnusedThemes(false);
 }
 
 ThemeSyncableService* ThemeService::GetThemeSyncableService() const {
   return theme_syncable_service_.get();
 }