Adding a prepoluated search preconnect field
This CL adds a way for other searh engines to opt into the DSE
preconnect feature.
Change-Id: I1b7ee8d98cfacbea8c839ff83cdd63940e034db1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3043087
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Commit-Queue: Ryan Sturm <ryansturm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#906456}
diff --git a/chrome/browser/navigation_predictor/navigation_predictor_preconnect_client_browsertest.cc b/chrome/browser/navigation_predictor/navigation_predictor_preconnect_client_browsertest.cc
index 34e519a7..cf33e75 100644
--- a/chrome/browser/navigation_predictor/navigation_predictor_preconnect_client_browsertest.cc
+++ b/chrome/browser/navigation_predictor/navigation_predictor_preconnect_client_browsertest.cc
@@ -367,8 +367,7 @@
public:
NavigationPredictorPreconnectClientBrowserTestWithSearch()
: NavigationPredictorPreconnectClientBrowserTest() {
- feature_list_.InitWithFeatures(
- {kPreconnectToSearchTest, features::kPreconnectToSearchNonGoogle}, {});
+ feature_list_.InitWithFeatures({kPreconnectToSearchTest}, {});
}
private:
@@ -395,6 +394,7 @@
data.SetShortName(kShortName);
data.SetKeyword(data.short_name());
data.SetURL(GetTestURL(kSearchURL).spec());
+ data.preconnect_to_search_url = true;
TemplateURL* template_url = model->Add(std::make_unique<TemplateURL>(data));
ASSERT_TRUE(template_url);
diff --git a/chrome/browser/navigation_predictor/search_engine_preconnector.cc b/chrome/browser/navigation_predictor/search_engine_preconnector.cc
index 198fa666e..c6539213 100644
--- a/chrome/browser/navigation_predictor/search_engine_preconnector.cc
+++ b/chrome/browser/navigation_predictor/search_engine_preconnector.cc
@@ -17,7 +17,6 @@
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/search_engines/template_url_service_factory.h"
#include "chrome/common/pref_names.h"
-#include "components/google/core/common/google_util.h"
#include "components/prefs/pref_service.h"
#include "components/search_engines/template_url_service.h"
#include "content/public/browser/browser_context.h"
@@ -27,7 +26,7 @@
// Feature to control preconnect to search.
const base::Feature kPreconnectToSearch {
"PreconnectToSearch",
-
+// Experiments are still ongoing on Desktop, but Android is launched for now.
#if defined(OS_ANDROID)
base::FEATURE_ENABLED_BY_DEFAULT
#else
@@ -35,9 +34,6 @@
#endif
};
-// Feature to limit experimentation to Google search only.
-const base::Feature kPreconnectToSearchNonGoogle{
- "PreconnectToSearchNonGoogle", base::FEATURE_DISABLED_BY_DEFAULT};
} // namespace features
SearchEnginePreconnector::SearchEnginePreconnector(
@@ -85,13 +81,6 @@
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_));
@@ -141,7 +130,7 @@
if (!template_service)
return GURL();
const auto* search_provider = template_service->GetDefaultSearchProvider();
- if (!search_provider)
+ if (!search_provider || !search_provider->data().preconnect_to_search_url)
return GURL();
return search_provider->GenerateSearchURL({}).GetOrigin();
}
diff --git a/chrome/browser/navigation_predictor/search_engine_preconnector_browsertest.cc b/chrome/browser/navigation_predictor/search_engine_preconnector_browsertest.cc
index 1738421..98a6417 100644
--- a/chrome/browser/navigation_predictor/search_engine_preconnector_browsertest.cc
+++ b/chrome/browser/navigation_predictor/search_engine_preconnector_browsertest.cc
@@ -112,8 +112,6 @@
private:
std::unique_ptr<net::EmbeddedTestServer> https_server_;
std::map<GURL, std::unique_ptr<base::RunLoop>> run_loops_;
-
- DISALLOW_COPY_AND_ASSIGN(SearchEnginePreconnectorBrowserTest);
};
// static
@@ -126,16 +124,12 @@
SearchEnginePreconnectorNoDelaysBrowserTest() {
feature_list_.InitWithFeaturesAndParameters(
{{features::kPreconnectToSearch, {{"startup_delay_ms", "1000000"}}},
- {features::kPreconnectToSearchNonGoogle, {{}}},
{net::features::kNetUnusedIdleSocketTimeout,
{{"unused_idle_socket_timeout_seconds", "0"}}}},
{});
}
~SearchEnginePreconnectorNoDelaysBrowserTest() override = default;
-
- private:
- DISALLOW_COPY_AND_ASSIGN(SearchEnginePreconnectorNoDelaysBrowserTest);
};
// Test routinely flakes on the Mac10.11 Tests bot (https://crbug.com/1141028).
@@ -165,6 +159,7 @@
data.SetShortName(kShortName);
data.SetKeyword(data.short_name());
data.SetURL(GetTestURL(kSearchURL).spec());
+ data.preconnect_to_search_url = true;
TemplateURL* template_url = model->Add(std::make_unique<TemplateURL>(data));
ASSERT_TRUE(template_url);
@@ -201,6 +196,7 @@
data.SetShortName(kShortName);
data.SetKeyword(data.short_name());
data.SetURL(GetTestURL(kSearchURL).spec());
+ data.preconnect_to_search_url = true;
// Set the DSE to the test URL.
TemplateURL* template_url = model->Add(std::make_unique<TemplateURL>(data));
@@ -214,6 +210,7 @@
data_fake_search.SetShortName(kShortName);
data_fake_search.SetKeyword(data.short_name());
data_fake_search.SetURL(kFakeSearch);
+ data_fake_search.preconnect_to_search_url = true;
template_url = model->Add(std::make_unique<TemplateURL>(data_fake_search));
ASSERT_TRUE(template_url);
@@ -246,7 +243,6 @@
{features::kPreconnectToSearch,
{{"startup_delay_ms", "1000000"},
{"skip_in_background", "true"}}},
- {features::kPreconnectToSearchNonGoogle, {{}}},
},
{});
} else {
@@ -255,7 +251,6 @@
{features::kPreconnectToSearch,
{{"startup_delay_ms", "1000000"},
{"skip_in_background", "false"}}},
- {features::kPreconnectToSearchNonGoogle, {{}}},
},
{});
}
@@ -269,9 +264,6 @@
~SearchEnginePreconnectorForegroundBrowserTest() override = default;
base::SimpleTestTickClock tick_clock_;
-
- private:
- DISALLOW_COPY_AND_ASSIGN(SearchEnginePreconnectorForegroundBrowserTest);
};
INSTANTIATE_TEST_SUITE_P(All,
@@ -300,6 +292,7 @@
data.SetShortName(kShortName);
data.SetKeyword(data.short_name());
data.SetURL(GetTestURL(kSearchURL).spec());
+ data.preconnect_to_search_url = true;
// Set the DSE to the test URL.
TemplateURL* template_url = model->Add(std::make_unique<TemplateURL>(data));
@@ -314,6 +307,7 @@
data_fake_search.SetKeyword(data.short_name());
const GURL fake_search_url(kFakeSearch);
data_fake_search.SetURL(kFakeSearch);
+ data_fake_search.preconnect_to_search_url = true;
template_url = model->Add(std::make_unique<TemplateURL>(data_fake_search));
ASSERT_TRUE(template_url);
@@ -359,16 +353,12 @@
SearchEnginePreconnectorKeepSocketBrowserTest() {
feature_list_.InitWithFeaturesAndParameters(
{{features::kPreconnectToSearch, {{"startup_delay_ms", "1000000"}}},
- {features::kPreconnectToSearchNonGoogle, {{}}},
{net::features::kNetUnusedIdleSocketTimeout,
{{"unused_idle_socket_timeout_seconds", "60"}}}},
{});
}
~SearchEnginePreconnectorKeepSocketBrowserTest() override = default;
-
- private:
- DISALLOW_COPY_AND_ASSIGN(SearchEnginePreconnectorKeepSocketBrowserTest);
};
IN_PROC_BROWSER_TEST_F(SearchEnginePreconnectorKeepSocketBrowserTest,
@@ -388,6 +378,7 @@
data.SetShortName(kShortName);
data.SetKeyword(data.short_name());
data.SetURL(GetTestURL(kSearchURL).spec());
+ data.preconnect_to_search_url = true;
// Set the DSE to the test URL.
TemplateURL* template_url = model->Add(std::make_unique<TemplateURL>(data));
@@ -431,9 +422,6 @@
}
~SearchEnginePreconnectorDesktopAutoStartBrowserTest() override = default;
-
- private:
- DISALLOW_COPY_AND_ASSIGN(SearchEnginePreconnectorDesktopAutoStartBrowserTest);
};
IN_PROC_BROWSER_TEST_F(SearchEnginePreconnectorDesktopAutoStartBrowserTest,
@@ -442,29 +430,24 @@
WaitForPreresolveCountForURL(GURL(kGoogleSearch), 2);
}
-class SearchEnginePreconnectorGoogleOnlyBrowserTest
+class SearchEnginePreconnectorEnabledOnlyBrowserTest
: public SearchEnginePreconnectorBrowserTest {
public:
- SearchEnginePreconnectorGoogleOnlyBrowserTest() {
+ SearchEnginePreconnectorEnabledOnlyBrowserTest() {
{
feature_list_.InitWithFeaturesAndParameters(
{{features::kPreconnectToSearch, {{"startup_delay_ms", "1000000"}}},
{net::features::kNetUnusedIdleSocketTimeout,
{{"unused_idle_socket_timeout_seconds", "60"}}}},
- {
- features::kPreconnectToSearchNonGoogle,
- });
+ {});
}
}
- ~SearchEnginePreconnectorGoogleOnlyBrowserTest() override = default;
-
- private:
- DISALLOW_COPY_AND_ASSIGN(SearchEnginePreconnectorGoogleOnlyBrowserTest);
+ ~SearchEnginePreconnectorEnabledOnlyBrowserTest() override = default;
};
-IN_PROC_BROWSER_TEST_F(SearchEnginePreconnectorGoogleOnlyBrowserTest,
- GoogleOnly) {
+IN_PROC_BROWSER_TEST_F(SearchEnginePreconnectorEnabledOnlyBrowserTest,
+ AllowedSearch) {
constexpr char16_t kShortName[] = u"test";
constexpr char kSearchURL[] = "/anchors_different_area.html?q={searchTerms}";
TemplateURLService* model =
@@ -477,6 +460,7 @@
data.SetShortName(kShortName);
data.SetKeyword(data.short_name());
data.SetURL(GetTestURL(kSearchURL).spec());
+ data.preconnect_to_search_url = false;
// Set the DSE to the test URL.
TemplateURL* template_url = model->Add(std::make_unique<TemplateURL>(data));
@@ -488,12 +472,13 @@
->search_engine_preconnector()
->StartPreconnecting(/*with_startup_delay=*/false);
- TemplateURLData data_google_search;
- data_google_search.SetShortName(kShortName);
- data_google_search.SetKeyword(data.short_name());
- data_google_search.SetURL(kGoogleSearch);
+ TemplateURLData data_allowed_search;
+ data_allowed_search.SetShortName(kShortName);
+ data_allowed_search.SetKeyword(data.short_name());
+ data_allowed_search.SetURL(kGoogleSearch);
+ data_allowed_search.preconnect_to_search_url = true;
- template_url = model->Add(std::make_unique<TemplateURL>(data_google_search));
+ template_url = model->Add(std::make_unique<TemplateURL>(data_allowed_search));
ASSERT_TRUE(template_url);
model->SetUserSelectedDefaultSearchProvider(template_url);
diff --git a/components/search_engines/default_search_manager.cc b/components/search_engines/default_search_manager.cc
index ce9d885..88a5694 100644
--- a/components/search_engines/default_search_manager.cc
+++ b/components/search_engines/default_search_manager.cc
@@ -77,6 +77,8 @@
const char DefaultSearchManager::kDisabledByPolicy[] = "disabled_by_policy";
const char DefaultSearchManager::kCreatedFromPlayAPI[] =
"created_from_play_api";
+const char DefaultSearchManager::kPreconnectToSearchUrl[] =
+ "preconnect_to_search_url";
DefaultSearchManager::DefaultSearchManager(
PrefService* pref_service,
diff --git a/components/search_engines/default_search_manager.h b/components/search_engines/default_search_manager.h
index 8c3f400..9c56805 100644
--- a/components/search_engines/default_search_manager.h
+++ b/components/search_engines/default_search_manager.h
@@ -61,6 +61,7 @@
static const char kCreatedByPolicy[];
static const char kDisabledByPolicy[];
static const char kCreatedFromPlayAPI[];
+ static const char kPreconnectToSearchUrl[];
enum Source {
// Default search engine chosen either from prepopulated engines set for
diff --git a/components/search_engines/prepopulated_engines.json b/components/search_engines/prepopulated_engines.json
index 269a489..3197f2707 100644
--- a/components/search_engines/prepopulated_engines.json
+++ b/components/search_engines/prepopulated_engines.json
@@ -28,7 +28,7 @@
// Increment this if you change the data in ways that mean users with
// existing data should get a new version. Otherwise, existing data may
// continue to be used and updates made here will not always appear.
- "kCurrentDataVersion": 125
+ "kCurrentDataVersion": 126
},
// The following engines are included in country lists and are added to the
@@ -129,6 +129,7 @@
"{google:baseURL}s?q={searchTerms}"
],
"type": "SEARCH_ENGINE_GOOGLE",
+ "preconnect_to_search_url" : "ALLOWED",
"id": 1
},
diff --git a/components/search_engines/prepopulated_engines_schema.json b/components/search_engines/prepopulated_engines_schema.json
index c46435e..9d6a797f 100644
--- a/components/search_engines/prepopulated_engines_schema.json
+++ b/components/search_engines/prepopulated_engines_schema.json
@@ -65,6 +65,10 @@
"default": "SEARCH_ENGINE_OTHER",
"optional": true
},
+ // Whether a connection to search_url should regularly be established when
+ // the engine is set as the "default search engine". "ALLOWED" is the only
+ // value that will enable the pre-connections.
+ { "field": "preconnect_to_search_url", "type": "string", "optional": true },
// Unique id for this prepopulate engine (corresponds to
// TemplateURL::prepopulate_id). This ID must be greater than zero and must
// remain the same for a particular site regardless of how the url changes;
diff --git a/components/search_engines/template_url_data.cc b/components/search_engines/template_url_data.cc
index 9e38747..3198d61 100644
--- a/components/search_engines/template_url_data.cc
+++ b/components/search_engines/template_url_data.cc
@@ -68,6 +68,7 @@
base::StringPiece favicon_url,
base::StringPiece encoding,
const base::ListValue& alternate_urls_list,
+ bool preconnect_to_search_url,
int prepopulate_id)
: suggestions_url(suggest_url),
image_url(image_url),
@@ -87,7 +88,8 @@
created_from_play_api(false),
usage_count(0),
prepopulate_id(prepopulate_id),
- sync_guid(GenerateGUID(prepopulate_id)) {
+ sync_guid(GenerateGUID(prepopulate_id)),
+ preconnect_to_search_url(preconnect_to_search_url) {
SetShortName(name);
SetKeyword(keyword);
SetURL(std::string(search_url));
diff --git a/components/search_engines/template_url_data.h b/components/search_engines/template_url_data.h
index 352aec2..d9b842c 100644
--- a/components/search_engines/template_url_data.h
+++ b/components/search_engines/template_url_data.h
@@ -45,6 +45,7 @@
base::StringPiece favicon_url,
base::StringPiece encoding,
const base::ListValue& alternate_urls_list,
+ bool preconnect_to_search_url,
int prepopulate_id);
~TemplateURLData();
@@ -153,6 +154,10 @@
// search terms from a URL.
std::vector<std::string> alternate_urls;
+ // Whether a connection to |url_| should regularly be established when this is
+ // set as the "default search engine".
+ bool preconnect_to_search_url = false;
+
private:
// Private so we can enforce using the setters and thus enforce that these
// fields are never empty.
diff --git a/components/search_engines/template_url_data_unittest.cc b/components/search_engines/template_url_data_unittest.cc
index 869a9c0..7ae1958 100644
--- a/components/search_engines/template_url_data_unittest.cc
+++ b/components/search_engines/template_url_data_unittest.cc
@@ -15,7 +15,7 @@
base::StringPiece(), base::StringPiece(), base::StringPiece(),
base::StringPiece(), base::StringPiece(), base::StringPiece(),
base::StringPiece(), base::StringPiece(), base::StringPiece(),
- base::StringPiece(), base::StringPiece(), base::ListValue(), 0);
+ base::StringPiece(), base::StringPiece(), base::ListValue(), false, 0);
EXPECT_EQ(u"shortname", data.short_name());
EXPECT_EQ(u"keyword", data.keyword());
diff --git a/components/search_engines/template_url_data_util.cc b/components/search_engines/template_url_data_util.cc
index 8c2255c..840b051 100644
--- a/components/search_engines/template_url_data_util.cc
+++ b/components/search_engines/template_url_data_util.cc
@@ -120,6 +120,8 @@
&result->created_by_policy);
dict.GetBoolean(DefaultSearchManager::kCreatedFromPlayAPI,
&result->created_from_play_api);
+ dict.GetBoolean(DefaultSearchManager::kPreconnectToSearchUrl,
+ &result->preconnect_to_search_url);
return result;
}
@@ -184,6 +186,8 @@
data.created_by_policy);
url_dict->SetBoolean(DefaultSearchManager::kCreatedFromPlayAPI,
data.created_from_play_api);
+ url_dict->SetBoolean(DefaultSearchManager::kPreconnectToSearchUrl,
+ data.preconnect_to_search_url);
return url_dict;
}
@@ -205,7 +209,8 @@
ToStringPiece(engine.suggest_url_post_params),
ToStringPiece(engine.image_url_post_params),
ToStringPiece(engine.favicon_url), ToStringPiece(engine.encoding),
- alternate_urls, engine.id);
+ alternate_urls,
+ ToStringPiece(engine.preconnect_to_search_url) == "ALLOWED", engine.id);
}
std::unique_ptr<TemplateURLData> TemplateURLDataFromOverrideDictionary(
@@ -233,6 +238,7 @@
std::string search_url_post_params;
std::string suggest_url_post_params;
std::string image_url_post_params;
+ std::string preconnect_to_search_url;
base::ListValue empty_list;
const base::ListValue* alternate_urls = &empty_list;
engine.GetString("suggest_url", &suggest_url);
@@ -245,11 +251,12 @@
engine.GetString("suggest_url_post_params", &suggest_url_post_params);
engine.GetString("image_url_post_params", &image_url_post_params);
engine.GetList("alternate_urls", &alternate_urls);
+ engine.GetString("preconnect_to_search_url", &preconnect_to_search_url);
return std::make_unique<TemplateURLData>(
name, keyword, search_url, suggest_url, image_url, new_tab_url,
contextual_search_url, logo_url, doodle_url, search_url_post_params,
suggest_url_post_params, image_url_post_params, favicon_url, encoding,
- *alternate_urls, id);
+ *alternate_urls, preconnect_to_search_url.compare("ALLOWED") == 0, id);
}
return nullptr;
}
diff --git a/components/search_engines/template_url_service_util_unittest.cc b/components/search_engines/template_url_service_util_unittest.cc
index d18f7680..e8e822ef 100644
--- a/components/search_engines/template_url_service_util_unittest.cc
+++ b/components/search_engines/template_url_service_util_unittest.cc
@@ -26,7 +26,8 @@
"" /* contextual_search_url */, "" /* logo_url */, "" /* doodle_url */,
"" /* search_url_post_params */, "" /* suggest_url_post_params */,
"" /* image_url_post_params */, "" /* favicon_url */, "UTF-8",
- base::ListValue() /* alternate_urls_list */, prepopulate_id);
+ base::ListValue() /* alternate_urls_list */,
+ false /* preconnect_to_search_url */, prepopulate_id);
}
// Creates a TemplateURL with default values except for the prepopulate ID,