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

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);
 }

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_;
 

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);
 }

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;

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.

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,

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);

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) {}