[FLEDGE] Component auctions CL10: Add component reportResult() fields.

In FLEDGE, the reportResult() methods of nested component sellers need
two additional parameters:  The modified bid, if the component seller's
scoreAd() method modified the bid, and the output of the top-level
sellers reportResult() method.

These fields are called modifiedBid and topLevelSellerSignals
respectively, and appear in reportResult's browserSignals input.

This CL adds support for both these fields.

Bug: 1288865
Change-Id: I11afd961f1f5b684fb1a84037e4be8590bf40ac2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3499152
Reviewed-by: Maks Orlovich <[email protected]>
Reviewed-by: Daniel Cheng <[email protected]>
Commit-Queue: Matt Menke <[email protected]>
Cr-Commit-Position: refs/heads/main@{#981985}

Author: Matt Menke

content/browser/interest_group/auction_runner.cc

@@ -283,6 +283,7 @@ void AuctionRunner::Auction::StartBiddingAndScoringPhase(
 }
 
 void AuctionRunner::Auction::StartReportingPhase(
+    absl::optional<std::string> top_seller_signals,
     AuctionPhaseCompletionCallback reporting_phase_callback) {
   DCHECK(reporting_phase_callback);
   DCHECK(!load_interest_groups_phase_callback_);
@@ -297,18 +298,21 @@ void AuctionRunner::Auction::StartReportingPhase(
   // reload the seller worklet in the case of a component auction.
   if (!seller_worklet_handle_) {
     DCHECK(parent_);
+    DCHECK(top_seller_signals);
     if (!auction_worklet_manager_->RequestSellerWorklet(
             config_->decision_logic_url, config_->trusted_scoring_signals_url,
-            base::BindOnce(&Auction::ReportSellerResult,
-                           base::Unretained(this)),
+            base::BindOnce(&Auction::ReportSellerResult, base::Unretained(this),
+                           top_seller_signals),
             base::BindOnce(&Auction::OnWinningComponentSellerWorkletFatalError,
                            base::Unretained(this)),
             seller_worklet_handle_)) {
       return;
     }
+  } else {
+    DCHECK(!parent_);
+    DCHECK(!top_seller_signals);
   }
-
-  ReportSellerResult();
+  ReportSellerResult(std::move(top_seller_signals));
 }
 
 void AuctionRunner::Auction::ClosePipes() {
@@ -984,14 +988,30 @@ void AuctionRunner::Auction::OnBiddingAndScoringComplete(
   std::move(bidding_and_scoring_phase_callback_).Run(success);
 }
 
-void AuctionRunner::Auction::ReportSellerResult() {
+void AuctionRunner::Auction::ReportSellerResult(
+    absl::optional<std::string> top_seller_signals) {
   DCHECK(seller_worklet_handle_);
   DCHECK(reporting_phase_callback_);
 
+  auction_worklet::mojom::ComponentAuctionReportResultParamsPtr
+      browser_signals_component_auction_report_result_params;
+  if (parent_) {
+    DCHECK(top_seller_signals);
+    DCHECK(top_bid_->component_auction_modified_bid_params);
+    browser_signals_component_auction_report_result_params =
+        auction_worklet::mojom::ComponentAuctionReportResultParams::New(
+            /*top_level_seller_signals=*/std::move(top_seller_signals).value(),
+            /*modified_bid=*/
+            top_bid_->component_auction_modified_bid_params->bid,
+            /*has_modified_bid=*/
+            top_bid_->component_auction_modified_bid_params->has_bid);
+  }
+
   seller_worklet_handle_->GetSellerWorklet()->ReportResult(
       config_->auction_ad_config_non_shared_params.Clone(),
       GetOtherSellerParam(*top_bid_->bid), top_bid_->bid->interest_group->owner,
       top_bid_->bid->render_url, top_bid_->bid->bid, top_bid_->score,
+      std::move(browser_signals_component_auction_report_result_params),
       top_bid_->scoring_signals_data_version.value_or(0),
       top_bid_->scoring_signals_data_version.has_value(),
       base::BindOnce(&Auction::OnReportSellerResultComplete,
@@ -1023,21 +1043,28 @@ void AuctionRunner::Auction::OnReportSellerResultComplete(
 
   errors_.insert(errors_.end(), errors.begin(), errors.end());
 
+  // Treat a null `signals_for_winner` value as a null JS response.
+  //
+  // TODO(mmenke): Consider making `signals_for_winner` itself non-optional, and
+  // clean this up.
+  std::string fixed_up_signals_for_winner = signals_for_winner.value_or("null");
+
   // If a the winning bid is from a nested component auction, need to call into
   // that Auction's report logic (which will invoke both that seller's
   // ReportResult() method, and the bidder's ReportWin()).
   if (top_bid_->bid->auction != this) {
     top_bid_->bid->auction->StartReportingPhase(
+        std::move(fixed_up_signals_for_winner),
         base::BindOnce(&Auction::OnComponentAuctionReportingPhaseComplete,
                        base::Unretained(this)));
     return;
   }
 
-  LoadBidderWorkletToReportBidWin(signals_for_winner);
+  LoadBidderWorkletToReportBidWin(std::move(fixed_up_signals_for_winner));
 }
 
 void AuctionRunner::Auction::LoadBidderWorkletToReportBidWin(
-    const absl::optional<std::string>& signals_for_winner) {
+    const std::string& signals_for_winner) {
   // Worklet handle should have been destroyed once the bid was generated.
   DCHECK(!top_bid_->bid->bid_state->worklet_handle);
 
@@ -1052,25 +1079,13 @@ void AuctionRunner::Auction::LoadBidderWorkletToReportBidWin(
 }
 
 void AuctionRunner::Auction::ReportBidWin(
-    const absl::optional<std::string>& signals_for_winner) {
+    const std::string& signals_for_winner) {
   DCHECK(top_bid_);
 
-  std::string signals_for_winner_arg;
-  if (signals_for_winner) {
-    signals_for_winner_arg = *signals_for_winner;
-  } else {
-    // `signals_for_winner_arg` is passed as JSON, so need to pass "null" when
-    // it's not provided. Pass in "null" instead of making the API take an
-    // optional to limit the information provided to the untrusted BidderWorklet
-    // process that's not part of the FLEDGE API. Unlikely to matter, but best
-    // to be safe.
-    signals_for_winner_arg = "null";
-  }
-
   top_bid_->bid->bid_state->worklet_handle->GetBidderWorklet()->ReportWin(
       top_bid_->bid->interest_group->name,
       config_->auction_ad_config_non_shared_params->auction_signals,
-      PerBuyerSignals(top_bid_->bid->bid_state), signals_for_winner_arg,
+      PerBuyerSignals(top_bid_->bid->bid_state), signals_for_winner,
       top_bid_->bid->render_url, top_bid_->bid->bid, config_->seller,
       parent_ ? parent_->config_->seller : absl::optional<url::Origin>(),
       top_bid_->bid->bidding_signals_data_version.value_or(0),
@@ -1358,8 +1373,10 @@ void AuctionRunner::OnBidsGeneratedAndScored(bool success) {
     return;
   }
 
-  auction_.StartReportingPhase(base::BindOnce(
-      &AuctionRunner::OnReportingPhaseComplete, base::Unretained(this)));
+  auction_.StartReportingPhase(
+      /*top_seller_signals=*/absl::nullopt,
+      base::BindOnce(&AuctionRunner::OnReportingPhaseComplete,
+                     base::Unretained(this)));
 }
 
 void AuctionRunner::OnReportingPhaseComplete(bool success) {

content/browser/interest_group/auction_runner.h

@@ -380,7 +380,11 @@ class CONTENT_EXPORT AuctionRunner {
     // when all reporting URLs (if any) have been retrieved from the applicable
     // worklets. `success` is true if the final status of the auction is
     // `kSuccess`.
+    //
+    // If this is a component auction, `top_seller_signals` must populated and
+    // be the output from the top-level seller's reportResult() method.
     void StartReportingPhase(
+        absl::optional<std::string> top_seller_signals,
         AuctionPhaseCompletionCallback reporting_phase_callback);
 
     // Close all Mojo pipes and release all weak pointers. Called when an
@@ -572,14 +576,13 @@ class CONTENT_EXPORT AuctionRunner {
     // Sequence of asynchronous methods to call into the seller and then bidder
     // worklet to report a a win. Will ultimately invoke
     // `reporting_phase_callback_`, which will delete the auction.
-    void ReportSellerResult();
+    void ReportSellerResult(absl::optional<std::string> top_seller_signals);
     void OnReportSellerResultComplete(
         const absl::optional<std::string>& signals_for_winner,
         const absl::optional<GURL>& seller_report_url,
         const std::vector<std::string>& error_msgs);
-    void LoadBidderWorkletToReportBidWin(
-        const absl::optional<std::string>& signals_for_winner);
-    void ReportBidWin(const absl::optional<std::string>& signals_for_winner);
+    void LoadBidderWorkletToReportBidWin(const std::string& signals_for_winner);
+    void ReportBidWin(const std::string& signals_for_winner);
     void OnReportBidWinComplete(const absl::optional<GURL>& bidder_report_url,
                                 const std::vector<std::string>& error_msgs);
 

content/browser/interest_group/auction_runner_unittest.cc

@@ -211,6 +211,24 @@ std::string MakeBidScript(const url::Origin& seller,
         throw new Error("wrong renderUrl");
       if (sellerSignals.bid !== bid)
         throw new Error("wrong bid");
+      // `sellerSignals` is the `browserSignals` for the seller that's
+      // associated with the bid. If it's the top-level seller, the seller's
+      // `browserSignals` should have no `componentSeller`, since the bid
+      // was made directly to the top-level seller. If it's the component
+      // seller, the seller's `browserSignals` should have a `topLevelSeller`
+      // instead of a `componentSeller`, so `componentSeller` should never
+      // be present in `sellerSignals` here.
+      if ("componentSeller" in sellerSignals)
+        throw new Error("wrong componentSeller in sellerSignals");
+      if (browserSignals.seller === topLevelSeller) {
+        if ("topLevelSeller" in sellerSignals)
+          throw new Error("wrong topLevelSeller in sellerSignals");
+      } else {
+        // If the seller is a component seller, then then the seller's
+        // `browserSignals` should have the top-level seller.
+        if (sellerSignals.topLevelSeller !== topLevelSeller)
+          throw new Error("wrong topLevelSeller in browserSignals");
+      }
 
       if (browserSignals.topWindowHostname !== 'publisher1.com')
         throw new Error("wrong browserSignals.topWindowHostname");
@@ -393,25 +411,39 @@ std::string MakeDecisionScript(
       if (decisionLogicUrl.startsWith(topLevelSeller)) {
         // Top-level sellers should receive component sellers, but only for
         // bids received from component auctions.
-          if ("topLevelSeller" in browserSignals)
-            throw new Error("Expected no topLevelSeller in browserSignals.");
+        if ("topLevelSeller" in browserSignals)
+          throw new Error("Expected no topLevelSeller in browserSignals.");
         if (bidFromComponentAuctionWins) {
           if (!browserSignals.componentSeller.includes("component"))
             throw new Error("Incorrect componentSeller in browserSignals.");
         } else {
           if ("componentSeller" in browserSignals)
             throw new Error("Expected no componentSeller in browserSignals.");
         }
+
+        if ("topLevelSellerSignals" in browserSignals)
+          throw new Error("Unexpected browserSignals.topLevelSellerSignals");
       } else {
         // Component sellers should receive only the top-level seller.
         if (browserSignals.topLevelSeller !== topLevelSeller)
           throw new Error("Incorrect topLevelSeller in browserSignals.");
         if ("componentSeller" in browserSignals)
           throw new Error("Expected no componentSeller in browserSignals.");
+
+        // Component sellers should get the return value of the top-level
+        // seller's `reportResult()` call, which is, in this case, the
+        // `browserSignals` of the top-level seller.
+        if (browserSignals.topLevelSellerSignals.componentSeller !=
+                auctionConfig.seller) {
+          throw new Error("Unexpected browserSignals.topLevelSellerSignals");
+        }
       }
 
       if (browserSignals.desirability != computeScore(browserSignals.bid))
         throw new Error("wrong bid or desirability in browserSignals");
+      // The default scoreAd() script does not modify bids.
+      if ("modifiedBid" in browserSignals)
+        throw new Error("modifiedBid unexpectedly in browserSignals");
       if (browserSignals.dataVersion !== undefined)
         throw new Error(`wrong dataVersion (${browserSignals.dataVersion})`);
       if (sendReportUrl)
@@ -766,17 +798,20 @@ class MockSellerWorklet : public auction_worklet::mojom::SellerWorklet {
     send_pending_signals_requests_called_ = true;
   }
 
-  void ReportResult(blink::mojom::AuctionAdConfigNonSharedParamsPtr
-                        auction_ad_config_non_shared_params,
-                    auction_worklet::mojom::ComponentAuctionOtherSellerPtr
-                        browser_signals_other_seller,
-                    const url::Origin& browser_signal_interest_group_owner,
-                    const GURL& browser_signal_render_url,
-                    double browser_signal_bid,
-                    double browser_signal_desirability,
-                    uint32_t browser_signal_data_version,
-                    bool browser_signal_has_data_version,
-                    ReportResultCallback report_result_callback) override {
+  void ReportResult(
+      blink::mojom::AuctionAdConfigNonSharedParamsPtr
+          auction_ad_config_non_shared_params,
+      auction_worklet::mojom::ComponentAuctionOtherSellerPtr
+          browser_signals_other_seller,
+      const url::Origin& browser_signal_interest_group_owner,
+      const GURL& browser_signal_render_url,
+      double browser_signal_bid,
+      double browser_signal_desirability,
+      auction_worklet::mojom::ComponentAuctionReportResultParamsPtr
+          browser_signals_component_auction_report_result_params,
+      uint32_t browser_signal_data_version,
+      bool browser_signal_has_data_version,
+      ReportResultCallback report_result_callback) override {
     report_result_callback_ = std::move(report_result_callback);
     if (report_result_run_loop_)
       report_result_run_loop_->Quit();
@@ -2495,6 +2530,80 @@ TEST_F(AuctionRunnerTest, ComponentAuctionOneComponentTwoBidders) {
                   /*expected_interest_groups=*/2, /*expected_owners=*/2);
 }
 
+// Test the case a top-level seller returns no signals in its reportResult
+// method. The default scripts return signals, so only need to individually test
+// the no-value case.
+TEST_F(AuctionRunnerTest, ComponentAuctionNoTopLevelReportResultSignals) {
+  // Basic bid script.
+  const char kBidScript[] = R"(
+      function generateBid(interestGroup, auctionSignals, perBuyerSignals,
+                           trustedBiddingSignals, browserSignals) {
+        return {ad: [], bid: 2, render: interestGroup.ads[0].renderUrl,
+                allowComponentAuction: true};
+      }
+
+    function reportWin(auctionSignals, perBuyerSignals, sellerSignals,
+                       browserSignals) {
+      sendReportTo("https://buyer-reporting.example.com/" + browserSignals.bid);
+    }
+  )";
+
+  // Component seller script that makes a report to a URL based on whether the
+  // top-level seller signals are null.
+  const std::string kComponentSellerScript = R"(
+    function scoreAd(adMetadata, bid, auctionConfig, browserSignals) {
+      return {desirability: 10, allowComponentAuction: true};
+    }
+
+    function reportResult(auctionConfig, browserSignals) {
+      sendReportTo(auctionConfig.seller + "/" +
+                   (browserSignals.topLevelSellerSignals === null));
+    }
+  )";
+
+  // Top-level seller script with a reportResult method that has no return
+  // value.
+  const std::string kTopLevelSellerScript = R"(
+    function scoreAd(adMetadata, bid, auctionConfig, browserSignals) {
+      return {desirability: 10, allowComponentAuction: true};
+    }
+
+    function reportResult(auctionConfig, browserSignals) {
+      sendReportTo(auctionConfig.seller + "/" + browserSignals.bid);
+      // Note that there's no return value here.
+    }
+  )";
+
+  auction_worklet::AddJavascriptResponse(&url_loader_factory_, kBidder1Url,
+                                         kBidScript);
+  auction_worklet::AddJavascriptResponse(&url_loader_factory_, kSellerUrl,
+                                         kTopLevelSellerScript);
+  auction_worklet::AddJavascriptResponse(
+      &url_loader_factory_, kComponentSeller1Url, kComponentSellerScript);
+
+  interest_group_buyers_.reset();
+  component_auctions_.emplace_back(
+      CreateAuctionConfig(kComponentSeller1Url, {{kBidder1}}));
+
+  std::vector<StorageInterestGroup> bidders;
+  bidders.emplace_back(MakeInterestGroup(
+      kBidder1, kBidder1Name, kBidder1Url,
+      /*trusted_bidding_signals_url=*/absl::nullopt,
+      /*trusted_bidding_signals_keys=*/{}, GURL("https://ad1.com")));
+
+  StartAuction(kSellerUrl, std::move(bidders));
+  auction_run_loop_->Run();
+  EXPECT_EQ(GURL("https://ad1.com"), result_.ad_url);
+  EXPECT_THAT(result_.errors, testing::ElementsAre());
+  EXPECT_THAT(result_.report_urls,
+              testing::UnorderedElementsAre(
+                  GURL("https://buyer-reporting.example.com/2"),
+                  GURL("https://component.seller1.test/true"),
+                  GURL("https://adstuff.publisher1.com/2")));
+  CheckHistograms(AuctionRunner::AuctionResult::kSuccess,
+                  /*expected_interest_groups=*/1, /*expected_owners=*/1);
+}
+
 TEST_F(AuctionRunnerTest, ComponentAuctionModifiesBid) {
   // Basic bid script.
   const char kBidScript[] = R"(
@@ -2517,7 +2626,8 @@ TEST_F(AuctionRunnerTest, ComponentAuctionModifiesBid) {
     }
 
     function reportResult(auctionConfig, browserSignals) {
-      sendReportTo(auctionConfig.seller + "/" + browserSignals.bid);
+      sendReportTo(auctionConfig.seller + "/" + browserSignals.bid +
+                   "_" + browserSignals.modifiedBid);
     }
   )";
 
@@ -2558,13 +2668,10 @@ TEST_F(AuctionRunnerTest, ComponentAuctionModifiesBid) {
   EXPECT_THAT(result_.errors, testing::ElementsAre());
   // The reporting URLs contain the bids - the top-level seller report should
   // see the modified bid, the other worklets see the original bid.
-  //
-  // TODO(https://crbug.com/1288865): The component seller worklet should also
-  // get the modified bid, in another field.
   EXPECT_THAT(result_.report_urls,
               testing::UnorderedElementsAre(
                   GURL("https://buyer-reporting.example.com/2"),
-                  GURL("https://component.seller1.test/2"),
+                  GURL("https://component.seller1.test/2_3"),
                   GURL("https://adstuff.publisher1.com/3")));
   CheckHistograms(AuctionRunner::AuctionResult::kSuccess,
                   /*expected_interest_groups=*/1, /*expected_owners=*/1);