Skip to content

Commit

Permalink
Implement data clearing for Private Aggregation API budgeting
Browse files Browse the repository at this point in the history
Ensures budgeting data is cleared when a user initiates data clearing.
However, note that site-initiated data clearing does not affect
budgeting data. This also makes a few drive-by changes to other data
removal constants.

Bug: 1328439
Change-Id: I6bd8aaa2b2e0d005317ce6bba1a498d14da5b408
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3825990
Reviewed-by: Austin Sullivan <asully@chromium.org>
Reviewed-by: John Delaney <johnidel@chromium.org>
Reviewed-by: Theodore Olsauskas-Warren <sauski@google.com>
Commit-Queue: Alex Turner <alexmt@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Christian Dullweber <dullweber@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1039323}
  • Loading branch information
alexmturner authored and Chromium LUCI CQ committed Aug 25, 2022
1 parent 76e9e61 commit 5ccf828
Show file tree
Hide file tree
Showing 25 changed files with 1,003 additions and 43 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -139,10 +139,12 @@ void DeleteStoragePartitionDataWithFilter(
: base::NullCallback();

const uint32_t removal_mask =
content::StoragePartition::REMOVE_DATA_MASK_AGGREGATION_SERVICE |
content::StoragePartition::
REMOVE_DATA_MASK_ATTRIBUTION_REPORTING_SITE_CREATED |
content::StoragePartition::
REMOVE_DATA_MASK_ATTRIBUTION_REPORTING_INTERNAL;
REMOVE_DATA_MASK_ATTRIBUTION_REPORTING_INTERNAL |
content::StoragePartition::REMOVE_DATA_MASK_PRIVATE_AGGREGATION_INTERNAL;
const uint32_t quota_removal_mask = 0;
storage_partition->ClearData(
removal_mask, quota_removal_mask, std::move(storage_key_matcher),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,10 +82,13 @@ TEST_F(BrowsingDataHistoryObserverServiceTest, AllHistoryDeleted_DataCleared) {

const absl::optional<RemovalData>& removal_data = partition.GetRemovalData();
EXPECT_TRUE(removal_data.has_value());
EXPECT_EQ(content::StoragePartition::
EXPECT_EQ(content::StoragePartition::REMOVE_DATA_MASK_AGGREGATION_SERVICE |
content::StoragePartition::
REMOVE_DATA_MASK_ATTRIBUTION_REPORTING_SITE_CREATED |
content::StoragePartition::
REMOVE_DATA_MASK_ATTRIBUTION_REPORTING_INTERNAL,
REMOVE_DATA_MASK_ATTRIBUTION_REPORTING_INTERNAL |
content::StoragePartition::
REMOVE_DATA_MASK_PRIVATE_AGGREGATION_INTERNAL,
removal_data->removal_mask);
EXPECT_EQ(0u, removal_data->quota_storage_removal_mask);
EXPECT_EQ(base::Time(), removal_data->begin);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,7 @@ constexpr DataType DATA_TYPE_SITE_DATA =
#endif
DATA_TYPE_SITE_USAGE_DATA | DATA_TYPE_DURABLE_PERMISSION |
DATA_TYPE_EXTERNAL_PROTOCOL_DATA | DATA_TYPE_ISOLATED_ORIGINS |
content::BrowsingDataRemover::DATA_TYPE_PRIVACY_SANDBOX |
content::BrowsingDataRemover::DATA_TYPE_ATTRIBUTION_REPORTING;
content::BrowsingDataRemover::DATA_TYPE_PRIVACY_SANDBOX;

// Datatypes protected by Important Sites.
constexpr DataType IMPORTANT_SITES_DATA_TYPES =
Expand Down
5 changes: 1 addition & 4 deletions chrome/browser/privacy_sandbox/privacy_sandbox_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -300,10 +300,7 @@ void PrivacySandboxService::OnPrivacySandboxV2PrefChanged() {
if (browsing_data_remover_) {
browsing_data_remover_->Remove(
base::Time::Min(), base::Time::Max(),
content::BrowsingDataRemover::DATA_TYPE_INTEREST_GROUPS |
content::BrowsingDataRemover::DATA_TYPE_AGGREGATION_SERVICE |
content::BrowsingDataRemover::DATA_TYPE_ATTRIBUTION_REPORTING |
content::BrowsingDataRemover::DATA_TYPE_TRUST_TOKENS,
content::BrowsingDataRemover::DATA_TYPE_PRIVACY_SANDBOX,
content::BrowsingDataRemover::ORIGIN_TYPE_UNPROTECTED_WEB);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1286,10 +1286,7 @@ TEST_F(PrivacySandboxServiceTest, DisablingV2SandboxClearsData) {
// Disabling should start a task clearing all kAPI information.
EXPECT_CALL(*mock_browsing_topics_service(), ClearAllTopicsData()).Times(1);
prefs()->SetBoolean(prefs::kPrivacySandboxApisEnabledV2, false);
EXPECT_EQ(content::BrowsingDataRemover::DATA_TYPE_INTEREST_GROUPS |
content::BrowsingDataRemover::DATA_TYPE_AGGREGATION_SERVICE |
content::BrowsingDataRemover::DATA_TYPE_ATTRIBUTION_REPORTING |
content::BrowsingDataRemover::DATA_TYPE_TRUST_TOKENS,
EXPECT_EQ(content::BrowsingDataRemover::DATA_TYPE_PRIVACY_SANDBOX,
browsing_data_remover()->GetLastUsedRemovalMaskForTesting());
EXPECT_EQ(base::Time::Min(),
browsing_data_remover()->GetLastUsedBeginTimeForTesting());
Expand Down
4 changes: 4 additions & 0 deletions content/browser/browsing_data/browsing_data_remover_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -429,6 +429,10 @@ void BrowsingDataRemoverImpl::RemoveImpl(
storage_partition_remove_mask |=
StoragePartition::REMOVE_DATA_MASK_AGGREGATION_SERVICE;
}
if (remove_mask & DATA_TYPE_PRIVATE_AGGREGATION_INTERNAL) {
storage_partition_remove_mask |=
StoragePartition::REMOVE_DATA_MASK_PRIVATE_AGGREGATION_INTERNAL;
}
if (remove_mask & DATA_TYPE_INTEREST_GROUPS) {
storage_partition_remove_mask |=
StoragePartition::REMOVE_DATA_MASK_INTEREST_GROUPS;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1371,6 +1371,15 @@ TEST_F(BrowsingDataRemoverImplTest, RemoveAggregationServiceData) {
StoragePartition::REMOVE_DATA_MASK_AGGREGATION_SERVICE);
}

TEST_F(BrowsingDataRemoverImplTest, RemovePrivateAggregationData) {
BlockUntilBrowsingDataRemoved(
base::Time(), base::Time::Max(),
BrowsingDataRemover::DATA_TYPE_PRIVATE_AGGREGATION_INTERNAL, false);
StoragePartitionRemovalData removal_data = GetStoragePartitionRemovalData();
EXPECT_EQ(removal_data.remove_mask,
StoragePartition::REMOVE_DATA_MASK_PRIVATE_AGGREGATION_INTERNAL);
}

class MultipleTasksObserver {
public:
// A simple implementation of BrowsingDataRemover::Observer.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ class TestBrowsingDataRemoverDelegate : public MockBrowsingDataRemoverDelegate {
: 0) |
(cache ? BrowsingDataRemover::DATA_TYPE_CACHE : 0);
data_type_mask &=
~BrowsingDataRemover::DATA_TYPE_ATTRIBUTION_REPORTING_INTERNAL;
~BrowsingDataRemover::DATA_TYPE_PRIVACY_SANDBOX_INTERNAL;

BrowsingDataFilterBuilderImpl filter_builder(
BrowsingDataFilterBuilder::Mode::kDelete);
Expand Down
3 changes: 1 addition & 2 deletions content/browser/browsing_data/clear_site_data_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,7 @@ class SiteDataClearer : public BrowsingDataRemover::Observer {
remove_mask |= BrowsingDataRemover::DATA_TYPE_DOM_STORAGE;
remove_mask |= BrowsingDataRemover::DATA_TYPE_PRIVACY_SANDBOX;
// Internal data should not be removed by site-initiated deletions.
remove_mask &=
~BrowsingDataRemover::DATA_TYPE_ATTRIBUTION_REPORTING_INTERNAL;
remove_mask &= ~BrowsingDataRemover::DATA_TYPE_PRIVACY_SANDBOX_INTERNAL;
}
if (clear_cache_)
remove_mask |= BrowsingDataRemover::DATA_TYPE_CACHE;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ class CONTENT_EXPORT PrivateAggregationBudgetKey {
// Represents a period of time for which budget usage is recorded. This
// interval includes the `start_time()` instant but excludes the end time
// (`start_time() + kDuration`) instant.
// TODO(crbug.com/1353473): Ensure `base::Time::Min()` and nearby times are
// handled correctly.
class TimeWindow {
public:
static constexpr base::TimeDelta kDuration = base::Hours(1);
Expand Down
126 changes: 120 additions & 6 deletions content/browser/private_aggregation/private_aggregation_budgeter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,10 @@
#include "content/browser/private_aggregation/private_aggregation_budget_key.h"
#include "content/browser/private_aggregation/private_aggregation_budget_storage.h"
#include "content/browser/private_aggregation/proto/private_aggregation_budgets.pb.h"
#include "third_party/blink/public/common/storage_key/storage_key.h"
#include "third_party/protobuf/src/google/protobuf/repeated_field.h"
#include "url/gurl.h"
#include "url/origin.h"

namespace content {

Expand Down Expand Up @@ -77,21 +80,38 @@ void PrivateAggregationBudgeter::ConsumeBudget(
const PrivateAggregationBudgetKey& budget_key,
base::OnceCallback<void(bool)> on_done) {
if (storage_status_ == StorageStatus::kInitializing) {
if (pending_consume_budget_calls_.size() >= kMaxPendingCalls) {
if (pending_calls_.size() >= kMaxPendingCalls) {
std::move(on_done).Run(false);
return;
}

// `base::Unretained` is safe as `pending_consume_budget_calls_` is owned by
// `this`.
pending_consume_budget_calls_.push_back(base::BindOnce(
// `base::Unretained` is safe as `pending_calls_` is owned by `this`.
pending_calls_.push_back(base::BindOnce(
&PrivateAggregationBudgeter::ConsumeBudgetImpl, base::Unretained(this),
budget, budget_key, std::move(on_done)));
} else {
ConsumeBudgetImpl(budget, budget_key, std::move(on_done));
}
}

void PrivateAggregationBudgeter::ClearData(
base::Time delete_begin,
base::Time delete_end,
StoragePartition::StorageKeyMatcherFunction filter,
base::OnceClosure done) {
if (storage_status_ == StorageStatus::kInitializing) {
// To ensure that data deletion always succeeds, we don't check
// `pending_calls.size()` here.

// `base::Unretained` is safe as `pending_calls_` is owned by `this`.
pending_calls_.push_back(base::BindOnce(
&PrivateAggregationBudgeter::ClearDataImpl, base::Unretained(this),
delete_begin, delete_end, std::move(filter), std::move(done)));
} else {
ClearDataImpl(delete_begin, delete_end, std::move(filter), std::move(done));
}
}

void PrivateAggregationBudgeter::OnStorageDoneInitializing(
std::unique_ptr<PrivateAggregationBudgetStorage> storage) {
DCHECK(shutdown_initializing_storage_);
Expand All @@ -110,10 +130,10 @@ void PrivateAggregationBudgeter::OnStorageDoneInitializing(
}

void PrivateAggregationBudgeter::ProcessAllPendingCalls() {
for (base::OnceClosure& cb : pending_consume_budget_calls_) {
for (base::OnceClosure& cb : pending_calls_) {
std::move(cb).Run();
}
pending_consume_budget_calls_.clear();
pending_calls_.clear();
}

// TODO(crbug.com/1336733): Consider enumerating different error cases and log
Expand Down Expand Up @@ -215,4 +235,98 @@ void PrivateAggregationBudgeter::ConsumeBudgetImpl(
std::move(on_done).Run(budget_increase_allowed);
}

void PrivateAggregationBudgeter::ClearDataImpl(
base::Time delete_begin,
base::Time delete_end,
StoragePartition::StorageKeyMatcherFunction filter,
base::OnceClosure done) {
switch (storage_status_) {
case StorageStatus::kInitializing:
NOTREACHED();
break;
case StorageStatus::kInitializationFailed:
std::move(done).Run();
return;
case StorageStatus::kOpen:
break;
}

// TODO(alexmt): Delay `done` being run until after the database task is
// complete.

// Treat null times as unbounded lower or upper range. This is used by
// browsing data remover.
if (delete_begin.is_null())
delete_begin = base::Time::Min();

if (delete_end.is_null())
delete_end = base::Time::Max();

bool is_all_time_covered = delete_begin.is_min() && delete_end.is_max();

if (is_all_time_covered && filter.is_null()) {
storage_->budgets_data()->DeleteAllData();
std::move(done).Run();
return;
}

std::vector<std::string> origins_to_delete;

for (const auto& [origin_key, budgets] :
storage_->budgets_data()->GetAllCached()) {
if (filter.is_null() ||
filter.Run(blink::StorageKey(url::Origin::Create(GURL(origin_key))))) {
origins_to_delete.push_back(origin_key);
}
}

if (is_all_time_covered) {
storage_->budgets_data()->DeleteData(origins_to_delete);
std::move(done).Run();
return;
}

// Ensure we round down to capture any time windows that partially overlap.
int64_t serialized_delete_begin =
delete_begin.is_min()
? SerializeTimeForStorage(base::Time::Min())
: SerializeTimeForStorage(
PrivateAggregationBudgetKey::TimeWindow(delete_begin)
.start_time());

// No need to round up as we compare against the time window's start time.
int64_t serialized_delete_end = SerializeTimeForStorage(delete_end);

for (const std::string& origin_key : origins_to_delete) {
proto::PrivateAggregationBudgets budgets;
storage_->budgets_data()->TryGetData(origin_key, &budgets);

static constexpr PrivateAggregationBudgetKey::Api kAllApis[] = {
PrivateAggregationBudgetKey::Api::kFledge,
PrivateAggregationBudgetKey::Api::kSharedStorage};

for (PrivateAggregationBudgetKey::Api api : kAllApis) {
google::protobuf::RepeatedPtrField<
proto::PrivateAggregationBudgetPerHour>* hourly_budgets =
GetHourlyBudgets(api, budgets);
DCHECK(hourly_budgets);

auto new_end = std::remove_if(
hourly_budgets->begin(), hourly_budgets->end(),
[=](const proto::PrivateAggregationBudgetPerHour& elem) {
return elem.hour_start_timestamp() >= serialized_delete_begin &&
elem.hour_start_timestamp() <= serialized_delete_end;
});
hourly_budgets->erase(new_end, hourly_budgets->end());
}
storage_->budgets_data()->UpdateData(origin_key, budgets);
}

// A no-op call to force the database to be flushed immediately instead of
// waiting up to `PrivateAggregationBudgetStorage::kFlushDelay`.
storage_->budgets_data()->DeleteData({});

std::move(done).Run();
}

} // namespace content
30 changes: 24 additions & 6 deletions content/browser/private_aggregation/private_aggregation_budgeter.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include "base/time/time.h"
#include "content/browser/private_aggregation/private_aggregation_budget_key.h"
#include "content/common/content_export.h"
#include "content/public/browser/storage_partition.h"

template <class T>
class scoped_refptr;
Expand Down Expand Up @@ -44,8 +45,9 @@ class CONTENT_EXPORT PrivateAggregationBudgeter {
// Maximum budget allowed to be claimed per-origin per-day per-API.
static constexpr int kMaxBudgetPerScope = 65536;

// To avoid unbounded memory growth, limit the number of pending consume
// budget calls during initialization.
// To avoid unbounded memory growth, limit the number of pending calls during
// initialization. Data clearing calls can be posted even if it would exceed
// this limit.
static constexpr int kMaxPendingCalls = 1000;

// The total length of time that per-origin per-API budgets are enforced
Expand Down Expand Up @@ -87,6 +89,17 @@ class CONTENT_EXPORT PrivateAggregationBudgeter {
const PrivateAggregationBudgetKey& budget_key,
base::OnceCallback<void(bool)> on_done);

// Deletes all data in storage for any budgets that could have been set
// between `delete_begin` and `delete_end` time (inclusive). Note that the
// discrete time windows used may lead to more data being deleted than
// strictly necessary. Null times are treated as unbounded lower or upper
// range. If `!filter.is_null()`, budget keys with an origin that does *not*
// match the `filter` are retained (i.e. not cleared).
virtual void ClearData(base::Time delete_begin,
base::Time delete_end,
StoragePartition::StorageKeyMatcherFunction filter,
base::OnceClosure done);

// TODO(crbug.com/1328439): Clear stale data periodically and on startup.

protected:
Expand All @@ -104,13 +117,18 @@ class CONTENT_EXPORT PrivateAggregationBudgeter {
void ConsumeBudgetImpl(int additional_budget,
const PrivateAggregationBudgetKey& budget_key,
base::OnceCallback<void(bool)> on_done);
void ClearDataImpl(base::Time delete_begin,
base::Time delete_end,
StoragePartition::StorageKeyMatcherFunction filter,
base::OnceClosure done);

void ProcessAllPendingCalls();

// While the storage initializes, queues calls to ConsumeBudget() in the
// order the calls are received. Should be empty after storage is initialized.
// The size is limited to `kMaxPendingCalls`.
std::vector<base::OnceClosure> pending_consume_budget_calls_;
// While the storage initializes, queues calls (e.g. to `ConsumeBudget()`) in
// the order the calls are received. Should be empty after storage is
// initialized. The size is limited to `kMaxPendingCalls` except that
// `ClearData()` can store additional tasks beyond that limit.
std::vector<base::OnceClosure> pending_calls_;

// `nullptr` until initialization is complete or if initialization failed.
// Otherwise, owned by this class until destruction. Iff present,
Expand Down
Loading

0 comments on commit 5ccf828

Please sign in to comment.