Allow triggering of same-origin preconnects on same-document navigations

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

chrome/browser/BUILD.gn

@@ -830,6 +830,8 @@ static_library("browser") {
     "native_window_notification_source.h",
     "navigation_predictor/navigation_predictor.cc",
     "navigation_predictor/navigation_predictor.h",
+    "navigation_predictor/navigation_predictor_features.cc",
+    "navigation_predictor/navigation_predictor_features.h",
     "navigation_predictor/navigation_predictor_keyed_service.cc",
     "navigation_predictor/navigation_predictor_keyed_service.h",
     "navigation_predictor/navigation_predictor_keyed_service_factory.cc",

chrome/browser/navigation_predictor/navigation_predictor_features.cc

+// Copyright 2020 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/navigation_predictor/navigation_predictor_features.h"
+
+#include "build/build_config.h"
+
+namespace features {
+
+// A holdback that prevents the preconnect to measure benefit of the feature.
+const base::Feature kNavigationPredictorPreconnectHoldback {
+  "NavigationPredictorPreconnectHoldback",
+#if defined(OS_ANDROID)
+      base::FEATURE_DISABLED_BY_DEFAULT
+#else
+      base::FEATURE_ENABLED_BY_DEFAULT
+#endif
+};
+
+// Enables triggering of same-origin preconnects on same-document navigations.
+const base::Feature
+    kNavigationPredictorEnablePreconnectOnSameDocumentNavigations{
+        "NavigationPredictorEnablePreconnectOnSameDocumentNavigations",
+        base::FEATURE_DISABLED_BY_DEFAULT};
+
+}  // namespace features

chrome/browser/navigation_predictor/navigation_predictor_features.h

+// Copyright 2020 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_NAVIGATION_PREDICTOR_NAVIGATION_PREDICTOR_FEATURES_H_
+#define CHROME_BROWSER_NAVIGATION_PREDICTOR_NAVIGATION_PREDICTOR_FEATURES_H_
+
+#include "base/feature_list.h"
+
+namespace features {
+
+// All features in alphabetical order. The features should be documented
+// alongside the definition of their values in the .cc file.
+extern const base::Feature kNavigationPredictorPreconnectHoldback;
+extern const base::Feature
+    kNavigationPredictorEnablePreconnectOnSameDocumentNavigations;
+
+}  // namespace features
+
+#endif  // CHROME_BROWSER_NAVIGATION_PREDICTOR_NAVIGATION_PREDICTOR_FEATURES_H_

chrome/browser/navigation_predictor/navigation_predictor_preconnect_client.cc

@@ -11,6 +11,7 @@
 #include "base/metrics/field_trial_params.h"
 #include "base/time/time.h"
 #include "build/build_config.h"
+#include "chrome/browser/navigation_predictor/navigation_predictor_features.h"
 #include "chrome/browser/navigation_predictor/search_engine_preconnector.h"
 #include "chrome/browser/predictors/loading_predictor.h"
 #include "chrome/browser/predictors/loading_predictor_factory.h"
@@ -23,19 +24,6 @@
 #include "content/public/browser/web_contents.h"
 #include "net/base/features.h"
 
-namespace features {
-
-// A holdback that prevents the preconnect to measure benefit of the feature.
-const base::Feature kNavigationPredictorPreconnectHoldback {
-  "NavigationPredictorPreconnectHoldback",
-#if defined(OS_ANDROID)
-      base::FEATURE_DISABLED_BY_DEFAULT
-#else
-      base::FEATURE_ENABLED_BY_DEFAULT
-#endif
-};
-}  // namespace features
-
 namespace {
 
 // Experiment with which event triggers the preconnect after commit.
@@ -56,13 +44,22 @@ NavigationPredictorPreconnectClient::~NavigationPredictorPreconnectClient() =
 void NavigationPredictorPreconnectClient::DidFinishNavigation(
     content::NavigationHandle* navigation_handle) {
   if (!navigation_handle->IsInMainFrame() ||
-      !navigation_handle->HasCommitted() || navigation_handle->IsSameDocument())
+      !navigation_handle->HasCommitted() ||
+      (!base::FeatureList::IsEnabled(
+           features::
+               kNavigationPredictorEnablePreconnectOnSameDocumentNavigations) &&
+       navigation_handle->IsSameDocument())) {
+    return;
+  }
+
+  if (!navigation_handle->GetURL().SchemeIsHTTPOrHTTPS())
     return;
 
   // New page, so stop the preconnect timer.
   timer_.Stop();
 
-  if (base::FeatureList::IsEnabled(kPreconnectOnDidFinishNavigation)) {
+  if (base::FeatureList::IsEnabled(kPreconnectOnDidFinishNavigation) ||
+      navigation_handle->IsSameDocument()) {
     int delay_ms = base::GetFieldTrialParamByFeatureAsInt(
         kPreconnectOnDidFinishNavigation, "delay_after_commit_in_ms", 3000);
     if (delay_ms <= 0) {

chrome/browser/navigation_predictor/navigation_predictor_preconnect_client.h

@@ -19,10 +19,6 @@ class BrowserContext;
 class RenderFrameHost;
 }  // namespace content
 
-namespace features {
-extern const base::Feature kNavigationPredictorPreconnectHoldback;
-}
-
 class NavigationPredictorPreconnectClient
     : public content::WebContentsObserver,
       public content::WebContentsUserData<NavigationPredictorPreconnectClient> {

chrome/browser/navigation_predictor/navigation_predictor_preconnect_client_browsertest.cc

@@ -8,6 +8,7 @@
 #include "base/test/scoped_feature_list.h"
 #include "base/test/test_timeouts.h"
 #include "build/build_config.h"
+#include "chrome/browser/navigation_predictor/navigation_predictor_features.h"
 #include "chrome/browser/navigation_predictor/navigation_predictor_keyed_service.h"
 #include "chrome/browser/navigation_predictor/navigation_predictor_keyed_service_factory.h"
 #include "chrome/browser/navigation_predictor/search_engine_preconnector.h"
@@ -27,6 +28,7 @@
 #include "net/base/features.h"
 #include "net/dns/mock_host_resolver.h"
 #include "net/test/embedded_test_server/embedded_test_server.h"
+#include "url/origin.h"
 
 namespace {
 
@@ -70,6 +72,12 @@ class NavigationPredictorPreconnectClientBrowserTest
       const GURL& url,
       const net::NetworkIsolationKey& network_isolation_key,
       bool success) override {
+    // The tests do not care about preresolves to non-test server (e.g., hard
+    // coded preconnects to google.com).
+    if (url::Origin::Create(url) !=
+        url::Origin::Create(https_server_->base_url())) {
+      return;
+    }
     EXPECT_TRUE(success);
     preresolve_done_count_++;
     if (run_loop_)
@@ -199,6 +207,14 @@ IN_PROC_BROWSER_TEST_F(
   run_loop.Run();
 
   EXPECT_EQ(6, preresolve_done_count_);
+
+  // By default, same document navigation should not trigger new preconnects.
+  const GURL& same_document_url =
+      GetTestURL("/page_with_same_host_anchor_element.html#foobar");
+  ui_test_utils::NavigateToURL(browser(), same_document_url);
+  // Expect another one.
+  WaitForPreresolveCount(6);
+  EXPECT_EQ(6, preresolve_done_count_);
 }
 
 class NavigationPredictorPreconnectClientBrowserTestWithHoldback
@@ -284,6 +300,54 @@ IN_PROC_BROWSER_TEST_F(
   EXPECT_EQ(3, preresolve_done_count_);
 }
 
+class NavigationPredictorSameDocumentPreconnectClientBrowserTest
+    : public NavigationPredictorPreconnectClientBrowserTest {
+ public:
+  NavigationPredictorSameDocumentPreconnectClientBrowserTest()
+      : NavigationPredictorPreconnectClientBrowserTest() {
+    // Configure kDelayRequestsOnMultiplexedConnections experiment params.
+    base::FieldTrialParams params_kNetUnusedIdleSocketTimeout;
+    params_kNetUnusedIdleSocketTimeout["unused_idle_socket_timeout_seconds"] =
+        "0";
+
+    // Configure kThrottleDelayable experiment params.
+    base::FieldTrialParams
+        params_kNavigationPredictorEnablePreconnectOnSameDocumentNavigations;
+    feature_list_.InitWithFeaturesAndParameters(
+        {{net::features::kNetUnusedIdleSocketTimeout,
+          params_kNetUnusedIdleSocketTimeout},
+         {features::
+              kNavigationPredictorEnablePreconnectOnSameDocumentNavigations,
+          params_kNavigationPredictorEnablePreconnectOnSameDocumentNavigations}},
+        {});
+  }
+
+ private:
+  base::test::ScopedFeatureList feature_list_;
+};
+
+// Test that we preconnect after the last preconnect timed out.
+IN_PROC_BROWSER_TEST_F(
+    NavigationPredictorSameDocumentPreconnectClientBrowserTest,
+    SameDocumentNavigation) {
+  const GURL& url = GetTestURL("/page_with_same_host_anchor_element.html");
+  ui_test_utils::NavigateToURL(browser(), url);
+
+  WaitForPreresolveCount(3);
+  EXPECT_LE(3, preresolve_done_count_);
+
+  // Expect another one.
+  WaitForPreresolveCount(4);
+  EXPECT_LE(4, preresolve_done_count_);
+
+  const GURL& same_document_url =
+      GetTestURL("/page_with_same_host_anchor_element.html#foobar");
+  ui_test_utils::NavigateToURL(browser(), same_document_url);
+  // Expect another one.
+  WaitForPreresolveCount(8);
+  EXPECT_LE(8, preresolve_done_count_);
+}
+
 namespace {
 // Feature to control preconnect to search.
 const base::Feature kPreconnectToSearchTest{"PreconnectToSearch",

chrome/browser/predictors/loading_predictor_browsertest.cc

@@ -22,6 +22,7 @@
 #include "base/test/scoped_feature_list.h"
 #include "build/build_config.h"
 #include "chrome/browser/browser_features.h"
+#include "chrome/browser/navigation_predictor/navigation_predictor_features.h"
 #include "chrome/browser/navigation_predictor/navigation_predictor_preconnect_client.h"
 #include "chrome/browser/optimization_guide/optimization_guide_keyed_service.h"
 #include "chrome/browser/optimization_guide/optimization_guide_keyed_service_factory.h"