Fix flaky Lite Page Browser test

Removes the random query param hack and prevents the timeout logic from
running in all tests.

Bug: 887962, 885252
Change-Id: Icc001d2e6d584228027ae56a12813c02dcaf0b42
Reviewed-on: https://chromium-review.googlesource.com/1239393Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
Commit-Queue: Robert Ogden <robertogden@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593338}

chrome/browser/previews/previews_lite_page_browsertest.cc

@@ -58,6 +58,12 @@ class PreviewsLitePageServerBrowserTest : public InProcessBrowserTest {
   }
 
   void SetUp() override {
+    SetUpLitePageTest(false);
+
+    InProcessBrowserTest::SetUp();
+  }
+
+  void SetUpLitePageTest(bool use_timeout) {
     https_server_ = std::make_unique<net::EmbeddedTestServer>(
         net::EmbeddedTestServer::TYPE_HTTPS);
     https_server_->ServeFilesFromSourceDirectory("chrome/test/data");
@@ -121,7 +127,7 @@ class PreviewsLitePageServerBrowserTest : public InProcessBrowserTest {
           {"previews_host", previews_server().spec()},
           {"blacklisted_path_suffixes", ".mp4,.jpg"},
           {"trigger_on_localhost", "true"},
-          {"navigation_timeout_milliseconds", "2000"}};
+          {"navigation_timeout_milliseconds", use_timeout ? "2000" : "0"}};
       base::FieldTrialParamAssociator::GetInstance()->AssociateFieldTrialParams(
           "TrialName1", "GroupName1", feature_parameters);
 
@@ -138,8 +144,6 @@ class PreviewsLitePageServerBrowserTest : public InProcessBrowserTest {
           base::FeatureList::OVERRIDE_ENABLE_FEATURE, trial.get());
     }
     scoped_feature_list_.InitWithFeatureList(std::move(feature_list));
