Remove NOTIFICATION_EXTENSION_ENABLED.

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 <stddef.h>
 
 #include <algorithm>
 
@@ skipping 31 matching lines @@
 #include "extensions/common/extension_set.h"
 #include "extensions/features/features.h"
 #include "ui/base/layout.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/gfx/color_palette.h"
 #include "ui/gfx/image/image_skia.h"
 #include "ui/native_theme/common_theme.h"
 #include "ui/native_theme/native_theme.h"
 
 #if BUILDFLAG(ENABLE_EXTENSIONS)
+#include "base/scoped_observer.h"
 #include "extensions/browser/extension_registry_observer.h"
 #endif
 
 #if BUILDFLAG(ENABLE_SUPERVISED_USERS)
 #include "chrome/browser/supervised_user/supervised_user_theme.h"
 #endif
 
 using base::UserMetricsAction;
 using content::BrowserThread;
 using extensions::Extension;
@@ skipping 91 matching lines @@
   return theme_service_.GetRawData(id, scale_factor);
 }
 
 
 // ThemeService::ThemeObserver ------------------------------------------------
 
 #if BUILDFLAG(ENABLE_EXTENSIONS)
 class ThemeService::ThemeObserver
     : public extensions::ExtensionRegistryObserver {
  public:
-  explicit ThemeObserver(ThemeService* service) : theme_service_(service) {
-    extensions::ExtensionRegistry::Get(theme_service_->profile_)
-        ->AddObserver(this);
+  explicit ThemeObserver(ThemeService* service)
+      : theme_service_(service), extension_registry_observer_(this) {
+    extension_registry_observer_.Add(
+        extensions::ExtensionRegistry::Get(theme_service_->profile_));
   }
 
   ~ThemeObserver() override {
-    extensions::ExtensionRegistry::Get(theme_service_->profile_)
-        ->RemoveObserver(this);
   }
 
  private:
-  void OnExtensionWillBeInstalled(content::BrowserContext* browser_context,
-                                  const extensions::Extension* extension,
-                                  bool is_update,
-                                  const std::string& old_name) override {
-    if (extension->is_theme()) {
-      // The theme may be initially disabled. Wait till it is loaded (if ever).
-      theme_service_->installed_pending_load_id_ = extension->id();
-    }
-  }
-
+  // extensions::ExtensionRegistryObserver::
   void OnExtensionLoaded(content::BrowserContext* browser_context,
                          const extensions::Extension* extension) override {
-    if (extension->is_theme() &&
-        theme_service_->installed_pending_load_id_ != kDefaultThemeID &&
-        theme_service_->installed_pending_load_id_ == extension->id()) {
-      theme_service_->SetTheme(extension);
-    }
-    theme_service_->installed_pending_load_id_ = kDefaultThemeID;
+    if (!extension->is_theme())
+      return;
+
+    // If NOTIFICATION_EXTENSIONS_READY_DEPRECATED already set this theme, bail
+    // out.

[pkotwicz, 2017/05/31 15:46:53]

Remove the comment because it is incorrect. ThemeService::SetTheme() should be
called when a theme installed not each time that Chrome is launched.

[lazyboy, 2017/05/31 21:07:41]

Done.

Also, as discussed offline, I have revived OnExtensionWillBeInstalled +
installed_pending_load_id_ to detect theme update. Now we can detect whether
|extension| is an updated theme or not, then we set the theme if it is an
updated theme. Otherwise, your suggestions applies: do not SetTheme if it is an
existing theme.
PTAL.

+    if (extension->id() == theme_service_->GetThemeID())
+      return;
+
+    // Set the new theme during extension load.
+    // Even if the theme was an update of a disabled extension, it shouldn't be
+    // loaded.
+    theme_service_->SetTheme(extension);
   }
 
   void OnExtensionUnloaded(
       content::BrowserContext* browser_context,
       const extensions::Extension* extension,
       extensions::UnloadedExtensionReason reason) override {
     if (reason != extensions::UnloadedExtensionReason::UPDATE &&
         reason != extensions::UnloadedExtensionReason::LOCK_ALL &&
         extension->is_theme() &&
         extension->id() == theme_service_->GetThemeID()) {
       theme_service_->UseDefaultTheme();
     }
   }
 
   ThemeService* theme_service_;
+
+  ScopedObserver<extensions::ExtensionRegistry,
+                 extensions::ExtensionRegistryObserver>
+      extension_registry_observer_;
+
+  DISALLOW_COPY_AND_ASSIGN(ThemeObserver);
 };
 #endif  // BUILDFLAG(ENABLE_EXTENSIONS)
 
 
 // ThemeService ---------------------------------------------------------------
 
 // The default theme if we haven't installed a theme yet or if we've clicked
 // the "Use Classic" button.
 const char ThemeService::kDefaultThemeID[] = "";
 
 ThemeService::ThemeService()
     : ready_(false),
       rb_(ResourceBundle::GetSharedInstance()),
       profile_(nullptr),
-      installed_pending_load_id_(kDefaultThemeID),
       number_of_infobars_(0),
       original_theme_provider_(*this, false),
       incognito_theme_provider_(*this, true),
       weak_ptr_factory_(this) {}
 
 ThemeService::~ThemeService() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   FreePlatformCaches();
 }
 
@@ skipping 20 matching lines @@
                            const content::NotificationSource& source,
                            const content::NotificationDetails& details) {
   using content::Details;
   switch (type) {
     case extensions::NOTIFICATION_EXTENSIONS_READY_DEPRECATED:
       registrar_.Remove(this,
                         extensions::NOTIFICATION_EXTENSIONS_READY_DEPRECATED,
                         content::Source<Profile>(profile_));
       OnExtensionServiceReady();
       break;
-    case extensions::NOTIFICATION_EXTENSION_ENABLED: {
-      const Extension* extension = Details<const Extension>(details).ptr();
-      if (extension->is_theme())
-        SetTheme(extension);
-      break;
-    }
     default:
       NOTREACHED();
   }
 }
 
 void ThemeService::SetTheme(const Extension* extension) {
   DCHECK(extension->is_theme());
   ExtensionService* service =
       extensions::ExtensionSystem::Get(profile_)->extension_service();
   if (!service->IsExtensionEnabled(extension->id())) {
@@ skipping 507 matching lines @@
     // be recreated from the extension.
     MigrateTheme();
     set_ready();
 
     // Send notification in case anyone requested data and cached it when the
     // theme service was not ready yet.
     NotifyThemeChanged();
   }
 
 #if BUILDFLAG(ENABLE_EXTENSIONS)
-  theme_observer_.reset(new ThemeObserver(this));
+  theme_observer_ = base::MakeUnique<ThemeObserver>(this);
 #endif
 
-  registrar_.Add(this,
-                 extensions::NOTIFICATION_EXTENSION_ENABLED,
-                 content::Source<Profile>(profile_));
-
   base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
       FROM_HERE, base::Bind(&ThemeService::RemoveUnusedThemes,
                             weak_ptr_factory_.GetWeakPtr(), false),
       base::TimeDelta::FromSeconds(kRemoveUnusedThemesStartupDelay));
 }
 
 void ThemeService::MigrateTheme() {
   // TODO(erg): We need to pop up a dialog informing the user that their
   // theme is being migrated.
   ExtensionService* service =
@@ skipping 57 matching lines @@
 
 #if BUILDFLAG(ENABLE_SUPERVISED_USERS)
 bool ThemeService::IsSupervisedUser() const {
   return profile_->IsSupervised();
 }
 
 void ThemeService::SetSupervisedUserTheme() {
   SetCustomDefaultTheme(new SupervisedUserTheme);
 }
 #endif