Skip to content

Commit

Permalink
Protected Audiences: Use InterestGroupCachingStorage
Browse files Browse the repository at this point in the history
This change is a follow up to https://chromium-review.googlesource.com/c/chromium/src/+/4916973,
which created a new class -- InterestGroupCachingStorage.
Use the new class in  InterestManagerImpl, but keep the caching functionality gated by a new Finch feature.
In addition, a number of downstream classes had references and pointers
to StorageInterestGroup which would now point to values inside a scoped_refpr<StorageInterestGroups>. Wrap these inside a new class
StorageInterestGroups::SingleStorageInterestGroups which will ensure
proper destruction of scoped_refpr<StorageInterestGroups> and
StorageInterestGroup*(s).

Bug: 1440817
Change-Id: Id3ac4f25df5c496baae33d2bc837346b7cd75c7e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4935438
Reviewed-by: Russ Hamilton <behamilton@google.com>
Commit-Queue: Abigail Katcoff <abigailkatcoff@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1212242}
  • Loading branch information
Abigail Katcoff authored and Chromium LUCI CQ committed Oct 19, 2023
1 parent ca9ee18 commit 97f8d40
Show file tree
Hide file tree
Showing 21 changed files with 950 additions and 798 deletions.
786 changes: 417 additions & 369 deletions content/browser/interest_group/ad_auction_service_impl_unittest.cc

Large diffs are not rendered by default.

29 changes: 15 additions & 14 deletions content/browser/interest_group/additional_bids_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "base/base64.h"
#include "base/containers/flat_set.h"
#include "base/json/json_writer.h"
#include "base/memory/scoped_refptr.h"
#include "base/strings/strcat.h"
#include "base/time/time.h"
#include "base/types/expected.h"
Expand Down Expand Up @@ -161,11 +162,10 @@ base::expected<AdditionalBidDecodeResult, std::string> DecodeAdditionalBid(
"' rejected due to invalid origin of biddingLogicURL."}));
}

auto synth_interest_group = std::make_unique<StorageInterestGroup>();
synth_interest_group->interest_group.name = *ig_name;
synth_interest_group->interest_group.owner = std::move(ig_owner).value();
synth_interest_group->interest_group.bidding_url = std::move(ig_bidding_url);

auto synth_interest_group = StorageInterestGroup();
synth_interest_group.interest_group.name = *ig_name;
synth_interest_group.interest_group.owner = std::move(ig_owner).value();
synth_interest_group.interest_group.bidding_url = std::move(ig_bidding_url);
// Add ads.
const base::Value::Dict* bid_dict = result_dict->FindDict("bid");
if (!bid_dict) {
Expand All @@ -186,9 +186,9 @@ base::expected<AdditionalBidDecodeResult, std::string> DecodeAdditionalBid(
}

// Create ad vector and its first entry.
synth_interest_group->interest_group.ads.emplace();
synth_interest_group->interest_group.ads.value().emplace_back();
synth_interest_group->interest_group.ads.value()[0].render_url = render_url;
synth_interest_group.interest_group.ads.emplace();
synth_interest_group.interest_group.ads.value().emplace_back();
synth_interest_group.interest_group.ads.value()[0].render_url = render_url;

absl::optional<double> bid_val = bid_dict->FindDouble("bid");
if (!bid_val || bid_val.value() <= 0) {
Expand Down Expand Up @@ -264,7 +264,7 @@ base::expected<AdditionalBidDecodeResult, std::string> DecodeAdditionalBid(
"' rejected due to too many ad component URLs."}));
}

synth_interest_group->interest_group.ad_components.emplace();
synth_interest_group.interest_group.ad_components.emplace();
for (const base::Value& ad_component : *ad_components_list) {
const std::string* ad_component_str = ad_component.GetIfString();
GURL ad_component_url;
Expand All @@ -278,7 +278,7 @@ base::expected<AdditionalBidDecodeResult, std::string> DecodeAdditionalBid(
}
ad_components.emplace_back(ad_component_url);
// TODO(http://crbug.com/1464874): What's the story with dimensions?
synth_interest_group->interest_group.ad_components->emplace_back(
synth_interest_group.interest_group.ad_components->emplace_back(
std::move(ad_component_url), /*metadata=*/absl::nullopt);
}
}
Expand Down Expand Up @@ -348,11 +348,12 @@ base::expected<AdditionalBidDecodeResult, std::string> DecodeAdditionalBid(
result.negative_target_interest_group_names.push_back(*negative_ig_str);
}
}

