Fix WebstoreInlineInstaller ASAN bugs
Certain code paths which called back onto a raw pointer to a
WebstoreInlineInstaller could not guarantee the object's existence at
callback time.
Additional ref counts have been added to ensure that this is no longer the case.
This is a continuation of the reverted https://codereview.chromium.org/24839002/
due to a previously missed test case.
BUG=236513, 241431, 241432, 247774, 263004
Review URL: https://codereview.chromium.org/27034008
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@228522 0039d316-1c4b-4281-b951-d872f2087c98
diff --git a/chrome/browser/extensions/tab_helper.cc b/chrome/browser/extensions/tab_helper.cc
index b81f480..63ce4be 100644
--- a/chrome/browser/extensions/tab_helper.cc
+++ b/chrome/browser/extensions/tab_helper.cc
@@ -23,6 +23,7 @@
#include "chrome/browser/extensions/script_bubble_controller.h"
#include "chrome/browser/extensions/script_executor.h"
#include "chrome/browser/extensions/webstore_inline_installer.h"
+#include "chrome/browser/extensions/webstore_inline_installer_factory.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/sessions/session_id.h"
#include "chrome/browser/sessions/session_tab_helper.h"
@@ -86,7 +87,8 @@
pending_web_app_action_(NONE),
script_executor_(new ScriptExecutor(web_contents,
&script_execution_observers_)),
- image_loader_ptr_factory_(this) {
+ image_loader_ptr_factory_(this),
+ webstore_inline_installer_factory_(new WebstoreInlineInstallerFactory()) {
// The ActiveTabPermissionManager requires a session ID; ensure this
// WebContents has one.
SessionTabHelper::CreateForWebContents(web_contents);
@@ -109,7 +111,6 @@
new ScriptBubbleController(web_contents, this));
}
-
// If more classes need to listen to global content script activity, then
// a separate routing class with an observer interface should be written.
profile_ = Profile::FromBrowserContext(web_contents->GetBrowserContext());
@@ -300,7 +301,7 @@
base::Bind(&TabHelper::OnInlineInstallComplete, base::Unretained(this),
install_id, return_route_id);
scoped_refptr<WebstoreInlineInstaller> installer(
- new WebstoreInlineInstaller(
+ webstore_inline_installer_factory_->CreateInstaller(
web_contents(),
webstore_item_id,
requestor_url,
@@ -417,6 +418,11 @@
web_contents()->NotifyNavigationStateChanged(content::INVALIDATE_TYPE_TITLE);
}
+void TabHelper::SetWebstoreInlineInstallerFactoryForTests(
+ WebstoreInlineInstallerFactory* factory) {
+ webstore_inline_installer_factory_.reset(factory);
+}
+
void TabHelper::OnImageLoaded(const gfx::Image& image) {
if (!image.IsEmpty()) {
extension_app_icon_ = *image.ToSkBitmap();
diff --git a/chrome/browser/extensions/tab_helper.h b/chrome/browser/extensions/tab_helper.h
index dfd7c94..e3538cf5 100644
--- a/chrome/browser/extensions/tab_helper.h
+++ b/chrome/browser/extensions/tab_helper.h
@@ -10,6 +10,7 @@
#include <vector>
#include "base/memory/ref_counted.h"
+#include "base/memory/scoped_ptr.h"
#include "base/memory/weak_ptr.h"
#include "base/observer_list.h"
#include "chrome/browser/extensions/active_tab_permission_granter.h"
@@ -36,6 +37,7 @@
class ScriptBadgeController;
class ScriptBubbleController;
class ScriptExecutor;
+class WebstoreInlineInstallerFactory;
// Per-tab extension helper. Also handles non-extension apps.
class TabHelper : public content::WebContentsObserver,
@@ -155,6 +157,11 @@
// INVALIDATE_TYPE_TITLE navigation state change to trigger repaint of title.
void SetAppIcon(const SkBitmap& app_icon);
+ // Sets the factory used to create inline webstore item installers.
+ // Used for testing. Takes ownership of the factory instance.
+ void SetWebstoreInlineInstallerFactoryForTests(
+ WebstoreInlineInstallerFactory* factory);
+
private:
explicit TabHelper(content::WebContents* web_contents);
friend class content::WebContentsUserData<TabHelper>;
@@ -263,6 +270,9 @@
// Vend weak pointers that can be invalidated to stop in-progress loads.
base::WeakPtrFactory<TabHelper> image_loader_ptr_factory_;
+ // Creates WebstoreInlineInstaller instances for inline install triggers.
+ scoped_ptr<WebstoreInlineInstallerFactory> webstore_inline_installer_factory_;
+
DISALLOW_COPY_AND_ASSIGN(TabHelper);
};
diff --git a/chrome/browser/extensions/webstore_inline_installer_browsertest.cc b/chrome/browser/extensions/webstore_inline_installer_browsertest.cc
new file mode 100644
index 0000000..ee16a934
--- /dev/null
+++ b/chrome/browser/extensions/webstore_inline_installer_browsertest.cc
@@ -0,0 +1,138 @@
+// Copyright 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/extensions/extension_install_prompt.h"
+#include "chrome/browser/extensions/tab_helper.h"
+#include "chrome/browser/extensions/webstore_inline_installer.h"
+#include "chrome/browser/extensions/webstore_inline_installer_factory.h"
+#include "chrome/browser/extensions/webstore_installer_test.h"
+#include "chrome/browser/extensions/webstore_standalone_installer.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/test/base/ui_test_utils.h"
+#include "content/public/browser/render_view_host.h"
+#include "content/public/browser/web_contents.h"
+#include "url/gurl.h"
+
+using content::WebContents;
+using extensions::Extension;
+using extensions::TabHelper;
+using extensions::WebstoreInlineInstaller;
+using extensions::WebstoreInlineInstallerFactory;
+using extensions::WebstoreStandaloneInstaller;
+
+const char kWebstoreDomain[] = "cws.com";
+const char kAppDomain[] = "app.com";
+const char kNonAppDomain[] = "nonapp.com";
+const char kTestExtensionId[] = "ecglahbcnmdpdciemllbhojghbkagdje";
+const char kTestDataPath[] = "extensions/api_test/webstore_inline_install";
+const char kCrxFilename[] = "extension.crx";
+
+class WebstoreInlineInstallerTest : public WebstoreInstallerTest {
+ public:
+ WebstoreInlineInstallerTest()
+ : WebstoreInstallerTest(
+ kWebstoreDomain,
+ kTestDataPath,
+ kCrxFilename,
+ kAppDomain,
+ kNonAppDomain) {}
+};
+
+class ProgrammableInstallPrompt : public ExtensionInstallPrompt {
+ public:
+ explicit ProgrammableInstallPrompt(WebContents* contents)
+ : ExtensionInstallPrompt(contents)
+ {}
+
+ virtual ~ProgrammableInstallPrompt() {}
+
+ virtual void ConfirmStandaloneInstall(
+ Delegate* delegate,
+ const Extension* extension,
+ SkBitmap* icon,
+ const ExtensionInstallPrompt::Prompt& prompt) OVERRIDE {
+ delegate_ = delegate;
+ }
+
+ static bool Ready() {
+ return delegate_ != NULL;
+ }
+
+ static void Accept() {
+ delegate_->InstallUIProceed();
+ }
+
+ static void Reject() {
+ delegate_->InstallUIAbort(true);
+ }
+
+ private:
+ static Delegate* delegate_;
+};
+
+ExtensionInstallPrompt::Delegate* ProgrammableInstallPrompt::delegate_;
+
+
+// Fake inline installer which creates a programmable prompt in place of
+// the normal dialog UI.
+class WebstoreInlineInstallerForTest : public WebstoreInlineInstaller {
+ public:
+ WebstoreInlineInstallerForTest(WebContents* contents,
+ const std::string& extension_id,
+ const GURL& requestor_url,
+ const Callback& callback)
+ : WebstoreInlineInstaller(
+ contents,
+ kTestExtensionId,
+ requestor_url,
+ base::Bind(DummyCallback)),
+ programmable_prompt_(NULL) {
+ }
+
+ virtual scoped_ptr<ExtensionInstallPrompt> CreateInstallUI() OVERRIDE {
+ programmable_prompt_ = new ProgrammableInstallPrompt(web_contents());
+ return make_scoped_ptr(programmable_prompt_).
+ PassAs<ExtensionInstallPrompt>();
+ }
+
+ private:
+ virtual ~WebstoreInlineInstallerForTest() {}
+
+ friend class base::RefCountedThreadSafe<WebstoreStandaloneInstaller>;
+
+ static void DummyCallback(bool /*success*/, const std::string& /*error*/) {
+ }
+
+ ProgrammableInstallPrompt* programmable_prompt_;
+};
+
+class WebstoreInlineInstallerForTestFactory :
+ public WebstoreInlineInstallerFactory {
+ virtual ~WebstoreInlineInstallerForTestFactory() {}
+ virtual WebstoreInlineInstaller* CreateInstaller(
+ WebContents* contents,
+ 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);
+ }
+};
+
+IN_PROC_BROWSER_TEST_F(WebstoreInlineInstallerTest,
+ CloseTabBeforeInstallConfirmation) {
+ 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());
+ RunTestAsync("runTest");
+ while (!ProgrammableInstallPrompt::Ready())
+ base::RunLoop().RunUntilIdle();
+ web_contents->Close();
+ ProgrammableInstallPrompt::Accept();
+}
diff --git a/chrome/browser/extensions/webstore_inline_installer_factory.cc b/chrome/browser/extensions/webstore_inline_installer_factory.cc
new file mode 100644
index 0000000..06f4592
--- /dev/null
+++ b/chrome/browser/extensions/webstore_inline_installer_factory.cc
@@ -0,0 +1,21 @@
+// Copyright 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "base/memory/scoped_ptr.h"
+#include "chrome/browser/extensions/webstore_inline_installer.h"
+#include "chrome/browser/extensions/webstore_inline_installer_factory.h"
+#include "content/public/browser/web_contents.h"
+
+namespace extensions {
+
+WebstoreInlineInstaller* WebstoreInlineInstallerFactory::CreateInstaller(
+ content::WebContents* contents,
+ const std::string& webstore_item_id,
+ const GURL& requestor_url,
+ const WebstoreStandaloneInstaller::Callback& callback) {
+ return new WebstoreInlineInstaller(
+ contents, webstore_item_id, requestor_url, callback);
+}
+
+} // namespace extensions
diff --git a/chrome/browser/extensions/webstore_inline_installer_factory.h b/chrome/browser/extensions/webstore_inline_installer_factory.h
new file mode 100644
index 0000000..b125741
--- /dev/null
+++ b/chrome/browser/extensions/webstore_inline_installer_factory.h
@@ -0,0 +1,38 @@
+// Copyright 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_EXTENSIONS_WEBSTORE_INLINE_INSTALLER_FACTORY_H_
+#define CHROME_BROWSER_EXTENSIONS_WEBSTORE_INLINE_INSTALLER_FACTORY_H_
+
+#include <string>
+
+#include "base/memory/scoped_ptr.h"
+#include "chrome/browser/extensions/extension_install_prompt.h"
+#include "chrome/browser/extensions/webstore_standalone_installer.h"
+
+namespace content {
+ class WebContents;
+}
+
+class GURL;
+
+namespace extensions {
+
+class WebstoreInlineInstaller;
+
+class WebstoreInlineInstallerFactory {
+ public:
+ virtual ~WebstoreInlineInstallerFactory() {}
+
+ // Create a new WebstoreInlineInstallerInstance to be owned by the caller.
+ virtual WebstoreInlineInstaller* CreateInstaller(
+ content::WebContents* contents,
+ const std::string& webstore_item_id,
+ const GURL& requestor_url,
+ const WebstoreStandaloneInstaller::Callback& callback);
+};
+
+} // namespace extensions
+
+#endif // CHROME_BROWSER_EXTENSIONS_WEBSTORE_INLINE_INSTALLER_FACTORY_H_
diff --git a/chrome/browser/extensions/webstore_installer_test.cc b/chrome/browser/extensions/webstore_installer_test.cc
new file mode 100644
index 0000000..26d3549
--- /dev/null
+++ b/chrome/browser/extensions/webstore_installer_test.cc
@@ -0,0 +1,127 @@
+// Copyright 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "base/command_line.h"
+#include "base/strings/stringprintf.h"
+#include "base/strings/utf_string_conversions.h"
+#include "chrome/browser/extensions/extension_install_prompt.h"
+#include "chrome/browser/extensions/tab_helper.h"
+#include "chrome/browser/extensions/webstore_inline_installer.h"
+#include "chrome/browser/extensions/webstore_inline_installer_factory.h"
+#include "chrome/browser/extensions/webstore_installer_test.h"
+#include "chrome/browser/extensions/webstore_standalone_installer.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/common/chrome_switches.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "chrome/test/base/test_switches.h"
+#include "chrome/test/base/ui_test_utils.h"
+#include "content/public/browser/notification_registrar.h"
+#include "content/public/browser/notification_service.h"
+#include "content/public/browser/notification_types.h"
+#include "content/public/browser/render_view_host.h"
+#include "content/public/browser/web_contents.h"
+#include "content/public/test/browser_test_utils.h"
+#include "net/base/host_port_pair.h"
+#include "net/dns/mock_host_resolver.h"
+#include "url/gurl.h"
+
+using content::WebContents;
+using extensions::Extension;
+using extensions::TabHelper;
+using extensions::WebstoreInlineInstaller;
+using extensions::WebstoreInlineInstallerFactory;
+using extensions::WebstoreStandaloneInstaller;
+
+WebstoreInstallerTest::WebstoreInstallerTest(
+ const std::string& webstore_domain,
+ const std::string& test_data_path,
+ const std::string& crx_filename,
+ const std::string& verified_domain,
+ const std::string& unverified_domain)
+ : webstore_domain_(webstore_domain),
+ test_data_path_(test_data_path),
+ crx_filename_(crx_filename),
+ verified_domain_(verified_domain),
+ unverified_domain_(unverified_domain) {
+}
+
+WebstoreInstallerTest::~WebstoreInstallerTest() {}
+
+void WebstoreInstallerTest::SetUpCommandLine(CommandLine* command_line) {
+ // We start the test server now instead of in
+ // SetUpInProcessBrowserTestFixture so that we can get its port number.
+ ASSERT_TRUE(test_server()->Start());
+
+ net::HostPortPair host_port = test_server()->host_port_pair();
+ test_gallery_url_ = base::StringPrintf(
+ "http://%s:%d/files/%s",
+ webstore_domain_.c_str(), host_port.port(), test_data_path_.c_str());
+ command_line->AppendSwitchASCII(
+ switches::kAppsGalleryURL, test_gallery_url_);
+
+ GURL crx_url = GenerateTestServerUrl(webstore_domain_, crx_filename_);
+ CommandLine::ForCurrentProcess()->AppendSwitchASCII(
+ switches::kAppsGalleryUpdateURL, crx_url.spec());
+
+ // Allow tests to call window.gc(), so that we can check that callback
+ // functions don't get collected prematurely.
+ command_line->AppendSwitchASCII(switches::kJavaScriptFlags, "--expose-gc");
+}
+
+void WebstoreInstallerTest::SetUpInProcessBrowserTestFixture() {
+ host_resolver()->AddRule(webstore_domain_, "127.0.0.1");
+ host_resolver()->AddRule(verified_domain_, "127.0.0.1");
+ host_resolver()->AddRule(unverified_domain_, "127.0.0.1");
+}
+
+GURL WebstoreInstallerTest::GenerateTestServerUrl(
+ const std::string& domain,
+ const std::string& page_filename) {
+ GURL page_url = test_server()->GetURL(
+ base::StringPrintf("files/%s/%s",
+ test_data_path_.c_str(),
+ page_filename.c_str()));
+
+ GURL::Replacements replace_host;
+ replace_host.SetHostStr(domain);
+ return page_url.ReplaceComponents(replace_host);
+}
+
+void WebstoreInstallerTest::RunTest(const std::string& test_function_name) {
+ bool result = false;
+ std::string script = base::StringPrintf(
+ "%s('%s')", test_function_name.c_str(),
+ test_gallery_url_.c_str());
+ ASSERT_TRUE(content::ExecuteScriptAndExtractBool(
+ browser()->tab_strip_model()->GetActiveWebContents(),
+ script,
+ &result));
+ EXPECT_TRUE(result);
+}
+
+bool WebstoreInstallerTest::RunIndexedTest(
+ const std::string& test_function_name,
+ int i) {
+ std::string result = "FAILED";
+ std::string script = base::StringPrintf("%s('%s', %d)",
+ test_function_name.c_str(), test_gallery_url_.c_str(), i);
+ EXPECT_TRUE(content::ExecuteScriptAndExtractString(
+ browser()->tab_strip_model()->GetActiveWebContents(),
+ script,
+ &result));
+ EXPECT_TRUE(result != "FAILED");
+ return result == "KEEPGOING";
+}
+
+void WebstoreInstallerTest::RunTestAsync(
+ const std::string& test_function_name) {
+ std::string script = base::StringPrintf(
+ "%s('%s')", test_function_name.c_str(), test_gallery_url_.c_str());
+ browser()->tab_strip_model()->GetActiveWebContents()->GetRenderViewHost()->
+ ExecuteJavascriptInWebFrame(
+ UTF8ToUTF16(std::string()),
+ UTF8ToUTF16(script));
+}
diff --git a/chrome/browser/extensions/webstore_installer_test.h b/chrome/browser/extensions/webstore_installer_test.h
new file mode 100644
index 0000000..40518ba
--- /dev/null
+++ b/chrome/browser/extensions/webstore_installer_test.h
@@ -0,0 +1,54 @@
+// Copyright 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_EXTENSIONS_WEBSTORE_INSTALLER_TEST_H_
+#define CHROME_BROWSER_EXTENSIONS_WEBSTORE_INSTALLER_TEST_H_
+
+#include <string>
+
+#include "chrome/test/base/in_process_browser_test.h"
+#include "url/gurl.h"
+
+namespace base {
+class CommandLine;
+} // namespace base
+
+class WebstoreInstallerTest : public InProcessBrowserTest {
+ public:
+ WebstoreInstallerTest(const std::string& webstore_domain,
+ const std::string& test_data_path,
+ const std::string& crx_filename,
+ const std::string& verified_domain,
+ const std::string& unverified_domain);
+ virtual ~WebstoreInstallerTest();
+
+ virtual void SetUpCommandLine(CommandLine* command_line) OVERRIDE;
+ virtual void SetUpInProcessBrowserTestFixture() OVERRIDE;
+
+ protected:
+ GURL GenerateTestServerUrl(const std::string& domain,
+ const std::string& page_filename);
+
+ void RunTest(const std::string& test_function_name);
+
+ // Passes |i| to |test_function_name|, and expects that function to
+ // return one of "FAILED", "KEEPGOING" or "DONE". KEEPGOING should be
+ // returned if more tests remain to be run and the current test succeeded,
+ // FAILED is returned when a test fails, and DONE is returned by the last
+ // test if it succeeds.
+ // This methods returns true iff there are more tests that need to be run.
+ bool RunIndexedTest(const std::string& test_function_name, int i);
+
+ // Runs a test without waiting for any results from the renderer.
+ void RunTestAsync(const std::string& test_function_name);
+
+ std::string webstore_domain_;
+ std::string test_data_path_;
+ std::string crx_filename_;
+ std::string verified_domain_;
+ std::string unverified_domain_;
+ std::string test_gallery_url_;
+};
+
+#endif // CHROME_BROWSER_EXTENSIONS_WEBSTORE_INSTALLER_TEST_H_
diff --git a/chrome/browser/extensions/webstore_standalone_installer.cc b/chrome/browser/extensions/webstore_standalone_installer.cc
index c9a2f96..3100c95 100644
--- a/chrome/browser/extensions/webstore_standalone_installer.cc
+++ b/chrome/browser/extensions/webstore_standalone_installer.cc
@@ -57,7 +57,10 @@
//
void WebstoreStandaloneInstaller::BeginInstall() {
- AddRef(); // Balanced in CompleteInstall or WebContentsDestroyed.
+ // Add a ref to keep this alive for WebstoreDataFetcher.
+ // All code paths from here eventually lead to either CompleteInstall or
+ // AbortInstall, which both release this ref.
+ AddRef();
if (!Extension::IdIsValid(id_)) {
CompleteInstall(kInvalidWebstoreItemId);
@@ -188,6 +191,9 @@
ShowInstallUI();
// Control flow finishes up in InstallUIProceed or InstallUIAbort.
} else {
+ // Balanced in InstallUIAbort or indirectly in InstallUIProceed via
+ // OnExtensionInstallSuccess or OnExtensionInstallFailure.
+ AddRef();
InstallUIProceed();
}
}
@@ -227,12 +233,14 @@
void WebstoreStandaloneInstaller::InstallUIAbort(bool user_initiated) {
CompleteInstall(kUserCancelledError);
+ Release(); // Balanced in ShowInstallUI.
}
void WebstoreStandaloneInstaller::OnExtensionInstallSuccess(
const std::string& id) {
CHECK_EQ(id_, id);
CompleteInstall(std::string());
+ Release(); // Balanced in ShowInstallUI.
}
void WebstoreStandaloneInstaller::OnExtensionInstallFailure(
@@ -241,6 +249,7 @@
WebstoreInstaller::FailureReason cancelled) {
CHECK_EQ(id_, id);
CompleteInstall(error);
+ Release(); // Balanced in ShowInstallUI.
}
void WebstoreStandaloneInstaller::AbortInstall() {
@@ -253,13 +262,8 @@
}
void WebstoreStandaloneInstaller::CompleteInstall(const std::string& error) {
- // Clear webstore_data_fetcher_ so that WebContentsDestroyed will no longer
- // call Release in case the WebContents is destroyed before this object.
- scoped_ptr<WebstoreDataFetcher> webstore_data_fetcher(
- webstore_data_fetcher_.Pass());
if (!callback_.is_null())
callback_.Run(error.empty(), error);
-
Release(); // Matches the AddRef in BeginInstall.
}
@@ -279,6 +283,11 @@
return;
}
+ // Keep this alive as long as the install prompt lives.
+ // Balanced in InstallUIAbort or indirectly in InstallUIProceed via
+ // OnExtensionInstallSuccess or OnExtensionInstallFailure.
+ AddRef();
+
install_ui_ = CreateInstallUI();
install_ui_->ConfirmStandaloneInstall(
this, localized_extension_for_display_.get(), &icon_, *install_prompt_);
diff --git a/chrome/browser/extensions/webstore_startup_installer_browsertest.cc b/chrome/browser/extensions/webstore_startup_installer_browsertest.cc
index 25ed2774..24de760 100644
--- a/chrome/browser/extensions/webstore_startup_installer_browsertest.cc
+++ b/chrome/browser/extensions/webstore_startup_installer_browsertest.cc
@@ -3,15 +3,13 @@
// found in the LICENSE file.
#include "base/command_line.h"
-#include "base/strings/stringprintf.h"
-#include "base/strings/utf_string_conversions.h"
#include "chrome/browser/chrome_notification_types.h"
#include "chrome/browser/extensions/extension_host.h"
#include "chrome/browser/extensions/extension_install_ui.h"
#include "chrome/browser/extensions/extension_service.h"
#include "chrome/browser/extensions/extension_system.h"
#include "chrome/browser/extensions/startup_helper.h"
-#include "chrome/browser/extensions/webstore_standalone_installer.h"
+#include "chrome/browser/extensions/webstore_installer_test.h"
#include "chrome/browser/infobars/infobar_service.h"
#include "chrome/browser/managed_mode/managed_user_service.h"
#include "chrome/browser/managed_mode/managed_user_service_factory.h"
@@ -30,7 +28,6 @@
#include "content/public/browser/notification_types.h"
#include "content/public/browser/web_contents.h"
#include "content/public/test/browser_test_utils.h"
-#include "net/base/host_port_pair.h"
#include "net/dns/mock_host_resolver.h"
#include "url/gurl.h"
@@ -44,79 +41,18 @@
const char kAppDomain[] = "app.com";
const char kNonAppDomain[] = "nonapp.com";
const char kTestExtensionId[] = "ecglahbcnmdpdciemllbhojghbkagdje";
+const char kTestDataPath[] = "extensions/api_test/webstore_inline_install";
+const char kCrxFilename[] = "extension.crx";
-class WebstoreStartupInstallerTest : public InProcessBrowserTest {
+class WebstoreStartupInstallerTest : public WebstoreInstallerTest {
public:
- virtual void SetUpCommandLine(CommandLine* command_line) OVERRIDE {
- // We start the test server now instead of in
- // SetUpInProcessBrowserTestFixture so that we can get its port number.
- ASSERT_TRUE(test_server()->Start());
-
- net::HostPortPair host_port = test_server()->host_port_pair();
- test_gallery_url_ = base::StringPrintf(
- "http://%s:%d/files/extensions/api_test/webstore_inline_install",
- kWebstoreDomain, host_port.port());
- command_line->AppendSwitchASCII(
- switches::kAppsGalleryURL, test_gallery_url_);
-
- GURL crx_url = GenerateTestServerUrl(kWebstoreDomain, "extension.crx");
- CommandLine::ForCurrentProcess()->AppendSwitchASCII(
- switches::kAppsGalleryUpdateURL, crx_url.spec());
-
- // Allow tests to call window.gc(), so that we can check that callback
- // functions don't get collected prematurely.
- command_line->AppendSwitchASCII(switches::kJavaScriptFlags, "--expose-gc");
- }
-
- virtual void SetUpInProcessBrowserTestFixture() OVERRIDE {
- host_resolver()->AddRule(kWebstoreDomain, "127.0.0.1");
- host_resolver()->AddRule(kAppDomain, "127.0.0.1");
- host_resolver()->AddRule(kNonAppDomain, "127.0.0.1");
- }
-
- protected:
- GURL GenerateTestServerUrl(const std::string& domain,
- const std::string& page_filename) {
- GURL page_url = test_server()->GetURL(
- "files/extensions/api_test/webstore_inline_install/" + page_filename);
-
- GURL::Replacements replace_host;
- replace_host.SetHostStr(domain);
- return page_url.ReplaceComponents(replace_host);
- }
-
- void RunTest(const std::string& test_function_name) {
- bool result = false;
- std::string script = base::StringPrintf(
- "%s('%s')", test_function_name.c_str(),
- test_gallery_url_.c_str());
- ASSERT_TRUE(content::ExecuteScriptAndExtractBool(
- browser()->tab_strip_model()->GetActiveWebContents(),
- script,
- &result));
- EXPECT_TRUE(result);
- }
-
- // Passes |i| to |test_function_name|, and expects that function to
- // return one of "FAILED", "KEEPGOING" or "DONE". KEEPGOING should be
- // returned if more tests remain to be run and the current test succeeded,
- // FAILED is returned when a test fails, and DONE is returned by the last
- // test if it succeeds.
- // This methods returns true iff there are more tests that need to be run.
- bool RunIndexedTest(const std::string& test_function_name,
- int i) {
- std::string result = "FAILED";
- std::string script = base::StringPrintf("%s('%s', %d)",
- test_function_name.c_str(), test_gallery_url_.c_str(), i);
- EXPECT_TRUE(content::ExecuteScriptAndExtractString(
- browser()->tab_strip_model()->GetActiveWebContents(),
- script,
- &result));
- EXPECT_TRUE(result != "FAILED");
- return result == "KEEPGOING";
- }
-
- std::string test_gallery_url_;
+ WebstoreStartupInstallerTest()
+ : WebstoreInstallerTest(
+ kWebstoreDomain,
+ kTestDataPath,
+ kCrxFilename,
+ kAppDomain,
+ kNonAppDomain) {}
};
IN_PROC_BROWSER_TEST_F(WebstoreStartupInstallerTest, Install) {
diff --git a/chrome/chrome_browser_extensions.gypi b/chrome/chrome_browser_extensions.gypi
index b0c4e69..8067930c 100644
--- a/chrome/chrome_browser_extensions.gypi
+++ b/chrome/chrome_browser_extensions.gypi
@@ -855,6 +855,8 @@
'browser/extensions/webstore_data_fetcher_delegate.h',
'browser/extensions/webstore_inline_installer.cc',
'browser/extensions/webstore_inline_installer.h',
+ 'browser/extensions/webstore_inline_installer_factory.cc',
+ 'browser/extensions/webstore_inline_installer_factory.h',
'browser/extensions/webstore_install_helper.cc',
'browser/extensions/webstore_install_helper.h',
'browser/extensions/webstore_installer.cc',
diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi
index 06efc77..40e460f 100644
--- a/chrome/chrome_tests.gypi
+++ b/chrome/chrome_tests.gypi
@@ -1219,6 +1219,9 @@
'browser/extensions/test_extension_dir.cc',
'browser/extensions/test_extension_dir.h',
'browser/extensions/web_contents_browsertest.cc',
+ 'browser/extensions/webstore_inline_installer_browsertest.cc',
+ 'browser/extensions/webstore_installer_test.cc',
+ 'browser/extensions/webstore_installer_test.h',
'browser/extensions/webstore_startup_installer_browsertest.cc',
'browser/extensions/window_open_apitest.cc',
'browser/external_extension_browsertest.cc',
