Add TriggerOnUninstalled to ExtensionRegistry.

And change ExtensionToolbarModel to use it instead of NOTIFICATION_EXTENSION_UNINSTALLED.

R=kalman@chromium.org
BUG=354459
TEST=unit_test

Review URL: https://codereview.chromium.org/289073008

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@272649 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/extensions/extension_service.cc

@@ -768,6 +768,7 @@ bool ExtensionService::UninstallExtension(
       chrome::NOTIFICATION_EXTENSION_UNINSTALLED,
       content::Source<Profile>(profile_),
       content::Details<const Extension>(extension.get()));
+  ExtensionRegistry::Get(profile_)->TriggerOnUninstalled(extension.get());
 
   if (extension_sync_service_) {
     extension_sync_service_->ProcessSyncUninstallExtension(extension->id(),
@@ -1426,6 +1427,7 @@ void ExtensionService::RemoveComponentExtension(
         chrome::NOTIFICATION_EXTENSION_UNINSTALLED,
         content::Source<Profile>(profile_),
         content::Details<const Extension>(extension.get()));
+    ExtensionRegistry::Get(profile_)->TriggerOnUninstalled(extension.get());
   }
 }
 

chrome/browser/extensions/extension_toolbar_model.cc

@@ -191,32 +191,36 @@ void ExtensionToolbarModel::OnExtensionUnloaded(
   RemoveExtension(extension);
 }
 
+void ExtensionToolbarModel::OnExtensionUninstalled(
+    content::BrowserContext* browser_context,
+    const Extension* extension) {
+  // Remove the extension id from the ordered list, if it exists (the extension
+  // might not be represented in the list because it might not have an icon).
+  ExtensionIdList::iterator pos =
+      std::find(last_known_positions_.begin(),
+                last_known_positions_.end(), extension->id());
+
+  if (pos != last_known_positions_.end()) {
+    last_known_positions_.erase(pos);
+    UpdatePrefs();
+  }
+}
+
 void ExtensionToolbarModel::Observe(
     int type,
     const content::NotificationSource& source,
     const content::NotificationDetails& details) {
-  switch (type) {
-    case chrome::NOTIFICATION_EXTENSION_UNINSTALLED: {
-      const Extension* extension =
-          content::Details<const Extension>(details).ptr();
-      UninstalledExtension(extension);
-      break;
-    }
-    case chrome::NOTIFICATION_EXTENSION_BROWSER_ACTION_VISIBILITY_CHANGED: {
-      const Extension* extension =
-          ExtensionRegistry::Get(profile_)->GetExtensionById(
-              *content::Details<const std::string>(details).ptr(),
-              ExtensionRegistry::EVERYTHING);
-      if (ExtensionActionAPI::GetBrowserActionVisibility(extension_prefs_,
-                                                         extension->id())) {
-        AddExtension(extension);
-      } else {
-        RemoveExtension(extension);
-      }
-      break;
-    }
-    default:
-      NOTREACHED() << "Received unexpected notification";
+  DCHECK_EQ(chrome::NOTIFICATION_EXTENSION_BROWSER_ACTION_VISIBILITY_CHANGED,
+            type);
+  const Extension* extension =
+      ExtensionRegistry::Get(profile_)->GetExtensionById(
+          *content::Details<const std::string>(details).ptr(),
+          ExtensionRegistry::EVERYTHING);
+  if (ExtensionActionAPI::GetBrowserActionVisibility(extension_prefs_,
+                                                     extension->id())) {
+    AddExtension(extension);
+  } else {
+    RemoveExtension(extension);
   }
 }
 
@@ -227,9 +231,6 @@ void ExtensionToolbarModel::OnReady() {
   // changes so that the toolbar buttons can be shown in their stable ordering
   // taken from prefs.
   extension_registry_observer_.Add(registry);
-  registrar_.Add(this,
-                 chrome::NOTIFICATION_EXTENSION_UNINSTALLED,
-                 content::Source<Profile>(profile_));
   registrar_.Add(
       this,
       chrome::NOTIFICATION_EXTENSION_BROWSER_ACTION_VISIBILITY_CHANGED,
@@ -325,19 +326,6 @@ void ExtensionToolbarModel::RemoveExtension(const Extension* extension) {
   UpdatePrefs();
 }
 
-void ExtensionToolbarModel::UninstalledExtension(const Extension* extension) {
-  // Remove the extension id from the ordered list, if it exists (the extension
-  // might not be represented in the list because it might not have an icon).
-  ExtensionIdList::iterator pos =
-      std::find(last_known_positions_.begin(),
-                last_known_positions_.end(), extension->id());
-
-  if (pos != last_known_positions_.end()) {
-    last_known_positions_.erase(pos);
-    UpdatePrefs();
-  }
-}
-
 // Combine the currently enabled extensions that have browser actions (which
 // we get from the ExtensionRegistry) with the ordering we get from the
 // pref service. For robustness we use a somewhat inefficient process:
@@ -589,6 +577,6 @@ void ExtensionToolbarModel::StopHighlighting() {
     }
     FOR_EACH_OBSERVER(Observer, observers_, HighlightModeChanged(false));
   }
-};
+}
 
 }  // namespace extensions

