Don't allow inline install if frame is deleted before user accepts

chrome/browser/extensions/webstore_inline_installer_browsertest.cc

Index: chrome/browser/extensions/webstore_inline_installer_browsertest.cc
diff --git a/chrome/browser/extensions/webstore_inline_installer_browsertest.cc b/chrome/browser/extensions/webstore_inline_installer_browsertest.cc
index 90144793c8df955ef89101eade1e194f1fe69eae..f641d3fc6a994b7056a9912a661a095808c7231b 100644
--- a/chrome/browser/extensions/webstore_inline_installer_browsertest.cc
+++ b/chrome/browser/extensions/webstore_inline_installer_browsertest.cc
@@ -18,6 +18,7 @@
 #include "chrome/test/base/ui_test_utils.h"
 #include "components/content_settings/core/browser/host_content_settings_map.h"
 #include "content/public/browser/web_contents.h"
+#include "content/public/test/browser_test_utils.h"
 #include "extensions/browser/extension_registry.h"
 #include "extensions/browser/extension_system.h"
 #include "url/gurl.h"
@@ -87,46 +88,82 @@ ExtensionInstallPrompt::Delegate* ProgrammableInstallPrompt::delegate_;
 class WebstoreInlineInstallerForTest : public WebstoreInlineInstaller {
  public:
   WebstoreInlineInstallerForTest(WebContents* contents,
+                                 content::RenderFrameHost* host,
                                  const std::string& extension_id,
                                  const GURL& requestor_url,
                                  const Callback& callback)
       : WebstoreInlineInstaller(
             contents,
+            host,
             kTestExtensionId,
             requestor_url,
-            base::Bind(DummyCallback)),
-        programmable_prompt_(NULL) {
-  }
+            base::Bind(&WebstoreInlineInstallerForTest::InstallCallback)),
+        programmable_prompt_(NULL) {}
 
   scoped_ptr<ExtensionInstallPrompt> CreateInstallUI() override {
     programmable_prompt_ = new ProgrammableInstallPrompt(web_contents());
     return make_scoped_ptr(programmable_prompt_);
   }
 
+  // Added here to make it public so that test cases can call it below.
+  bool CheckRequestorAlive() const override {
+    return WebstoreInlineInstaller::CheckRequestorAlive();
+  }
+
+  // A struct for letting us store the actual parameters that were passed to
+  // the install callback.
+  struct InstallResult {
+    bool success;

[Devlin, 2015/11/20 01:27:19]

nitty nit: please initialize these (feel free to use default member
initialization to make it fewer lines).

[asargent_no_longer_on_chrome, 2015/11/20 23:56:46]

Done.

+    std::string error;
+    webstore_install::Result result;
+  };
+
+  static InstallResult* last_install_result() { return g_last_install_result_; }
+
  private:
   ~WebstoreInlineInstallerForTest() override {}
 
   friend class base::RefCountedThreadSafe<WebstoreStandaloneInstaller>;
 
-  static void DummyCallback(bool success,
-                            const std::string& error,
-                            webstore_install::Result result) {
+  static void InstallCallback(bool success,
+                              const std::string& error,
+                              webstore_install::Result result) {
+    g_last_install_result_ = new InstallResult;
+    g_last_install_result_->success = success;
+    g_last_install_result_->error = error;
+    g_last_install_result_->result = result;
   }
 
+  static InstallResult* g_last_install_result_;

[Devlin, 2015/11/20 01:27:19]

nitty nit: This isn't entirely POD (because of the string), and is never garbage
collected.  Does it matter in a test?  Probably not.  But is there a reason not
to have it as a non-static member on the test itself, either a scoped_ptr or
just a default-initialized struct?

[asargent_no_longer_on_chrome, 2015/11/20 23:56:46]

Yeah, that was probably a little too fast and loose. I've switched it to let
consumers of WebstoreInlineInstallerForTest specify a scoped_ptr to write this
into. 

+
   ProgrammableInstallPrompt* programmable_prompt_;
 };
 
