-
Notifications
You must be signed in to change notification settings - Fork 6.8k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
FLEDGE: Decouple nested blink::InterestGroup::Size.
This CL is a first step to add size fields to the return value of generateBid(). In https://crrev.com/c/4167800, blink::InterestGroup::Size is introduced for adding sizes to joinAdInterestGroup(). It is declared as a nested struct inside blink::InterestGroup. When working on adding size fields to the return value of generateBid(), we should reuse this Size struct, instead of creating a new one, for BidderWorkletBid. The way it being declared as a nested struct have a few drawbacks: 1. This struct seems to serve InterestGroup only at first glance, but it will be used to represent general size info for various FLEDGE APIs soon: runAdAuction(), scoreAd(), reportResult() and reportWin(). 2. Clients that only need the size struct's declaration must include interest_group.h entirely. 3. Cannot be forward declared. This CL decouples the nested blink::InterestGroup::Size, to a standalone struct blink::AdSize. Some minor notes: 1. Update Size's operator== to compare units as well. 2. Although the struct traits for Size was defined in the related CL, the type was not mapped because no mapping was specified the BUILD.gn. This CL added the type mapping. This will later be used in the follow-up CL on generateBid(). 3. Added unit tests for Size. See Turtledove issue: WICG/turtledove#312 See Turtledove PR: WICG/turtledove#417 Bug: http://b/239866637 Related CL on adding sizes to joinAdInterestGroup(): https://crrev.com/c/4167800 Change-Id: I4a4dd47607102599bbeb33d05aa1ca5e6928ec5b Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4296777 Commit-Queue: Xiaochen Zhou <xiaochenzh@chromium.org> Reviewed-by: Dominic Farolino <dom@chromium.org> Reviewed-by: Russ Hamilton <behamilton@google.com> Reviewed-by: Garrett Tanzer <gtanzer@chromium.org> Reviewed-by: Daniel Cheng <dcheng@chromium.org> Cr-Commit-Position: refs/heads/main@{#1112228}
- Loading branch information
1 parent
010ea64
commit c8cdce7
Showing
31 changed files
with
455 additions
and
327 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
42 changes: 42 additions & 0 deletions
42
third_party/blink/common/interest_group/ad_display_size.cc
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| // Copyright 2023 The Chromium Authors | ||
| // Use of this source code is governed by a BSD-style license that can be | ||
| // found in the LICENSE file. | ||
|
|
||
| #include <tuple> | ||
|
|
||
| #include "third_party/blink/public/common/interest_group/ad_display_size.h" | ||
|
|
||
| namespace blink { | ||
|
|
||
| AdSize::AdSize() = default; | ||
|
|
||
| AdSize::AdSize(double width, | ||
| LengthUnit width_units, | ||
| double height, | ||
| LengthUnit height_units) | ||
| : width(width), | ||
| width_units(width_units), | ||
| height(height), | ||
| height_units(height_units) {} | ||
|
|
||
| AdSize::AdSize(const AdSize&) = default; | ||
|
|
||
| AdSize::AdSize(AdSize&&) = default; | ||
|
|
||
| AdSize& AdSize::operator=(const AdSize&) = default; | ||
|
|
||
| AdSize& AdSize::operator=(AdSize&&) = default; | ||
|
|
||
| bool AdSize::operator==(const AdSize& other) const { | ||
| return std::tie(width, width_units, height, height_units) == | ||
| std::tie(other.width, other.width_units, other.height, | ||
| other.height_units); | ||
| } | ||
|
|
||
| bool AdSize::operator!=(const AdSize& other) const { | ||
| return !(*this == other); | ||
| } | ||
|
|
||
| AdSize::~AdSize() = default; | ||
|
|
||
| } // namespace blink |
21 changes: 21 additions & 0 deletions
21
third_party/blink/common/interest_group/ad_display_size_mojom_traits.cc
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| // Copyright 2023 The Chromium Authors | ||
| // Use of this source code is governed by a BSD-style license that can be | ||
| // found in the LICENSE file. | ||
|
|
||
| #include "third_party/blink/public/common/interest_group/ad_display_size_mojom_traits.h" | ||
|
|
||
| namespace mojo { | ||
|
|
||
| bool StructTraits<blink::mojom::AdSizeDataView, blink::AdSize>::Read( | ||
| blink::mojom::AdSizeDataView data, | ||
| blink::AdSize* out) { | ||
| if (!data.ReadWidthUnits(&out->width_units) || | ||
| !data.ReadHeightUnits(&out->height_units)) { | ||
| return false; | ||
| } | ||
| out->width = data.width(); | ||
| out->height = data.height(); | ||
| return true; | ||
| } | ||
|
|
||
| } // namespace mojo |
24 changes: 24 additions & 0 deletions
24
third_party/blink/common/interest_group/ad_display_size_mojom_traits_test.cc
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| // Copyright 2023 The Chromium Authors | ||
| // Use of this source code is governed by a BSD-style license that can be | ||
| // found in the LICENSE file. | ||
|
|
||
| #include "third_party/blink/public/common/interest_group/ad_display_size_mojom_traits.h" | ||
|
|
||
| #include "mojo/public/cpp/test_support/test_utils.h" | ||
| #include "testing/gtest/include/gtest/gtest.h" | ||
| #include "third_party/blink/public/common/interest_group/ad_display_size.h" | ||
| #include "third_party/blink/public/mojom/interest_group/ad_display_size.mojom.h" | ||
|
|
||
| namespace blink { | ||
|
|
||
| TEST(AdDisplaySizeStructTraitsTest, SerializeAndDeserializeAdSize) { | ||
| blink::AdSize ad_size(300, blink::AdSize::LengthUnit::kPixels, 150, | ||
| blink::AdSize::LengthUnit::kPixels); | ||
|
|
||
| blink::AdSize ad_size_clone; | ||
| ASSERT_TRUE(mojo::test::SerializeAndDeserialize<blink::mojom::AdSize>( | ||
| ad_size, ad_size_clone)); | ||
| EXPECT_EQ(ad_size, ad_size_clone); | ||
| } | ||
|
|
||
| } // namespace blink |
49 changes: 49 additions & 0 deletions
49
third_party/blink/common/interest_group/ad_display_size_unittest.cc
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,49 @@ | ||
| // Copyright 2023 The Chromium Authors | ||
| // Use of this source code is governed by a BSD-style license that can be | ||
| // found in the LICENSE file. | ||
|
|
||
| #include "third_party/blink/public/common/interest_group/ad_display_size.h" | ||
|
|
||
| #include "testing/gtest/include/gtest/gtest.h" | ||
| #include "url/gurl.h" | ||
|
|
||
| namespace blink { | ||
|
|
||
| namespace { | ||
|
|
||
| const GURL kUrl1("https://origin1.test/url1"); | ||
| const GURL kUrl2("https://origin1.test/url2"); | ||
|
|
||
| } // namespace | ||
|
|
||
| TEST(AdSizeTest, OperatorCompare) { | ||
| // AdSizes with different units. | ||
| AdSize ad_size_in_pixels(100, AdSize::LengthUnit::kPixels, 100, | ||
| AdSize::LengthUnit::kPixels); | ||
| AdSize ad_size_in_screenwidth(100, AdSize::LengthUnit::kScreenWidth, 100, | ||
| AdSize::LengthUnit::kScreenWidth); | ||
| AdSize ad_size_in_mix_units(100, AdSize::LengthUnit::kPixels, 100, | ||
| AdSize::LengthUnit::kScreenWidth); | ||
|
|
||
| EXPECT_FALSE(ad_size_in_pixels == ad_size_in_screenwidth); | ||
| EXPECT_TRUE(ad_size_in_pixels != ad_size_in_screenwidth); | ||
| EXPECT_FALSE(ad_size_in_pixels == ad_size_in_mix_units); | ||
| EXPECT_TRUE(ad_size_in_pixels != ad_size_in_mix_units); | ||
| EXPECT_FALSE(ad_size_in_screenwidth == ad_size_in_mix_units); | ||
| EXPECT_TRUE(ad_size_in_screenwidth != ad_size_in_mix_units); | ||
|
|
||
| // AdSizes with different numeric values. | ||
| AdSize ad_size_in_pixels_small(5, AdSize::LengthUnit::kPixels, 5, | ||
| AdSize::LengthUnit::kPixels); | ||
|
|
||
| EXPECT_FALSE(ad_size_in_pixels == ad_size_in_pixels_small); | ||
| EXPECT_TRUE(ad_size_in_pixels != ad_size_in_pixels_small); | ||
|
|
||
| // AdSizes with the same numeric values and units. | ||
| AdSize ad_size_in_pixels_clone = ad_size_in_pixels; | ||
|
|
||
| EXPECT_TRUE(ad_size_in_pixels == ad_size_in_pixels_clone); | ||
| EXPECT_FALSE(ad_size_in_pixels != ad_size_in_pixels_clone); | ||
| } | ||
|
|
||
| } // namespace blink |
Oops, something went wrong.