result.bid_state = std::make_unique<InterestGroupAuction::BidState>();
SingleStorageInterestGroup storage_interest_group(
std::move(synth_interest_group));
result.bid_state = std::make_unique<InterestGroupAuction::BidState>(
std::move(storage_interest_group));
result.bid_state->additional_bid_buyer =
synth_interest_group->interest_group.owner;
result.bid_state->bidder = std::move(synth_interest_group);
result.bid_state->bidder->interest_group.owner;
result.bid_state->made_bid = true;
result.bid_state->BeginTracing();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -494,7 +494,6 @@ TEST_F(AdditionalBidsUtilTest, MinimalValid) {
const InterestGroupAuction::Bid* bid = result->bid.get();

EXPECT_TRUE(bid_state->made_bid);
ASSERT_TRUE(bid_state->bidder);
EXPECT_EQ("trainfans", bid_state->bidder->interest_group.name);
ASSERT_TRUE(bid_state->additional_bid_buyer.has_value());
EXPECT_EQ(bid_state->bidder->interest_group.owner,
Expand Down
2 changes: 1 addition & 1 deletion content/browser/interest_group/auction_metrics_recorder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ void AuctionMetricsRecorder::SetNumSellersWithBidders(
GetExponentialBucketMinForCounts1000(num_sellers_with_bidders));
}

void AuctionMetricsRecorder::ReportBuyer(url::Origin& owner) {
void AuctionMetricsRecorder::ReportBuyer(const url::Origin& owner) {
buyers_.emplace(owner);
}

Expand Down
2 changes: 1 addition & 1 deletion content/browser/interest_group/auction_metrics_recorder.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ class CONTENT_EXPORT AuctionMetricsRecorder {

// Reports an InterestGroup owner, used to determine the number of distinct
// Buyers across all component auctions in a multi-seller auction.
void ReportBuyer(url::Origin& owner);
void ReportBuyer(const url::Origin& owner);

// Reports a Bidder WorkletKey, used to count the number of distinct
// BidderWorklets in use by this auction, which might be more than the number
Expand Down
62 changes: 34 additions & 28 deletions content/browser/interest_group/bidding_and_auction_serializer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
#include "components/cbor/diagnostic_writer.h"
#include "components/cbor/values.h"
#include "components/cbor/writer.h"
#include "content/browser/interest_group/interest_group_caching_storage.h"
#include "content/browser/interest_group/storage_interest_group.h"
#include "content/public/common/content_switches.h"
#include "content/services/auction_worklet/public/mojom/bidder_worklet.mojom.h"
#include "third_party/abseil-cpp/absl/numeric/bits.h"
Expand Down Expand Up @@ -61,48 +63,49 @@ cbor::Value SerializeAds(const std::vector<blink::InterestGroup::Ad>& ads,
// This serialization is sent to the B&A server, so the format is standardized.
// We can't add fields to this format without coordinating with the B&A team.
cbor::Value SerializeInterestGroup(base::Time start_time,
const StorageInterestGroup& group) {
const SingleStorageInterestGroup& group) {
cbor::Value::MapValue group_obj;
group_obj[cbor::Value("name")] = cbor::Value(group.interest_group.name);
if (group.interest_group.trusted_bidding_signals_keys) {
group_obj[cbor::Value("name")] = cbor::Value(group->interest_group.name);
if (group->interest_group.trusted_bidding_signals_keys) {
cbor::Value::ArrayValue bidding_signal_keys;
bidding_signal_keys.reserve(
group.interest_group.trusted_bidding_signals_keys->size());
for (const auto& key : *group.interest_group.trusted_bidding_signals_keys) {
group->interest_group.trusted_bidding_signals_keys->size());
for (const auto& key :
*group->interest_group.trusted_bidding_signals_keys) {
bidding_signal_keys.emplace_back(key);
}
group_obj[cbor::Value("biddingSignalsKeys")] =
cbor::Value(std::move(bidding_signal_keys));
}
if (group.interest_group.user_bidding_signals) {
if (group->interest_group.user_bidding_signals) {
group_obj[cbor::Value("userBiddingSignals")] =
cbor::Value(*group.interest_group.user_bidding_signals);
cbor::Value(*group->interest_group.user_bidding_signals);
}
if (!group.interest_group.auction_server_request_flags.Has(
if (!group->interest_group.auction_server_request_flags.Has(
blink::AuctionServerRequestFlagsEnum::kOmitAds)) {
if (group.interest_group.ads) {
if (group->interest_group.ads) {
group_obj[cbor::Value("ads")] = SerializeAds(
*group.interest_group.ads,
group.interest_group.auction_server_request_flags.Has(
*group->interest_group.ads,
group->interest_group.auction_server_request_flags.Has(
blink::AuctionServerRequestFlagsEnum::kIncludeFullAds));
}
if (group.interest_group.ad_components) {
if (group->interest_group.ad_components) {
group_obj[cbor::Value("components")] = SerializeAds(
*group.interest_group.ad_components,
group.interest_group.auction_server_request_flags.Has(
*group->interest_group.ad_components,
group->interest_group.auction_server_request_flags.Has(
blink::AuctionServerRequestFlagsEnum::kIncludeFullAds));
}
}
cbor::Value::MapValue browser_signals;
browser_signals[cbor::Value("bidCount")] =
cbor::Value(group.bidding_browser_signals->bid_count);
cbor::Value(group->bidding_browser_signals->bid_count);
// joinCount and recency are noised and binned on the server.
browser_signals[cbor::Value("joinCount")] =
cbor::Value(group.bidding_browser_signals->join_count);
cbor::Value(group->bidding_browser_signals->join_count);
browser_signals[cbor::Value("recency")] =
cbor::Value((start_time - group.join_time).InSeconds());
cbor::Value((start_time - group->join_time).InSeconds());
cbor::Value::ArrayValue prev_wins;
for (const auto& prev_win : group.bidding_browser_signals->prev_wins) {
for (const auto& prev_win : group->bidding_browser_signals->prev_wins) {
cbor::Value::ArrayValue tuple;
tuple.emplace_back((start_time - prev_win->time).InSeconds());
// We trust this ad_json because we wrote it ourselves.
Expand All @@ -119,7 +122,7 @@ cbor::Value SerializeInterestGroup(base::Time start_time,
// Just do our best regardless.
continue;
}
if (group.interest_group.auction_server_request_flags.Has(
if (group->interest_group.auction_server_request_flags.Has(
blink::AuctionServerRequestFlagsEnum::kIncludeFullAds)) {
cbor::Value::MapValue obj;
for (const auto kv : ad->GetDict()) {
Expand Down Expand Up @@ -177,14 +180,17 @@ BiddingAndAuctionSerializer::BiddingAndAuctionSerializer(
BiddingAndAuctionSerializer::~BiddingAndAuctionSerializer() = default;

void BiddingAndAuctionSerializer::AddGroups(
url::Origin owner,
std::vector<StorageInterestGroup> groups) {
base::EraseIf(groups, [](const StorageInterestGroup& group) {
return (!group.interest_group.ads) ||
(group.interest_group.ads->size() == 0);
const url::Origin& owner,
scoped_refptr<StorageInterestGroups> groups) {
std::vector<SingleStorageInterestGroup> groups_to_add =
groups->GetInterestGroups();
base::EraseIf(groups_to_add, [](const SingleStorageInterestGroup& group) {
return (!group->interest_group.ads) ||
(group->interest_group.ads->size() == 0);
});
if (groups.size() > 0) {
accumulated_groups_.emplace_back(std::move(owner), std::move(groups));
if (groups_to_add.size() > 0) {
accumulated_groups_.emplace_back(std::move(owner),
std::move(groups_to_add));
}
}

Expand All @@ -208,10 +214,10 @@ BiddingAndAuctionData BiddingAndAuctionSerializer::Build() {
for (const auto& bidder_groups : accumulated_groups_) {
cbor::Value::ArrayValue groups;
std::vector<std::string> names;
for (const auto& group : bidder_groups.second) {
for (const SingleStorageInterestGroup& group : bidder_groups.second) {
cbor::Value group_obj = SerializeInterestGroup(start_time_, group);
groups.emplace_back(std::move(group_obj));
names.push_back(group.interest_group.name);
names.push_back(group->interest_group.name);
}
cbor::Value groups_obj(std::move(groups));
absl::optional<std::vector<uint8_t>> maybe_sub_message =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,10 @@
#include "base/containers/flat_map.h"
#include "base/time/time.h"
#include "base/uuid.h"
#include "content/browser/interest_group/interest_group_caching_storage.h"
#include "content/browser/interest_group/storage_interest_group.h"
#include "content/common/content_export.h"
#include "url/origin.h"

namespace content {

Expand All @@ -38,14 +40,15 @@ class BiddingAndAuctionSerializer {
void SetGenerationId(base::Uuid generation_id) {
generation_id_ = generation_id;
}
void AddGroups(url::Origin owner, std::vector<StorageInterestGroup> groups);
void AddGroups(const url::Origin& owner,
scoped_refptr<StorageInterestGroups> groups);
BiddingAndAuctionData Build();

private:
base::Uuid generation_id_;
base::Time start_time_;
std::string publisher_;
std::vector<std::pair<url::Origin, std::vector<StorageInterestGroup>>>
std::vector<std::pair<url::Origin, std::vector<SingleStorageInterestGroup>>>
accumulated_groups_;
};

Expand Down

0 comments on commit 97f8d40

Please sign in to comment.