Limit idle users' speculative connections

When a user is idle (i.e., has not navigated or foreground/background
the tab for 5 minutes), we should stop attempting to preconnect to the
foreground tab.

Bug: 1055256
Change-Id: I76324c36508f0ee8720058db6f02235df482c693
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2076217Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Commit-Queue: Ryan Sturm <ryansturm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#745616}

chrome/browser/navigation_predictor/navigation_predictor_preconnect_client.cc

@@ -66,14 +66,14 @@ void NavigationPredictorPreconnectClient::DidFinishNavigation(
     int delay_ms = base::GetFieldTrialParamByFeatureAsInt(
         kPreconnectOnDidFinishNavigation, "delay_after_commit_in_ms", 3000);
     if (delay_ms <= 0) {
-      MaybePreconnectNow();
+      MaybePreconnectNow(/*preconnects_attempted=*/0u);
       return;
     }
 
     timer_.Start(
         FROM_HERE, base::TimeDelta::FromMilliseconds(delay_ms),
         base::BindOnce(&NavigationPredictorPreconnectClient::MaybePreconnectNow,
-                       base::Unretained(this)));
+                       base::Unretained(this), /*preconnects_attempted=*/0u));
   }
 }
 
@@ -99,7 +99,7 @@ void NavigationPredictorPreconnectClient::OnVisibilityChanged(
 
   // Previously, the visibility was HIDDEN, and now it is VISIBLE implying that
   // the web contents that was fully hidden is now fully visible.
-  MaybePreconnectNow();
+  MaybePreconnectNow(/*preconnects_attempted=*/0u);
 }
 
 void NavigationPredictorPreconnectClient::DidFinishLoad(
@@ -109,10 +109,11 @@ void NavigationPredictorPreconnectClient::DidFinishLoad(
   if (render_frame_host->GetParent())
     return;
 
-  MaybePreconnectNow();
+  MaybePreconnectNow(/*preconnects_attempted=*/0u);
 }
 
-void NavigationPredictorPreconnectClient::MaybePreconnectNow() {
+void NavigationPredictorPreconnectClient::MaybePreconnectNow(
+    size_t preconnects_attempted) {
   if (base::FeatureList::IsEnabled(
           features::kNavigationPredictorPreconnectHoldback))
     return;
@@ -124,10 +125,14 @@ void NavigationPredictorPreconnectClient::MaybePreconnectNow() {
   if (current_visibility_ != content::Visibility::VISIBLE)
     return;
 
-  // On search engine results page, next navigation is likely to be a different
-  // origin. Currently, the preconnect is only allowed for same origins. Hence,
-  // preconnect is currently disabled on search engine results page.
-  // If preconnect to DSE is enabled, skip this check.
+  // Only allow 5 preconnects per foreground/load.
+  if (preconnects_attempted >= 5u)
+    return;
+
+  // On search engine results page, next navigation is likely to be a
+  // different origin. Currently, the preconnect is only allowed for same
+  // origins. Hence, preconnect is currently disabled on search engine results
+  // page. If preconnect to DSE is enabled, skip this check.
   if (!base::FeatureList::IsEnabled(features::kPreconnectToSearch) &&
       IsSearchEnginePage())
     return;
@@ -164,7 +169,7 @@ void NavigationPredictorPreconnectClient::MaybePreconnectNow() {
           "unused_idle_socket_timeout_seconds", 60)) +
           base::TimeDelta::FromMilliseconds(retry_delay_ms),
       base::BindOnce(&NavigationPredictorPreconnectClient::MaybePreconnectNow,
-                     base::Unretained(this)));
+                     base::Unretained(this), preconnects_attempted + 1));
 }
 
 bool NavigationPredictorPreconnectClient::IsSearchEnginePage() const {

chrome/browser/navigation_predictor/navigation_predictor_preconnect_client.h

@@ -49,7 +49,7 @@ class NavigationPredictorPreconnectClient
       const GURL& document_url) const;
 
   // MaybePreconnectNow preconnects to an origin server if it's allowed.
-  void MaybePreconnectNow();
+  void MaybePreconnectNow(size_t preconnects_attempted);
 
   // Used to get keyed services.
   content::BrowserContext* const browser_context_;

chrome/browser/navigation_predictor/navigation_predictor_preconnect_client_browsertest.cc

@@ -6,6 +6,7 @@
 #include "base/run_loop.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/test/scoped_feature_list.h"
+#include "base/test/test_timeouts.h"
 #include "build/build_config.h"
 #include "chrome/browser/predictors/loading_predictor.h"
 #include "chrome/browser/predictors/loading_predictor_factory.h"
@@ -184,6 +185,26 @@ IN_PROC_BROWSER_TEST_F(
   EXPECT_EQ(4, preresolve_done_count_);
 }
 
+// Test that we preconnect after the last preconnect timed out.
+IN_PROC_BROWSER_TEST_F(
+    NavigationPredictorPreconnectClientBrowserTestWithUnusedIdleSocketTimeout,
+    CappedAtFiveAttempts) {
+  const GURL& url = GetTestURL("/page_with_same_host_anchor_element.html");
+  ui_test_utils::NavigateToURL(browser(), url);
+
+  // Expect 1 navigation preresolve and 5 repeated onLoad calls.
+  WaitForPreresolveCount(6);
+  EXPECT_EQ(6, preresolve_done_count_);
+
+  // We should not see additional preresolves.
+  base::RunLoop run_loop;
+  base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
+      FROM_HERE, run_loop.QuitClosure(), TestTimeouts::tiny_timeout());
+  run_loop.Run();
+
+  EXPECT_EQ(6, preresolve_done_count_);
+}
+
 class NavigationPredictorPreconnectClientBrowserTestWithHoldback
     : public NavigationPredictorPreconnectClientBrowserTest {
  public: