Invoke callback from WebstoreInstaller when installation is fully complete

ExtensionRegistry observers will be notified when an extension install
is fully complete, i.e. after the extension has been loaded or added
to one of the registry extension sets.

This allows the WebstoreInstaller completion to be delayed until the
extension has been loaded. Workarounds in EphemeralAppLauncher for
launching an app can now be cleaned up since we are now assured that
the app has been loaded.

BUG=None
TEST=browser_tests

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

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

8 files changed, 44 insertions, 75 deletions

diff --git a/chrome/browser/apps/ephemeral_app_launcher.cc b/chrome/browser/apps/ephemeral_app_launcher.cc
index d95e833..d46942e 100644
--- a/chrome/browser/apps/ephemeral_app_launcher.cc
+++ b/chrome/browser/apps/ephemeral_app_launcher.cc
@@ -5,19 +5,17 @@
 #include "chrome/browser/apps/ephemeral_app_launcher.h"
 
 #include "chrome/browser/extensions/extension_install_prompt.h"
-#include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_util.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/extensions/application_launch.h"
 #include "chrome/browser/ui/extensions/extension_enable_flow.h"
 #include "content/public/browser/web_contents.h"
 #include "extensions/browser/extension_registry.h"
-#include "extensions/browser/extension_system.h"
 #include "extensions/common/permissions/permissions_data.h"
 
 using content::WebContents;
 using extensions::Extension;