-
-    InProcessBrowserTest::SetUp();
   }
 
   void SetUpOnMainThread() override {
@@ -203,8 +207,7 @@ class PreviewsLitePageServerBrowserTest : public InProcessBrowserTest {
   GURL HttpLitePageURL(int return_code,
                        std::string* headers = nullptr,
                        int delay_ms = 0) const {
-    std::string query = "resp=" + base::IntToString(return_code) + "&rand=" +
-                        base::IntToString(base::RandInt(INT_MIN, INT_MAX));
+    std::string query = "resp=" + base::IntToString(return_code);
     if (delay_ms != 0)
       query += "&delay_ms=" + base::IntToString(delay_ms);
     if (headers)
@@ -221,8 +224,7 @@ class PreviewsLitePageServerBrowserTest : public InProcessBrowserTest {
   GURL HttpsLitePageURL(int return_code,
                         std::string* headers = nullptr,
                         int delay_ms = 0) const {
-    std::string query = "resp=" + base::IntToString(return_code) + "&rand=" +
-                        base::IntToString(base::RandInt(INT_MIN, INT_MAX));
+    std::string query = "resp=" + base::IntToString(return_code);
     if (delay_ms != 0)
       query += "&delay_ms=" + base::IntToString(delay_ms);
     if (headers)
@@ -551,37 +553,6 @@ IN_PROC_BROWSER_TEST_F(PreviewsLitePageServerBrowserTest,
   }
 }
 
-// Previews InfoBar (which these tests trigger) does not work on Mac.
-// See https://crbug.com/782322 for detail.
-// Also occasional flakes on win7 (https://crbug.com/789542).
-#if defined(OS_ANDROID) || defined(OS_LINUX)
-#define MAYBE_LitePagePreviewsTimeout LitePagePreviewsTimeout
-#else
-#define MAYBE_LitePagePreviewsTimeout DISABLED_LitePagePreviewsTimeout
-#endif
-IN_PROC_BROWSER_TEST_F(PreviewsLitePageServerBrowserTest,
-                       MAYBE_LitePagePreviewsTimeout) {
-  {
-    // Ensure that a hung previews navigation doesn't wind up at the previews
-    // server.
-    base::HistogramTester histogram_tester;
-    ui_test_utils::NavigateToURL(browser(), HttpsLitePageURL(200, nullptr, -1));
-    VerifyPreviewNotLoaded();
-    histogram_tester.ExpectBucketCount(
-        "Previews.ServerLitePage.ServerResponse",
-        PreviewsLitePageNavigationThrottle::ServerResponse::kTimeout, 1);
-  }
-
-  {
-    // Ensure that a hung normal navigation eventually loads.
-    base::HistogramTester histogram_tester;
-    ui_test_utils::NavigateToURL(browser(), slow_http_url());
-    VerifyPreviewNotLoaded();
-    histogram_tester.ExpectTotalCount("Previews.ServerLitePage.ServerResponse",
-                                      0);
-  }
-}
-
 // Previews InfoBar (which these tests trigger) does not work on Mac.
 // See https://crbug.com/782322 for detail.
 // Also occasional flakes on win7 (https://crbug.com/789542).
@@ -671,6 +642,51 @@ IN_PROC_BROWSER_TEST_F(PreviewsLitePageServerBrowserTest,
   VerifyPreviewLoaded();
 }
 
+class PreviewsLitePageServerTimeoutBrowserTest
+    : public PreviewsLitePageServerBrowserTest {
+ public:
+  PreviewsLitePageServerTimeoutBrowserTest() = default;
+
+  ~PreviewsLitePageServerTimeoutBrowserTest() override = default;
+
+  void SetUp() override {
+    SetUpLitePageTest(true);
+
+    InProcessBrowserTest::SetUp();
+  }
+};
+
+// Previews InfoBar (which these tests trigger) does not work on Mac.
+// See https://crbug.com/782322 for detail.
+// Also occasional flakes on win7 (https://crbug.com/789542).
+#if defined(OS_ANDROID) || defined(OS_LINUX)
+#define MAYBE_LitePagePreviewsTimeout LitePagePreviewsTimeout
+#else
+#define MAYBE_LitePagePreviewsTimeout DISABLED_LitePagePreviewsTimeout
+#endif
+IN_PROC_BROWSER_TEST_F(PreviewsLitePageServerTimeoutBrowserTest,
+                       MAYBE_LitePagePreviewsTimeout) {
+  {
+    // Ensure that a hung previews navigation doesn't wind up at the previews
+    // server.
+    base::HistogramTester histogram_tester;
+    ui_test_utils::NavigateToURL(browser(), HttpsLitePageURL(200, nullptr, -1));
+    VerifyPreviewNotLoaded();
+    histogram_tester.ExpectBucketCount(
+        "Previews.ServerLitePage.ServerResponse",
+        PreviewsLitePageNavigationThrottle::ServerResponse::kTimeout, 1);
+  }
+
+  {
+    // Ensure that a hung normal navigation eventually loads.
+    base::HistogramTester histogram_tester;
+    ui_test_utils::NavigateToURL(browser(), slow_http_url());
+    VerifyPreviewNotLoaded();
+    histogram_tester.ExpectTotalCount("Previews.ServerLitePage.ServerResponse",
+                                      0);
+  }
+}
+
 class PreviewsLitePageServerBadServerBrowserTest
     : public PreviewsLitePageServerBrowserTest {
  public:

chrome/browser/previews/previews_lite_page_navigation_throttle.cc

@@ -329,18 +329,22 @@ PreviewsLitePageNavigationThrottle::TriggerPreview() const {
   // timeout whether the previews navigation has finished (either in success or
   // failure). If not, the helper will stop the ongoing previews navigation and
   // load the original page.
-  base::PostDelayedTaskWithTraits(
-      FROM_HERE, {content::BrowserThread::UI},
-      base::BindOnce(
-          &WebContentsLifetimeHelper::CheckForHungNavigation,
-          helper->GetWeakPtr(), GetPreviewsURL(),
-          base::BindOnce(
-              &PreviewsLitePageNavigationThrottle::LoadAndBypass,
-              base::Unretained(web_contents), manager_,
-              MakeOpenURLParams(navigation_handle(),
-                                navigation_handle()->GetURL(), std::string()),
-              false)),
-      previews::params::LitePagePreviewsNavigationTimeoutDuration());
+  const base::TimeDelta timeout =
+      previews::params::LitePagePreviewsNavigationTimeoutDuration();
+  if (timeout > base::TimeDelta()) {
+    base::PostDelayedTaskWithTraits(
+        FROM_HERE, {content::BrowserThread::UI},
+        base::BindOnce(
+            &WebContentsLifetimeHelper::CheckForHungNavigation,
+            helper->GetWeakPtr(), GetPreviewsURL(),
+            base::BindOnce(
+                &PreviewsLitePageNavigationThrottle::LoadAndBypass,
+                base::Unretained(web_contents), manager_,
+                MakeOpenURLParams(navigation_handle(),
+                                  navigation_handle()->GetURL(), std::string()),
+                false)),
+        timeout);
+  }
 
   // The helper class and its weak pointer protect against the WebContents
   // dying in-between the PostTask and its execution, resulting in a use after