Skip to content

Commit

Permalink
[Canary branch 5965] Pass directFromSellerSignalsHeaderAdSlot results…
Browse files Browse the repository at this point in the history
… to worklets

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.

(cherry picked from commit 18b6294)

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-Original-Commit-Position: refs/heads/main@{#1186925}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4805284
Owners-Override: Krishna Govind <govind@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Reviewed-by: Krishna Govind <govind@chromium.org>
Cr-Commit-Position: refs/branch-heads/5965@{#4}
Cr-Branched-From: 033bbac-refs/heads/main@{#1186892}
  • Loading branch information
caraitto authored and Krishna Govind committed Aug 23, 2023
1 parent c1ee53f commit 62a8057
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 62a8057

Please sign in to comment.