Rework ordering in ExtensionsService::UninstallExtension()
BUG=39147
TEST=Existing tests in extensions_service_unittest.cc
Review URL: http://codereview.chromium.org/1278003
Patch from Mattias Nissler.
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@42752 0039d316-1c4b-4281-b951-d872f2087c98
diff --git a/chrome/browser/extensions/extension_prefs.cc b/chrome/browser/extensions/extension_prefs.cc
index 3040903..015d7cb 100644
--- a/chrome/browser/extensions/extension_prefs.cc
+++ b/chrome/browser/extensions/extension_prefs.cc
@@ -385,19 +385,19 @@
prefs_->SavePersistentPrefs();
}
-void ExtensionPrefs::OnExtensionUninstalled(const Extension* extension,
+void ExtensionPrefs::OnExtensionUninstalled(const std::string& extension_id,
+ const Extension::Location& location,
bool external_uninstall) {
// For external extensions, we save a preference reminding ourself not to try
// and install the extension anymore (except when |external_uninstall| is
// true, which signifies that the registry key was deleted or the pref file
// no longer lists the extension).
- if (!external_uninstall &&
- Extension::IsExternalLocation(extension->location())) {
- UpdateExtensionPref(extension->id(), kPrefState,
+ if (!external_uninstall && Extension::IsExternalLocation(location)) {
+ UpdateExtensionPref(extension_id, kPrefState,
Value::CreateIntegerValue(Extension::KILLBIT));
prefs_->ScheduleSavePersistentPrefs();
} else {
- DeleteExtensionPrefs(extension->id());
+ DeleteExtensionPrefs(extension_id);
}
}
diff --git a/chrome/browser/extensions/extension_prefs.h b/chrome/browser/extensions/extension_prefs.h
index 66faf77..e6de376 100644
--- a/chrome/browser/extensions/extension_prefs.h
+++ b/chrome/browser/extensions/extension_prefs.h
@@ -50,7 +50,8 @@
void OnExtensionInstalled(Extension* extension);
// Called when an extension is uninstalled, so that prefs get cleaned up.
- void OnExtensionUninstalled(const Extension* extension,
+ void OnExtensionUninstalled(const std::string& extension_id,
+ const Extension::Location& location,
bool external_uninstall);
// Returns the state (enabled/disabled) of the given extension.
@@ -162,4 +163,3 @@
};
#endif // CHROME_BROWSER_EXTENSIONS_EXTENSION_PREFS_H_
-
diff --git a/chrome/browser/extensions/extensions_service.cc b/chrome/browser/extensions/extensions_service.cc
index 6807f6a..9c24464 100644
--- a/chrome/browser/extensions/extensions_service.cc
+++ b/chrome/browser/extensions/extensions_service.cc
@@ -325,26 +325,32 @@
// Callers should not send us nonexistant extensions.
DCHECK(extension);
- GURL extension_url(extension->url());
- extension_prefs_->OnExtensionUninstalled(extension, external_uninstall);
+ // 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());
+
+ // Also copy the extension identifier since the reference might have been
+ // obtained via Extension::id().
+ std::string extension_id_copy(extension_id);
+
+ // Unload before doing more cleanup to ensure that nothing is hanging on to
+ // any of these resources.
+ UnloadExtension(extension_id);
+
+ extension_prefs_->OnExtensionUninstalled(extension_id_copy, location,
+ external_uninstall);
// Tell the backend to start deleting installed extensions on the file thread.
- if (Extension::LOAD != extension->location()) {
+ if (Extension::LOAD != location) {
ChromeThread::PostTask(
ChromeThread::FILE, FROM_HERE,
NewRunnableFunction(
- &extension_file_util::UninstallExtension, extension_id,
+ &extension_file_util::UninstallExtension, extension_id_copy,
install_directory_));
}
- ExtensionDOMUI::UnregisterChromeURLOverrides(profile_,
- extension->GetChromeURLOverrides());
-
- // TODO(mnissler, erikkay) Check whether we should really unload the extension
- // first, so we we're sure it's not running while we clean up.
- UnloadExtension(extension_id);
-
ClearExtensionData(extension_url);
}