+WebstoreInlineInstallerForTest::InstallResult*
+    WebstoreInlineInstallerForTest::g_last_install_result_ = nullptr;
+
 class WebstoreInlineInstallerForTestFactory :
     public WebstoreInlineInstallerFactory {
+ public:
+  WebstoreInlineInstallerForTestFactory() : last_installer_(nullptr) {}
   ~WebstoreInlineInstallerForTestFactory() override {}
+
+  WebstoreInlineInstallerForTest* last_installer() { return last_installer_; }
+
   WebstoreInlineInstaller* CreateInstaller(
       WebContents* contents,
+      content::RenderFrameHost* host,
       const std::string& webstore_item_id,
       const GURL& requestor_url,
       const WebstoreStandaloneInstaller::Callback& callback) override {
-    return new WebstoreInlineInstallerForTest(
-        contents, webstore_item_id, requestor_url, callback);
+    last_installer_ = new WebstoreInlineInstallerForTest(
+        contents, host, webstore_item_id, requestor_url, callback);
+    return last_installer_;
   }
+
+ private:
+  // The last installer that was created.
+  WebstoreInlineInstallerForTest* last_installer_;
 };
 
 IN_PROC_BROWSER_TEST_F(WebstoreInlineInstallerTest,
@@ -145,6 +182,42 @@ IN_PROC_BROWSER_TEST_F(WebstoreInlineInstallerTest,
   ProgrammableInstallPrompt::Accept();
 }
 
+IN_PROC_BROWSER_TEST_F(WebstoreInlineInstallerTest,
+                       NavigateBeforeInstallConfirmation) {
+  GURL install_url = GenerateTestServerUrl(kAppDomain, "install.html");
+  ui_test_utils::NavigateToURL(browser(), install_url);
+  WebContents* web_contents =
+      browser()->tab_strip_model()->GetActiveWebContents();
+  TabHelper* tab_helper = TabHelper::FromWebContents(web_contents);
+  WebstoreInlineInstallerForTestFactory* factory =
+      new WebstoreInlineInstallerForTestFactory();
+  tab_helper->SetWebstoreInlineInstallerFactoryForTests(factory);
+  RunTestAsync("runTest");
+  while (!ProgrammableInstallPrompt::Ready())
+    base::RunLoop().RunUntilIdle();
+  GURL new_url = GenerateTestServerUrl(kNonAppDomain, "empty.html");
+  web_contents->GetController().LoadURL(
+      new_url, content::Referrer(), ui::PAGE_TRANSITION_LINK, std::string());
+  EXPECT_TRUE(content::WaitForLoadStop(web_contents));
+  ASSERT_NE(factory->last_installer(), nullptr);
+  EXPECT_NE(factory->last_installer()->web_contents(), nullptr);
+  EXPECT_FALSE(factory->last_installer()->CheckRequestorAlive());
+
+  // Right now the way we handle navigations away from the frame that began the
+  // inline install is to just declare the requestor to be dead, but not to
+  // kill the prompt (that would be a better UX, but more complicated to
+  // implement). If we ever do change things to kill the prompt in this case,
+  // the following code can be removed (it verifies that clicking ok on the
+  // dialog does not result in an install).
+  ASSERT_TRUE(ProgrammableInstallPrompt::Ready());
+  ProgrammableInstallPrompt::Accept();
+  WebstoreInlineInstallerForTest::InstallResult* install_result =
+      WebstoreInlineInstallerForTest::last_install_result();
+  ASSERT_NE(install_result, nullptr);
+  EXPECT_EQ(install_result->success, false);
+  EXPECT_EQ(install_result->result, webstore_install::ABORTED);
+}
+
 // Flaky: https://crbug.com/537526.
 IN_PROC_BROWSER_TEST_F(WebstoreInlineInstallerTest,
                        DISABLED_ShouldBlockInlineInstallFromPopupWindow) {