Skip to content

Commit

Permalink
Modernize equality operators in //components/signin.
Browse files Browse the repository at this point in the history
This CL is a functional no-op that modernizes/cleans up the following:
- Where possible, it defaults equality operators. Inequality operators
  are removed since C++20 derives those from equality operators.
- Where possible, it replaces operator< by the spaceship operator.
- It fixes some declaration orders of assignment constructors and
  operators and defaults them, were applicable.
- In a couple of drive-bys, it replaces pass by const ref arguments
  by pass by value iff the constructor does nothing but assign the
  argument to a member. This avoids copies if the constructor is called
  with an r-value.

Bug: n/a
Change-Id: I9690d9a905400b78cc7332c615201f4d22158409
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5526612
Reviewed-by: David Roger <droger@chromium.org>
Reviewed-by: Mohamed Amir Yosef <mamir@chromium.org>
Commit-Queue: Jan Keitel <jkeitel@google.com>
Cr-Commit-Position: refs/heads/main@{#1299989}
  • Loading branch information
Jan Keitel authored and Chromium LUCI CQ committed May 13, 2024
1 parent 343f4af commit 593a01a
Show file tree
Hide file tree
Showing 17 changed files with 57 additions and 112 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,8 @@ std::vector<GaiaIdHash> DeserializeGaiaIdHashVector(const base::Pickle& p) {

base::PickleIterator iterator(p);
while (iterator.ReadString(&hash)) {
hashes.push_back(GaiaIdHash::FromBinary(hash));
hashes.push_back(GaiaIdHash::FromBinary(std::move(hash)));
hash = {};
}
return hashes;
}
Expand Down
9 changes: 4 additions & 5 deletions components/signin/core/browser/account_reconcilor_delegate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,10 @@ AccountReconcilorDelegate::CalculateParametersForMultilogin(
const gaia::MultiloginMode mode =
CalculateModeForReconcile(chrome_accounts, gaia_accounts, primary_account,
first_execution, primary_has_error);
const std::vector<CoreAccountId> accounts_to_send =
GetChromeAccountsForReconcile(chrome_accounts, primary_account,
gaia_accounts, first_execution,
primary_has_error, mode);
return {mode, accounts_to_send};
std::vector<CoreAccountId> accounts_to_send = GetChromeAccountsForReconcile(
chrome_accounts, primary_account, gaia_accounts, first_execution,
primary_has_error, mode);
return {mode, std::move(accounts_to_send)};
}

bool AccountReconcilorDelegate::RevokeSecondaryTokensBeforeMultiloginIfNeeded(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -255,9 +255,7 @@ struct Cookie {
std::string gaia_id;
bool is_valid;

bool operator==(const Cookie& other) const {
return gaia_id == other.gaia_id && is_valid == other.is_valid;
}
bool operator==(const Cookie& other) const = default;
};

// Converts CookieParams to ListedAccounts.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,10 +156,7 @@ class TrackingEvent {
gaia_id_(gaia_id),
email_(email) {}

bool operator==(const TrackingEvent& event) const {
return type_ == event.type_ && account_id_ == event.account_id_ &&
gaia_id_ == event.gaia_id_ && email_ == event.email_;
}
bool operator==(const TrackingEvent& event) const = default;

std::string ToString() const {
const char* typestr = "INVALID";
Expand Down
36 changes: 13 additions & 23 deletions components/signin/public/base/gaia_id_hash.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,38 +4,44 @@

#include "components/signin/public/base/gaia_id_hash.h"

#include <string_view>
#include <utility>

#include "base/base64.h"
#include "crypto/sha2.h"

namespace signin {

// static
GaiaIdHash GaiaIdHash::FromGaiaId(const std::string& gaia_id) {
GaiaIdHash GaiaIdHash::FromGaiaId(std::string_view gaia_id) {
return FromBinary(crypto::SHA256HashString(gaia_id));
}

// static
GaiaIdHash GaiaIdHash::FromBinary(const std::string& gaia_id_hash) {
return GaiaIdHash(gaia_id_hash);
GaiaIdHash GaiaIdHash::FromBinary(std::string gaia_id_hash) {
return GaiaIdHash(std::move(gaia_id_hash));
}

// static
GaiaIdHash GaiaIdHash::FromBase64(const std::string& gaia_id_base64_hash) {
GaiaIdHash GaiaIdHash::FromBase64(std::string_view gaia_id_base64_hash) {
std::string gaia_id_hash;
base::Base64Decode(gaia_id_base64_hash, &gaia_id_hash);
return FromBinary(gaia_id_hash);
return FromBinary(std::move(gaia_id_hash));
}

GaiaIdHash::GaiaIdHash() = default;

GaiaIdHash::GaiaIdHash(const GaiaIdHash& other) = default;
GaiaIdHash& GaiaIdHash::operator=(const GaiaIdHash& form) = default;

GaiaIdHash::GaiaIdHash(GaiaIdHash&& other) = default;

GaiaIdHash& GaiaIdHash::operator=(GaiaIdHash&& form) = default;

GaiaIdHash::~GaiaIdHash() = default;

GaiaIdHash::GaiaIdHash(const std::string& gaia_id_hash)
: gaia_id_hash_(gaia_id_hash) {}
GaiaIdHash::GaiaIdHash(std::string gaia_id_hash)
: gaia_id_hash_(std::move(gaia_id_hash)) {}

std::string GaiaIdHash::ToBinary() const {
return gaia_id_hash_;
Expand All @@ -49,20 +55,4 @@ bool GaiaIdHash::IsValid() const {
return gaia_id_hash_.size() == crypto::kSHA256Length;
}

bool operator<(const GaiaIdHash& lhs, const GaiaIdHash& rhs) {
return lhs.gaia_id_hash_ < rhs.gaia_id_hash_;
}

bool operator==(const GaiaIdHash& lhs, const GaiaIdHash& rhs) {
return lhs.gaia_id_hash_ == rhs.gaia_id_hash_;
}

bool operator!=(const GaiaIdHash& lhs, const GaiaIdHash& rhs) {
return !(lhs == rhs);
}

GaiaIdHash& GaiaIdHash::operator=(const GaiaIdHash& form) = default;

GaiaIdHash& GaiaIdHash::operator=(GaiaIdHash&& form) = default;

} // namespace signin
20 changes: 11 additions & 9 deletions components/signin/public/base/gaia_id_hash.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@
#ifndef COMPONENTS_SIGNIN_PUBLIC_BASE_GAIA_ID_HASH_H_
#define COMPONENTS_SIGNIN_PUBLIC_BASE_GAIA_ID_HASH_H_

#include <compare>
#include <string>
#include <string_view>

namespace signin {

Expand All @@ -16,33 +18,33 @@ namespace signin {
// be treated as a PII.
class GaiaIdHash {
public:
static GaiaIdHash FromGaiaId(const std::string& gaia_id);
static GaiaIdHash FromGaiaId(std::string_view gaia_id);
// |gaia_id_hash| is a string representing the binary hash of the gaia id. If
// the input isn't of length crypto::kSHA256Length, it returns an invalid
// GaiaIdHash object.
static GaiaIdHash FromBinary(const std::string& gaia_id_hash);
static GaiaIdHash FromBinary(std::string gaia_id_hash);
// If |gaia_id_base64_hash| isn't well-formed Base64 string, or doesn't decode
// to a hash of the correct length, it returns an invalid GaiaIdHash object.
static GaiaIdHash FromBase64(const std::string& gaia_id_base64_hash);
static GaiaIdHash FromBase64(std::string_view gaia_id_base64_hash);

GaiaIdHash();
GaiaIdHash(const GaiaIdHash& other);
GaiaIdHash& operator=(const GaiaIdHash& form);
GaiaIdHash(GaiaIdHash&& other);
GaiaIdHash& operator=(GaiaIdHash&& form);
~GaiaIdHash();

std::string ToBinary() const;
std::string ToBase64() const;

bool IsValid() const;

friend bool operator<(const GaiaIdHash& lhs, const GaiaIdHash& rhs);
friend bool operator==(const GaiaIdHash& lhs, const GaiaIdHash& rhs);
friend bool operator!=(const GaiaIdHash& lhs, const GaiaIdHash& rhs);
GaiaIdHash& operator=(const GaiaIdHash& form);
GaiaIdHash& operator=(GaiaIdHash&& form);
friend std::strong_ordering operator<=>(const GaiaIdHash&,
const GaiaIdHash&) = default;
friend bool operator==(const GaiaIdHash&, const GaiaIdHash&) = default;

private:
explicit GaiaIdHash(const std::string& gaia_id_hash);
explicit GaiaIdHash(std::string gaia_id_hash);
std::string gaia_id_hash_;
};

Expand Down
21 changes: 8 additions & 13 deletions components/signin/public/base/multilogin_parameters.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

#include <sstream>
#include <string>
#include <utility>

#include "base/strings/string_util.h"

Expand All @@ -27,15 +28,16 @@ MultiloginParameters::MultiloginParameters() = default;

MultiloginParameters::MultiloginParameters(
const gaia::MultiloginMode mode,
const std::vector<CoreAccountId>& accounts_to_send)
: mode(mode), accounts_to_send(accounts_to_send) {}
std::vector<CoreAccountId> accounts_to_send)
: mode(mode), accounts_to_send(std::move(accounts_to_send)) {}

MultiloginParameters::~MultiloginParameters() = default;

MultiloginParameters::MultiloginParameters(const MultiloginParameters& other) {
mode = other.mode;
accounts_to_send = other.accounts_to_send;
}
MultiloginParameters::MultiloginParameters(const MultiloginParameters&) =
default;

MultiloginParameters& MultiloginParameters::operator=(
const MultiloginParameters&) = default;

std::string MultiloginParameters::ToString() const {
std::stringstream ss;
Expand All @@ -46,13 +48,6 @@ std::string MultiloginParameters::ToString() const {
return ss.str();
}

MultiloginParameters& MultiloginParameters::operator=(
const MultiloginParameters& other) {
mode = other.mode;
accounts_to_send = other.accounts_to_send;
return *this;
}

std::ostream& operator<<(std::ostream& out, const MultiloginParameters& p) {
return out << p.ToString();
}
Expand Down
14 changes: 4 additions & 10 deletions components/signin/public/base/multilogin_parameters.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,20 +18,14 @@ struct MultiloginParameters {
// Parameters with UPDATE mode and empty accounts.
MultiloginParameters();
MultiloginParameters(gaia::MultiloginMode mode,
const std::vector<CoreAccountId>& accounts_to_send);
MultiloginParameters(const MultiloginParameters& other);
MultiloginParameters& operator=(const MultiloginParameters& other);
std::vector<CoreAccountId> accounts_to_send);
MultiloginParameters(const MultiloginParameters&);
MultiloginParameters& operator=(const MultiloginParameters&);
~MultiloginParameters();

std::string ToString() const;

bool operator==(const MultiloginParameters& other) const {
return mode == other.mode && accounts_to_send == other.accounts_to_send;
}

bool operator!=(const MultiloginParameters& other) const {
return !(*this == other);
}
bool operator==(const MultiloginParameters&) const = default;

gaia::MultiloginMode mode =
gaia::MultiloginMode::MULTILOGIN_UPDATE_COOKIE_ACCOUNTS_ORDER;
Expand Down
1 change: 0 additions & 1 deletion components/signin/public/identity_manager/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ source_set("identity_manager") {
"access_token_constants.h",
"access_token_fetcher.cc",
"access_token_fetcher.h",
"access_token_info.cc",
"access_token_info.h",
"account_capabilities.cc",
"account_capabilities.h",
Expand Down
15 changes: 0 additions & 15 deletions components/signin/public/identity_manager/access_token_info.cc

This file was deleted.

18 changes: 10 additions & 8 deletions components/signin/public/identity_manager/access_token_info.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#define COMPONENTS_SIGNIN_PUBLIC_IDENTITY_MANAGER_ACCESS_TOKEN_INFO_H_

#include <string>
#include <utility>

#include "base/time/time.h"

Expand All @@ -27,16 +28,17 @@ struct AccessTokenInfo {
std::string id_token;

AccessTokenInfo() = default;
AccessTokenInfo(const std::string& token_param,
const base::Time& expiration_time_param,
const std::string& id_token)
: token(token_param),
AccessTokenInfo(std::string token_param,
base::Time expiration_time_param,
std::string id_token)
: token(std::move(token_param)),
expiration_time(expiration_time_param),
id_token(id_token) {}
};
id_token(std::move(id_token)) {}

// Defined for testing purposes only.
bool operator==(const AccessTokenInfo& lhs, const AccessTokenInfo& rhs);
// Defined for testing purposes only.
friend bool operator==(const AccessTokenInfo&,
const AccessTokenInfo&) = default;
};

} // namespace signin

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,10 +157,6 @@ bool AccountCapabilities::operator==(const AccountCapabilities& other) const {
return true;
}

bool AccountCapabilities::operator!=(const AccountCapabilities& other) const {
return !(*this == other);
}

#if BUILDFLAG(IS_ANDROID)
// static
AccountCapabilities AccountCapabilities::ConvertFromJavaAccountCapabilities(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,6 @@ class AccountCapabilities {
bool UpdateWith(const AccountCapabilities& other);

bool operator==(const AccountCapabilities& other) const;
bool operator!=(const AccountCapabilities& other) const;

private:
friend std::optional<AccountCapabilities> AccountCapabilitiesFromValue(
Expand Down
3 changes: 0 additions & 3 deletions components/signin/public/identity_manager/account_info.cc
Original file line number Diff line number Diff line change
Expand Up @@ -168,9 +168,6 @@ bool operator==(const CoreAccountInfo& l, const CoreAccountInfo& r) {
gaia::AreEmailsSame(l.email, r.email) &&
l.is_under_advanced_protection == r.is_under_advanced_protection;
}
bool operator!=(const CoreAccountInfo& l, const CoreAccountInfo& r) {
return !(l == r);
}

std::ostream& operator<<(std::ostream& os, const CoreAccountInfo& account) {
os << "account_id: " << account.account_id << ", gaia: " << account.gaia
Expand Down
1 change: 0 additions & 1 deletion components/signin/public/identity_manager/account_info.h
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,6 @@ struct AccountInfo : public CoreAccountInfo {
};

bool operator==(const CoreAccountInfo& l, const CoreAccountInfo& r);
bool operator!=(const CoreAccountInfo& l, const CoreAccountInfo& r);
std::ostream& operator<<(std::ostream& os, const CoreAccountInfo& account);

// Comparing `AccountInfo`s is likely a mistake. You should compare either
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,12 +167,6 @@ bool PrimaryAccountChangeEvent::StatesAndEventSourceAreValid(
return true;
}

bool operator==(const PrimaryAccountChangeEvent::State& lhs,
const PrimaryAccountChangeEvent::State& rhs) {
return lhs.primary_account == rhs.primary_account &&
lhs.consent_level == rhs.consent_level;
}

std::ostream& operator<<(std::ostream& os,
const PrimaryAccountChangeEvent::State& state) {
os << "{ primary_account: " << state.primary_account.account_id << ", "
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,12 @@ class PrimaryAccountChangeEvent {

struct State {
State();
State(const State& other);
State(CoreAccountInfo account_info, ConsentLevel consent_level);
State(const State& other);
State& operator=(const State& other);
~State();

State& operator=(const State& other);
friend bool operator==(const State&, const State&) = default;

CoreAccountInfo primary_account;
ConsentLevel consent_level = ConsentLevel::kSignin;
Expand Down Expand Up @@ -73,9 +74,6 @@ class PrimaryAccountChangeEvent {
event_source_;
};

bool operator==(const PrimaryAccountChangeEvent::State& lhs,
const PrimaryAccountChangeEvent::State& rhs);

std::ostream& operator<<(std::ostream& os,
const PrimaryAccountChangeEvent::State& state);
std::ostream& operator<<(std::ostream& os,
Expand Down

0 comments on commit 593a01a

Please sign in to comment.