-using extensions::ExtensionSystem;
+using extensions::ExtensionRegistry;
 using extensions::WebstoreInstaller;
 
 namespace {
@@ -72,11 +70,9 @@ EphemeralAppLauncher::CreateForLink(
 }
 
 void EphemeralAppLauncher::Start() {
-  ExtensionService* extension_service =
-      extensions::ExtensionSystem::Get(profile())->extension_service();
-  DCHECK(extension_service);
-
-  const Extension* extension = extension_service->GetInstalledExtension(id());
+  const Extension* extension =
+      ExtensionRegistry::Get(profile())
+          ->GetExtensionById(id(), ExtensionRegistry::EVERYTHING);
   if (extension) {
     if (extensions::util::IsAppLaunchableWithoutEnabling(extension->id(),
                                                          profile())) {
@@ -101,7 +97,6 @@ void EphemeralAppLauncher::Start() {
   }
 
   // Fetch the app from the webstore.
-  StartObserving();
   BeginInstall();
 }
 
@@ -110,7 +105,6 @@ EphemeralAppLauncher::EphemeralAppLauncher(const std::string& webstore_item_id,
                                            gfx::NativeWindow parent_window,
                                            const Callback& callback)
     : WebstoreStandaloneInstaller(webstore_item_id, profile, callback),
-      extension_registry_observer_(this),
       parent_window_(parent_window),
       dummy_web_contents_(
           WebContents::Create(WebContents::CreateParams(profile))) {
@@ -123,17 +117,11 @@ EphemeralAppLauncher::EphemeralAppLauncher(const std::string& webstore_item_id,
                                   ProfileForWebContents(web_contents),
                                   callback),
       content::WebContentsObserver(web_contents),
-      extension_registry_observer_(this),
       parent_window_(NativeWindowForWebContents(web_contents)) {
 }
 
 EphemeralAppLauncher::~EphemeralAppLauncher() {}
 
-void EphemeralAppLauncher::StartObserving() {
-  extension_registry_observer_.Add(
-      extensions::ExtensionRegistry::Get(profile()));
-}
-
 void EphemeralAppLauncher::LaunchApp(const Extension* extension) const {
   DCHECK(extension);
   if (!extension->is_app()) {
@@ -238,46 +226,25 @@ EphemeralAppLauncher::CreateApproval() const {
 }
 
 void EphemeralAppLauncher::CompleteInstall(const std::string& error) {
-  if (!error.empty())
-    WebstoreStandaloneInstaller::CompleteInstall(error);
-
-  // If the installation succeeds, we reach this point as a result of
-  // chrome::NOTIFICATION_EXTENSION_INSTALLED_DEPRECATED, but this is
-  // broadcasted before
-  // ExtensionService has added the extension to its list of installed
-  // extensions and is too early to launch the app. Instead, we will launch at
-  // EphemeralAppLauncher::OnExtensionLoaded().
-  // TODO(tmdiep): Refactor extensions/WebstoreInstaller or
-  // WebstoreStandaloneInstaller to support this cleanly.
+  if (error.empty()) {
+    const Extension* extension =
+        ExtensionRegistry::Get(profile())
+            ->GetExtensionById(id(), ExtensionRegistry::ENABLED);
+    if (extension)
+      LaunchApp(extension);
+  }
+
+  WebstoreStandaloneInstaller::CompleteInstall(error);
 }
 
 void EphemeralAppLauncher::WebContentsDestroyed() {
   AbortInstall();
 }
 
-void EphemeralAppLauncher::OnExtensionLoaded(
-    content::BrowserContext* browser_context,
-    const Extension* extension) {
-  if (extension->id() == id()) {
-    LaunchApp(extension);
-    WebstoreStandaloneInstaller::CompleteInstall(std::string());
-  }
-}
-
 void EphemeralAppLauncher::ExtensionEnableFlowFinished() {
-  ExtensionService* extension_service =
-      extensions::ExtensionSystem::Get(profile())->extension_service();
-  DCHECK(extension_service);
-
-  const Extension* extension = extension_service->GetExtensionById(id(), false);
-  if (extension) {
-    LaunchApp(extension);
-    WebstoreStandaloneInstaller::CompleteInstall(std::string());
-  } else {
-    WebstoreStandaloneInstaller::CompleteInstall(kLaunchAbortedError);
-  }
+  CompleteInstall(std::string());
 }
 
 void EphemeralAppLauncher::ExtensionEnableFlowAborted(bool user_initiated) {
-  WebstoreStandaloneInstaller::CompleteInstall(kLaunchAbortedError);
+  CompleteInstall(kLaunchAbortedError);
 }
diff --git a/chrome/browser/apps/ephemeral_app_launcher.h b/chrome/browser/apps/ephemeral_app_launcher.h
index a68d611..92a82a5 100644
--- a/chrome/browser/apps/ephemeral_app_launcher.h
+++ b/chrome/browser/apps/ephemeral_app_launcher.h
@@ -12,7 +12,6 @@
 #include "chrome/browser/extensions/webstore_standalone_installer.h"
 #include "chrome/browser/ui/extensions/extension_enable_flow_delegate.h"
 #include "content/public/browser/web_contents_observer.h"
-#include "extensions/browser/extension_registry_observer.h"
 
 class ExtensionEnableFlow;
 class Profile;
@@ -31,7 +30,6 @@ class ExtensionRegistry;
 // launches the app.
 class EphemeralAppLauncher : public extensions::WebstoreStandaloneInstaller,
                              public content::WebContentsObserver,
-                             public extensions::ExtensionRegistryObserver,
                              public ExtensionEnableFlowDelegate {
  public:
   typedef WebstoreStandaloneInstaller::Callback Callback;
@@ -64,7 +62,6 @@ class EphemeralAppLauncher : public extensions::WebstoreStandaloneInstaller,
 
   virtual ~EphemeralAppLauncher();
 
-  void StartObserving();
   void LaunchApp(const extensions::Extension* extension) const;
 
   // WebstoreStandaloneInstaller implementation.
@@ -92,20 +89,10 @@ class EphemeralAppLauncher : public extensions::WebstoreStandaloneInstaller,
   // content::WebContentsObserver implementation.
   virtual void WebContentsDestroyed() OVERRIDE;
 
-  // ExtensionRegistryObserver implementation.
-  virtual void OnExtensionLoaded(
-      content::BrowserContext* browser_context,
-      const extensions::Extension* extension) OVERRIDE;
-
   // ExtensionEnableFlowDelegate implementation.
   virtual void ExtensionEnableFlowFinished() OVERRIDE;
   virtual void ExtensionEnableFlowAborted(bool user_initiated) OVERRIDE;
 
-  // Listen to extension unloaded notifications.
-  ScopedObserver<extensions::ExtensionRegistry,
-                 extensions::ExtensionRegistryObserver>
-      extension_registry_observer_;
-
   gfx::NativeWindow parent_window_;
   scoped_ptr<content::WebContents> dummy_web_contents_;
 
diff --git a/chrome/browser/extensions/extension_service.cc b/chrome/browser/extensions/extension_service.cc
index c124c72..3009017 100644
--- a/chrome/browser/extensions/extension_service.cc
+++ b/chrome/browser/extensions/extension_service.cc
@@ -1978,7 +1978,7 @@ void ExtensionService::FinishInstallation(
       content::Source<Profile>(profile_),
       content::Details<const extensions::InstalledExtensionInfo>(&details));
 
-  ExtensionRegistry::Get(profile_)->TriggerOnWillBeInstalled(
+  registry_->TriggerOnWillBeInstalled(
       extension, is_update, from_ephemeral, old_name);
 
   bool unacknowledged_external = IsUnacknowledgedExternalExtension(extension);
@@ -1992,6 +1992,9 @@ void ExtensionService::FinishInstallation(
 
   AddExtension(extension);
 
+  // Notify observers that need to know when an installation is complete.
+  registry_->TriggerOnInstalled(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.
@@ -2078,6 +2081,8 @@ void ExtensionService::PromoteEphemeralApp(
     registry_->TriggerOnLoaded(extension);
   }
 
+  registry_->TriggerOnInstalled(extension);
+
   if (!is_from_sync && extension_sync_service_)
     extension_sync_service_->SyncExtensionChangeIfNeeded(*extension);
 }
diff --git a/chrome/browser/extensions/webstore_installer.cc b/chrome/browser/extensions/webstore_installer.cc
index 7dd6017..79a5b69 100644
--- a/chrome/browser/extensions/webstore_installer.cc
+++ b/chrome/browser/extensions/webstore_installer.cc
@@ -399,12 +399,9 @@ void WebstoreInstaller::Observe(int type,
   }
 }
 
-void WebstoreInstaller::OnExtensionWillBeInstalled(
+void WebstoreInstaller::OnExtensionInstalled(
     content::BrowserContext* browser_context,
-    const Extension* extension,
-    bool is_update,
-    bool from_ephemeral,
-    const std::string& old_name) {
+    const Extension* extension) {
   CHECK(profile_->IsSameProfile(Profile::FromBrowserContext(browser_context)));
   if (pending_modules_.empty())
     return;
diff --git a/chrome/browser/extensions/webstore_installer.h b/chrome/browser/extensions/webstore_installer.h
index b35d1e1b..c31c4c1 100644
--- a/chrome/browser/extensions/webstore_installer.h
+++ b/chrome/browser/extensions/webstore_installer.h
@@ -160,7 +160,7 @@ class WebstoreInstaller : public content::NotificationObserver,
     // Required minimum version.
     scoped_ptr<Version> minimum_version;
 
-    // Ephemeral apps (experimental) are not permanently installed in Chrome.
+    // Ephemeral apps are transiently installed.
     bool is_ephemeral;
 
     // The authuser index required to download the item being installed. May be
@@ -199,12 +199,8 @@ class WebstoreInstaller : public content::NotificationObserver,
                        const content::NotificationDetails& details) OVERRIDE;
 
   // ExtensionRegistryObserver.
-  virtual void OnExtensionWillBeInstalled(
-      content::BrowserContext* browser_context,
-      const Extension* extension,
-      bool is_update,
-      bool from_ephemeral,
-      const std::string& old_name) OVERRIDE;
+  virtual void OnExtensionInstalled(content::BrowserContext* browser_context,
+                                    const Extension* extension) OVERRIDE;
 
   // Removes the reference to the delegate passed in the constructor. Used when
   // the delegate object must be deleted before this object.
diff --git a/extensions/browser/extension_registry.cc b/extensions/browser/extension_registry.cc
index a0175c2..bfd1fdd 100644
--- a/extensions/browser/extension_registry.cc
+++ b/extensions/browser/extension_registry.cc
@@ -67,6 +67,13 @@ void ExtensionRegistry::TriggerOnWillBeInstalled(const Extension* extension,
           browser_context_, extension, is_update, from_ephemeral, old_name));
 }
 
+void ExtensionRegistry::TriggerOnInstalled(const Extension* extension) {
+  DCHECK(GenerateInstalledExtensionsSet()->Contains(extension->id()));
+  FOR_EACH_OBSERVER(ExtensionRegistryObserver,
+                    observers_,
+                    OnExtensionInstalled(browser_context_, extension));
+}
+
 void ExtensionRegistry::TriggerOnUninstalled(const Extension* extension) {
   DCHECK(!GenerateInstalledExtensionsSet()->Contains(extension->id()));
   FOR_EACH_OBSERVER(ExtensionRegistryObserver,
diff --git a/extensions/browser/extension_registry.h b/extensions/browser/extension_registry.h
index 646863c..3d10763 100644
--- a/extensions/browser/extension_registry.h
+++ b/extensions/browser/extension_registry.h
@@ -86,6 +86,10 @@ class ExtensionRegistry : public KeyedService {
                                 bool from_ephemeral,
                                 const std::string& old_name);
 
+  // Invokes the observer method OnExtensionInstalled(). The extension must be
+  // contained in one of the registry's extension sets.
+  void TriggerOnInstalled(const Extension* extension);
+
   // Invokes the observer method OnExtensionUninstalled(). The extension must
   // not be any installed extension with |extension|'s ID.
   void TriggerOnUninstalled(const Extension* extension);
diff --git a/extensions/browser/extension_registry_observer.h b/extensions/browser/extension_registry_observer.h
index 8aa0d07..d0b10f3 100644
--- a/extensions/browser/extension_registry_observer.h
+++ b/extensions/browser/extension_registry_observer.h
@@ -59,7 +59,13 @@ class ExtensionRegistryObserver {
       bool from_ephemeral,
       const std::string& old_name) {}
 
-  // Called after an extension is uninstalled. The extension no longer exsit in
+  // Called when the installation of |extension| is complete. At this point the
+  // extension is tracked in one of the ExtensionRegistry sets, but is not
+  // necessarily enabled.
+  virtual void OnExtensionInstalled(content::BrowserContext* browser_context,
+                                    const Extension* extension) {}
+
+  // Called after an extension is uninstalled. The extension no longer exists in
   // any of the ExtensionRegistry sets (enabled, disabled, etc.).
   virtual void OnExtensionUninstalled(content::BrowserContext* browser_context,
                                       const Extension* extension) {}