Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1158)

Unified Diff: chrome/browser/extensions/webstore_inline_installer_browsertest.cc

Issue 2779643002: Use new SafeBrowsing redirect tracking code in CWS pings. (Closed)
Patch Set: Devlin feedback + rebase. Created 3 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « chrome/browser/extensions/webstore_inline_installer.cc ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
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 7d71fb4a0bffe71e614aaff45f16d7b010a73690..32a4bb5b72457b90beb2fab2bdf3fbc3c26911c8 100644
--- a/chrome/browser/extensions/webstore_inline_installer_browsertest.cc
+++ b/chrome/browser/extensions/webstore_inline_installer_browsertest.cc
@@ -118,7 +118,8 @@ class WebstoreInlineInstallerForTest : public WebstoreInlineInstaller {
content::RenderFrameHost* host,
const std::string& extension_id,
const GURL& requestor_url,
- const Callback& callback)
+ const Callback& callback,
+ bool enable_safebrowsing_redirects)
: WebstoreInlineInstaller(
contents,
host,
@@ -139,6 +140,10 @@ class WebstoreInlineInstallerForTest : public WebstoreInlineInstaller {
return WebstoreInlineInstaller::CheckRequestorAlive();
}
+ bool SafeBrowsingNavigationEventsEnabled() const override {
+ return enable_safebrowsing_redirects_;
+ }
+
// Tests that care about the actual arguments to the install callback can use
// this to receive a copy in |install_result_target|.
void set_install_result_target(
@@ -166,13 +171,22 @@ class WebstoreInlineInstallerForTest : public WebstoreInlineInstaller {
// arguments.
std::unique_ptr<InstallResult>* install_result_target_;
+ // This can be set by tests that want to use the new SafeBrowsing redirect
+ // tracker.
+ bool enable_safebrowsing_redirects_;
+
ProgrammableInstallPrompt* programmable_prompt_;
};
class WebstoreInlineInstallerForTestFactory :
public WebstoreInlineInstallerFactory {
public:
- WebstoreInlineInstallerForTestFactory() : last_installer_(nullptr) {}
+ WebstoreInlineInstallerForTestFactory()
+ : last_installer_(nullptr), enable_safebrowsing_redirects_(false) {}
+ explicit WebstoreInlineInstallerForTestFactory(
+ bool enable_safebrowsing_redirects)
+ : last_installer_(nullptr),
+ enable_safebrowsing_redirects_(enable_safebrowsing_redirects) {}
~WebstoreInlineInstallerForTestFactory() override {}
WebstoreInlineInstallerForTest* last_installer() { return last_installer_; }
@@ -184,13 +198,16 @@ class WebstoreInlineInstallerForTestFactory :
const GURL& requestor_url,
const WebstoreStandaloneInstaller::Callback& callback) override {
last_installer_ = new WebstoreInlineInstallerForTest(
- contents, host, webstore_item_id, requestor_url, callback);
+ contents, host, webstore_item_id, requestor_url, callback,
+ enable_safebrowsing_redirects_);
return last_installer_;
}
private:
// The last installer that was created.
WebstoreInlineInstallerForTest* last_installer_;
+
+ bool enable_safebrowsing_redirects_;
};
IN_PROC_BROWSER_TEST_F(WebstoreInlineInstallerTest,
@@ -373,7 +390,9 @@ IN_PROC_BROWSER_TEST_F(WebstoreInlineInstallerTest, DoubleInlineInstallTest) {
RunTest("runTest");
}
-class WebstoreInlineInstallerRedirectTest : public WebstoreInlineInstallerTest {
+class WebstoreInlineInstallerRedirectTest
+ : public WebstoreInlineInstallerTest,
+ public ::testing::WithParamInterface<bool> {
public:
WebstoreInlineInstallerRedirectTest() : cws_request_received_(false) {}
~WebstoreInlineInstallerRedirectTest() override {}
@@ -401,8 +420,16 @@ class WebstoreInlineInstallerRedirectTest : public WebstoreInlineInstallerTest {
// Test that an install from a page arrived at via redirects includes the
// redirect information in the webstore request.
-IN_PROC_BROWSER_TEST_F(WebstoreInlineInstallerRedirectTest,
+IN_PROC_BROWSER_TEST_P(WebstoreInlineInstallerRedirectTest,
IncludesRedirectData) {
+ const bool using_safe_browsing_tracker = GetParam();
+ WebContents* web_contents =
+ browser()->tab_strip_model()->GetActiveWebContents();
+ TabHelper* tab_helper = TabHelper::FromWebContents(web_contents);
+ WebstoreInlineInstallerForTestFactory* factory =
+ new WebstoreInlineInstallerForTestFactory(using_safe_browsing_tracker);
+ tab_helper->SetWebstoreInlineInstallerFactoryForTests(factory);
+
// Hand craft a url that will cause the test server to issue redirects.
const std::vector<std::string> redirects = {kRedirect1Domain,
kRedirect2Domain};
@@ -419,7 +446,11 @@ IN_PROC_BROWSER_TEST_F(WebstoreInlineInstallerRedirectTest,
AutoAcceptInstall();
ui_test_utils::NavigateToURL(browser(), install_url);
- RunTest("runTest");
+
+ RunTestAsync("runTest");
+ while (!ProgrammableInstallPrompt::Ready())
+ base::RunLoop().RunUntilIdle();
+ web_contents->Close();
EXPECT_TRUE(cws_request_received_);
ASSERT_NE(nullptr, cws_request_json_data_);
@@ -429,15 +460,21 @@ IN_PROC_BROWSER_TEST_F(WebstoreInlineInstallerRedirectTest,
ASSERT_NE(nullptr, redirect_list);
// Check that the expected domains are in the redirect list.
- const std::vector<std::string> expected_redirect_domains = {
+ const std::set<std::string> expected_redirect_domains = {
kRedirect1Domain, kRedirect2Domain, kAppDomain};
- ASSERT_EQ(expected_redirect_domains.size(), redirect_list->GetSize());
- int i = 0;
+
+ // The SafeBrowsing tracker has a much more liberal definition of "redirect"
+ // and it may (based on timing) pick up additional navigations that occur
+ // shortly before the navigation we mainly care about here. Be somewhat
+ // permissive in what we accept as redirect results.
+ ASSERT_LE(expected_redirect_domains.size(), redirect_list->GetSize());
+
for (const auto& value : *redirect_list) {
std::string value_string;
ASSERT_TRUE(value.GetAsString(&value_string));
GURL redirect_url(value_string);
- EXPECT_EQ(expected_redirect_domains[i++], redirect_url.host());
+ EXPECT_TRUE(expected_redirect_domains.find(redirect_url.host()) !=
+ expected_redirect_domains.end());
}
}
@@ -473,6 +510,13 @@ IN_PROC_BROWSER_TEST_F(WebstoreInlineInstallerRedirectTest,
ASSERT_EQ(nullptr, cws_request_json_data_);
}
+INSTANTIATE_TEST_CASE_P(NetRedirectTracking,
+ WebstoreInlineInstallerRedirectTest,
+ testing::Values(false));
+INSTANTIATE_TEST_CASE_P(SafeBrowsingRedirectTracking,
+ WebstoreInlineInstallerRedirectTest,
+ testing::Values(true));
+
class WebstoreInlineInstallerListenerTest : public WebstoreInlineInstallerTest {
public:
WebstoreInlineInstallerListenerTest() {}
« no previous file with comments | « chrome/browser/extensions/webstore_inline_installer.cc ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698