Add a feature to limit DSE preconnect to Google only

This feature allows us to experiment on Google without possibly creating
issues with non-Google DSEs. This will hopefully prevent problems or
regressions with other search engines.

Bug: 1036489
Change-Id: I499b580b1bc0a2d25bed1c8727ba8b36ef6259b2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2013748
Commit-Queue: Ryan Sturm <ryansturm@chromium.org>
Reviewed-by: Robert Ogden <robertogden@chromium.org>
Cr-Commit-Position: refs/heads/master@{#734601}

chrome/browser/navigation_predictor/search_engine_preconnector.cc

@@ -12,6 +12,7 @@
 #include "chrome/browser/predictors/loading_predictor_factory.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/search_engines/template_url_service_factory.h"
+#include "components/google/core/common/google_util.h"
 #include "components/search_engines/template_url_service.h"
 #include "content/public/browser/browser_context.h"
 #include "net/base/features.h"
@@ -20,6 +21,10 @@ namespace features {
 // Feature to control preconnect to search.
 const base::Feature kPreconnectToSearch{"PreconnectToSearch",
                                         base::FEATURE_DISABLED_BY_DEFAULT};
+
+// Feature to limit experimentation to Google search only.
+const base::Feature kPreconnectToSearchNonGoogle{
+    "PreconnectToSearchNonGoogle", base::FEATURE_DISABLED_BY_DEFAULT};
 }  // namespace features
 
 SearchEnginePreconnector::SearchEnginePreconnector(
@@ -36,6 +41,7 @@ SearchEnginePreconnector::SearchEnginePreconnector(
 #endif  // defined(OS_ANDROID)
       browser_context_(browser_context),
       currently_in_foreground_(true) {
+
   DCHECK(!browser_context_->IsOffTheRecord());
 
 #if defined(OS_ANDROID)
@@ -119,6 +125,13 @@ void SearchEnginePreconnector::PreconnectDSE() {
       preconnect_url.scheme() != url::kHttpsScheme) {
     return;
   }
+  // Limit experimentation to [www].google.com only.
+  if (!base::FeatureList::IsEnabled(features::kPreconnectToSearchNonGoogle) &&
+      !google_util::IsGoogleDomainUrl(preconnect_url,
+                                      google_util::DISALLOW_SUBDOMAIN,
+                                      google_util::ALLOW_NON_STANDARD_PORTS)) {
+    return;
+  }
 
   auto* loading_predictor = predictors::LoadingPredictorFactory::GetForProfile(
       Profile::FromBrowserContext(browser_context_));

chrome/browser/navigation_predictor/search_engine_preconnector.h

@@ -21,6 +21,7 @@ class BrowserContext;
 
 namespace features {
 extern const base::Feature kPreconnectToSearch;
+extern const base::Feature kPreconnectToSearchNonGoogle;
 }  // namespace features
 
 // Class to preconnect to the user's default search engine at regular intervals.

chrome/browser/navigation_predictor/search_engine_preconnector_browsertest.cc

@@ -28,10 +28,6 @@
 
 namespace {
 
-// Feature to control preconnect to search.
-const base::Feature kPreconnectToSearchTest{"PreconnectToSearch",
-                                            base::FEATURE_DISABLED_BY_DEFAULT};
-
 // TODO(https://crbug.com/1042727): Fix test GURL scoping and remove this getter
 // function.
 GURL FakeSearch() {
@@ -129,7 +125,8 @@ class SearchEnginePreconnectorNoDelaysBrowserTest
   SearchEnginePreconnectorNoDelaysBrowserTest() {
     {
       feature_list_.InitWithFeaturesAndParameters(
-          {{kPreconnectToSearchTest, {{"startup_delay_ms", "0"}}},
+          {{features::kPreconnectToSearch, {{"startup_delay_ms", "0"}}},
+           {features::kPreconnectToSearchNonGoogle, {{}}},
            {net::features::kNetUnusedIdleSocketTimeout,
             {{"unused_idle_socket_timeout_seconds", "0"}}}},
           {});
@@ -239,7 +236,8 @@ class SearchEnginePreconnectorKeepSocketBrowserTest
   SearchEnginePreconnectorKeepSocketBrowserTest() {
     {
       feature_list_.InitWithFeaturesAndParameters(
-          {{kPreconnectToSearchTest, {{"startup_delay_ms", "0"}}},
+          {{features::kPreconnectToSearch, {{"startup_delay_ms", "0"}}},
+           {features::kPreconnectToSearchNonGoogle, {{}}},
            {net::features::kNetUnusedIdleSocketTimeout,
             {{"unused_idle_socket_timeout_seconds", "60"}}}},
           {});
@@ -330,4 +328,67 @@ IN_PROC_BROWSER_TEST_F(SearchEnginePreconnectorKeepSocketBrowserTest,
   }
 }
 
+class SearchEnginePreconnectorGoogleOnlyBrowserTest
+    : public SearchEnginePreconnectorBrowserTest {
+ public:
+  SearchEnginePreconnectorGoogleOnlyBrowserTest() {
+    {
+      feature_list_.InitWithFeaturesAndParameters(
+          {{features::kPreconnectToSearch, {{"startup_delay_ms", "0"}}},
+           {net::features::kNetUnusedIdleSocketTimeout,
+            {{"unused_idle_socket_timeout_seconds", "60"}}}},
+          {
+              {features::kPreconnectToSearchNonGoogle, {{}}},
+          });
+    }
+  }
+
+  ~SearchEnginePreconnectorGoogleOnlyBrowserTest() override = default;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(SearchEnginePreconnectorGoogleOnlyBrowserTest);
+};
+
+IN_PROC_BROWSER_TEST_F(SearchEnginePreconnectorGoogleOnlyBrowserTest,
+                       GoogleOnly) {
+  static const char kShortName[] = "test";
+  static const char kSearchURL[] =
+      "/anchors_different_area.html?q={searchTerms}";
+  TemplateURLService* model =
+      TemplateURLServiceFactory::GetForProfile(browser()->profile());
+  ASSERT_TRUE(model);
+  search_test_utils::WaitForTemplateURLServiceToLoad(model);
+  ASSERT_TRUE(model->loaded());
+
+  TemplateURLData data;
+  data.SetShortName(base::ASCIIToUTF16(kShortName));
+  data.SetKeyword(data.short_name());
+  data.SetURL(GetTestURL(kSearchURL).spec());
+
+  // Set the DSE to the test URL.
+  TemplateURL* template_url = model->Add(std::make_unique<TemplateURL>(data));
+  ASSERT_TRUE(template_url);
+  model->SetUserSelectedDefaultSearchProvider(template_url);
+
+  // Ensure that we wait long enough to trigger preconnects.
+  WaitForDelay(base::TimeDelta::FromMilliseconds(200));
+
+  TemplateURLData data_google_search;
+  data_google_search.SetShortName(base::ASCIIToUTF16(kShortName));
+  data_google_search.SetKeyword(data.short_name());
+  data_google_search.SetURL(GoogleSearch().spec());
+
+  template_url = model->Add(std::make_unique<TemplateURL>(data_google_search));
+  ASSERT_TRUE(template_url);
+  model->SetUserSelectedDefaultSearchProvider(template_url);
+
+  WaitForPreresolveCountForURL(GoogleSearch(), 2);
+
+  // Preconnect should occur for Google search (2 since there are 2 NIKs).
+  EXPECT_EQ(2, preresolve_counts_[GoogleSearch()]);
+
+  // No preconnects should have been issued for the test URL.
+  EXPECT_EQ(0, preresolve_counts_[GetTestURL("/").GetOrigin()]);
+}
+
 }  // namespace