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..984084899af90f8596950b50c2a8ed916734d501 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"
@@ -26,6 +27,8 @@ using content::WebContents;
 
 namespace extensions {
 
+class WebstoreInlineInstallerForTest;
+
 namespace {
 
 const char kWebstoreDomain[] = "cws.com";
@@ -35,17 +38,31 @@ const char kTestExtensionId[] = "ecglahbcnmdpdciemllbhojghbkagdje";
 const char kTestDataPath[] = "extensions/api_test/webstore_inline_install";
 const char kCrxFilename[] = "extension.crx";
 
+class InstallerFactoryObserver {

[Devlin, 2015/11/05 17:40:40]

nit: I think this is a little more than we need.  Can the test factory itself
just store the last installer, and we can keep a raw ptr to the factory in the
test?

[asargent_no_longer_on_chrome, 2015/11/18 21:42:44]

Much simpler, I like it. Done. 

+ public:
+  virtual void OnInstallerCreated(
+      WebstoreInlineInstallerForTest* installer) = 0;
+};
+
 }  // namespace
 
-class WebstoreInlineInstallerTest : public WebstoreInstallerTest {
+class WebstoreInlineInstallerTest : public WebstoreInstallerTest,
+                                    public InstallerFactoryObserver {
  public:
   WebstoreInlineInstallerTest()
-      : WebstoreInstallerTest(
-            kWebstoreDomain,
-            kTestDataPath,
-            kCrxFilename,
-            kAppDomain,
-            kNonAppDomain) {}
+      : WebstoreInstallerTest(kWebstoreDomain,
+                              kTestDataPath,
+                              kCrxFilename,
+                              kAppDomain,
+                              kNonAppDomain),
+        last_installer_(nullptr) {}
+
+  void OnInstallerCreated(WebstoreInlineInstallerForTest* installer) override {
+    last_installer_ = installer;
+  }
+
+ protected:
+  WebstoreInlineInstallerForTest* last_installer_;
 };
 
 class ProgrammableInstallPrompt : public ExtensionInstallPrompt {
@@ -87,22 +104,26 @@ 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,
-            kTestExtensionId,
-            requestor_url,
-            base::Bind(DummyCallback)),
-        programmable_prompt_(NULL) {
-  }
+      : WebstoreInlineInstaller(contents,
+                                host,
+                                kTestExtensionId,
+                                requestor_url,
+                                base::Bind(DummyCallback)),
+        programmable_prompt_(NULL) {}
 
   scoped_ptr<ExtensionInstallPrompt> CreateInstallUI() override {
     programmable_prompt_ = new ProgrammableInstallPrompt(web_contents());
     return make_scoped_ptr(programmable_prompt_);
   }
 
+  bool CheckRequestorAlive() const override {
+    return WebstoreInlineInstaller::CheckRequestorAlive();

[Devlin, 2015/11/05 17:40:40]

Is this needed?

[asargent_no_longer_on_chrome, 2015/11/18 21:42:44]

It's just needed to make the method publicly visible so the test can call it
(it's a protected method of WebstoreInlineInstaller). 

Added a comment to this effect.

+  }
+
  private:
   ~WebstoreInlineInstallerForTest() override {}
 
@@ -118,15 +139,27 @@ class WebstoreInlineInstallerForTest : public WebstoreInlineInstaller {
 
 class WebstoreInlineInstallerForTestFactory :
     public WebstoreInlineInstallerFactory {
+ public:
+  explicit WebstoreInlineInstallerForTestFactory(
+      InstallerFactoryObserver* observer)
+      : observer_(observer) {}
+
   ~WebstoreInlineInstallerForTestFactory() override {}
   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);
+    WebstoreInlineInstallerForTest* installer =
+        new WebstoreInlineInstallerForTest(contents, host, webstore_item_id,
+                                           requestor_url, callback);
+    observer_->OnInstallerCreated(installer);
+    return installer;
   }
+
+ protected:
+  InstallerFactoryObserver* observer_;
 };
 
 IN_PROC_BROWSER_TEST_F(WebstoreInlineInstallerTest,
@@ -137,7 +170,7 @@ IN_PROC_BROWSER_TEST_F(WebstoreInlineInstallerTest,
       browser()->tab_strip_model()->GetActiveWebContents();
   TabHelper* tab_helper = TabHelper::FromWebContents(web_contents);
   tab_helper->SetWebstoreInlineInstallerFactoryForTests(
-      new WebstoreInlineInstallerForTestFactory());
+      new WebstoreInlineInstallerForTestFactory(this));
   RunTestAsync("runTest");
   while (!ProgrammableInstallPrompt::Ready())
     base::RunLoop().RunUntilIdle();
@@ -145,6 +178,27 @@ 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);
+  tab_helper->SetWebstoreInlineInstallerFactoryForTests(
+      new WebstoreInlineInstallerForTestFactory(this));
+  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(last_installer_, nullptr);
+  EXPECT_NE(last_installer_->web_contents(), nullptr);
+  EXPECT_FALSE(last_installer_->CheckRequestorAlive());

[Devlin, 2015/11/05 17:40:40]

Can we also test inline installation from a frame, and navigating the frame? 
(If it seems like it'll be a pain, don't worry about it.)

[asargent_no_longer_on_chrome, 2015/11/18 21:42:44]

I wrote up a test for it, and in the process of trying to get the test working I
found out that we don't allow calling the API from frames! =)

https://code.google.com/p/chromium/codesearch#chromium/src/chrome/renderer/ex...

https://code.google.com/p/chromium/codesearch#chromium/src/chrome/renderer/ex...

+}
+
 // Flaky: https://crbug.com/537526.
 IN_PROC_BROWSER_TEST_F(WebstoreInlineInstallerTest,
                        DISABLED_ShouldBlockInlineInstallFromPopupWindow) {