Reland "Use new SafeBrowsing redirect tracking code in CWS pings.""

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 c7f6a627e41a7af84edba5a33a2c41800737cf73..bf9effed2ed60bffda7c5963a9db8000f30a5052 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,
@@ -127,6 +128,7 @@ class WebstoreInlineInstallerForTest : public WebstoreInlineInstaller {
             base::Bind(&WebstoreInlineInstallerForTest::InstallCallback,
                        base::Unretained(this))),
         install_result_target_(nullptr),
+        enable_safebrowsing_redirects_(enable_safebrowsing_redirects),
         programmable_prompt_(nullptr) {}
 
   std::unique_ptr<ExtensionInstallPrompt> CreateInstallUI() override {
@@ -139,6 +141,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 +172,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 +199,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 +391,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 +421,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 +447,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 +461,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 +511,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() {}