Skip to content

Commit

Permalink
Address Web Discovery feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
DJAndries committed Oct 30, 2024
1 parent 4844e82 commit 83195a9
Show file tree
Hide file tree
Showing 7 changed files with 27 additions and 19 deletions.
2 changes: 1 addition & 1 deletion components/web_discovery/browser/double_fetcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ void DoubleFetcher::ScheduleDoubleFetch(const GURL& url,
fetch_dict.Set(kUrlKey, url.spec());
fetch_dict.Set(kAssociatedDataKey, std::move(associated_data));

request_queue_.ScheduleRequest(base::Value(std::move(fetch_dict)));
request_queue_.ScheduleRequest(std::move(fetch_dict));
}

void DoubleFetcher::OnFetchTimer(const base::Value& request_data) {
Expand Down
2 changes: 1 addition & 1 deletion components/web_discovery/browser/ecdh_aes.cc
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ std::optional<AESEncryptResult> DeriveAESKeyAndEncrypt(
}

std::array<uint8_t, kKeyMaterialSize> shared_key_material;
if (!ECDH_compute_key(shared_key_material.data(), kKeyMaterialSize,
if (!ECDH_compute_key(shared_key_material.data(), shared_key_material.size(),
server_public_point.get(), client_private_key.get(),
nullptr)) {
VLOG(1) << "Failed to set derive key via ECDH";
Expand Down
13 changes: 9 additions & 4 deletions components/web_discovery/browser/pref_names.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,13 @@
#ifndef BRAVE_COMPONENTS_WEB_DISCOVERY_BROWSER_PREF_NAMES_H_
#define BRAVE_COMPONENTS_WEB_DISCOVERY_BROWSER_PREF_NAMES_H_

#include "base/types/strong_alias.h"

namespace web_discovery {

using RequestQueuePrefName =
base::StrongAlias<class QueuePrefNameTag, std::string_view>;

// Profile prefs
inline constexpr char kWebDiscoveryEnabled[] = "brave.web_discovery_enabled";

Expand All @@ -20,10 +25,10 @@ inline constexpr char kCredentialRSAPrivateKey[] =
inline constexpr char kAnonymousCredentialsDict[] =
"brave.web_discovery.anon_creds";

inline constexpr char kScheduledDoubleFetches[] =
"brave.web_discovery.scheduled_double_fetches";
inline constexpr char kScheduledReports[] =
"brave.web_discovery.scheduled_reports";
inline constexpr auto kScheduledDoubleFetches =
RequestQueuePrefName("brave.web_discovery.scheduled_double_fetches");
inline constexpr auto kScheduledReports =
RequestQueuePrefName("brave.web_discovery.scheduled_reports");
inline constexpr char kUsedBasenameCounts[] =
"brave.web_discovery.used_basename_counts";
inline constexpr char kPageCounts[] = "brave.web_discovery.page_counts";
Expand Down
2 changes: 1 addition & 1 deletion components/web_discovery/browser/reporter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ Reporter::Reporter(PrefService* profile_prefs,
Reporter::~Reporter() = default;

void Reporter::ScheduleSend(base::Value::Dict payload) {
request_queue_.ScheduleRequest(base::Value(std::move(payload)));
request_queue_.ScheduleRequest(std::move(payload));
}

void Reporter::PrepareRequest(const base::Value& request_data) {
Expand Down
10 changes: 5 additions & 5 deletions components/web_discovery/browser/request_queue.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ constexpr char kDataKey[] = "data";

RequestQueue::RequestQueue(
PrefService* profile_prefs,
const char* list_pref_name,
RequestQueuePrefName list_pref_name,
base::TimeDelta request_max_age,
base::TimeDelta min_request_interval,
base::TimeDelta max_request_interval,
Expand All @@ -42,13 +42,13 @@ RequestQueue::RequestQueue(

RequestQueue::~RequestQueue() = default;

void RequestQueue::ScheduleRequest(base::Value request_data) {
void RequestQueue::ScheduleRequest(base::Value::Dict request_data) {
base::Value::Dict fetch_dict;
fetch_dict.Set(kDataKey, std::move(request_data));
fetch_dict.Set(kRequestTimeKey,
static_cast<double>(base::Time::Now().ToTimeT()));

ScopedListPrefUpdate update(profile_prefs_, list_pref_name_);
ScopedListPrefUpdate update(profile_prefs_, list_pref_name_.value());
update->Append(std::move(fetch_dict));

if (!fetch_timer_.IsRunning()) {
Expand All @@ -59,7 +59,7 @@ void RequestQueue::ScheduleRequest(base::Value request_data) {
std::optional<base::Value> RequestQueue::NotifyRequestComplete(bool success) {
backoff_entry_.InformOfRequest(success);

ScopedListPrefUpdate update(profile_prefs_, list_pref_name_);
ScopedListPrefUpdate update(profile_prefs_, list_pref_name_.value());
auto& request_dict = update->front().GetDict();

std::optional<base::Value> removed_value;
Expand Down Expand Up @@ -87,7 +87,7 @@ std::optional<base::Value> RequestQueue::NotifyRequestComplete(bool success) {
}

void RequestQueue::OnFetchTimer() {
ScopedListPrefUpdate update(profile_prefs_, list_pref_name_);
ScopedListPrefUpdate update(profile_prefs_, list_pref_name_.value());
for (auto it = update->begin(); it != update->end();) {
const auto* fetch_dict = it->GetIfDict();
const auto request_time =
Expand Down
9 changes: 6 additions & 3 deletions components/web_discovery/browser/request_queue.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,13 @@
#ifndef BRAVE_COMPONENTS_WEB_DISCOVERY_BROWSER_REQUEST_QUEUE_H_
#define BRAVE_COMPONENTS_WEB_DISCOVERY_BROWSER_REQUEST_QUEUE_H_

#include <string_view>

#include "base/functional/callback.h"
#include "base/memory/raw_ptr.h"
#include "base/timer/timer.h"
#include "base/values.h"
#include "brave/components/web_discovery/browser/pref_names.h"
#include "net/base/backoff_entry.h"

class PrefService;
Expand All @@ -24,7 +27,7 @@ class RequestQueue {
public:
RequestQueue(
PrefService* profile_prefs,
const char* list_pref_name,
RequestQueuePrefName list_pref_name,
base::TimeDelta request_max_age,
base::TimeDelta min_request_interval,
base::TimeDelta max_request_interval,
Expand All @@ -37,7 +40,7 @@ class RequestQueue {

// Persist and schedule a request. The arbitrary data will be passed
// to `start_request_callback` on the scheduled interval.
void ScheduleRequest(base::Value request_data);
void ScheduleRequest(base::Value::Dict request_data);
// Returns data value if request is deleted from queue, due to the retry limit
// or success
std::optional<base::Value> NotifyRequestComplete(bool success);
Expand All @@ -47,7 +50,7 @@ class RequestQueue {
void StartFetchTimer(bool use_backoff_delta);

raw_ptr<PrefService> profile_prefs_;
const char* list_pref_name_;
RequestQueuePrefName list_pref_name_;

net::BackoffEntry backoff_entry_;

Expand Down
8 changes: 4 additions & 4 deletions components/web_discovery/browser/web_discovery_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ void WebDiscoveryService::RegisterLocalStatePrefs(
void WebDiscoveryService::RegisterProfilePrefs(PrefRegistrySimple* registry) {
registry->RegisterDictionaryPref(kAnonymousCredentialsDict);
registry->RegisterStringPref(kCredentialRSAPrivateKey, {});
registry->RegisterListPref(kScheduledDoubleFetches);
registry->RegisterListPref(kScheduledReports);
registry->RegisterListPref(kScheduledDoubleFetches.value());
registry->RegisterListPref(kScheduledReports.value());
registry->RegisterDictionaryPref(kUsedBasenameCounts);
registry->RegisterDictionaryPref(kPageCounts);
}
Expand Down Expand Up @@ -96,8 +96,8 @@ void WebDiscoveryService::Stop() {
void WebDiscoveryService::ClearPrefs() {
profile_prefs_->ClearPref(kAnonymousCredentialsDict);
profile_prefs_->ClearPref(kCredentialRSAPrivateKey);
profile_prefs_->ClearPref(kScheduledDoubleFetches);
profile_prefs_->ClearPref(kScheduledReports);
profile_prefs_->ClearPref(kScheduledDoubleFetches.value());
profile_prefs_->ClearPref(kScheduledReports.value());
profile_prefs_->ClearPref(kUsedBasenameCounts);
profile_prefs_->ClearPref(kPageCounts);
}
Expand Down

0 comments on commit 83195a9

Please sign in to comment.