From 6f13dc8a2613a723a05a52e9e508179ec0be4110 Mon Sep 17 00:00:00 2001 From: Darnell Andries Date: Mon, 27 Jan 2025 20:22:51 -0800 Subject: [PATCH] Use `BackupResultsService` in Web Discovery native double fetcher --- browser/web_discovery/BUILD.gn | 1 + .../web_discovery_service_factory.cc | 9 +++- components/web_discovery/browser/BUILD.gn | 1 + .../web_discovery/browser/double_fetcher.cc | 52 ++++++++++++++++--- .../web_discovery/browser/double_fetcher.h | 14 ++++- .../browser/double_fetcher_unittest.cc | 2 +- .../browser/web_discovery_service.cc | 8 ++- .../browser/web_discovery_service.h | 8 ++- 8 files changed, 81 insertions(+), 14 deletions(-) diff --git a/browser/web_discovery/BUILD.gn b/browser/web_discovery/BUILD.gn index 4b940c79c18d..9cce8c2740d3 100644 --- a/browser/web_discovery/BUILD.gn +++ b/browser/web_discovery/BUILD.gn @@ -14,6 +14,7 @@ if (enable_web_discovery_native) { ] deps = [ "//base", + "//brave/browser/brave_search", "//brave/components/web_discovery/browser", "//brave/components/web_discovery/common", "//chrome/browser:browser_process", diff --git a/browser/web_discovery/web_discovery_service_factory.cc b/browser/web_discovery/web_discovery_service_factory.cc index e0354e93ebc8..b7eac9502e95 100644 --- a/browser/web_discovery/web_discovery_service_factory.cc +++ b/browser/web_discovery/web_discovery_service_factory.cc @@ -6,6 +6,7 @@ #include "brave/browser/web_discovery/web_discovery_service_factory.h" #include "base/path_service.h" +#include "brave/browser/brave_search/backup_results_service_factory.h" #include "brave/components/web_discovery/browser/web_discovery_service.h" #include "brave/components/web_discovery/common/features.h" #include "chrome/browser/browser_process.h" @@ -30,7 +31,9 @@ WebDiscoveryServiceFactory* WebDiscoveryServiceFactory::GetInstance() { WebDiscoveryServiceFactory::WebDiscoveryServiceFactory() : ProfileKeyedServiceFactory("WebDiscoveryService", - CreateProfileSelections()) {} + CreateProfileSelections()) { + DependsOn(brave_search::BackupResultsServiceFactory::GetInstance()); +} WebDiscoveryServiceFactory::~WebDiscoveryServiceFactory() = default; @@ -48,11 +51,13 @@ WebDiscoveryServiceFactory::BuildServiceInstanceForBrowserContext( auto* default_storage_partition = context->GetDefaultStoragePartition(); auto shared_url_loader_factory = default_storage_partition->GetURLLoaderFactoryForBrowserProcess(); + auto* backup_results_service = + brave_search::BackupResultsServiceFactory::GetForBrowserContext(context); base::FilePath user_data_dir = base::PathService::CheckedGet(chrome::DIR_USER_DATA); return std::make_unique( g_browser_process->local_state(), user_prefs::UserPrefs::Get(context), - user_data_dir, shared_url_loader_factory); + user_data_dir, shared_url_loader_factory, backup_results_service); } bool WebDiscoveryServiceFactory::ServiceIsCreatedWithBrowserContext() const { diff --git a/components/web_discovery/browser/BUILD.gn b/components/web_discovery/browser/BUILD.gn index 289457ba342b..c7085a7a9062 100644 --- a/components/web_discovery/browser/BUILD.gn +++ b/components/web_discovery/browser/BUILD.gn @@ -71,6 +71,7 @@ source_set("internal") { "document_extractor", "//base", "//brave/brave_domains", + "//brave/components/brave_search/browser", "//brave/components/constants", "//brave/components/web_discovery/common:mojom", "//components/prefs", diff --git a/components/web_discovery/browser/double_fetcher.cc b/components/web_discovery/browser/double_fetcher.cc index ef801d6ea422..c30e1e0d8bf2 100644 --- a/components/web_discovery/browser/double_fetcher.cc +++ b/components/web_discovery/browser/double_fetcher.cc @@ -7,6 +7,9 @@ #include +#include "base/containers/contains.h" +#include "brave/components/brave_search/browser/backup_results_allowed_urls.h" +#include "brave/components/brave_search/browser/backup_results_service.h" #include "brave/components/web_discovery/browser/pref_names.h" #include "brave/components/web_discovery/browser/request_queue.h" #include "brave/components/web_discovery/browser/util.h" @@ -22,6 +25,7 @@ namespace web_discovery { namespace { constexpr char kUrlKey[] = "url"; constexpr char kAssociatedDataKey[] = "assoc_data"; +constexpr char kSearchPath[] = "/search"; constexpr base::TimeDelta kRequestMaxAge = base::Hours(1); constexpr base::TimeDelta kMinRequestInterval = @@ -55,9 +59,11 @@ constexpr net::NetworkTrafficAnnotationTag kFetchNetworkTrafficAnnotation = DoubleFetcher::DoubleFetcher( PrefService* profile_prefs, network::SharedURLLoaderFactory* shared_url_loader_factory, + brave_search::BackupResultsService* backup_results_service, FetchedCallback callback) : profile_prefs_(profile_prefs), shared_url_loader_factory_(shared_url_loader_factory), + backup_results_service_(backup_results_service), request_queue_(profile_prefs, kScheduledDoubleFetches, kRequestMaxAge, @@ -88,24 +94,56 @@ void DoubleFetcher::OnFetchTimer(const base::Value& request_data) { } GURL url(*url_str); + + if (brave_search::IsBackupResultURLAllowed(url) && + base::StartsWith(url.path_piece(), kSearchPath)) { + CHECK(backup_results_service_); + backup_results_service_->FetchBackupResults( + url, {}, + base::BindOnce(&DoubleFetcher::OnRenderedResponse, + weak_ptr_factory_.GetWeakPtr(), url)); + return; + } + auto resource_request = CreateResourceRequest(url); url_loader_ = network::SimpleURLLoader::Create( std::move(resource_request), kFetchNetworkTrafficAnnotation); url_loader_->DownloadToString( shared_url_loader_factory_.get(), - base::BindOnce(&DoubleFetcher::OnRequestComplete, base::Unretained(this), - url), + base::BindOnce(&DoubleFetcher::OnURLLoaderResponse, + base::Unretained(this), url), kMaxDoubleFetchResponseSize); } -void DoubleFetcher::OnRequestComplete( - GURL url, +void DoubleFetcher::OnURLLoaderResponse( + const GURL& url, std::optional response_body) { - bool result = false; auto* response_info = url_loader_->ResponseInfo(); + std::optional response_code; if (response_info) { - auto response_code = response_info->headers->response_code(); - if (!network::IsSuccessfulStatus(response_code)) { + response_code = response_info->headers->response_code(); + } + + OnRequestComplete(url, response_code, std::move(response_body)); +} + +void DoubleFetcher::OnRenderedResponse( + const GURL& url, + std::optional results) { + if (results) { + OnRequestComplete(url, results->final_status_code, results->html); + } else { + OnRequestComplete(url, std::nullopt, std::nullopt); + } +} + +void DoubleFetcher::OnRequestComplete( + const GURL& url, + std::optional response_code, + std::optional response_body) { + bool result = false; + if (response_code) { + if (!network::IsSuccessfulStatus(*response_code)) { if (response_code >= net::HttpStatusCode::HTTP_BAD_REQUEST && response_code < net::HttpStatusCode::HTTP_INTERNAL_SERVER_ERROR) { // Only retry failures due to server error diff --git a/components/web_discovery/browser/double_fetcher.h b/components/web_discovery/browser/double_fetcher.h index 774c46d48dc9..4ceac02705b9 100644 --- a/components/web_discovery/browser/double_fetcher.h +++ b/components/web_discovery/browser/double_fetcher.h @@ -12,6 +12,7 @@ #include "base/memory/raw_ptr.h" #include "base/values.h" +#include "brave/components/brave_search/browser/backup_results_service.h" #include "brave/components/web_discovery/browser/request_queue.h" #include "url/gurl.h" @@ -37,6 +38,7 @@ class DoubleFetcher { std::optional response_body)>; DoubleFetcher(PrefService* profile_prefs, network::SharedURLLoaderFactory* shared_url_loader_factory, + brave_search::BackupResultsService* backup_results_service, FetchedCallback callback); ~DoubleFetcher(); @@ -50,15 +52,25 @@ class DoubleFetcher { private: void OnFetchTimer(const base::Value& request_data); - void OnRequestComplete(GURL url, std::optional response_body); + void OnURLLoaderResponse(const GURL& url, + std::optional response_body); + void OnRenderedResponse( + const GURL& url, + std::optional results); + void OnRequestComplete(const GURL& url, + std::optional response_code, + std::optional response_body); raw_ptr profile_prefs_; raw_ptr shared_url_loader_factory_; std::unique_ptr url_loader_; + raw_ptr backup_results_service_; RequestQueue request_queue_; FetchedCallback callback_; + + base::WeakPtrFactory weak_ptr_factory_{this}; }; } // namespace web_discovery diff --git a/components/web_discovery/browser/double_fetcher_unittest.cc b/components/web_discovery/browser/double_fetcher_unittest.cc index b1997ad1381d..18c33fd08192 100644 --- a/components/web_discovery/browser/double_fetcher_unittest.cc +++ b/components/web_discovery/browser/double_fetcher_unittest.cc @@ -46,7 +46,7 @@ class WebDiscoveryDoubleFetcherTest : public testing::Test { protected: void InitDoubleFetcher() { double_fetcher_ = std::make_unique( - &profile_prefs_, shared_url_loader_factory_.get(), + &profile_prefs_, shared_url_loader_factory_.get(), nullptr, base::BindRepeating(&WebDiscoveryDoubleFetcherTest::HandleDoubleFetch, base::Unretained(this))); } diff --git a/components/web_discovery/browser/web_discovery_service.cc b/components/web_discovery/browser/web_discovery_service.cc index 5f0a50aa4733..be83948593f6 100644 --- a/components/web_discovery/browser/web_discovery_service.cc +++ b/components/web_discovery/browser/web_discovery_service.cc @@ -30,11 +30,14 @@ WebDiscoveryService::WebDiscoveryService( PrefService* local_state, PrefService* profile_prefs, base::FilePath user_data_dir, - scoped_refptr shared_url_loader_factory) + scoped_refptr shared_url_loader_factory, + brave_search::BackupResultsService* backup_results_service) : local_state_(local_state), profile_prefs_(profile_prefs), user_data_dir_(user_data_dir), - shared_url_loader_factory_(shared_url_loader_factory) { + shared_url_loader_factory_(shared_url_loader_factory), + backup_results_service_(backup_results_service) { + CHECK(backup_results_service); pref_change_registrar_.Init(profile_prefs); pref_change_registrar_.Add( kWebDiscoveryEnabled, @@ -123,6 +126,7 @@ void WebDiscoveryService::OnPatternsLoaded() { if (!double_fetcher_) { double_fetcher_ = std::make_unique( profile_prefs_.get(), shared_url_loader_factory_.get(), + backup_results_service_.get(), base::BindRepeating(&WebDiscoveryService::OnDoubleFetched, base::Unretained(this))); } diff --git a/components/web_discovery/browser/web_discovery_service.h b/components/web_discovery/browser/web_discovery_service.h index 726734e60247..3472d1a8d6e5 100644 --- a/components/web_discovery/browser/web_discovery_service.h +++ b/components/web_discovery/browser/web_discovery_service.h @@ -33,6 +33,10 @@ namespace network { class SharedURLLoaderFactory; } // namespace network +namespace brave_search { +class BackupResultsService; +} // namespace brave_search + namespace web_discovery { // The main service for the native re-implementation of Web Discovery Project. @@ -43,7 +47,8 @@ class WebDiscoveryService : public KeyedService { PrefService* local_state, PrefService* profile_prefs, base::FilePath user_data_dir, - scoped_refptr shared_url_loader_factory); + scoped_refptr shared_url_loader_factory, + brave_search::BackupResultsService* backup_results_service); ~WebDiscoveryService() override; WebDiscoveryService(const WebDiscoveryService&) = delete; @@ -88,6 +93,7 @@ class WebDiscoveryService : public KeyedService { base::FilePath user_data_dir_; scoped_refptr shared_url_loader_factory_; + raw_ptr backup_results_service_; mojo::RemoteSet document_extractor_remotes_;