SearchPrefetchService supports Incognito profile Search prefetch feature is currently available only on non-Incognito profiles without obvious reasons. This CL changes SearchPrefetchServiceFactory to return the instance even for Incognito profile, not only for regular profiles, which allows the browser to trigger search prefetch in omnibox. The actual prefetch triggers are managed by existing feature params. There is no change. This is implemented behind the feature param named `allow_incognito`. Here is the design doc (internal): https://docs.google.com/document/d/1yP49fFU4VIlrIBOgDSqYqqpnH46Xx2iv_Ckh9censfs/edit?resourcekey=0-DeNv8KgxA-sl-VpLzbw10w&tab=t.0 Change-Id: Ife4ae3e937395e253ae1ce574cf743c1076cfbd5 Bug: 394716358 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6186527 Reviewed-by: Lingqi Chi <lingqi@chromium.org> Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org> Commit-Queue: Shunya Shishido <sisidovski@chromium.org> Cr-Commit-Position: refs/heads/main@{#1417988}
diff --git a/chrome/browser/preloading/prefetch/search_prefetch/field_trial_settings.cc b/chrome/browser/preloading/prefetch/search_prefetch/field_trial_settings.cc index d02445b..43bc42ef 100644 --- a/chrome/browser/preloading/prefetch/search_prefetch/field_trial_settings.cc +++ b/chrome/browser/preloading/prefetch/search_prefetch/field_trial_settings.cc
@@ -105,6 +105,13 @@ kSearchPrefetchOnlyAllowDefaultMatchPreloading); } +bool IsPrefetchIncognitoEnabled() { + return SearchPrefetchServicePrefetchingIsEnabled() && + IsSearchNavigationPrefetchEnabled() && + base::GetFieldTrialParamByFeatureAsBool(kSearchNavigationPrefetch, + "allow_incognito", false); +} + BASE_FEATURE(kAutocompleteDictionaryPreload, "AutocompleteDictionaryPreload", base::FEATURE_ENABLED_BY_DEFAULT);
diff --git a/chrome/browser/preloading/prefetch/search_prefetch/field_trial_settings.h b/chrome/browser/preloading/prefetch/search_prefetch/field_trial_settings.h index 7eaf69e..133a8513 100644 --- a/chrome/browser/preloading/prefetch/search_prefetch/field_trial_settings.h +++ b/chrome/browser/preloading/prefetch/search_prefetch/field_trial_settings.h
@@ -71,6 +71,13 @@ // being the default match. bool OnlyAllowDefaultMatchPreloading(); +// Allows the omnibox search prefetch in Incognito. +// +// Note SearchPrefetchService partially supports Incognito profile. For now, +// it supports the on-press triggered search prefetch only. Other prefetches +// must not be triggered in Incognito. crbug.com/394716358 for more details. +bool IsPrefetchIncognitoEnabled(); + // When this feature is enabled, SearchPrefetchService will send a request to // the network service to preload shared dictionary from the disk storage for // the AutocompleteResult's `destination_url`.
diff --git a/chrome/browser/preloading/prefetch/search_prefetch/search_prefetch_service.cc b/chrome/browser/preloading/prefetch/search_prefetch/search_prefetch_service.cc index a7fa23d..c3b30842 100644 --- a/chrome/browser/preloading/prefetch/search_prefetch/search_prefetch_service.cc +++ b/chrome/browser/preloading/prefetch/search_prefetch/search_prefetch_service.cc
@@ -238,7 +238,7 @@ SearchPrefetchService::SearchPrefetchService(Profile* profile) : profile_(profile) { - DCHECK(!profile_->IsOffTheRecord()); + CHECK(!profile_->IsOffTheRecord() || IsPrefetchIncognitoEnabled()); if (LoadFromPrefs()) SaveToPrefs();
diff --git a/chrome/browser/preloading/prefetch/search_prefetch/search_prefetch_service_browsertest.cc b/chrome/browser/preloading/prefetch/search_prefetch/search_prefetch_service_browsertest.cc index d1d92af..843fcbb9 100644 --- a/chrome/browser/preloading/prefetch/search_prefetch/search_prefetch_service_browsertest.cc +++ b/chrome/browser/preloading/prefetch/search_prefetch/search_prefetch_service_browsertest.cc
@@ -4158,3 +4158,81 @@ SendMemoryPressureToNetworkService(); EXPECT_FALSE(HasPreloadedSharedDictionaryInfo()); } + +class SearchNavigationPrefetchIncognitoBrowserTest + : public SearchPrefetchBaseBrowserTest { + public: + SearchNavigationPrefetchIncognitoBrowserTest() { + std::vector<base::test::FeatureRefAndParams> enabled_features = { + {kSearchPrefetchServicePrefetching, + {{"max_attempts_per_caching_duration", "3"}, + {"cache_size", "1"}, + {"device_memory_threshold_MB", "0"}}}, + { + kSearchNavigationPrefetch, + {{"allow_incognito", "true"}}, + }}; + feature_list_.InitWithFeaturesAndParameters(enabled_features, {}); + } + + void SetUpOnMainThread() override { + // Close normal browser and switch the test's browser instance to an + // incognito instance. + Browser* incognito = CreateIncognitoBrowser(browser()->profile()); + CloseBrowserSynchronously(browser()); + SelectFirstBrowser(); + ASSERT_EQ(browser(), incognito); + + SearchPrefetchBaseBrowserTest::SetUpOnMainThread(); + + ASSERT_TRUE(browser()->profile()->IsOffTheRecord()); + } + + private: + base::test::ScopedFeatureList feature_list_; +}; + +IN_PROC_BROWSER_TEST_F(SearchNavigationPrefetchIncognitoBrowserTest, + NavigationPrefetchIsServedMouseDown) { + SetDSEWithURL( + GetSearchServerQueryURL( + "{searchTerms}&{google:assistedQueryStats}{google:prefetchSource}"), + true); + + auto* search_prefetch_service = + SearchPrefetchServiceFactory::GetForProfile(browser()->profile()); + ASSERT_NE(search_prefetch_service, nullptr); + std::string search_terms = "terms of service"; + std::string user_input = "terms"; + + AutocompleteMatch autocomplete_match = + CreateSearchSuggestionMatch(search_terms, search_terms, false); + search_prefetch_service->OnNavigationLikely( + 1, autocomplete_match, NavigationPredictor::kMouseDown, GetWebContents()); + + WaitUntilStatusChangesTo( + GetCanonicalSearchURL(autocomplete_match.destination_url), + SearchPrefetchStatus::kComplete); + + auto prefetch_status = + search_prefetch_service->GetSearchPrefetchStatusForTesting( + GetCanonicalSearchURL(autocomplete_match.destination_url)); + ASSERT_TRUE(prefetch_status.has_value()); + EXPECT_EQ(SearchPrefetchStatus::kComplete, prefetch_status.value()); + + auto [prefetch_url, search_url] = + GetSearchPrefetchAndNonPrefetch(search_terms); + GURL canonical_search_url = GetCanonicalSearchURL(prefetch_url); + ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), search_url)); + + prefetch_status = search_prefetch_service->GetSearchPrefetchStatusForTesting( + canonical_search_url); + EXPECT_FALSE(prefetch_status.has_value()); + + auto inner_html = + content::EvalJs(browser()->tab_strip_model()->GetActiveWebContents(), + "document.documentElement.innerHTML") + .ExtractString(); + EXPECT_FALSE(base::Contains(inner_html, "regular")); + EXPECT_TRUE(base::Contains(inner_html, "prefetch")); +}
diff --git a/chrome/browser/preloading/prefetch/search_prefetch/search_prefetch_service_factory.cc b/chrome/browser/preloading/prefetch/search_prefetch/search_prefetch_service_factory.cc index 97cf80bb..84bc2c5 100644 --- a/chrome/browser/preloading/prefetch/search_prefetch/search_prefetch_service_factory.cc +++ b/chrome/browser/preloading/prefetch/search_prefetch/search_prefetch_service_factory.cc
@@ -5,10 +5,32 @@ #include "chrome/browser/preloading/prefetch/search_prefetch/search_prefetch_service_factory.h" #include "base/no_destructor.h" +#include "chrome/browser/preloading/prefetch/search_prefetch/field_trial_settings.h" #include "chrome/browser/preloading/prefetch/search_prefetch/search_prefetch_service.h" #include "chrome/browser/profiles/profile.h" +#include "chrome/browser/profiles/profile_selections.h" #include "content/public/browser/browser_context.h" +namespace { +ProfileSelections GetProfileSelections() { + // Note SearchPrefetchService partially supports Incognito profile. For now, + // it supports the on-press triggered search prefetch only. Other prefetches + // must not be triggered in Incognito. crbug.com/394716358 for more details. + ProfileSelection profile_selection = IsPrefetchIncognitoEnabled() + ? ProfileSelection::kOwnInstance + : ProfileSelection::kOriginalOnly; + return ProfileSelections::Builder() + .WithRegular(profile_selection) + // TODO(crbug.com/40257657): Check if this + // service is needed in Guest mode. + .WithGuest(profile_selection) + // TODO(crbug.com/41488885): Check if this + // service is needed for Ash Internals. + .WithAshInternals(profile_selection) + .Build(); +} +} // namespace + // static SearchPrefetchService* SearchPrefetchServiceFactory::GetForProfile( Profile* profile) { @@ -23,17 +45,8 @@ } SearchPrefetchServiceFactory::SearchPrefetchServiceFactory() - : ProfileKeyedServiceFactory( - "SearchPrefetchService", - ProfileSelections::Builder() - .WithRegular(ProfileSelection::kOriginalOnly) - // TODO(crbug.com/40257657): Check if this service is needed in - // Guest mode. - .WithGuest(ProfileSelection::kOriginalOnly) - // TODO(crbug.com/41488885): Check if this service is needed for - // Ash Internals. - .WithAshInternals(ProfileSelection::kOriginalOnly) - .Build()) {} + : ProfileKeyedServiceFactory("SearchPrefetchService", + GetProfileSelections()) {} SearchPrefetchServiceFactory::~SearchPrefetchServiceFactory() = default;
diff --git a/chrome/browser/preloading/prefetch/search_prefetch/search_prefetch_service_factory.h b/chrome/browser/preloading/prefetch/search_prefetch/search_prefetch_service_factory.h index 64a126d..e6520f58 100644 --- a/chrome/browser/preloading/prefetch/search_prefetch/search_prefetch_service_factory.h +++ b/chrome/browser/preloading/prefetch/search_prefetch/search_prefetch_service_factory.h
@@ -21,7 +21,11 @@ public: // Gets the SearchPrefetchService for the profile. // - // Returns null if the features if not enabled or incognito. + // Returns null if the features is not enabled. + // + // Note it also returns null for Incognito by default, but returns + // SearchPrefetchService if the Incognito support is enabled via the feature + // param. See crbug.com/394716358. static SearchPrefetchService* GetForProfile(Profile* profile); // Gets the LazyInstance that owns all SearchPrefetchService(s).