Skip to content

Commit

Permalink
Pass directFromSellerSignalsHeaderAdSlot results to worklets
Browse files Browse the repository at this point in the history
Since the same directFromSellerSignals parameter object (for
generateBid(), scoreAd(), reportResult(), reportWin()) that was used for
the subresource bundle version of directFromSellerSignals will also be
used for the header ad-slot version, it's invalid for the browser
process to pass both types of directFromSellerSignals.

Bug: 1462720
Change-Id: I212a7bc2af99f84d5076e3eaad3c192052c7421b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4744259
Reviewed-by: Maks Orlovich <morlovich@chromium.org>
Commit-Queue: Caleb Raitto <caraitto@chromium.org>
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1186925}
  • Loading branch information
caraitto authored and Chromium LUCI CQ committed Aug 22, 2023
1 parent bd56715 commit 18b6294
Show file tree
Hide file tree
Showing 26 changed files with 1,176 additions and 195 deletions.
12 changes: 12 additions & 0 deletions content/browser/interest_group/auction_worklet_manager_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,11 @@ class MockBidderWorklet : public auction_worklet::mojom::BidderWorklet {
const absl::optional<std::string>& auction_signals_json,
const absl::optional<std::string>& per_buyer_signals_json,
const absl::optional<GURL>& direct_from_seller_per_buyer_signals,
const absl::optional<std::string>&
direct_from_seller_per_buyer_signals_header_ad_slot,
const absl::optional<GURL>& direct_from_seller_auction_signals,
const absl::optional<std::string>&
direct_from_seller_auction_signals_header_ad_slot,
const std::string& seller_signals_json,
auction_worklet::mojom::KAnonymityBidMode kanon_mode,
bool bid_is_kanon,
Expand Down Expand Up @@ -337,7 +341,11 @@ class MockSellerWorklet : public auction_worklet::mojom::SellerWorklet {
const blink::AuctionConfig::NonSharedParams&
auction_ad_config_non_shared_params,
const absl::optional<GURL>& direct_from_seller_seller_signals,
const absl::optional<std::string>&
direct_from_seller_seller_signals_header_ad_slot,
const absl::optional<GURL>& direct_from_seller_auction_signals,
const absl::optional<std::string>&
direct_from_seller_auction_signals_header_ad_slot,
auction_worklet::mojom::ComponentAuctionOtherSellerPtr
browser_signals_other_seller,
const absl::optional<blink::AdCurrency>& component_expect_bid_currency,
Expand All @@ -364,7 +372,11 @@ class MockSellerWorklet : public auction_worklet::mojom::SellerWorklet {
const blink::AuctionConfig::NonSharedParams&
auction_ad_config_non_shared_params,
const absl::optional<GURL>& direct_from_seller_seller_signals,
const absl::optional<std::string>&
direct_from_seller_seller_signals_header_ad_slot,
const absl::optional<GURL>& direct_from_seller_auction_signals,
const absl::optional<std::string>&
direct_from_seller_auction_signals_header_ad_slot,
auction_worklet::mojom::ComponentAuctionOtherSellerPtr
browser_signals_other_seller,
const url::Origin& browser_signal_interest_group_owner,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,17 +71,17 @@ class CONTENT_EXPORT HeaderDirectFromSellerSignals {
CompletionCallback callback);

// Results of the `sellerSignals` JSON dictionary field.
const absl::optional<std::string>& seller_signals() {
const absl::optional<std::string>& seller_signals() const {
return seller_signals_;
}

// Results of the `auctionSignals` JSON dictionary field.
const absl::optional<std::string>& auction_signals() {
const absl::optional<std::string>& auction_signals() const {
return auction_signals_;
}

// Results of the `perBuyerSignals` JSON dictionary field.
const base::flat_map<url::Origin, std::string>& per_buyer_signals() {
const base::flat_map<url::Origin, std::string>& per_buyer_signals() const {
return per_buyer_signals_;
}

Expand Down
64 changes: 60 additions & 4 deletions content/browser/interest_group/interest_group_auction.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1333,7 +1333,12 @@ class InterestGroupAuction::BuyerHelper
PerBuyerCurrency(owner_, *auction_->config_),
GetDirectFromSellerPerBuyerSignals(
url_builder, bid_state->bidder->interest_group.owner),
GetDirectFromSellerAuctionSignals(url_builder));
GetDirectFromSellerPerBuyerSignalsHeaderAdSlot(
*auction_->direct_from_seller_signals_header_ad_slot(),
bid_state->bidder->interest_group.owner),
GetDirectFromSellerAuctionSignals(url_builder),
GetDirectFromSellerAuctionSignalsHeaderAdSlot(
*auction_->direct_from_seller_signals_header_ad_slot()));
bid_state->bid_finalizer.reset();
}

Expand Down Expand Up @@ -2105,6 +2110,7 @@ void InterestGroupAuction::StartBiddingAndScoringPhase(

bidding_and_scoring_phase_start_time_ = base::TimeTicks::Now();

CHECK_EQ(num_scoring_dependencies_, 0);
num_scoring_dependencies_ =
buyer_helpers_.size() + component_auctions_.size();

Expand All @@ -2113,6 +2119,11 @@ void InterestGroupAuction::StartBiddingAndScoringPhase(
++num_scoring_dependencies_;
}

// Also wait for directFromSellerSignalsHeaderAdSlot to finish JSON parsing.
if (direct_from_seller_signals_header_ad_slot_pending_) {
++num_scoring_dependencies_;
}

// TODO(morlovich): If the config already resolved additional_bids here,
// may start work on it.

Expand Down Expand Up @@ -2301,8 +2312,11 @@ InterestGroupAuction::CreateReporter(
top_level_seller_winning_bid_info;
top_level_seller_winning_bid_info.auction_config = config_;
DCHECK(subresource_url_builder_); // Must have been created by scoring.
CHECK(direct_from_seller_signals_header_ad_slot_); // Should never be null.
top_level_seller_winning_bid_info.subresource_url_builder =
std::move(subresource_url_builder_);
top_level_seller_winning_bid_info.direct_from_seller_signals_header_ad_slot =
std::move(direct_from_seller_signals_header_ad_slot_);
top_level_seller_winning_bid_info.bid = winner->bid->bid;

if (winner->bid->auction == this) {
Expand Down Expand Up @@ -2353,8 +2367,13 @@ InterestGroupAuction::CreateReporter(
component_seller_winning_bid_info->auction_config =
component_auction->config_;
DCHECK(component_auction->subresource_url_builder_);
// Should never be null.
CHECK(component_auction->direct_from_seller_signals_header_ad_slot_);
component_seller_winning_bid_info->subresource_url_builder =
std::move(component_auction->subresource_url_builder_);
component_seller_winning_bid_info
->direct_from_seller_signals_header_ad_slot = std::move(
component_auction->direct_from_seller_signals_header_ad_slot_);
const LeaderInfo& component_leader = component_auction->leader_info();
component_seller_winning_bid_info->bid = component_leader.top_bid->bid->bid;
// The bidder in this auction was the actual bidder, so the currency comes
Expand Down Expand Up @@ -2500,10 +2519,13 @@ void InterestGroupAuction::NotifyDirectFromSellerSignalsHeaderAdSlotConfig(
AdAuctionPageData* auction_page_data,
const absl::optional<std::string>&
direct_from_seller_signals_header_ad_slot) {
CHECK(!direct_from_seller_signals_header_ad_slot_pending_);
if (!direct_from_seller_signals_header_ad_slot) {
return;
}

if (started_bidding_and_scoring_phase_) {
++num_scoring_dependencies_;
}
direct_from_seller_signals_header_ad_slot_pending_ = true;
HeaderDirectFromSellerSignals::ParseAndFind(
auction_page_data->GetAuctionSignalsForOrigin(config_->seller),
Expand Down Expand Up @@ -3098,6 +3120,12 @@ absl::optional<GURL> InterestGroupAuction::GetDirectFromSellerAuctionSignals(
return absl::nullopt;
}

absl::optional<std::string>
InterestGroupAuction::GetDirectFromSellerAuctionSignalsHeaderAdSlot(
const HeaderDirectFromSellerSignals& signals) {
return signals.auction_signals();
}

absl::optional<GURL> InterestGroupAuction::GetDirectFromSellerPerBuyerSignals(
const SubresourceUrlBuilder* subresource_url_builder,
const url::Origin& owner) {
Expand All @@ -3112,6 +3140,17 @@ absl::optional<GURL> InterestGroupAuction::GetDirectFromSellerPerBuyerSignals(
return it->second.subresource_url;
}

absl::optional<std::string>
InterestGroupAuction::GetDirectFromSellerPerBuyerSignalsHeaderAdSlot(
const HeaderDirectFromSellerSignals& signals,
const url::Origin& owner) {
auto it = signals.per_buyer_signals().find(owner);
if (it == signals.per_buyer_signals().end()) {
return absl::nullopt;
}
return it->second;
}

absl::optional<GURL> InterestGroupAuction::GetDirectFromSellerSellerSignals(
const SubresourceUrlBuilder* subresource_url_builder) {
if (subresource_url_builder && subresource_url_builder->seller_signals()) {
Expand All @@ -3120,6 +3159,12 @@ absl::optional<GURL> InterestGroupAuction::GetDirectFromSellerSellerSignals(
return absl::nullopt;
}

absl::optional<std::string>
InterestGroupAuction::GetDirectFromSellerSellerSignalsHeaderAdSlot(
const HeaderDirectFromSellerSignals& signals) {
return signals.seller_signals();
}

InterestGroupAuction::LeaderInfo::LeaderInfo() = default;
InterestGroupAuction::LeaderInfo::~LeaderInfo() = default;

Expand Down Expand Up @@ -3527,7 +3572,11 @@ void InterestGroupAuction::ScoreBidIfReady(std::unique_ptr<Bid> bid) {
seller_worklet_handle_->GetSellerWorklet()->ScoreAd(
bid_raw->ad_metadata, bid_raw->bid, bid_raw->bid_currency,
config_->non_shared_params, GetDirectFromSellerSellerSignals(url_builder),
GetDirectFromSellerSellerSignalsHeaderAdSlot(
*direct_from_seller_signals_header_ad_slot_),
GetDirectFromSellerAuctionSignals(url_builder),
GetDirectFromSellerAuctionSignalsHeaderAdSlot(
*direct_from_seller_signals_header_ad_slot_),
GetOtherSellerParam(*bid_raw),
parent_ ? PerBuyerCurrency(config_->seller, *parent_->config_)
: absl::nullopt,
Expand Down Expand Up @@ -4133,12 +4182,19 @@ void InterestGroupAuction::OnLoadedWinningGroup(
void InterestGroupAuction::OnDirectFromSellerSignalHeaderAdSlotResolved(
std::unique_ptr<HeaderDirectFromSellerSignals> signals,
std::vector<std::string> errors) {
CHECK(direct_from_seller_signals_header_ad_slot_pending_);
CHECK(direct_from_seller_signals_header_ad_slot_);
direct_from_seller_signals_header_ad_slot_ = std::move(signals);
errors_.insert(errors_.end(), errors.begin(), errors.end());

direct_from_seller_signals_header_ad_slot_pending_ = false;
for (const auto& buyer_helper : buyer_helpers_) {
buyer_helper->NotifyConfigDependencyResolved();

if (started_bidding_and_scoring_phase_) {
for (const auto& buyer_helper : buyer_helpers_) {
buyer_helper->NotifyConfigDependencyResolved();
}
OnScoringDependencyDone();
ScoreQueuedBidsIfReady();
}
}

Expand Down
41 changes: 34 additions & 7 deletions content/browser/interest_group/interest_group_auction.h
Original file line number Diff line number Diff line change
Expand Up @@ -710,22 +710,41 @@ class CONTENT_EXPORT InterestGroupAuction
const blink::AuctionConfig& config,
const url::Origin& buyer);

// Gets the buyer DirectFromSellerSignals auction-signals in `config` for
// buyer. Public so that InterestGroupAuctionReporter can use it.
// Gets the DirectFromSellerSignals auction-signals. Public so that
// InterestGroupAuctionReporter can use it.
static absl::optional<GURL> GetDirectFromSellerAuctionSignals(
const SubresourceUrlBuilder* subresource_url_builder);

// Gets the DirectFromSellerSignalsHeaderAdSlot auction-signals. Public so
// that InterestGroupAuctionReporter can use it.
static absl::optional<std::string>
GetDirectFromSellerAuctionSignalsHeaderAdSlot(
const HeaderDirectFromSellerSignals& signals);

// Gets the buyer DirectFromSellerSignals per-buyer-signals in `config` for
// buyer. Public so that InterestGroupAuctionReporter can use it.
static absl::optional<GURL> GetDirectFromSellerPerBuyerSignals(
const SubresourceUrlBuilder* subresource_url_builder,
const url::Origin& owner);

// Gets the buyer DirectFromSellerSignals seller-signals in `config` for
// buyer. Public so that InterestGroupAuctionReporter can use it.
// Gets the buyer DirectFromSellerSignalsHeaderAdSlot per-buyer-signals
// for `owner`. Public so that InterestGroupAuctionReporter can use it.
static absl::optional<std::string>
GetDirectFromSellerPerBuyerSignalsHeaderAdSlot(
const HeaderDirectFromSellerSignals& signals,
const url::Origin& owner);

// Gets DirectFromSellerSignals seller-signals. Public so that
// InterestGroupAuctionReporter can use it.
static absl::optional<GURL> GetDirectFromSellerSellerSignals(
const SubresourceUrlBuilder* subresource_url_builder);

// Gets DirectFromSellerSignalsHeaderAdSlot seller-signals. Public so that
// InterestGroupAuctionReporter can use it.
static absl::optional<std::string>
GetDirectFromSellerSellerSignalsHeaderAdSlot(
const HeaderDirectFromSellerSignals& signals);

// Replaces `${}` placeholders in a debug report URL's query string for post
// auction signals if exist. Only replaces unescaped placeholder ${}, but
// not escaped placeholder (i.e., %24%7B%7D).
Expand Down Expand Up @@ -821,7 +840,8 @@ class CONTENT_EXPORT InterestGroupAuction
// True if all async prerequisites for calling ScoreBid on the SellerWorklet
// are done.
bool ReadyToScoreBids() const {
return seller_worklet_received_ && config_promises_resolved_;
return seller_worklet_received_ && config_promises_resolved_ &&
!direct_from_seller_signals_header_ad_slot_pending_;
}

// Called when RequestSellerWorklet() returns. Starts scoring bids, if there
Expand All @@ -842,6 +862,7 @@ class CONTENT_EXPORT InterestGroupAuction
// True if all bids have been generated (or decoded from additional_bids) and
// scored and all config promises resolved.
bool IsBiddingAndScoringPhaseComplete() const {
CHECK(started_bidding_and_scoring_phase_);
return num_scoring_dependencies_ == 0 && bids_being_scored_ == 0 &&
unscored_bids_.empty();
}
Expand Down Expand Up @@ -1007,6 +1028,11 @@ class CONTENT_EXPORT InterestGroupAuction
// creating it if needed.
SubresourceUrlBuilder* SubresourceUrlBuilderIfReady();

const HeaderDirectFromSellerSignals*
direct_from_seller_signals_header_ad_slot() const {
return direct_from_seller_signals_header_ad_slot_.get();
}

void OnDecompressedServerResponse(
std::unique_ptr<data_decoder::DataDecoder> decoder,
AdAuctionRequestContext* request_context,
Expand Down Expand Up @@ -1110,7 +1136,8 @@ class CONTENT_EXPORT InterestGroupAuction
// This includes bidders that are still attempting to generate bids ---
// both BuyerHelpers and component auctions. BuyerHelpers may generate
// multiple bids (or no bids). It also includes waiting for promises in
// configuration to resolve.
// configuration to resolve, and waiting for
// directFromSellerSignalsHeaderAdSlot to parse.
// TODO(morlovich): And will wait for additional_bids.
//
// When this reaches 0, the SellerWorklet's SendPendingSignalsRequests()
Expand Down Expand Up @@ -1161,7 +1188,7 @@ class CONTENT_EXPORT InterestGroupAuction
std::unique_ptr<SubresourceUrlBuilder> subresource_url_builder_;

// Stores the loaded HeaderDirectFromSellerSignals, if there were any. Should
// never be null.
// never be null until moved to the reporter.
//
// After `direct_from_seller_signals_header_ad_slot_` has been
// set to true, the default constructed value gets replaced with the found
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -456,8 +456,12 @@ void InterestGroupAuctionReporter::OnSellerWorkletReceived(
seller_info->auction_config->non_shared_params,
InterestGroupAuction::GetDirectFromSellerSellerSignals(
seller_info->subresource_url_builder.get()),
InterestGroupAuction::GetDirectFromSellerSellerSignalsHeaderAdSlot(
*seller_info->direct_from_seller_signals_header_ad_slot),
InterestGroupAuction::GetDirectFromSellerAuctionSignals(
seller_info->subresource_url_builder.get()),
InterestGroupAuction::GetDirectFromSellerAuctionSignalsHeaderAdSlot(
*seller_info->direct_from_seller_signals_header_ad_slot),
std::move(other_seller),
winning_bid_info_.storage_interest_group->interest_group.owner,
/*browser_signal_buyer_and_seller_reporting_id=*/
Expand Down Expand Up @@ -718,8 +722,13 @@ void InterestGroupAuctionReporter::OnBidderWorkletReceived(
InterestGroupAuction::GetDirectFromSellerPerBuyerSignals(
seller_info.subresource_url_builder.get(),
winning_bid_info_.storage_interest_group->interest_group.owner),
InterestGroupAuction::GetDirectFromSellerPerBuyerSignalsHeaderAdSlot(
*seller_info.direct_from_seller_signals_header_ad_slot,
winning_bid_info_.storage_interest_group->interest_group.owner),
InterestGroupAuction::GetDirectFromSellerAuctionSignals(
seller_info.subresource_url_builder.get()),
InterestGroupAuction::GetDirectFromSellerAuctionSignalsHeaderAdSlot(
*seller_info.direct_from_seller_signals_header_ad_slot),
signals_for_winner, kanon_mode_, bid_is_kanon_,
winning_bid_info_.render_url,
RoundStochasticallyToKBits(winning_bid_info_.bid,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ namespace content {
class AuctionWorkletManager;
struct BiddingAndAuctionResponse;
class BrowserContext;
class HeaderDirectFromSellerSignals;
class InterestGroupManagerImpl;
class PrivateAggregationManager;

Expand Down Expand Up @@ -118,6 +119,8 @@ class CONTENT_EXPORT InterestGroupAuctionReporter {
auction_config;

std::unique_ptr<SubresourceUrlBuilder> subresource_url_builder;
std::unique_ptr<HeaderDirectFromSellerSignals>
direct_from_seller_signals_header_ad_slot;

// Bid fed as input to the seller. If this is the top level seller and the
// bid came from a component auction, it's the (optionally) modified bid
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include "base/test/scoped_feature_list.h"
#include "content/browser/fenced_frame/fenced_frame_reporter.h"
#include "content/browser/interest_group/auction_worklet_manager.h"
#include "content/browser/interest_group/header_direct_from_seller_signals.h"
#include "content/browser/interest_group/interest_group_k_anonymity_manager.h"
#include "content/browser/interest_group/interest_group_manager_impl.h"
#include "content/browser/interest_group/mock_auction_process_manager.h"
Expand Down Expand Up @@ -65,6 +66,9 @@ InterestGroupAuctionReporter::SellerWinningBidInfo CreateSellerWinningBidInfo(
// return nothing, though.
out.subresource_url_builder = std::make_unique<SubresourceUrlBuilder>(
/*direct_from_seller_signals=*/absl::nullopt);
// Also must not be null.
out.direct_from_seller_signals_header_ad_slot =
std::make_unique<HeaderDirectFromSellerSignals>();

// The specific values these are assigned to don't matter for these tests, but
// they don't have default initializers, so have to set them to placate memory
Expand Down

0 comments on commit 18b6294

Please sign in to comment.