Commit 1b5d2b01 authored by Ryan Sturm's avatar Ryan Sturm Committed by Commit Bot

Search Prefetch Services clears cache when DSE changes

Any change to DSE should cause the search prefetch cache to be cleared.
While it is only necessary to clean up when url or search post params
change, it is healthier to clean up whenever DSE changes state.

Bug: 1138647
Change-Id: I6f995b5423464decd823a33b9c71262cf3999c7b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2515365
Commit-Queue: Ryan Sturm <ryansturm@chromium.org>
Reviewed-by: default avatarRobert Ogden <robertogden@chromium.org>
Cr-Commit-Position: refs/heads/master@{#826979}
parent d4a75917
...@@ -12,6 +12,7 @@ ...@@ -12,6 +12,7 @@
#include "chrome/browser/prefetch/search_prefetch/prefetched_response_container.h" #include "chrome/browser/prefetch/search_prefetch/prefetched_response_container.h"
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
#include "chrome/browser/search_engines/template_url_service_factory.h" #include "chrome/browser/search_engines/template_url_service_factory.h"
#include "chrome/browser/search_engines/ui_thread_search_terms_data.h"
#include "components/omnibox/browser/autocomplete_controller.h" #include "components/omnibox/browser/autocomplete_controller.h"
#include "components/omnibox/browser/base_search_provider.h" #include "components/omnibox/browser/base_search_provider.h"
#include "components/search_engines/template_url_service.h" #include "components/search_engines/template_url_service.h"
...@@ -145,6 +146,11 @@ SearchPrefetchService::SearchPrefetchService(Profile* profile) ...@@ -145,6 +146,11 @@ SearchPrefetchService::SearchPrefetchService(Profile* profile)
SearchPrefetchService::~SearchPrefetchService() = default; SearchPrefetchService::~SearchPrefetchService() = default;
void SearchPrefetchService::Shutdown() {
if (observer_.IsObserving())
observer_.RemoveObservation();
}
bool SearchPrefetchService::MaybePrefetchURL(const GURL& url) { bool SearchPrefetchService::MaybePrefetchURL(const GURL& url) {
if (!SearchPrefetchServicePrefetchingIsEnabled()) if (!SearchPrefetchServicePrefetchingIsEnabled())
return false; return false;
...@@ -158,6 +164,17 @@ bool SearchPrefetchService::MaybePrefetchURL(const GURL& url) { ...@@ -158,6 +164,17 @@ bool SearchPrefetchService::MaybePrefetchURL(const GURL& url) {
if (!template_url_service || if (!template_url_service ||
!template_url_service->GetDefaultSearchProvider()) !template_url_service->GetDefaultSearchProvider())
return false; return false;
// Lazily observe Template URL Service.
if (!observer_.IsObserving()) {
observer_.Observe(template_url_service);
const TemplateURL* template_url =
template_url_service->GetDefaultSearchProvider();
if (template_url) {
template_url_service_data_ = template_url->data();
}
}
base::string16 search_terms; base::string16 search_terms;
// Extract the terms directly to make sure this string will match the URL // Extract the terms directly to make sure this string will match the URL
...@@ -325,3 +342,34 @@ void SearchPrefetchService::OnResultChanged( ...@@ -325,3 +342,34 @@ void SearchPrefetchService::OnResultChanged(
} }
} }
} }
void SearchPrefetchService::OnTemplateURLServiceChanged() {
auto* template_url_service =
TemplateURLServiceFactory::GetForProfile(profile_);
DCHECK(template_url_service);
base::Optional<TemplateURLData> template_url_service_data;
const TemplateURL* template_url =
template_url_service->GetDefaultSearchProvider();
if (template_url) {
template_url_service_data = template_url->data();
}
if (!template_url_service_data_.has_value() &&
!template_url_service_data.has_value()) {
return;
}
UIThreadSearchTermsData search_data;
if (template_url_service_data_.has_value() &&
template_url_service_data.has_value() &&
TemplateURL::MatchesData(
template_url, &(template_url_service_data_.value()), search_data)) {
return;
}
template_url_service_data_ = template_url_service_data;
ClearPrefetches();
}
...@@ -9,9 +9,13 @@ ...@@ -9,9 +9,13 @@
#include "base/callback.h" #include "base/callback.h"
#include "base/optional.h" #include "base/optional.h"
#include "base/scoped_observation.h"
#include "base/strings/string16.h" #include "base/strings/string16.h"
#include "base/timer/timer.h" #include "base/timer/timer.h"
#include "components/keyed_service/core/keyed_service.h" #include "components/keyed_service/core/keyed_service.h"
#include "components/search_engines/template_url_data.h"
#include "components/search_engines/template_url_service.h"
#include "components/search_engines/template_url_service_observer.h"
#include "services/network/public/cpp/simple_url_loader.h" #include "services/network/public/cpp/simple_url_loader.h"
#include "url/gurl.h" #include "url/gurl.h"
...@@ -32,7 +36,8 @@ enum class SearchPrefetchStatus { ...@@ -32,7 +36,8 @@ enum class SearchPrefetchStatus {
kRequestCancelled = 4, kRequestCancelled = 4,
}; };
class SearchPrefetchService : public KeyedService { class SearchPrefetchService : public KeyedService,
public TemplateURLServiceObserver {
public: public:
explicit SearchPrefetchService(Profile* profile); explicit SearchPrefetchService(Profile* profile);
~SearchPrefetchService() override; ~SearchPrefetchService() override;
...@@ -40,6 +45,13 @@ class SearchPrefetchService : public KeyedService { ...@@ -40,6 +45,13 @@ class SearchPrefetchService : public KeyedService {
SearchPrefetchService(const SearchPrefetchService&) = delete; SearchPrefetchService(const SearchPrefetchService&) = delete;
SearchPrefetchService& operator=(const SearchPrefetchService&) = delete; SearchPrefetchService& operator=(const SearchPrefetchService&) = delete;
// KeyedService:
void Shutdown() override;
// TemplateURLServiceObserver:
// Monitors changes to DSE. If a change occurs, clears prefetches.
void OnTemplateURLServiceChanged() override;
// Called when |controller| has updated information. // Called when |controller| has updated information.
void OnResultChanged(AutocompleteController* controller); void OnResultChanged(AutocompleteController* controller);
...@@ -122,6 +134,12 @@ class SearchPrefetchService : public KeyedService { ...@@ -122,6 +134,12 @@ class SearchPrefetchService : public KeyedService {
// The time of the last prefetch network/server error. // The time of the last prefetch network/server error.
base::TimeTicks last_error_time_ticks_; base::TimeTicks last_error_time_ticks_;
// The current state of the DSE.
base::Optional<TemplateURLData> template_url_service_data_;
base::ScopedObservation<TemplateURLService, TemplateURLServiceObserver>
observer_{this};
Profile* profile_; Profile* profile_;
}; };
......
...@@ -83,17 +83,7 @@ class SearchPrefetchBaseBrowserTest : public InProcessBrowserTest { ...@@ -83,17 +83,7 @@ class SearchPrefetchBaseBrowserTest : public InProcessBrowserTest {
search_test_utils::WaitForTemplateURLServiceToLoad(model); search_test_utils::WaitForTemplateURLServiceToLoad(model);
ASSERT_TRUE(model->loaded()); ASSERT_TRUE(model->loaded());
TemplateURLData data; SetDSEWithURL(GetSearchServerQueryURL("{searchTerms}"));
data.SetShortName(base::ASCIIToUTF16(kSearchDomain));
data.SetKeyword(data.short_name());
data.SetURL(GetSearchServerQueryURL("{searchTerms}").spec());
data.suggestions_url =
search_suggest_server_->GetURL(kSuggestDomain, "/?q={searchTerms}")
.spec();
TemplateURL* template_url = model->Add(std::make_unique<TemplateURL>(data));
ASSERT_TRUE(template_url);
model->SetUserSelectedDefaultSearchProvider(template_url);
} }
void SetUpCommandLine(base::CommandLine* cmd) override { void SetUpCommandLine(base::CommandLine* cmd) override {
...@@ -194,6 +184,40 @@ class SearchPrefetchBaseBrowserTest : public InProcessBrowserTest { ...@@ -194,6 +184,40 @@ class SearchPrefetchBaseBrowserTest : public InProcessBrowserTest {
std::move(filter), &completion_observer); std::move(filter), &completion_observer);
} }
void SetDSEWithURL(const GURL& url) {
TemplateURLService* model =
TemplateURLServiceFactory::GetForProfile(browser()->profile());
TemplateURLData data;
data.SetShortName(base::ASCIIToUTF16(kSearchDomain));
data.SetKeyword(data.short_name());
data.SetURL(url.spec());
data.suggestions_url =
search_suggest_server_->GetURL(kSuggestDomain, "/?q={searchTerms}")
.spec();
TemplateURL* template_url = model->Add(std::make_unique<TemplateURL>(data));
ASSERT_TRUE(template_url);
model->SetUserSelectedDefaultSearchProvider(template_url);
}
// This is sufficient to cause observer calls about updated template URL, but
// doesn't change DSE at all.
void UpdateButChangeNothingInDSE() {
TemplateURLService* model =
TemplateURLServiceFactory::GetForProfile(browser()->profile());
TemplateURLData data;
data.SetShortName(base::ASCIIToUTF16(kSuggestDomain));
data.SetKeyword(data.short_name());
data.SetURL(
search_suggest_server_->GetURL(kSuggestDomain, "/?q={searchTerms}")
.spec());
data.suggestions_url =
search_suggest_server_->GetURL(kSuggestDomain, "/?q={searchTerms}")
.spec();
model->Add(std::make_unique<TemplateURL>(data));
}
private: private:
std::unique_ptr<net::test_server::HttpResponse> HandleSearchRequest( std::unique_ptr<net::test_server::HttpResponse> HandleSearchRequest(
const net::test_server::HttpRequest& request) { const net::test_server::HttpRequest& request) {
...@@ -905,6 +929,78 @@ IN_PROC_BROWSER_TEST_F(SearchPrefetchServiceEnabledBrowserTest, ...@@ -905,6 +929,78 @@ IN_PROC_BROWSER_TEST_F(SearchPrefetchServiceEnabledBrowserTest,
EXPECT_TRUE(prefetch_status.has_value()); EXPECT_TRUE(prefetch_status.has_value());
} }
IN_PROC_BROWSER_TEST_F(SearchPrefetchServiceEnabledBrowserTest,
ChangeDSESameOriginClearsPrefetches) {
auto* search_prefetch_service =
SearchPrefetchServiceFactory::GetForProfile(browser()->profile());
EXPECT_NE(nullptr, search_prefetch_service);
std::string search_terms = "prefetch_content";
GURL prefetch_url = GetSearchServerQueryURL(search_terms);
EXPECT_TRUE(search_prefetch_service->MaybePrefetchURL(prefetch_url));
auto prefetch_status =
search_prefetch_service->GetSearchPrefetchStatusForTesting(
base::ASCIIToUTF16(search_terms));
EXPECT_TRUE(prefetch_status.has_value());
SetDSEWithURL(GetSearchServerQueryURL("/q={searchTerms}&extra_stuff"));
prefetch_status = search_prefetch_service->GetSearchPrefetchStatusForTesting(
base::ASCIIToUTF16(search_terms));
EXPECT_FALSE(prefetch_status.has_value());
}
IN_PROC_BROWSER_TEST_F(SearchPrefetchServiceEnabledBrowserTest,
ChangeDSECrossOriginClearsPrefetches) {
auto* search_prefetch_service =
SearchPrefetchServiceFactory::GetForProfile(browser()->profile());
EXPECT_NE(nullptr, search_prefetch_service);
std::string search_terms = "prefetch_content";
GURL prefetch_url = GetSearchServerQueryURL(search_terms);
EXPECT_TRUE(search_prefetch_service->MaybePrefetchURL(prefetch_url));
auto prefetch_status =
search_prefetch_service->GetSearchPrefetchStatusForTesting(
base::ASCIIToUTF16(search_terms));
EXPECT_TRUE(prefetch_status.has_value());
SetDSEWithURL(GetSuggestServerURL("/q={searchTerms}"));
prefetch_status = search_prefetch_service->GetSearchPrefetchStatusForTesting(
base::ASCIIToUTF16(search_terms));
EXPECT_FALSE(prefetch_status.has_value());
}
IN_PROC_BROWSER_TEST_F(SearchPrefetchServiceEnabledBrowserTest,
ChangeDSESameDoesntClearPrefetches) {
auto* search_prefetch_service =
SearchPrefetchServiceFactory::GetForProfile(browser()->profile());
EXPECT_NE(nullptr, search_prefetch_service);
std::string search_terms = "prefetch_content";
GURL prefetch_url = GetSearchServerQueryURL(search_terms);
EXPECT_TRUE(search_prefetch_service->MaybePrefetchURL(prefetch_url));
auto prefetch_status =
search_prefetch_service->GetSearchPrefetchStatusForTesting(
base::ASCIIToUTF16(search_terms));
EXPECT_TRUE(prefetch_status.has_value());
UpdateButChangeNothingInDSE();
prefetch_status = search_prefetch_service->GetSearchPrefetchStatusForTesting(
base::ASCIIToUTF16(search_terms));
EXPECT_TRUE(prefetch_status.has_value());
}
class SearchPrefetchServiceZeroCacheTimeBrowserTest class SearchPrefetchServiceZeroCacheTimeBrowserTest
: public SearchPrefetchBaseBrowserTest { : public SearchPrefetchBaseBrowserTest {
public: public:
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment