[M115] Add prevWinsMs browser signal

The time field in prevWins was reported in seconds, but the web standard convention is to use milliseconds instead of seconds. To maintain compatibility with existing API users, both prevWins and the new prevWinsMs will be provided -- support for the deprecated prevWins can be removed once API users have switched. **NOTE**: Test expectations in bidder_worklet_unittest.cc have been changed from the version of this CL in main since crrev.com/c/4566565 and crrev.com/c/4580612 were not cherry-picked for M115. (cherry picked from commit 88997c3) Bug: 1451033 Change-Id: I67248f92030c91d6e1f76e283ba43d57e2f2e4a0 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4590273 Reviewed-by: Russ Hamilton <behamilton@google.com> Commit-Queue: Caleb Raitto <caraitto@chromium.org> Cr-Original-Commit-Position: refs/heads/main@{#1153994} Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4605148 Auto-Submit: Caleb Raitto <caraitto@chromium.org> Commit-Queue: Russ Hamilton <behamilton@google.com> Cr-Commit-Position: refs/branch-heads/5790@{#544} Cr-Branched-From: 1d71a33-refs/heads/main@{#1148114}
chromium · Jun 9, 2023 · 9ca5e98 · 9ca5e98
1 parent 5ab580a
commit 9ca5e98
Show file tree

Hide file tree

Showing 7 changed files with 114 additions and 12 deletions.
diff --git a/content/browser/interest_group/auction_runner_unittest.cc b/content/browser/interest_group/auction_runner_unittest.cc
@@ -330,6 +330,16 @@ std::string MakeBidScript(const url::Origin& seller,
         if (browserSignals.prevWins[i][1].winner !== -i)
           throw new Error("prevWin MD not what passed in");
       }
+      if (browserSignals.prevWinsMs.length !== 3)
+        throw new Error("prevWinsMs");
+      for (let i = 0; i < browserSignals.prevWinsMs.length; ++i) {
+        if (!(browserSignals.prevWinsMs[i] instanceof Array))
+          throw new Error("prevWinsMs entry not an array");
+        if (typeof browserSignals.prevWinsMs[i][0] != "number")
+          throw new Error("Not a Number in prevWin?");
+        if (browserSignals.prevWinsMs[i][1].winner !== -i)
+          throw new Error("prevWin MD not what passed in");
+      }
       if (debugLossReportUrl) {
         if (reportPostAuctionSignals)
           debugLossReportUrl += postAuctionSignalsPlaceholder;

diff --git a/content/services/auction_worklet/bidder_lazy_filler.cc b/content/services/auction_worklet/bidder_lazy_filler.cc
@@ -21,6 +21,7 @@ namespace {
 // wins is only used for a single bid in a single auction, and its order is
 // unspecified, anyways.
 v8::MaybeLocal<v8::Value> CreatePrevWinsArray(
+    PrevWinsType prev_wins_type,
     AuctionV8Helper* v8_helper,
     v8::Local<v8::Context> context,
     base::Time auction_start_time,
@@ -33,12 +34,24 @@ v8::MaybeLocal<v8::Value> CreatePrevWinsArray(
   std::vector<v8::Local<v8::Value>> prev_wins_v8;
   v8::Isolate* isolate = v8_helper->isolate();
   for (const auto& prev_win : prev_wins) {
-    int64_t time_delta = (auction_start_time - prev_win->time).InSeconds();
+    base::TimeDelta time_delta = auction_start_time - prev_win->time;
+    int time_delta_int;
+    switch (prev_wins_type) {
+      case PrevWinsType::kSeconds:
+        time_delta_int = time_delta.InSeconds();
+        break;
+      case PrevWinsType::kMilliseconds:
+        // Truncate to the nearest second, rather than providing the result in
+        // millisecond granularity.
+        time_delta_int = time_delta.InSeconds() * 1000;
+        break;
+    }
     // Don't give negative times if clock has changed since last auction win.
-    if (time_delta < 0)
-      time_delta = 0;
+    if (time_delta_int < 0) {
+      time_delta_int = 0;
+    }
     v8::Local<v8::Value> win_values[2];
-    win_values[0] = v8::Number::New(isolate, time_delta);
+    win_values[0] = v8::Number::New(isolate, time_delta_int);
     if (!v8_helper->CreateValueFromJson(context, prev_win->ad_json)
              .ToLocal(&win_values[1])) {
       return v8::MaybeLocal<v8::Value>();
@@ -169,8 +182,12 @@ void BiddingBrowserSignalsLazyFiller::ReInitialize(
 
 bool BiddingBrowserSignalsLazyFiller::FillInObject(
     v8::Local<v8::Object> object) {
-  if (!DefineLazyAttribute(object, "prevWins", &HandlePrevWins))
+  if (!DefineLazyAttribute(object, "prevWins", &HandlePrevWins)) {
+    return false;
+  }
+  if (!DefineLazyAttribute(object, "prevWinsMs", &HandlePrevWinsMs)) {
     return false;
+  }
   return true;
 }
 
@@ -182,15 +199,32 @@ void BiddingBrowserSignalsLazyFiller::Reset() {
 void BiddingBrowserSignalsLazyFiller::HandlePrevWins(
     v8::Local<v8::Name> name,
     const v8::PropertyCallbackInfo<v8::Value>& info) {
+  HandlePrevWinsInternal(name, info, PrevWinsType::kSeconds);
+}
+
+// static
+void BiddingBrowserSignalsLazyFiller::HandlePrevWinsMs(
+    v8::Local<v8::Name> name,
+    const v8::PropertyCallbackInfo<v8::Value>& info) {
+  HandlePrevWinsInternal(name, info, PrevWinsType::kMilliseconds);
+}
+
+// TODO(crbug.com/1451034): Clean up support for deprecated seconds-based
+// version after API users migrate, and remove this indirection function.
+// static
+void BiddingBrowserSignalsLazyFiller::HandlePrevWinsInternal(
+    v8::Local<v8::Name> name,
+    const v8::PropertyCallbackInfo<v8::Value>& info,
+    PrevWinsType prev_wins_type) {
   BiddingBrowserSignalsLazyFiller* self =
       GetSelf<BiddingBrowserSignalsLazyFiller>(info);
   AuctionV8Helper* v8_helper = self->v8_helper();
   v8::Isolate* isolate = v8_helper->isolate();
   v8::Local<v8::Value> value;
   if (self->bidder_browser_signals_ &&
-      CreatePrevWinsArray(v8_helper, isolate->GetCurrentContext(),
-                          self->auction_start_time_,
-                          self->bidder_browser_signals_->prev_wins)
+      CreatePrevWinsArray(
+          prev_wins_type, v8_helper, isolate->GetCurrentContext(),
+          self->auction_start_time_, self->bidder_browser_signals_->prev_wins)
           .ToLocal(&value)) {
     SetResult(info, value);
   } else {

diff --git a/content/services/auction_worklet/bidder_lazy_filler.h b/content/services/auction_worklet/bidder_lazy_filler.h
@@ -40,6 +40,10 @@ class CONTENT_EXPORT InterestGroupLazyFiller : public LazyFiller {
       bidder_worklet_non_shared_params_ = nullptr;
 };
 
+// TODO(crbug.com/1451034): Clean up support for deprecated seconds-based
+// version after API users migrate.
+enum class PrevWinsType { kSeconds, kMilliseconds };
+
 class CONTENT_EXPORT BiddingBrowserSignalsLazyFiller : public LazyFiller {
  public:
   explicit BiddingBrowserSignalsLazyFiller(AuctionV8Helper* v8_helper);
@@ -53,6 +57,12 @@ class CONTENT_EXPORT BiddingBrowserSignalsLazyFiller : public LazyFiller {
  private:
   static void HandlePrevWins(v8::Local<v8::Name> name,
                              const v8::PropertyCallbackInfo<v8::Value>& info);
+  static void HandlePrevWinsMs(v8::Local<v8::Name> name,
+                               const v8::PropertyCallbackInfo<v8::Value>& info);
+  static void HandlePrevWinsInternal(
+      v8::Local<v8::Name> name,
+      const v8::PropertyCallbackInfo<v8::Value>& info,
+      PrevWinsType prev_wins_type);
 
   raw_ptr<mojom::BiddingBrowserSignals> bidder_browser_signals_ = nullptr;
   base::Time auction_start_time_;

diff --git a/content/services/auction_worklet/bidder_worklet_unittest.cc b/content/services/auction_worklet/bidder_worklet_unittest.cc
@@ -3751,6 +3751,48 @@ TEST_F(BidderWorkletTest, GenerateBidPrevWins) {
           "browserSignals.prevWins",
           R"([[200,"ad1"],[100,["ad2"]],[0,"future_ad"]])",
       },
+      // Same as above, but for prevWinsMs.
+      {
+          {},
+          "browserSignals.prevWinsMs",
+          "[]",
+      },
+      {
+          CreateWinList(win1),
+          "browserSignals.prevWinsMs",
+          R"([[200000,"ad1"]])",
+      },
+      // Make sure it's passed on as an object and not a string.
+      {
+          CreateWinList(win1),
+          "browserSignals.prevWinsMs[0]",
+          R"([200000,"ad1"])",
+      },
+      // Test rounding.
+      {
+          CreateWinList(win2),
+          "browserSignals.prevWinsMs",
+          R"([[100000,["ad2"]]])",
+      },
+      // Multiple previous wins.
+      {
+          CreateWinList(win1, win2),
+          "browserSignals.prevWinsMs",
+          R"([[200000,"ad1"],[100000,["ad2"]]])",
+      },
+      // Times are trimmed at 0.
+      {
+          CreateWinList(future_win),
+          "browserSignals.prevWinsMs",
+          R"([[0,"future_ad"]])",
+      },
+      // Out of order wins should be sorted.
+      {
+          CreateWinList(win2, future_win, win1),
+          "browserSignals.prevWinsMs",
+          R"([[200000,"ad1"],[100000,["ad2"]],[0,"future_ad"]])",
+      },
+
   };
 
   for (auto& test_case : test_cases) {

diff --git a/content/services/auction_worklet/context_recycler_unittest.cc b/content/services/auction_worklet/context_recycler_unittest.cc
@@ -860,7 +860,8 @@ TEST_F(ContextRecyclerTest, BidderLazyFiller) {
         "{\"userBiddingSignals\":{\"k\":2},"
         "\"trustedBiddingSignalsKeys\":[\"c\",\"d\"],"
         "\"priorityVector\":{\"e\":12},"
-        "\"prevWins\":[[240,[\"d\"]],[180,[\"c\"]]]}",
+        "\"prevWins\":[[240,[\"d\"]],[180,[\"c\"]]],"
+        "\"prevWinsMs\":[[240000,[\"d\"]],[180000,[\"c\"]]]}",
         str_result);
   }
 }
@@ -953,7 +954,8 @@ TEST_F(ContextRecyclerTest, BidderLazyFiller2) {
         "{\"userBiddingSignals\":null,"
         "\"trustedBiddingSignalsKeys\":null,"
         "\"priorityVector\":null,"
-        "\"prevWins\":[]}",
+        "\"prevWins\":[],"
+        "\"prevWinsMs\":[]}",
         str_result);
   }
 }

diff --git a/content/test/data/interest_group/bidding_argument_validator.js b/content/test/data/interest_group/bidding_argument_validator.js
@@ -198,7 +198,7 @@ function validateBrowserSignals(browserSignals, isGenerateBid) {
     throw 'Wrong topLevelSeller ' + browserSignals.topLevelSeller;
 
   if (isGenerateBid) {
-    if (Object.keys(browserSignals).length !== 5) {
+    if (Object.keys(browserSignals).length !== 6) {
       throw 'Wrong number of browser signals fields ' +
           JSON.stringify(browserSignals);
     }
@@ -208,6 +208,8 @@ function validateBrowserSignals(browserSignals, isGenerateBid) {
       throw 'Wrong bidCount ' + bidCount;
     if (browserSignals.prevWins.length !== 0)
       throw 'Wrong prevWins ' + JSON.stringify(browserSignals.prevWins);
+    if (browserSignals.prevWinsMs.length !== 0)
+      throw 'Wrong prevWinsMs ' + JSON.stringify(browserSignals.prevWinsMs);
   } else {
     if (Object.keys(browserSignals).length !== 15) {
       throw 'Wrong number of browser signals fields ' +

diff --git a/content/test/data/interest_group/component_auction_bidding_argument_validator.js b/content/test/data/interest_group/component_auction_bidding_argument_validator.js
@@ -180,7 +180,7 @@ function validateBrowserSignals(browserSignals, isGenerateBid) {
     throw 'Wrong topLevelSeller ' + browserSignals.topLevelSeller;
 
   if (isGenerateBid) {
-    if (Object.keys(browserSignals).length !== 6) {
+    if (Object.keys(browserSignals).length !== 7) {
       throw 'Wrong number of browser signals fields ' +
           JSON.stringify(browserSignals);
     }
@@ -190,6 +190,8 @@ function validateBrowserSignals(browserSignals, isGenerateBid) {
       throw 'Wrong bidCount ' + browserSignals.bidCount;
     if (browserSignals.prevWins.length !== 0)
       throw 'Wrong prevWins ' + JSON.stringify(browserSignals.prevWins);
+    if (browserSignals.prevWinsMs.length !== 0)
+      throw 'Wrong prevWinsMs ' + JSON.stringify(browserSignals.prevWinsMs);
   } else {
     if (Object.keys(browserSignals).length !== 16) {
       throw 'Wrong number of browser signals fields ' +