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

3 files changed, 23 insertions, 17 deletions

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 @@ void ExtensionPrefs::OnExtensionInstalled(Extension* extension) {
   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 @@ class ExtensionPrefs {
   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 @@ class ExtensionPrefs {
 };
 
 #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 @@ void ExtensionsService::UninstallExtension(const std::string& extension_id,
 
   // Callers should not send us nonexistant extensions.
   DCHECK(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());
+
+  // Also copy the extension identifier since the reference might have been
+  // obtained via Extension::id().
+  std::string extension_id_copy(extension_id);
 
-  extension_prefs_->OnExtensionUninstalled(extension, external_uninstall);
+  // 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);
 }