Make inline installation callbacks fire when the item is downloaded and installed.

By having WebstoreInlineInstaller implement WebstoreInstaller::Delegate, we can
respond to the callback when the item is actually installed, instead of as soon
as the download is initiated.

R=jstritar@chromium.org

Review URL: http://codereview.chromium.org/8348006

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

8 files changed, 93 insertions, 17 deletions

diff --git a/chrome/browser/extensions/webstore_inline_install_browsertest.cc b/chrome/browser/extensions/webstore_inline_install_browsertest.cc
index ac58795..b43e6d4 100644
--- a/chrome/browser/extensions/webstore_inline_install_browsertest.cc
+++ b/chrome/browser/extensions/webstore_inline_install_browsertest.cc
@@ -81,20 +81,14 @@ class WebstoreInlineInstallTest : public InProcessBrowserTest {
 IN_PROC_BROWSER_TEST_F(WebstoreInlineInstallTest, Install) {
   SetExtensionInstallDialogForManifestAutoConfirmForTests(true);
 
-  ui_test_utils::WindowedNotificationObserver load_signal(
-        chrome::NOTIFICATION_EXTENSION_LOADED,
-        Source<Profile>(browser()->profile()));
-
   ui_test_utils::NavigateToURL(
       browser(), GenerateTestServerUrl(kAppDomain, "install.html"));
 
   RunInlineInstallTest("runTest");
 
-  load_signal.Wait();
-
   const Extension* extension = browser()->profile()->GetExtensionService()->
       GetExtensionById("ecglahbcnmdpdciemllbhojghbkagdje", false);
-  EXPECT_TRUE(extension != NULL);
+  EXPECT_TRUE(extension);
 }
 
 IN_PROC_BROWSER_TEST_F(
@@ -102,7 +96,7 @@ IN_PROC_BROWSER_TEST_F(
   SetExtensionInstallDialogForManifestAutoConfirmForTests(false);
   ui_test_utils::NavigateToURL(
       browser(),
-      GenerateTestServerUrl(kNonAppDomain, "install-non-verified-domain.html"));
+      GenerateTestServerUrl(kNonAppDomain, "install_non_verified_domain.html"));
 
   RunInlineInstallTest("runTest1");
   RunInlineInstallTest("runTest2");
@@ -127,7 +121,7 @@ IN_PROC_BROWSER_TEST_F(WebstoreInlineInstallTest, InstallNotSupported) {
   SetExtensionInstallDialogForManifestAutoConfirmForTests(false);
   ui_test_utils::NavigateToURL(
       browser(),
-      GenerateTestServerUrl(kAppDomain, "install-not-supported.html"));
+      GenerateTestServerUrl(kAppDomain, "install_not_supported.html"));
 
   RunInlineInstallTest("runTest");
 
@@ -139,3 +133,27 @@ IN_PROC_BROWSER_TEST_F(WebstoreInlineInstallTest, InstallNotSupported) {
   TabContents* tab_contents = browser()->GetSelectedTabContents();
   EXPECT_EQ(GURL("http://cws.com/show-me-the-money"), tab_contents->GetURL());
 }
+
+// The unpack failure test needs to use a different install .crx, which is
+// specified via a command-line flag, so it needs its own test subclass.
+class WebstoreInlineInstallUnpackFailureTest
+    : public WebstoreInlineInstallTest {
+ public:
+  virtual void SetUpCommandLine(CommandLine* command_line) OVERRIDE {
+    WebstoreInlineInstallTest::SetUpCommandLine(command_line);
+
+    GURL crx_url = GenerateTestServerUrl(
+        kWebstoreDomain, "malformed_extension.crx");
+    CommandLine::ForCurrentProcess()->AppendSwitchASCII(
+        switches::kAppsGalleryUpdateURL, crx_url.spec());
+  }
+};
+
+IN_PROC_BROWSER_TEST_F(WebstoreInlineInstallUnpackFailureTest, Test) {
+  SetExtensionInstallDialogForManifestAutoConfirmForTests(true);
+
+  ui_test_utils::NavigateToURL(browser(),
+      GenerateTestServerUrl(kAppDomain, "install_unpack_failure.html"));
+
+  RunInlineInstallTest("runTest");
+}
diff --git a/chrome/browser/extensions/webstore_inline_installer.cc b/chrome/browser/extensions/webstore_inline_installer.cc
index 967dba1..4a2ddfe 100644
--- a/chrome/browser/extensions/webstore_inline_installer.cc
+++ b/chrome/browser/extensions/webstore_inline_installer.cc
@@ -13,7 +13,6 @@
 #include "chrome/browser/extensions/crx_installer.h"
 #include "chrome/browser/extensions/extension_install_dialog.h"
 #include "chrome/browser/extensions/extension_service.h"
-#include "chrome/browser/extensions/webstore_installer.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/chrome_utility_messages.h"
 #include "chrome/common/extensions/extension.h"
@@ -382,13 +381,8 @@ void WebstoreInlineInstaller::InstallUIProceed() {
 
   WebstoreInstaller* installer =
       profile->GetExtensionService()->webstore_installer();
-  installer->InstallExtension(id_, NULL,
-                              WebstoreInstaller::FLAG_INLINE_INSTALL);
-
-  // TODO(mihaip): the success message should happen later, when the extension
-  // is actually downloaded and installed (by using the callbacks on
-  // ExtensionInstaller::Delegate).
-  CompleteInstall("");
+  installer->InstallExtension(
+      id_, this, WebstoreInstaller::FLAG_INLINE_INSTALL);
 }
 
 void WebstoreInlineInstaller::InstallUIAbort(bool user_initiated) {
@@ -403,6 +397,17 @@ void WebstoreInlineInstaller::TabContentsDestroyed(TabContents* tab_contents) {
   }
 }
 
+void WebstoreInlineInstaller::OnExtensionInstallSuccess(const std::string& id) {
+  CHECK_EQ(id_, id);
+  CompleteInstall("");
+}
+
+void WebstoreInlineInstaller::OnExtensionInstallFailure(
+    const std::string& id, const std::string& error) {
+  CHECK_EQ(id_, id);
+  CompleteInstall(error);
+}
+
 void WebstoreInlineInstaller::CompleteInstall(const std::string& error) {
   // Only bother responding if there's still a tab contents to send back the
   // response to.
diff --git a/chrome/browser/extensions/webstore_inline_installer.h b/chrome/browser/extensions/webstore_inline_installer.h
index 7f11aa8..2b2cf05 100644
--- a/chrome/browser/extensions/webstore_inline_installer.h
+++ b/chrome/browser/extensions/webstore_inline_installer.h
@@ -12,6 +12,7 @@
 #include "base/memory/scoped_ptr.h"
 #include "base/values.h"
 #include "chrome/browser/extensions/extension_install_ui.h"
+#include "chrome/browser/extensions/webstore_installer.h"
 #include "chrome/browser/extensions/webstore_install_helper.h"
 #include "content/browser/tab_contents/tab_contents_observer.h"
 #include "content/common/net/url_fetcher.h"
@@ -32,6 +33,7 @@ class WebstoreInlineInstaller
       public ExtensionInstallUI::Delegate,
       public TabContentsObserver,
       public URLFetcher::Delegate,
+      public WebstoreInstaller::Delegate,
       public WebstoreInstallHelper::Delegate {
  public:
   class Delegate {
@@ -89,6 +91,11 @@ class WebstoreInlineInstaller
   // TabContentsObserver interface implementation.
   virtual void TabContentsDestroyed(TabContents* tab_contents) OVERRIDE;
 
+  // WebstoreInstaller::Delegate interface implementation.
+  virtual void OnExtensionInstallSuccess(const std::string& id) OVERRIDE;
+  virtual void OnExtensionInstallFailure(const std::string& id,
+                                         const std::string& error) OVERRIDE;
+
   void CompleteInstall(const std::string& error);
 
   int install_id_;
diff --git a/chrome/test/data/extensions/api_test/webstore_inline_install/inlineinstall/detail/cccccccccccccccccccccccccccccccc b/chrome/test/data/extensions/api_test/webstore_inline_install/inlineinstall/detail/cccccccccccccccccccccccccccccccc
new file mode 100644
index 0000000..f9c1f89
--- /dev/null
+++ b/chrome/test/data/extensions/api_test/webstore_inline_install/inlineinstall/detail/cccccccccccccccccccccccccccccccc
@@ -0,0 +1,8 @@
+{
+  "icon_url": "webstore_inline_install/extension/icon.png",
+  "users": "0",
+  "average_rating": 5,
+  "rating_count": 0,
+  "verified_site": "app.com",
+  "manifest": "{\"name\":\"Install Failure Test Extension\",\"version\":\"0.1\"}"
+}
diff --git a/chrome/test/data/extensions/api_test/webstore_inline_install/install-non-verified-domain.html b/chrome/test/data/extensions/api_test/webstore_inline_install/install_non_verified_domain.html
index 8f74754..8f74754 100644
--- a/chrome/test/data/extensions/api_test/webstore_inline_install/install-non-verified-domain.html
+++ b/chrome/test/data/extensions/api_test/webstore_inline_install/install_non_verified_domain.html
diff --git a/chrome/test/data/extensions/api_test/webstore_inline_install/install-not-supported.html b/chrome/test/data/extensions/api_test/webstore_inline_install/install_not_supported.html
index f86fc40..f86fc40 100644
--- a/chrome/test/data/extensions/api_test/webstore_inline_install/install-not-supported.html
+++ b/chrome/test/data/extensions/api_test/webstore_inline_install/install_not_supported.html
diff --git a/chrome/test/data/extensions/api_test/webstore_inline_install/install_unpack_failure.html b/chrome/test/data/extensions/api_test/webstore_inline_install/install_unpack_failure.html
new file mode 100644
index 0000000..d638582
--- /dev/null
+++ b/chrome/test/data/extensions/api_test/webstore_inline_install/install_unpack_failure.html
@@ -0,0 +1,38 @@
+<!DOCTYPE html>
+<html>
+<head>
+  <link rel="chrome-webstore-item">
+</head>
+<body>
+<script>
+  function runTest(galleryUrl) {
+    // Link URL has to be generated dynamically in order to include the right
+    // port number. The ID corresponds to the data in the "extension" directory.
+    document.getElementsByTagName('link')[0].href =
+        galleryUrl + '/detail/cccccccccccccccccccccccccccccccc';
+
+    try {
+      chrome.webstore.install(
+          undefined,
+          function() {
+            console.log('Unexpected install success');
+            window.domAutomationController.send(false);
+          },
+          function(error) {
+            if (error.indexOf('Package is invalid') != -1) {
+              window.domAutomationController.send(true);
+            } else {
+              console.log('Unexpected install error: ' + error);
+              window.domAutomationController.send(false);
+            }
+          });
+    } catch (e) {
+      console.log('Unexpected exception: ' + e);
+      window.domAutomationController.send(false);
+      throw e;
+    }
+  }
+</script>
+
+</body>
+</html>
diff --git a/chrome/test/data/extensions/api_test/webstore_inline_install/malformed_extension.crx b/chrome/test/data/extensions/api_test/webstore_inline_install/malformed_extension.crx
new file mode 100644
index 0000000..e69de29
--- /dev/null
+++ b/chrome/test/data/extensions/api_test/webstore_inline_install/malformed_extension.crx