Skip to content

Commit

Permalink
Fixes for C++20 support.
Browse files Browse the repository at this point in the history
* Types on both sides of comparison operators should be the same.
  Switch to the NVI pattern to avoid polymorphic comparisons.
  Explicitly convert types where necessary.
* Structs with user-declared constructors are no longer aggregates.
  Remove the declarations where possible, or else add constructors.
* u8"" no longer produces const char*.  Switch to "", which can also
  accept UTF-8 literals.
* Math between disparate enums is deprecated.  Switch to constexprs.
* Some #include/forward-decl fixups.

Bug: 1284275
Change-Id: I7c4dfa62c92d66344c1ceb63eea105e8b69e7530
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3631265
Auto-Submit: Peter Kasting <pkasting@chromium.org>
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Reviewed-by: Nektarios Paisios <nektar@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1001513}
  • Loading branch information
pkasting authored and Chromium LUCI CQ committed May 10, 2022
1 parent 5d7b331 commit eef552b
Show file tree
Hide file tree
Showing 24 changed files with 227 additions and 207 deletions.
10 changes: 0 additions & 10 deletions content/browser/accessibility/browser_accessibility.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1558,16 +1558,6 @@ BrowserAccessibility::PlatformChildIterator::PlatformChildIterator(

BrowserAccessibility::PlatformChildIterator::~PlatformChildIterator() = default;

bool BrowserAccessibility::PlatformChildIterator::operator==(
const ChildIterator& rhs) const {
return GetIndexInParent() == rhs.GetIndexInParent();
}

bool BrowserAccessibility::PlatformChildIterator::operator!=(
const ChildIterator& rhs) const {
return GetIndexInParent() != rhs.GetIndexInParent();
}

BrowserAccessibility::PlatformChildIterator&
BrowserAccessibility::PlatformChildIterator::operator++() {
++platform_iterator;
Expand Down
2 changes: 0 additions & 2 deletions content/browser/accessibility/browser_accessibility.h
Original file line number Diff line number Diff line change
Expand Up @@ -125,8 +125,6 @@ class CONTENT_EXPORT BrowserAccessibility : public ui::AXPlatformNodeDelegate {
BrowserAccessibility* child);
PlatformChildIterator(const PlatformChildIterator& it);
~PlatformChildIterator() override;
bool operator==(const ChildIterator& rhs) const override;
bool operator!=(const ChildIterator& rhs) const override;
PlatformChildIterator& operator++() override;
PlatformChildIterator& operator++(int) override;
PlatformChildIterator& operator--() override;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -479,34 +479,23 @@ TEST_F(AttributionDataHostManagerImplTest, TriggerDataHost_TriggerRegistered) {
auto reporting_origin = url::Origin::Create(GURL("https://reporter.example"));
EXPECT_CALL(
mock_manager_,
HandleTrigger(AttributionTriggerMatches({
.destination_origin = destination_origin,
.reporting_origin = reporting_origin,
.filters = *AttributionFilterData::FromTriggerFilterValues({
HandleTrigger(AttributionTriggerMatches(AttributionTriggerMatcherConfig(
destination_origin, reporting_origin,
*AttributionFilterData::FromTriggerFilterValues({
{"a", {"b"}},
}),
.debug_key = Optional(789),
.event_triggers = ElementsAre(
EventTriggerDataMatches({
.data = 1,
.priority = 2,
.dedup_key = Optional(3),
.filters = *AttributionFilterData::FromTriggerFilterValues({
{"c", {"d"}},
}),
.not_filters =
*AttributionFilterData::FromTriggerFilterValues({
{"e", {"f"}},
}),
}),
EventTriggerDataMatches({
.data = 4,
.priority = 5,
.dedup_key = Eq(absl::nullopt),
.filters = AttributionFilterData(),
.not_filters = AttributionFilterData(),
})),
})));
Optional(789),
ElementsAre(EventTriggerDataMatches(EventTriggerDataMatcherConfig(
1, 2, Optional(3),
*AttributionFilterData::FromTriggerFilterValues({
{"c", {"d"}},
}),
*AttributionFilterData::FromTriggerFilterValues({
{"e", {"f"}},
}))),
EventTriggerDataMatches(EventTriggerDataMatcherConfig(
4, 5, Eq(absl::nullopt), AttributionFilterData(),
AttributionFilterData())))))));

{
RemoteDataHost data_host_remote{.task_environment = task_environment_};
Expand Down Expand Up @@ -1359,11 +1348,13 @@ TEST_F(AttributionDataHostManagerImplTest,

EXPECT_CALL(mock_manager_, HandleTrigger).Times(0);
EXPECT_CALL(checkpoint, Call(1));
EXPECT_CALL(mock_manager_, HandleTrigger(AttributionTriggerMatches(
{.reporting_origin = reporting_origin1})));
EXPECT_CALL(mock_manager_,
HandleTrigger(AttributionTriggerMatches(
AttributionTriggerMatcherConfig(_, reporting_origin1))));
EXPECT_CALL(checkpoint, Call(2));
EXPECT_CALL(mock_manager_, HandleTrigger(AttributionTriggerMatches(
{.reporting_origin = reporting_origin2})));
EXPECT_CALL(mock_manager_,
HandleTrigger(AttributionTriggerMatches(
AttributionTriggerMatcherConfig(_, reporting_origin2))));
}

mojo::Remote<blink::mojom::AttributionDataHost> source_data_host_remote;
Expand Down Expand Up @@ -1478,9 +1469,9 @@ TEST_F(AttributionDataHostManagerImplTest,
url::Origin reporting_origin = url::Origin::Create(GURL(
base::StrCat({"https://report", base::NumberToString(i), ".test"})));

EXPECT_CALL(mock_manager_, HandleTrigger(AttributionTriggerMatches({
.reporting_origin = reporting_origin,
})))
EXPECT_CALL(mock_manager_,
HandleTrigger(AttributionTriggerMatches(
AttributionTriggerMatcherConfig(_, reporting_origin))))
.WillOnce([&](AttributionTrigger trigger) { barrier.Run(); });

send_trigger(std::move(reporting_origin));
Expand Down
25 changes: 25 additions & 0 deletions content/browser/attribution_reporting/attribution_test_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1315,6 +1315,18 @@ std::ostream& operator<<(
return out << "]}";
}

EventTriggerDataMatcherConfig::EventTriggerDataMatcherConfig(
::testing::Matcher<uint64_t> data,
::testing::Matcher<int64_t> priority,
::testing::Matcher<absl::optional<uint64_t>> dedup_key,
::testing::Matcher<const AttributionFilterData&> filters,
::testing::Matcher<const AttributionFilterData&> not_filters)
: data(std::move(data)),
priority(std::move(priority)),
dedup_key(std::move(dedup_key)),
filters(std::move(filters)),
not_filters(std::move(not_filters)) {}

EventTriggerDataMatcherConfig::~EventTriggerDataMatcherConfig() = default;

::testing::Matcher<const AttributionTrigger::EventTriggerData&>
Expand All @@ -1331,6 +1343,19 @@ EventTriggerDataMatches(const EventTriggerDataMatcherConfig& cfg) {
cfg.not_filters));
}

AttributionTriggerMatcherConfig::AttributionTriggerMatcherConfig(
::testing::Matcher<const url::Origin&> destination_origin,
::testing::Matcher<const url::Origin&> reporting_origin,
::testing::Matcher<const AttributionFilterData&> filters,
::testing::Matcher<absl::optional<uint64_t>> debug_key,
::testing::Matcher<const std::vector<AttributionTrigger::EventTriggerData>&>
event_triggers)
: destination_origin(std::move(destination_origin)),
reporting_origin(std::move(reporting_origin)),
filters(std::move(filters)),
debug_key(std::move(debug_key)),
event_triggers(std::move(event_triggers)) {}

AttributionTriggerMatcherConfig::~AttributionTriggerMatcherConfig() = default;

::testing::Matcher<AttributionTrigger> AttributionTriggerMatches(
Expand Down
25 changes: 20 additions & 5 deletions content/browser/attribution_reporting/attribution_test_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -915,13 +915,20 @@ MATCHER_P(NewAggregatableReportIs, matcher, "") {
}

struct EventTriggerDataMatcherConfig {
::testing::Matcher<uint64_t> data = ::testing::_;
::testing::Matcher<int64_t> priority = ::testing::_;
::testing::Matcher<absl::optional<uint64_t>> dedup_key = ::testing::_;
::testing::Matcher<const AttributionFilterData&> filters = ::testing::_;
::testing::Matcher<const AttributionFilterData&> not_filters = ::testing::_;
::testing::Matcher<uint64_t> data;
::testing::Matcher<int64_t> priority;
::testing::Matcher<absl::optional<uint64_t>> dedup_key;
::testing::Matcher<const AttributionFilterData&> filters;
::testing::Matcher<const AttributionFilterData&> not_filters;

EventTriggerDataMatcherConfig() = delete;
EventTriggerDataMatcherConfig(
::testing::Matcher<uint64_t> data = ::testing::_,
::testing::Matcher<int64_t> priority = ::testing::_,
::testing::Matcher<absl::optional<uint64_t>> dedup_key = ::testing::_,
::testing::Matcher<const AttributionFilterData&> filters = ::testing::_,
::testing::Matcher<const AttributionFilterData&> not_filters =
::testing::_);
~EventTriggerDataMatcherConfig();
};

Expand All @@ -937,6 +944,14 @@ struct AttributionTriggerMatcherConfig {
event_triggers = ::testing::_;

AttributionTriggerMatcherConfig() = delete;
AttributionTriggerMatcherConfig(
::testing::Matcher<const url::Origin&> destination_origin = ::testing::_,
::testing::Matcher<const url::Origin&> reporting_origin = ::testing::_,
::testing::Matcher<const AttributionFilterData&> filters = ::testing::_,
::testing::Matcher<absl::optional<uint64_t>> debug_key = ::testing::_,
::testing::Matcher<
const std::vector<AttributionTrigger::EventTriggerData>&>
event_triggers = ::testing::_);
~AttributionTriggerMatcherConfig();
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,12 @@ class InitializationSubTask : public DatabaseTask {
// Holds data used by all SubTasks.
struct SubTaskInit {
SubTaskInit() = delete;
SubTaskInit(int64_t service_worker_registration_id,
std::string unique_id,
BackgroundFetchInitializationData* initialization_data)
: service_worker_registration_id(service_worker_registration_id),
unique_id(std::move(unique_id)),
initialization_data(initialization_data) {}
~SubTaskInit() = default;

// Service Worker Database metadata.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -253,15 +253,14 @@ BrowsingDataFilterBuilderImpl::Copy() {
return std::move(copy);
}

bool BrowsingDataFilterBuilderImpl::operator==(
const BrowsingDataFilterBuilder& other) {
bool BrowsingDataFilterBuilderImpl::IsEqual(
const BrowsingDataFilterBuilder& other) const {
// This is the only implementation of BrowsingDataFilterBuilder, so we can
// downcast |other|.
const BrowsingDataFilterBuilderImpl* other_impl =
static_cast<const BrowsingDataFilterBuilderImpl*>(&other);

return origins_ == other_impl->origins_ &&
domains_ == other_impl->domains_ &&
return origins_ == other_impl->origins_ && domains_ == other_impl->domains_ &&
mode_ == other_impl->mode_;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,10 @@ class CONTENT_EXPORT BrowsingDataFilterBuilderImpl
override;
Mode GetMode() override;
std::unique_ptr<BrowsingDataFilterBuilder> Copy() override;
bool operator==(const BrowsingDataFilterBuilder& other) override;

private:
bool IsEqual(const BrowsingDataFilterBuilder& other) const override;

Mode mode_;

std::set<url::Origin> origins_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,10 @@ TEST(ParseBaseFrequencyFromCpuidTest, InvalidStrings) {
"GHz",
"FooMHz", // Units preceded by non-numbers.
"FooGHz",
u8"\U0001f41bGHz",
u8"1\U0001f41bGHz",
u8"1.\U0001f41bGHz",
u8"1.0\U0001f41bGHz",
"\U0001f41bGHz",
"1\U0001f41bGHz",
"1.\U0001f41bGHz",
"1.0\U0001f41bGHz",
"1", // Numbers without units.
"1000MH", // Numbers without units.
"1000Mz",
Expand Down Expand Up @@ -98,8 +98,8 @@ TEST(ParseBaseFrequencyFromCpuidTest, ValidStrings) {
{"3Ghz", 3'000'000'000},
{"4GHZ", 4'000'000'000},
{"@2GHz", 2'000'000'000}, // Text right before the number.
{u8"\U0001f5782GHz", 2'000'000'000},
{u8"\U0001f578 2GHz", 2'000'000'000},
{"\U0001f5782GHz", 2'000'000'000},
{"\U0001f578 2GHz", 2'000'000'000},
{"933 MHz", 933'000'000}, // Spaces between unit and frequency.
{"1.2 GHz", 1'200'000'000},
{"0.1GHz", 100'000'000}, // Smallest value for GHz.
Expand All @@ -112,7 +112,7 @@ TEST(ParseBaseFrequencyFromCpuidTest, ValidStrings) {
{"9223372036800Mhz", 9'223'372'036'800'000'000},
{"0GHz 0.00GHz 1.0.0Ghz 1.1Ghz 1.2Ghz", 1'100'000'000}, // False starts.
{"100MHzabc", 100'000'000}, // Text after the frequency
{u8"1GHz\U0001f578", 1'000'000'000},
{"1GHz\U0001f578", 1'000'000'000},
};

for (const TestCase& test_case : test_cases) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -925,11 +925,11 @@ TEST_F(PlatformNotificationContextTest, WriteReadNotificationResources) {

// Store resources for the new notification.
std::vector<NotificationResourceData> resources;
resources.push_back(
{notification_id, origin, blink::NotificationResources()});
resources.emplace_back(notification_id, origin,
blink::NotificationResources());
// Also try inserting resources for an invalid notification id.
std::string invalid_id = "invalid-id";
resources.push_back({invalid_id, origin, blink::NotificationResources()});
resources.emplace_back(invalid_id, origin, blink::NotificationResources());
// Writing resources should succeed.
ASSERT_TRUE(
WriteNotificationResourcesSync(context.get(), std::move(resources)));
Expand Down Expand Up @@ -966,8 +966,8 @@ TEST_F(PlatformNotificationContextTest, ReDisplayNotifications) {
std::string notification_id =
WriteNotificationDataSync(context.get(), origin, data2);
std::vector<NotificationResourceData> resources;
resources.push_back(
{notification_id, origin, blink::NotificationResources()});
resources.emplace_back(notification_id, origin,
blink::NotificationResources());
WriteNotificationResourcesSync(context.get(), std::move(resources));
// 1 notification without resources.
NotificationDatabaseData data3;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,10 @@ struct FontExpectation {
uint16_t ttc_index;
};

constexpr FontExpectation kExpectedTestFonts[] = {{u8"CambriaMath", 1},
{u8"Ming-Lt-HKSCS-ExtB", 2},
{u8"NSimSun", 1},
{u8"calibri-bolditalic", 0}};
constexpr FontExpectation kExpectedTestFonts[] = {{"CambriaMath", 1},
{"Ming-Lt-HKSCS-ExtB", 2},
{"NSimSun", 1},
{"calibri-bolditalic", 0}};

constexpr base::TimeDelta kTestingTimeout = base::Seconds(10);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,10 @@ struct FontExpectation {
uint16_t ttc_index;
};

constexpr FontExpectation kExpectedTestFonts[] = {{u8"CambriaMath", 1},
{u8"Ming-Lt-HKSCS-ExtB", 2},
{u8"NSimSun", 1},
{u8"calibri-bolditalic", 0}};
constexpr FontExpectation kExpectedTestFonts[] = {{"CambriaMath", 1},
{"Ming-Lt-HKSCS-ExtB", 2},
{"NSimSun", 1},
{"calibri-bolditalic", 0}};

// DirectWrite on Windows supports IDWriteFontSet API which allows for querying
// by PostScript name and full font name directly. In the implementation of
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ class SignedExchangeCertFetcherTest : public testing::Test {
cbor::Value("OCSP", cbor::Value::Type::BYTE_STRING);

cbor::Value::ArrayValue cbor_array;
cbor_array.push_back(cbor::Value(u8"\U0001F4DC\u26D3"));
cbor_array.push_back(cbor::Value("\U0001F4DC\u26D3"));
cbor_array.push_back(cbor::Value(std::move(cbor_map)));

absl::optional<std::vector<uint8_t>> serialized =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ TEST(SignedExchangeCertificateParseTest, Empty) {

TEST(SignedExchangeCertificateParseTest, EmptyChain) {
cbor::Value::ArrayValue cbor_array;
cbor_array.push_back(cbor::Value(u8"\U0001F4DC\u26D3"));
cbor_array.push_back(cbor::Value("\U0001F4DC\u26D3"));

auto serialized = cbor::Writer::Write(cbor::Value(std::move(cbor_array)));
ASSERT_TRUE(serialized.has_value());
Expand All @@ -62,7 +62,7 @@ TEST(SignedExchangeCertificateParseTest, MissingCert) {
cbor_map[cbor::Value("ocsp")] = CBORByteString("OCSP");

cbor::Value::ArrayValue cbor_array;
cbor_array.push_back(cbor::Value(u8"\U0001F4DC\u26D3"));
cbor_array.push_back(cbor::Value("\U0001F4DC\u26D3"));
cbor_array.push_back(cbor::Value(std::move(cbor_map)));

auto serialized = cbor::Writer::Write(cbor::Value(std::move(cbor_array)));
Expand All @@ -87,7 +87,7 @@ TEST(SignedExchangeCertificateParseTest, OneCert) {
cbor_map[cbor::Value("ocsp")] = CBORByteString("OCSP");

cbor::Value::ArrayValue cbor_array;
cbor_array.push_back(cbor::Value(u8"\U0001F4DC\u26D3"));
cbor_array.push_back(cbor::Value("\U0001F4DC\u26D3"));
cbor_array.push_back(cbor::Value(std::move(cbor_map)));

auto serialized = cbor::Writer::Write(cbor::Value(std::move(cbor_array)));
Expand Down Expand Up @@ -116,7 +116,7 @@ TEST(SignedExchangeCertificateParseTest, MissingOCSPInFirstCert) {
cbor_map[cbor::Value("cert")] = CBORByteString(cert_der);

cbor::Value::ArrayValue cbor_array;
cbor_array.push_back(cbor::Value(u8"\U0001F4DC\u26D3"));
cbor_array.push_back(cbor::Value("\U0001F4DC\u26D3"));
cbor_array.push_back(cbor::Value(std::move(cbor_map)));

auto serialized = cbor::Writer::Write(cbor::Value(std::move(cbor_array)));
Expand Down Expand Up @@ -146,7 +146,7 @@ TEST(SignedExchangeCertificateParseTest, TwoCerts) {
cbor_map2[cbor::Value("cert")] = CBORByteString(cert2_der);

cbor::Value::ArrayValue cbor_array;
cbor_array.push_back(cbor::Value(u8"\U0001F4DC\u26D3"));
cbor_array.push_back(cbor::Value("\U0001F4DC\u26D3"));
cbor_array.push_back(cbor::Value(std::move(cbor_map1)));
cbor_array.push_back(cbor::Value(std::move(cbor_map2)));

Expand Down Expand Up @@ -185,7 +185,7 @@ TEST(SignedExchangeCertificateParseTest, HavingOCSPInSecondCert) {
cbor_map2[cbor::Value("ocsp")] = CBORByteString("OCSP2");

cbor::Value::ArrayValue cbor_array;
cbor_array.push_back(cbor::Value(u8"\U0001F4DC\u26D3"));
cbor_array.push_back(cbor::Value("\U0001F4DC\u26D3"));
cbor_array.push_back(cbor::Value(std::move(cbor_map1)));
cbor_array.push_back(cbor::Value(std::move(cbor_map2)));

Expand Down
2 changes: 1 addition & 1 deletion content/browser/web_package/signed_exchange_consts.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ constexpr char kIntegrity[] = "integrity";
constexpr char kSig[] = "sig";
constexpr char kStatusKey[] = ":status";
constexpr char kValidityUrlKey[] = "validity-url";
constexpr char kCertChainCborMagic[] = u8"\U0001F4DC\u26D3"; // "📜⛓"
constexpr char kCertChainCborMagic[] = "📜⛓";
constexpr char kCertKey[] = "cert";
constexpr char kOcspKey[] = "ocsp";
constexpr char kSctKey[] = "sct";
Expand Down

0 comments on commit eef552b

Please sign in to comment.