Commit 18b3188a authored by Jared Saul's avatar Jared Saul Committed by Commit Bot

[Autofill] Action base::Optional suggestions from seblalancette@

I opted to use ".has_value() &&" instead of ".value_or(~)" in
some places for generally readability reasons.  We don't want to
log certain metrics or strikes if the value was missing (which
SHOULD almost never happen, but rare race conditions are possible)
and I don't want to switch between _or(true) and _or(false) as
necessary in those situations.

Change-Id: Ieec56f5cdf9cae8855b17d49ed78a9850a2a8c91
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1576107Reviewed-by: default avatarSebastien Lalancette <seblalancette@chromium.org>
Commit-Queue: Jared Saul <jsaul@google.com>
Cr-Commit-Position: refs/heads/master@{#653381}
parent 1ad7b466
...@@ -106,7 +106,7 @@ void CreditCardSaveManager::AttemptToOfferCardLocalSave( ...@@ -106,7 +106,7 @@ void CreditCardSaveManager::AttemptToOfferCardLocalSave(
bool has_non_focusable_field, bool has_non_focusable_field,
const CreditCard& card) { const CreditCard& card) {
local_card_save_candidate_ = card; local_card_save_candidate_ = card;
show_save_prompt_ = base::nullopt; show_save_prompt_.reset();
has_non_focusable_field_ = has_non_focusable_field; has_non_focusable_field_ = has_non_focusable_field;
from_dynamic_change_form_ = from_dynamic_change_form; from_dynamic_change_form_ = from_dynamic_change_form;
...@@ -143,7 +143,7 @@ void CreditCardSaveManager::AttemptToOfferCardUploadSave( ...@@ -143,7 +143,7 @@ void CreditCardSaveManager::AttemptToOfferCardUploadSave(
upload_request_ = payments::PaymentsClient::UploadRequestDetails(); upload_request_ = payments::PaymentsClient::UploadRequestDetails();
upload_request_.card = card; upload_request_.card = card;
uploading_local_card_ = uploading_local_card; uploading_local_card_ = uploading_local_card;
show_save_prompt_ = base::nullopt; show_save_prompt_.reset();
// In an ideal scenario, when uploading a card, we would have: // In an ideal scenario, when uploading a card, we would have:
// 1) Card number and expiration // 1) Card number and expiration
...@@ -373,7 +373,7 @@ void CreditCardSaveManager::OnDidUploadCard( ...@@ -373,7 +373,7 @@ void CreditCardSaveManager::OnDidUploadCard(
base::DoNothing()); base::DoNothing());
} }
} else { } else {
if (show_save_prompt_.value()) { if (show_save_prompt_.has_value() && show_save_prompt_.value()) {
if (base::FeatureList::IsEnabled( if (base::FeatureList::IsEnabled(
features::kAutofillSaveCreditCardUsesStrikeSystemV2)) { features::kAutofillSaveCreditCardUsesStrikeSystemV2)) {
// If the upload failed and the bubble was actually shown (NOT just the // If the upload failed and the bubble was actually shown (NOT just the
...@@ -471,7 +471,7 @@ void CreditCardSaveManager::OnDidGetUploadDetails( ...@@ -471,7 +471,7 @@ void CreditCardSaveManager::OnDidGetUploadDetails(
// LegacyStrikeDatabase have returned their decisions. Use presence of // LegacyStrikeDatabase have returned their decisions. Use presence of
// |show_save_prompt_| as an indicator of LegacyStrikeDatabase retrieving // |show_save_prompt_| as an indicator of LegacyStrikeDatabase retrieving
// its data. // its data.
if (show_save_prompt_ != base::nullopt) if (show_save_prompt_.has_value())
OfferCardUploadSave(); OfferCardUploadSave();
} else { } else {
// If the upload details request failed and we *know* we have all possible // If the upload details request failed and we *know* we have all possible
...@@ -514,16 +514,16 @@ void CreditCardSaveManager::OfferCardLocalSave() { ...@@ -514,16 +514,16 @@ void CreditCardSaveManager::OfferCardLocalSave() {
bool is_mobile_build = false; bool is_mobile_build = false;
#endif // #if defined(OS_ANDROID) || defined(OS_IOS) #endif // #if defined(OS_ANDROID) || defined(OS_IOS)
// If |show_save_prompt_.value()| is false, desktop builds will still offer // If |show_save_prompt_|'s value is false, desktop builds will still offer
// save in the omnibox without popping-up the bubble. Mobile builds, however, // save in the omnibox without popping-up the bubble. Mobile builds, however,
// should not display the offer-to-save infobar at all. // should not display the offer-to-save infobar at all.
if (!is_mobile_build || show_save_prompt_.value()) { if (!is_mobile_build || show_save_prompt_.value_or(true)) {
if (observer_for_testing_) if (observer_for_testing_)
observer_for_testing_->OnOfferLocalSave(); observer_for_testing_->OnOfferLocalSave();
client_->ConfirmSaveCreditCardLocally( client_->ConfirmSaveCreditCardLocally(
local_card_save_candidate_, local_card_save_candidate_,
AutofillClient::SaveCreditCardOptions().with_show_prompt( AutofillClient::SaveCreditCardOptions().with_show_prompt(
show_save_prompt_.value()), show_save_prompt_.value_or(true)),
base::BindOnce(&CreditCardSaveManager::OnUserDidDecideOnLocalSave, base::BindOnce(&CreditCardSaveManager::OnUserDidDecideOnLocalSave,
weak_ptr_factory_.GetWeakPtr())); weak_ptr_factory_.GetWeakPtr()));
...@@ -532,7 +532,7 @@ void CreditCardSaveManager::OfferCardLocalSave() { ...@@ -532,7 +532,7 @@ void CreditCardSaveManager::OfferCardLocalSave() {
AutofillMetrics::LogSaveCardWithFirstAndLastNameOffered( AutofillMetrics::LogSaveCardWithFirstAndLastNameOffered(
/*is_local=*/true); /*is_local=*/true);
} }
if (!show_save_prompt_.value()) { if (show_save_prompt_.has_value() && !show_save_prompt_.value()) {
AutofillMetrics::LogCreditCardSaveNotOfferedDueToMaxStrikesMetric( AutofillMetrics::LogCreditCardSaveNotOfferedDueToMaxStrikesMetric(
AutofillMetrics::SaveTypeMetric::LOCAL); AutofillMetrics::SaveTypeMetric::LOCAL);
} }
...@@ -544,10 +544,10 @@ void CreditCardSaveManager::OfferCardUploadSave() { ...@@ -544,10 +544,10 @@ void CreditCardSaveManager::OfferCardUploadSave() {
#else #else
bool is_mobile_build = false; bool is_mobile_build = false;
#endif // #if defined(OS_ANDROID) || defined(OS_IOS) #endif // #if defined(OS_ANDROID) || defined(OS_IOS)
// If |show_save_prompt_.value()| is false, desktop builds will still offer // If |show_save_prompt_|'s value is false, desktop builds will still offer
// save in the omnibox without popping-up the bubble. Mobile builds, however, // save in the omnibox without popping-up the bubble. Mobile builds, however,
// should not display the offer-to-save infobar at all. // should not display the offer-to-save infobar at all.
if (!is_mobile_build || show_save_prompt_.value()) { if (!is_mobile_build || show_save_prompt_.value_or(true)) {
user_did_accept_upload_prompt_ = false; user_did_accept_upload_prompt_ = false;
client_->ConfirmSaveCreditCardToCloud( client_->ConfirmSaveCreditCardToCloud(
upload_request_.card, std::move(legal_message_), upload_request_.card, std::move(legal_message_),
...@@ -557,7 +557,7 @@ void CreditCardSaveManager::OfferCardUploadSave() { ...@@ -557,7 +557,7 @@ void CreditCardSaveManager::OfferCardUploadSave() {
.with_should_request_name_from_user(should_request_name_from_user_) .with_should_request_name_from_user(should_request_name_from_user_)
.with_should_request_expiration_date_from_user( .with_should_request_expiration_date_from_user(
should_request_expiration_date_from_user_) should_request_expiration_date_from_user_)
.with_show_prompt(show_save_prompt_.value()), .with_show_prompt(show_save_prompt_.value_or(true)),
base::BindOnce(&CreditCardSaveManager::OnUserDidDecideOnUploadSave, base::BindOnce(&CreditCardSaveManager::OnUserDidDecideOnUploadSave,
weak_ptr_factory_.GetWeakPtr())); weak_ptr_factory_.GetWeakPtr()));
client_->LoadRiskData( client_->LoadRiskData(
...@@ -581,7 +581,7 @@ void CreditCardSaveManager::OfferCardUploadSave() { ...@@ -581,7 +581,7 @@ void CreditCardSaveManager::OfferCardUploadSave() {
AutofillMetrics::UPLOAD_NOT_OFFERED_MAX_STRIKES_ON_MOBILE; AutofillMetrics::UPLOAD_NOT_OFFERED_MAX_STRIKES_ON_MOBILE;
} }
LogCardUploadDecisions(upload_decision_metrics_); LogCardUploadDecisions(upload_decision_metrics_);
if (!show_save_prompt_.value()) { if (show_save_prompt_.has_value() && !show_save_prompt_.value()) {
AutofillMetrics::LogCreditCardSaveNotOfferedDueToMaxStrikesMetric( AutofillMetrics::LogCreditCardSaveNotOfferedDueToMaxStrikesMetric(
AutofillMetrics::SaveTypeMetric::SERVER); AutofillMetrics::SaveTypeMetric::SERVER);
} }
...@@ -992,7 +992,7 @@ void CreditCardSaveManager::SendUploadCardRequest() { ...@@ -992,7 +992,7 @@ void CreditCardSaveManager::SendUploadCardRequest() {
void CreditCardSaveManager::OnUserDidIgnoreOrDeclineSave( void CreditCardSaveManager::OnUserDidIgnoreOrDeclineSave(
const base::string16& card_last_four_digits) { const base::string16& card_last_four_digits) {
if (show_save_prompt_ != base::nullopt && show_save_prompt_.value()) { if (show_save_prompt_.has_value() && show_save_prompt_.value()) {
if (base::FeatureList::IsEnabled( if (base::FeatureList::IsEnabled(
features::kAutofillSaveCreditCardUsesStrikeSystemV2)) { features::kAutofillSaveCreditCardUsesStrikeSystemV2)) {
// If the user rejected or ignored save and the offer-to-save bubble or // If the user rejected or ignored save and the offer-to-save bubble or
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment