Changed EXTENSION_UNINSTALLED notification to happen after uninstallation.
The important part is that it comes after the EXTENSION_UNLOADED
notification is sent. This makes it easier on the listeners, as they
can assume that extension notifications other than EXTENSION_UNINSTALLED
are sent for currently-installed extensions.
BUG=54415
TEST=BackgroundModeManagerTest
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=60834
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=60848
Review URL: http://codereview.chromium.org/3461025
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@61089 0039d316-1c4b-4281-b951-d872f2087c98
diff --git a/chrome/browser/background_mode_manager.cc b/chrome/browser/background_mode_manager.cc
index ba952cb..8946e3c 100644
--- a/chrome/browser/background_mode_manager.cc
+++ b/chrome/browser/background_mode_manager.cc
@@ -188,6 +188,20 @@
allowed);
}
+namespace {
+
+bool HasBackgroundAppPermission(
+ const std::set<std::string>& api_permissions) {
+ return Extension::HasApiPermission(
+ api_permissions, Extension::kBackgroundPermission);
+}
+
+bool IsBackgroundApp(const Extension& extension) {
+ return HasBackgroundAppPermission(extension.api_permissions());
+}
+
+} // namespace
+
void BackgroundModeManager::Observe(NotificationType type,
const NotificationSource& source,
const NotificationDetails& details) {
@@ -202,19 +216,21 @@
#endif
break;
case NotificationType::EXTENSION_LOADED:
- if (IsBackgroundApp(Details<Extension>(details).ptr()))
+ if (IsBackgroundApp(*Details<Extension>(details).ptr()))
OnBackgroundAppLoaded();
break;
case NotificationType::EXTENSION_UNLOADED:
- if (IsBackgroundApp(Details<Extension>(details).ptr()))
+ if (IsBackgroundApp(*Details<Extension>(details).ptr()))
OnBackgroundAppUnloaded();
break;
case NotificationType::EXTENSION_INSTALLED:
- if (IsBackgroundApp(Details<Extension>(details).ptr()))
+ if (IsBackgroundApp(*Details<Extension>(details).ptr()))
OnBackgroundAppInstalled();
break;
case NotificationType::EXTENSION_UNINSTALLED:
- if (IsBackgroundApp(Details<Extension>(details).ptr()))
+ if (HasBackgroundAppPermission(
+ Details<UninstalledExtensionInfo>(details).ptr()->
+ extension_api_permissions))
OnBackgroundAppUninstalled();
break;
case NotificationType::APP_TERMINATING:
@@ -236,11 +252,6 @@
}
}
-bool BackgroundModeManager::IsBackgroundApp(Extension* extension) {
- return extension->HasApiPermission(Extension::kBackgroundPermission);
-}
-
-
void BackgroundModeManager::OnBackgroundModePrefChanged() {
// Background mode has been enabled/disabled in preferences, so update our
// state accordingly.
@@ -308,9 +319,9 @@
}
void BackgroundModeManager::OnBackgroundAppUninstalled() {
- // When uninstalling a background app, disable launch on startup if it's the
- // last one.
- if (IsBackgroundModeEnabled() && background_app_count_ == 1)
+ // When uninstalling a background app, disable launch on startup if
+ // we have no more background apps.
+ if (IsBackgroundModeEnabled() && background_app_count_ == 0)
EnableLaunchOnStartup(false);
}
diff --git a/chrome/browser/background_mode_manager.h b/chrome/browser/background_mode_manager.h
index 52f37da..9c3de16 100644
--- a/chrome/browser/background_mode_manager.h
+++ b/chrome/browser/background_mode_manager.h
@@ -86,9 +86,6 @@
// Invoked when the kBackgroundModeEnabled preference has changed.
void OnBackgroundModePrefChanged();
- // Returns true if the passed extension is a background app.
- bool IsBackgroundApp(Extension* extension);
-
// Returns true if the background mode preference is enabled
bool IsBackgroundModeEnabled();
diff --git a/chrome/browser/background_mode_manager_unittest.cc b/chrome/browser/background_mode_manager_unittest.cc
index 075db814..4c43610 100644
--- a/chrome/browser/background_mode_manager_unittest.cc
+++ b/chrome/browser/background_mode_manager_unittest.cc
@@ -56,15 +56,15 @@
TestingProfile profile;
TestBackgroundModeManager manager(&profile, command_line_.get());
// Call to AppInstalled() will cause chrome to be set to launch on startup,
- // and call to AppUninstalling() set chrome to not launch on startup.
+ // and call to AppUninstalled() set chrome to not launch on startup.
EXPECT_CALL(manager, EnableLaunchOnStartup(true));
EXPECT_CALL(manager, CreateStatusTrayIcon());
- EXPECT_CALL(manager, EnableLaunchOnStartup(false));
EXPECT_CALL(manager, RemoveStatusTrayIcon());
+ EXPECT_CALL(manager, EnableLaunchOnStartup(false));
manager.OnBackgroundAppInstalled();
manager.OnBackgroundAppLoaded();
- manager.OnBackgroundAppUninstalled();
manager.OnBackgroundAppUnloaded();
+ manager.OnBackgroundAppUninstalled();
}
TEST_F(BackgroundModeManagerTest, BackgroundPrefDisabled) {
@@ -72,15 +72,15 @@
TestingProfile profile;
profile.GetPrefs()->SetBoolean(prefs::kBackgroundModeEnabled, false);
TestBackgroundModeManager manager(&profile, command_line_.get());
+ EXPECT_CALL(manager, CreateStatusTrayIcon()).Times(0);
// Should not change launch on startup status when installing/uninstalling
// if background mode is disabled.
EXPECT_CALL(manager, EnableLaunchOnStartup(true)).Times(0);
- EXPECT_CALL(manager, CreateStatusTrayIcon()).Times(0);
manager.OnBackgroundAppInstalled();
manager.OnBackgroundAppLoaded();
EXPECT_FALSE(BrowserList::WillKeepAlive());
- manager.OnBackgroundAppUninstalled();
manager.OnBackgroundAppUnloaded();
+ manager.OnBackgroundAppUninstalled();
}
TEST_F(BackgroundModeManagerTest, BackgroundPrefDynamicDisable) {
diff --git a/chrome/browser/extensions/extension_management_api.cc b/chrome/browser/extensions/extension_management_api.cc
index 738c6cf..fe6888a 100644
--- a/chrome/browser/extensions/extension_management_api.cc
+++ b/chrome/browser/extensions/extension_management_api.cc
@@ -215,9 +215,8 @@
ListValue args;
if (event_name == events::kOnExtensionUninstalled) {
- // TODO(akalin) - change this to get the id from UninstalledExtensionInfo
- // when re-landing change to how we send the uninstall notification.
- std::string extension_id = Details<Extension>(details).ptr()->id();
+ const std::string& extension_id =
+ Details<UninstalledExtensionInfo>(details).ptr()->extension_id;
args.Append(Value::CreateStringValue(extension_id));
} else {
Extension* extension = Details<Extension>(details).ptr();
diff --git a/chrome/browser/extensions/extensions_service.cc b/chrome/browser/extensions/extensions_service.cc
index 737f6894..03699fa8 100644
--- a/chrome/browser/extensions/extensions_service.cc
+++ b/chrome/browser/extensions/extensions_service.cc
@@ -768,16 +768,11 @@
// Callers should not send us nonexistent extensions.
DCHECK(extension);
- // Notify interested parties that we're uninstalling this extension.
- NotificationService::current()->Notify(
- NotificationType::EXTENSION_UNINSTALLED,
- Source<Profile>(profile_),
- Details<Extension>(extension));
-
// Get hold of information we need after unloading, since the extension
// pointer will be invalid then.
GURL extension_url(extension->url());
Extension::Location location(extension->location());
+ UninstalledExtensionInfo uninstalled_extension_info(*extension);
// Also copy the extension identifier since the reference might have been
// obtained via Extension::id().
@@ -804,6 +799,12 @@
}
ClearExtensionData(extension_url);
+
+ // Notify interested parties that we've uninstalled this extension.
+ NotificationService::current()->Notify(
+ NotificationType::EXTENSION_UNINSTALLED,
+ Source<Profile>(profile_),
+ Details<UninstalledExtensionInfo>(&uninstalled_extension_info));
}
void ExtensionsService::ClearExtensionData(const GURL& extension_url) {
