From 1d7f248f583f82237a373154ec9726238036aecc Mon Sep 17 00:00:00 2001 From: "limasdf@gmail.com" Date: Sat, 24 May 2014 00:11:51 +0000 Subject: [PATCH] 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 | 2 + .../browser/extensions/extension_toolbar_model.cc | 66 +++++++++------------- .../browser/extensions/extension_toolbar_model.h | 3 +- extensions/browser/extension_registry.cc | 7 +++ extensions/browser/extension_registry.h | 4 ++ extensions/browser/extension_registry_observer.h | 5 ++ extensions/browser/extension_registry_unittest.cc | 12 ++++ 7 files changed, 59 insertions(+), 40 deletions(-) diff --git a/chrome/browser/extensions/extension_service.cc b/chrome/browser/extensions/extension_service.cc index e069715dca05..2390554fa8c0 100644 --- a/chrome/browser/extensions/extension_service.cc +++ b/chrome/browser/extensions/extension_service.cc @@ -768,6 +768,7 @@ bool ExtensionService::UninstallExtension( chrome::NOTIFICATION_EXTENSION_UNINSTALLED, content::Source(profile_), content::Details(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_), content::Details(extension.get())); + ExtensionRegistry::Get(profile_)->TriggerOnUninstalled(extension.get()); } } diff --git a/chrome/browser/extensions/extension_toolbar_model.cc b/chrome/browser/extensions/extension_toolbar_model.cc index 7c9d5bfdd08b..62050ab606cd 100644 --- a/chrome/browser/extensions/extension_toolbar_model.cc +++ b/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(details).ptr(); - UninstalledExtension(extension); - break; - } - case chrome::NOTIFICATION_EXTENSION_BROWSER_ACTION_VISIBILITY_CHANGED: { - const Extension* extension = - ExtensionRegistry::Get(profile_)->GetExtensionById( - *content::Details(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(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_)); 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 diff --git a/chrome/browser/extensions/extension_toolbar_model.h b/chrome/browser/extensions/extension_toolbar_model.h index df5710706e73..3626f846a28e 100644 --- a/chrome/browser/extensions/extension_toolbar_model.h +++ b/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_; diff --git a/extensions/browser/extension_registry.cc b/extensions/browser/extension_registry.cc index 76783f0d336d..2b4d005d4db8 100644 --- a/extensions/browser/extension_registry.cc +++ b/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); diff --git a/extensions/browser/extension_registry.h b/extensions/browser/extension_registry.h index 65ee7517b99b..0087645d0894 100644 --- a/extensions/browser/extension_registry.h +++ b/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 diff --git a/extensions/browser/extension_registry_observer.h b/extensions/browser/extension_registry_observer.h index 6ad066614d3e..b6cf8e0907fe 100644 --- a/extensions/browser/extension_registry_observer.h +++ b/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 diff --git a/extensions/browser/extension_registry_unittest.cc b/extensions/browser/extension_registry_unittest.cc index 8281ee0d5145..5566fffa241d 100644 --- a/extensions/browser/extension_registry_unittest.cc +++ b/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); } -- 2.11.4.GIT