chrome/browser/extensions/extension_toolbar_model.h

@@ -151,6 +151,8 @@ class ExtensionToolbarModel : public content::NotificationObserver,
       content::BrowserContext* browser_context,
       const Extension* extension,
       UnloadedExtensionInfo::Reason reason) OVERRIDE;
+  virtual void OnExtensionUninstalled(content::BrowserContext* browser_context,
+                                      const Extension* extension) OVERRIDE;
 
   // To be called after the extension service is ready; gets loaded extensions
   // from the extension service and their saved order from the pref service
@@ -171,7 +173,6 @@ class ExtensionToolbarModel : public content::NotificationObserver,
 
   void AddExtension(const Extension* extension);
   void RemoveExtension(const Extension* extension);
-  void UninstalledExtension(const Extension* extension);
 
   // The Profile this toolbar model is for.
   Profile* profile_;

extensions/browser/extension_registry.cc

@@ -65,6 +65,13 @@ void ExtensionRegistry::TriggerOnWillBeInstalled(const Extension* extension,
                         browser_context_, extension, is_update, old_name));
 }
 
+void ExtensionRegistry::TriggerOnUninstalled(const Extension* extension) {
+  DCHECK(!GenerateInstalledExtensionsSet()->Contains(extension->id()));
+  FOR_EACH_OBSERVER(ExtensionRegistryObserver,
+                    observers_,
+                    OnExtensionUninstalled(browser_context_, extension));
+}
+
 const Extension* ExtensionRegistry::GetExtensionById(const std::string& id,
                                                      int include_mask) const {
   std::string lowercase_id = StringToLowerASCII(id);

extensions/browser/extension_registry.h

@@ -82,6 +82,10 @@ class ExtensionRegistry : public KeyedService {
                                 bool is_update,
                                 const std::string& old_name);
 
+  // Invokes the observer method OnExtensionUninstalled(). The extension must
+  // not be any installed extension with |extension|'s ID.
+  void TriggerOnUninstalled(const Extension* extension);
+
   // Find an extension by ID using |include_mask| to pick the sets to search:
   //  * enabled_extensions()     --> ExtensionRegistry::ENABLED
   //  * disabled_extensions()    --> ExtensionRegistry::DISABLED

extensions/browser/extension_registry_observer.h

@@ -48,6 +48,11 @@ class ExtensionRegistryObserver {
       const Extension* extension,
       bool is_update,
       const std::string& old_name) {}
+
+  // Called after an extension is uninstalled. The extension no longer exsit in
+  // any of the ExtensionRegistry sets (enabled, disabled, etc.).
+  virtual void OnExtensionUninstalled(content::BrowserContext* browser_context,
+                                      const Extension* extension) {}
 };
 
 }  // namespace extensions

extensions/browser/extension_registry_unittest.cc

@@ -39,11 +39,13 @@ class TestObserver : public ExtensionRegistryObserver {
     loaded_.clear();
     unloaded_.clear();
     installed_.clear();
+    uninstalled_.clear();
   }
 
   const ExtensionList& loaded() { return loaded_; }
   const ExtensionList& unloaded() { return unloaded_; }
   const ExtensionList& installed() { return installed_; }
+  const ExtensionList& uninstalled() { return uninstalled_; }
 
  private:
   virtual void OnExtensionLoaded(content::BrowserContext* browser_context,
@@ -66,9 +68,15 @@ class TestObserver : public ExtensionRegistryObserver {
     installed_.push_back(extension);
   }
 
+  virtual void OnExtensionUninstalled(content::BrowserContext* browser_context,
+                                      const Extension* extension) OVERRIDE {
+    uninstalled_.push_back(extension);
+  }
+
   ExtensionList loaded_;
   ExtensionList unloaded_;
   ExtensionList installed_;
+  ExtensionList uninstalled_;
 };
 
 TEST_F(ExtensionRegistryTest, FillAndClearRegistry) {
@@ -251,6 +259,10 @@ TEST_F(ExtensionRegistryTest, Observer) {
   EXPECT_TRUE(HasSingleExtension(observer.unloaded(), extension.get()));
   observer.Reset();
 
+  registry.TriggerOnUninstalled(extension);
+  EXPECT_TRUE(observer.installed().empty());
+  EXPECT_TRUE(HasSingleExtension(observer.uninstalled(), extension.get()));
+
   registry.RemoveObserver(&observer);
 }