Unify the post-installation steps between delayed installs and regular ones.

Also did some cleanup by removing unnecessary extra scoped_refptrs. They were
leftovers from the days before Extensions were refcounted.

BUG=37971

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

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

5 files changed, 41 insertions, 49 deletions

diff --git a/chrome/browser/extensions/extension_service.cc b/chrome/browser/extensions/extension_service.cc
index 64654c9..09f50f2 100644
--- a/chrome/browser/extensions/extension_service.cc
+++ b/chrome/browser/extensions/extension_service.cc
@@ -752,7 +752,7 @@ void ExtensionService::ReloadExtensionWithEvents(
   }
 
   if (pending_extension_updates_.Contains(extension_id)) {
-    FinishInstallation(extension_id);
+    FinishDelayedInstallation(extension_id);
     return;
   }
 
@@ -2039,9 +2039,6 @@ void ExtensionService::OnLoadedInstalledExtensions() {
 }
 
 void ExtensionService::AddExtension(const Extension* extension) {
-  // Ensure extension is deleted unless we transfer ownership.
-  scoped_refptr<const Extension> scoped_extension(extension);
-
   // TODO(jstritar): We may be able to get rid of this branch by overriding the
   // default extension state to DISABLED when the --disable-extensions flag
   // is set (http://crbug.com/29067).
@@ -2073,7 +2070,7 @@ void ExtensionService::AddExtension(const Extension* extension) {
   MaybeWipeout(extension);
 
   if (extension_prefs_->IsExtensionDisabled(extension->id())) {
-    disabled_extensions_.Insert(scoped_extension);
+    disabled_extensions_.Insert(extension);
     SyncExtensionChangeIfNeeded(*extension);
     content::NotificationService::current()->Notify(
         chrome::NOTIFICATION_EXTENSION_UPDATE_DISABLED,
@@ -2094,7 +2091,7 @@ void ExtensionService::AddExtension(const Extension* extension) {
         extension->id(), syncer::StringOrdinal());
   }
 
-  extensions_.Insert(scoped_extension);
+  extensions_.Insert(extension);
   SyncExtensionChangeIfNeeded(*extension);
   NotifyExtensionLoaded(extension);
   DoPostLoadTasks(extension);
@@ -2282,8 +2279,6 @@ void ExtensionService::OnExtensionInstalled(
     bool wait_for_idle) {
   CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
-  // Ensure extension is deleted unless we transfer ownership.
-  scoped_refptr<const Extension> scoped_extension(extension);
   const std::string& id = extension->id();
   bool initial_enable = ShouldEnableOnInstall(extension);
   const extensions::PendingExtensionInfo* pending_extension_info = NULL;
@@ -2362,7 +2357,7 @@ void ExtensionService::OnExtensionInstalled(
         initial_enable ? Extension::ENABLED : Extension::DISABLED);
 
     // Transfer ownership of |extension|.
-    pending_extension_updates_.Insert(scoped_extension);
+    pending_extension_updates_.Insert(extension);
 
     // Notify extension of available update.
     extensions::RuntimeEventRouter::DispatchOnUpdateAvailableEvent(
@@ -2382,28 +2377,10 @@ void ExtensionService::OnExtensionInstalled(
     extension_prefs_->SetAllowFileAccess(id, true);
   }
 
-  content::NotificationService::current()->Notify(
-      chrome::NOTIFICATION_EXTENSION_INSTALLED,
-      content::Source<Profile>(profile_),
-      content::Details<const Extension>(extension));
-
-  bool unacknowledged_external = IsUnacknowledgedExternalExtension(extension);
-
-  // Transfer ownership of |extension| to AddExtension.
-  AddExtension(scoped_extension);
-
-  // If this is a new external extension that was disabled, alert the user
-  // so he can reenable it. We do this last so that it has already been
-  // added to our list of extensions.
-  if (unacknowledged_external) {
-    UpdateExternalExtensionAlert();
-    UMA_HISTOGRAM_ENUMERATION("Extensions.ExternalExtensionEvent",
-                              EXTERNAL_EXTENSION_INSTALLED,
-                              EXTERNAL_EXTENSION_BUCKET_BOUNDARY);
-  }
+  FinishInstallation(extension);
 }
 
-void ExtensionService::MaybeFinishInstallation(
+void ExtensionService::MaybeFinishDelayedInstallation(
     const std::string& extension_id) {
   // Check if the extension already got updated.
   if (!pending_extension_updates_.Contains(extension_id))
@@ -2412,32 +2389,41 @@ void ExtensionService::MaybeFinishInstallation(
   if (!IsExtensionIdle(extension_id))
     return;
 
-  FinishInstallation(extension_id);
+  FinishDelayedInstallation(extension_id);
 }
 
-void ExtensionService::FinishInstallation(const std::string& extension_id) {
-  const Extension* extension = GetPendingExtensionUpdate(extension_id);
+void ExtensionService::FinishDelayedInstallation(
+    const std::string& extension_id) {
+  scoped_refptr<const Extension> extension(
+      GetPendingExtensionUpdate(extension_id));
   CHECK(extension);
-  // Ensure extension is deleted unless we transfer ownership.
-  scoped_refptr<const Extension> scoped_extension(extension);
   pending_extension_updates_.Remove(extension_id);
 
   if (!extension_prefs_->FinishIdleInstallInfo(extension_id))
     NOTREACHED();
 
+  FinishInstallation(extension);
+}
+
+void ExtensionService::FinishInstallation(const Extension* extension) {
   content::NotificationService::current()->Notify(
       chrome::NOTIFICATION_EXTENSION_INSTALLED,
       content::Source<Profile>(profile_),
       content::Details<const Extension>(extension));
 
-  // Transfer ownership of |extension| to AddExtension.
-  AddExtension(scoped_extension);
+  bool unacknowledged_external = IsUnacknowledgedExternalExtension(extension);
+
+  AddExtension(extension);
 
   // If this is a new external extension that was disabled, alert the user
-  // so he can reenable it.
-  if (Extension::IsExternalLocation(extension->location()) &&
-      extension_prefs_->IsExtensionDisabled(extension_id))
+  // so he can reenable it. We do this last so that it has already been
+  // added to our list of extensions.
+  if (unacknowledged_external) {
     UpdateExternalExtensionAlert();
+    UMA_HISTOGRAM_ENUMERATION("Extensions.ExternalExtensionEvent",
+                              EXTERNAL_EXTENSION_INSTALLED,
+                              EXTERNAL_EXTENSION_BUCKET_BOUNDARY);
+  }
 }
 
 const Extension* ExtensionService::GetPendingExtensionUpdate(
@@ -2711,7 +2697,7 @@ void ExtensionService::Observe(int type,
         // so maybe finish installation.
         MessageLoop::current()->PostDelayedTask(
             FROM_HERE,
-            base::Bind(&ExtensionService::MaybeFinishInstallation,
+            base::Bind(&ExtensionService::MaybeFinishDelayedInstallation,
                        AsWeakPtr(), extension_id),
             base::TimeDelta::FromSeconds(kUpdateIdleDelay));
       }
diff --git a/chrome/browser/extensions/extension_service.h b/chrome/browser/extensions/extension_service.h
index 38ec4ac..6aa659d 100644
--- a/chrome/browser/extensions/extension_service.h
+++ b/chrome/browser/extensions/extension_service.h
@@ -115,7 +115,7 @@ class ExtensionServiceInterface : public syncer::SyncableService {
 
   virtual const extensions::Extension* GetPendingExtensionUpdate(
       const std::string& extension_id) const = 0;
-  virtual void FinishInstallation(const std::string& extension_id) = 0;
+  virtual void FinishDelayedInstallation(const std::string& extension_id) = 0;
 
   virtual bool IsExtensionEnabled(const std::string& extension_id) const = 0;
   virtual bool IsExternalExtensionUninstalled(
@@ -430,14 +430,15 @@ class ExtensionService
       bool has_requirement_errors,
       bool wait_for_idle);
 
+  // Similar to FinishInstallation, but first checks if there still is an update
+  // pending for the extension, and makes sure the extension is still idle.
+  void MaybeFinishDelayedInstallation(const std::string& extension_id);
+
   // Finishes installation of an update for an extension with the specified id,
   // when installation of that extension was previously delayed because the
   // extension was in use.
-  virtual void FinishInstallation(const std::string& extension_id) OVERRIDE;
-
-  // Similar to FinishInstallation, but first checks if there still is an update
-  // pending for the extension, and makes sure the extension is still idle.
-  void MaybeFinishInstallation(const std::string& extension_id);
+  virtual void FinishDelayedInstallation(
+     const std::string& extension_id) OVERRIDE;
 
   // Returns an update for an extension with the specified id, if installation
   // of that update was previously delayed because the extension was in use. If
@@ -769,6 +770,9 @@ class ExtensionService
   void NotifyExtensionUnloaded(const extensions::Extension* extension,
                                extension_misc::UnloadedExtensionReason reason);
 
+  // Common helper to finish installing the given extension.
+  void FinishInstallation(const extensions::Extension* extension);
+
   // Reloads |extension_id| and then dispatches to it the PostReloadEvents
   // indicated by |events|.
   void ReloadExtensionWithEvents(const std::string& extension_id,
diff --git a/chrome/browser/extensions/test_extension_service.cc b/chrome/browser/extensions/test_extension_service.cc
index 8dfedfb..0c31d3e 100644
--- a/chrome/browser/extensions/test_extension_service.cc
+++ b/chrome/browser/extensions/test_extension_service.cc
@@ -55,7 +55,8 @@ const Extension* TestExtensionService::GetPendingExtensionUpdate(
   return NULL;
 }
 
-void TestExtensionService::FinishInstallation(const std::string& extension_id) {
+void TestExtensionService::FinishDelayedInstallation(
+    const std::string& extension_id) {
   ADD_FAILURE();
 }
 
diff --git a/chrome/browser/extensions/test_extension_service.h b/chrome/browser/extensions/test_extension_service.h
index 0121dab..b152678 100644
--- a/chrome/browser/extensions/test_extension_service.h
+++ b/chrome/browser/extensions/test_extension_service.h
@@ -43,7 +43,8 @@ class TestExtensionService : public ExtensionServiceInterface {
       const std::string& id) const OVERRIDE;
   virtual const extensions::Extension* GetPendingExtensionUpdate(
       const std::string& extension_id) const OVERRIDE;
-  virtual void FinishInstallation(const std::string& extension_id) OVERRIDE;
+  virtual void FinishDelayedInstallation(
+      const std::string& extension_id) OVERRIDE;
   virtual bool IsExtensionEnabled(
       const std::string& extension_id) const OVERRIDE;
   virtual bool IsExternalExtensionUninstalled(
diff --git a/chrome/browser/extensions/updater/extension_updater.cc b/chrome/browser/extensions/updater/extension_updater.cc
index 35bce9e..a11f718 100644
--- a/chrome/browser/extensions/updater/extension_updater.cc
+++ b/chrome/browser/extensions/updater/extension_updater.cc
@@ -476,7 +476,7 @@ void ExtensionUpdater::OnExtensionDownloadFailed(
   // current update check has |install_immediately| set the previously
   // queued update should be installed now.
   if (install_immediately && service_->GetPendingExtensionUpdate(id))
-    service_->FinishInstallation(id);
+    service_->FinishDelayedInstallation(id);
 }
 
 void ExtensionUpdater::OnExtensionDownloadFinished(