Skip to content

Commit

Permalink
Use BackupResultsService in Web Discovery native double fetcher
Browse files Browse the repository at this point in the history
  • Loading branch information
DJAndries committed Jan 28, 2025
1 parent c174d7e commit 523564f
Show file tree
Hide file tree
Showing 8 changed files with 80 additions and 13 deletions.
1 change: 1 addition & 0 deletions browser/web_discovery/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
9 changes: 7 additions & 2 deletions browser/web_discovery/web_discovery_service_factory.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -30,7 +31,9 @@ WebDiscoveryServiceFactory* WebDiscoveryServiceFactory::GetInstance() {

WebDiscoveryServiceFactory::WebDiscoveryServiceFactory()
: ProfileKeyedServiceFactory("WebDiscoveryService",
CreateProfileSelections()) {}
CreateProfileSelections()) {
DependsOn(brave_search::BackupResultsServiceFactory::GetInstance());
}

WebDiscoveryServiceFactory::~WebDiscoveryServiceFactory() = default;

Expand All @@ -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<WebDiscoveryService>(
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 {
Expand Down
1 change: 1 addition & 0 deletions components/web_discovery/browser/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
50 changes: 44 additions & 6 deletions components/web_discovery/browser/double_fetcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@

#include <utility>

#include "base/containers/contains.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"
Expand All @@ -22,6 +24,8 @@ namespace web_discovery {
namespace {
constexpr char kUrlKey[] = "url";
constexpr char kAssociatedDataKey[] = "assoc_data";
constexpr char kGoogleSLD[] = "google";
constexpr char kSearchPath[] = "/search";

constexpr base::TimeDelta kRequestMaxAge = base::Hours(1);
constexpr base::TimeDelta kMinRequestInterval =
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -88,24 +94,56 @@ void DoubleFetcher::OnFetchTimer(const base::Value& request_data) {
}

GURL url(*url_str);

if (base::Contains(url.host_piece(), kGoogleSLD) &&
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(
void DoubleFetcher::OnURLLoaderResponse(
GURL url,
std::optional<std::string> response_body) {
bool result = false;
auto* response_info = url_loader_->ResponseInfo();
std::optional<int> 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(
GURL url,
std::optional<brave_search::BackupResultsService::BackupResults> results) {
if (results) {
OnRequestComplete(url, results->final_status_code, results->html);
} else {
OnRequestComplete(url, std::nullopt, std::nullopt);
}
}

void DoubleFetcher::OnRequestComplete(
GURL url,
std::optional<int> response_code,
std::optional<std::string> 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
Expand Down
13 changes: 12 additions & 1 deletion components/web_discovery/browser/double_fetcher.h
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand All @@ -37,6 +38,7 @@ class DoubleFetcher {
std::optional<std::string> response_body)>;
DoubleFetcher(PrefService* profile_prefs,
network::SharedURLLoaderFactory* shared_url_loader_factory,
brave_search::BackupResultsService* backup_results_service,
FetchedCallback callback);
~DoubleFetcher();

Expand All @@ -50,15 +52,24 @@ class DoubleFetcher {

private:
void OnFetchTimer(const base::Value& request_data);
void OnRequestComplete(GURL url, std::optional<std::string> response_body);
void OnURLLoaderResponse(GURL url, std::optional<std::string> response_body);
void OnRenderedResponse(
GURL url,
std::optional<brave_search::BackupResultsService::BackupResults> results);
void OnRequestComplete(GURL url,
std::optional<int> response_code,
std::optional<std::string> response_body);

raw_ptr<PrefService> profile_prefs_;
raw_ptr<network::SharedURLLoaderFactory> shared_url_loader_factory_;
std::unique_ptr<network::SimpleURLLoader> url_loader_;
raw_ptr<brave_search::BackupResultsService> backup_results_service_;

RequestQueue request_queue_;

FetchedCallback callback_;

base::WeakPtrFactory<DoubleFetcher> weak_ptr_factory_{this};
};

} // namespace web_discovery
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ class WebDiscoveryDoubleFetcherTest : public testing::Test {
protected:
void InitDoubleFetcher() {
double_fetcher_ = std::make_unique<DoubleFetcher>(
&profile_prefs_, shared_url_loader_factory_.get(),
&profile_prefs_, shared_url_loader_factory_.get(), nullptr,
base::BindRepeating(&WebDiscoveryDoubleFetcherTest::HandleDoubleFetch,
base::Unretained(this)));
}
Expand Down
9 changes: 7 additions & 2 deletions components/web_discovery/browser/web_discovery_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,14 @@ WebDiscoveryService::WebDiscoveryService(
PrefService* local_state,
PrefService* profile_prefs,
base::FilePath user_data_dir,
scoped_refptr<network::SharedURLLoaderFactory> shared_url_loader_factory)
scoped_refptr<network::SharedURLLoaderFactory> 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,
Expand Down Expand Up @@ -65,6 +68,7 @@ void WebDiscoveryService::RegisterProfilePrefs(PrefRegistrySimple* registry) {
void WebDiscoveryService::Shutdown() {
Stop();
pref_change_registrar_.RemoveAll();
backup_results_service_ = nullptr;
}

void WebDiscoveryService::Start() {
Expand Down Expand Up @@ -123,6 +127,7 @@ void WebDiscoveryService::OnPatternsLoaded() {
if (!double_fetcher_) {
double_fetcher_ = std::make_unique<DoubleFetcher>(
profile_prefs_.get(), shared_url_loader_factory_.get(),
backup_results_service_.get(),
base::BindRepeating(&WebDiscoveryService::OnDoubleFetched,
base::Unretained(this)));
}
Expand Down
8 changes: 7 additions & 1 deletion components/web_discovery/browser/web_discovery_service.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -43,7 +47,8 @@ class WebDiscoveryService : public KeyedService {
PrefService* local_state,
PrefService* profile_prefs,
base::FilePath user_data_dir,
scoped_refptr<network::SharedURLLoaderFactory> shared_url_loader_factory);
scoped_refptr<network::SharedURLLoaderFactory> shared_url_loader_factory,
brave_search::BackupResultsService* backup_results_service);
~WebDiscoveryService() override;

WebDiscoveryService(const WebDiscoveryService&) = delete;
Expand Down Expand Up @@ -88,6 +93,7 @@ class WebDiscoveryService : public KeyedService {
base::FilePath user_data_dir_;

scoped_refptr<network::SharedURLLoaderFactory> shared_url_loader_factory_;
raw_ptr<brave_search::BackupResultsService> backup_results_service_;

mojo::RemoteSet<mojom::DocumentExtractor> document_extractor_remotes_;

Expand Down

0 comments on commit 523564f

Please sign in to comment.