Commit 4d310087 authored by Jared Saul's avatar Jared Saul Committed by Commit Bot

[Autofill] Add strikes when CC save ignored/denied/fails

This is a retry/relanding of
https://chromium-review.googlesource.com/c/chromium/src/+/1269524.
We're having a *very* hard time reproducing the flakes that the trybots
experienced, but I've made some improvements to attempt to reduce
possible race conditions in the browsertests.

Patch set 1 is the original CL; patch set 2 contains the new changes.

(When a card accrues 3 strikes, future offers to save for that card
should show the omnibox save icon, but NOT pop up the bubble.
On Android, no offer to save is shown at all.)

TBR=mahmadi@chromium.org

Bug: 884817
Change-Id: If641cff300cab6122bfd0a52942376b8b6449fed
Reviewed-on: https://chromium-review.googlesource.com/c/1292169
Commit-Queue: Jared Saul <jsaul@google.com>
Reviewed-by: default avatarEvan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#601278}
parent 2faba286
......@@ -118,6 +118,7 @@ AutofillSaveCardInfoBarDelegateMobileTest::CreateDelegateWithLegalMessage(
std::unique_ptr<ConfirmInfoBarDelegate> delegate(
new AutofillSaveCardInfoBarDelegateMobile(
is_uploading, credit_card, std::move(legal_message),
/*strike_database=*/nullptr,
/*upload_save_card_callback=*/
base::BindOnce(&AutofillSaveCardInfoBarDelegateMobileTest::
UploadSaveCardCallback,
......@@ -130,6 +131,7 @@ AutofillSaveCardInfoBarDelegateMobileTest::CreateDelegateWithLegalMessage(
std::unique_ptr<ConfirmInfoBarDelegate> delegate(
new AutofillSaveCardInfoBarDelegateMobile(
is_uploading, credit_card, std::move(legal_message),
/*strike_database=*/nullptr,
/*upload_save_card_callback=*/
base::OnceCallback<void(const base::string16&)>(),
/*local_save_card_callback=*/
......
......@@ -263,6 +263,7 @@ void ChromeAutofillClient::ConfirmSaveCreditCardLocally(
->AddInfoBar(CreateSaveCardInfoBarMobile(
std::make_unique<AutofillSaveCardInfoBarDelegateMobile>(
false, card, std::unique_ptr<base::DictionaryValue>(nullptr),
GetStrikeDatabase(),
/*upload_save_card_callback=*/
base::OnceCallback<void(const base::string16&)>(),
/*local_save_card_callback=*/std::move(callback), GetPrefs())));
......@@ -287,7 +288,7 @@ void ChromeAutofillClient::ConfirmSaveCreditCardToCloud(
std::unique_ptr<AutofillSaveCardInfoBarDelegateMobile>
save_card_info_bar_delegate_mobile =
std::make_unique<AutofillSaveCardInfoBarDelegateMobile>(
true, card, std::move(legal_message),
true, card, std::move(legal_message), GetStrikeDatabase(),
/*upload_save_card_callback=*/std::move(callback),
/*local_save_card_callback=*/base::Closure(), GetPrefs());
if (save_card_info_bar_delegate_mobile->LegalMessagesParsedSuccessfully()) {
......
......@@ -8,6 +8,7 @@
#include <utility>
#include "base/strings/utf_string_conversions.h"
#include "chrome/browser/autofill/strike_database_factory.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/signin/account_consistency_mode_manager.h"
#include "chrome/browser/signin/account_tracker_service_factory.h"
......@@ -27,6 +28,7 @@
#include "chrome/common/url_constants.h"
#include "components/autofill/core/browser/autofill_experiments.h"
#include "components/autofill/core/browser/autofill_metrics.h"
#include "components/autofill/core/browser/strike_database.h"
#include "components/autofill/core/common/autofill_clock.h"
#include "components/autofill/core/common/autofill_constants.h"
#include "components/autofill/core/common/autofill_features.h"
......@@ -48,7 +50,8 @@ SaveCardBubbleControllerImpl::SaveCardBubbleControllerImpl(
: content::WebContentsObserver(web_contents),
web_contents_(web_contents),
pref_service_(
user_prefs::UserPrefs::Get(web_contents->GetBrowserContext())) {
user_prefs::UserPrefs::Get(web_contents->GetBrowserContext())),
weak_ptr_factory_(this) {
security_state::SecurityInfo security_info;
SecurityStateTabHelper::FromWebContents(web_contents)
->GetSecurityInfo(&security_info);
......@@ -71,12 +74,13 @@ void SaveCardBubbleControllerImpl::OfferLocalSave(
is_upload_save_ = false;
is_reshow_ = false;
should_request_name_from_user_ = false;
show_bubble_ = show_bubble;
legal_message_lines_.clear();
card_ = card;
local_save_card_callback_ = std::move(save_card_callback);
current_bubble_type_ = BubbleType::LOCAL_SAVE;
if (show_bubble) {
if (show_bubble_) {
ShowBubble();
AutofillMetrics::LogSaveCardPromptMetric(
AutofillMetrics::SAVE_CARD_PROMPT_SHOW_REQUESTED, is_upload_save_,
......@@ -106,8 +110,9 @@ void SaveCardBubbleControllerImpl::OfferUploadSave(
is_upload_save_ = true;
is_reshow_ = false;
should_request_name_from_user_ = should_request_name_from_user;
if (show_bubble) {
// Can't move this into the other "if (show_bubble)" below because an
show_bubble_ = show_bubble;
if (show_bubble_) {
// Can't move this into the other "if (show_bubble_)" below because an
// invalid legal message would skip it.
AutofillMetrics::LogSaveCardPromptMetric(
AutofillMetrics::SAVE_CARD_PROMPT_SHOW_REQUESTED, is_upload_save_,
......@@ -132,7 +137,7 @@ void SaveCardBubbleControllerImpl::OfferUploadSave(
upload_save_card_callback_ = std::move(save_card_callback);
current_bubble_type_ = BubbleType::UPLOAD_SAVE;
if (show_bubble)
if (show_bubble_)
ShowBubble();
else
ShowIconOnly();
......@@ -374,6 +379,19 @@ void SaveCardBubbleControllerImpl::OnCancelButton() {
pref_service_->SetInteger(
prefs::kAutofillAcceptSaveCreditCardPromptState,
prefs::PREVIOUS_SAVE_CREDIT_CARD_PROMPT_USER_DECISION_DENIED);
if (show_bubble_ &&
base::FeatureList::IsEnabled(
features::kAutofillSaveCreditCardUsesStrikeSystem)) {
// If save was cancelled and the bubble was actually shown (NOT just the
// icon), count that as a strike against offering save in the future.
StrikeDatabase* strike_database = GetStrikeDatabase();
strike_database->AddStrike(
strike_database->GetKeyForCreditCardSave(
base::UTF16ToUTF8(card_.LastFourDigits())),
base::BindRepeating(
&SaveCardBubbleControllerImpl::OnStrikeChangeComplete,
weak_ptr_factory_.GetWeakPtr()));
}
}
}
......@@ -409,6 +427,8 @@ void SaveCardBubbleControllerImpl::OnBubbleClosed() {
if (current_bubble_type_ == BubbleType::SIGN_IN_PROMO)
current_bubble_type_ = BubbleType::MANAGE_CARDS;
UpdateIcon();
if (observer_for_testing_)
observer_for_testing_->OnBubbleClosed();
}
void SaveCardBubbleControllerImpl::OnAnimationEnded() {
......@@ -479,6 +499,20 @@ void SaveCardBubbleControllerImpl::DidFinishNavigation(
pref_service_->GetInteger(
prefs::kAutofillAcceptSaveCreditCardPromptState),
GetSecurityLevel());
if (base::FeatureList::IsEnabled(
features::kAutofillSaveCreditCardUsesStrikeSystem) &&
show_bubble_) {
// If the save offer was ignored and the bubble was actually shown (NOT
// just the icon), count that as a strike against offering save in the
// future.
StrikeDatabase* strike_database = GetStrikeDatabase();
strike_database->AddStrike(
strike_database->GetKeyForCreditCardSave(
base::UTF16ToUTF8(card_.LastFourDigits())),
base::BindRepeating(
&SaveCardBubbleControllerImpl::OnStrikeChangeComplete,
weak_ptr_factory_.GetWeakPtr()));
}
}
}
......@@ -506,6 +540,15 @@ void SaveCardBubbleControllerImpl::FetchAccountInfo() {
signin_manager->GetAuthenticatedAccountId());
}
StrikeDatabase* SaveCardBubbleControllerImpl::GetStrikeDatabase() {
Profile* profile = GetProfile();
// No need to return a StrikeDatabase in incognito mode. We don't allow saving
// of Autofill data while in incognito, so an incognito code path should never
// get this far.
DCHECK(profile && !profile->IsOffTheRecord());
return StrikeDatabaseFactory::GetForProfile(profile);
}
void SaveCardBubbleControllerImpl::ShowBubble() {
DCHECK(current_bubble_type_ != BubbleType::INACTIVE);
// Upload save callback should not be null for UPLOAD_SAVE state.
......@@ -598,6 +641,12 @@ void SaveCardBubbleControllerImpl::OpenUrl(const GURL& url) {
ui::PAGE_TRANSITION_LINK, false));
}
void SaveCardBubbleControllerImpl::OnStrikeChangeComplete(
const int num_strikes) {
if (observer_for_testing_)
observer_for_testing_->OnSCBCStrikeChangeComplete();
}
security_state::SecurityLevel SaveCardBubbleControllerImpl::GetSecurityLevel()
const {
return security_level_;
......
......@@ -21,6 +21,7 @@ class PrefService;
namespace autofill {
enum class BubbleType;
class StrikeDatabase;
// Implementation of per-tab class to control the save credit card bubble and
// Omnibox icon.
......@@ -34,6 +35,8 @@ class SaveCardBubbleControllerImpl
class ObserverForTest {
public:
virtual void OnBubbleShown() = 0;
virtual void OnBubbleClosed() = 0;
virtual void OnSCBCStrikeChangeComplete() = 0;
};
~SaveCardBubbleControllerImpl() override;
......@@ -131,6 +134,9 @@ class SaveCardBubbleControllerImpl
void FetchAccountInfo();
// Fetches the Autofill StrikeDatabase for the current profile.
StrikeDatabase* GetStrikeDatabase();
// Displays both the offer-to-save bubble and is associated omnibox icon.
void ShowBubble();
......@@ -140,6 +146,10 @@ class SaveCardBubbleControllerImpl
// Update the visibility and toggled state of the Omnibox save card icon.
void UpdateIcon();
// Used for browsertests. Gives the |observer_for_testing_| a notification
// a strike change has been made.
void OnStrikeChangeComplete(const int num_strikes);
void OpenUrl(const GURL& url);
// For testing.
......@@ -184,6 +194,11 @@ class SaveCardBubbleControllerImpl
// requesting the cardholder name.
bool should_request_name_from_user_ = false;
// Whether the offer-to-save bubble should be shown or not. If true, behaves
// normally. If false, the omnibox icon will be displayed when offering credit
// card save, but the bubble itself will not pop up.
bool show_bubble_ = true;
// The account info of the signed-in user.
AccountInfo account_info_;
......@@ -203,6 +218,8 @@ class SaveCardBubbleControllerImpl
// Observer for when a bubble is created. Initialized only during tests.
ObserverForTest* observer_for_testing_ = nullptr;
base::WeakPtrFactory<SaveCardBubbleControllerImpl> weak_ptr_factory_;
DISALLOW_COPY_AND_ASSIGN(SaveCardBubbleControllerImpl);
};
......
......@@ -59,6 +59,8 @@ const char kResponseGetUploadDetailsSuccess[] =
const char kResponseGetUploadDetailsFailure[] =
"{\"error\":{\"code\":\"FAILED_PRECONDITION\",\"user_error_message\":\"An "
"unexpected error has occurred. Please try again later.\"}}";
const char kURLUploadCardRequest[] =
"https://payments.google.com/payments/apis/chromepaymentsservice/savecard";
const double kFakeGeolocationLatitude = 1.23;
const double kFakeGeolocationLongitude = 4.56;
......@@ -140,11 +142,31 @@ void SaveCardBubbleViewsBrowserTestBase::OnSentUploadCardRequest() {
event_waiter_->OnEvent(DialogEvent::SENT_UPLOAD_CARD_REQUEST);
}
void SaveCardBubbleViewsBrowserTestBase::OnReceivedUploadCardResponse() {
if (event_waiter_)
event_waiter_->OnEvent(DialogEvent::RECEIVED_UPLOAD_CARD_RESPONSE);
}
void SaveCardBubbleViewsBrowserTestBase::OnCCSMStrikeChangeComplete() {
if (event_waiter_)
event_waiter_->OnEvent(DialogEvent::STRIKE_CHANGE_COMPLETE);
}
void SaveCardBubbleViewsBrowserTestBase::OnBubbleShown() {
if (event_waiter_)
event_waiter_->OnEvent(DialogEvent::BUBBLE_SHOWN);
}
void SaveCardBubbleViewsBrowserTestBase::OnBubbleClosed() {
if (event_waiter_)
event_waiter_->OnEvent(DialogEvent::BUBBLE_CLOSED);
}
void SaveCardBubbleViewsBrowserTestBase::OnSCBCStrikeChangeComplete() {
if (event_waiter_)
event_waiter_->OnEvent(DialogEvent::STRIKE_CHANGE_COMPLETE);
}
void SaveCardBubbleViewsBrowserTestBase::SetUpInProcessBrowserTestFixture() {
will_create_browser_context_services_subscription_ =
BrowserContextDependencyManager::GetInstance()
......@@ -348,6 +370,11 @@ void SaveCardBubbleViewsBrowserTestBase::SetUploadDetailsRpcServerError() {
net::HTTP_INTERNAL_SERVER_ERROR);
}
void SaveCardBubbleViewsBrowserTestBase::SetUploadCardRpcPaymentsFails() {
test_url_loader_factory()->AddResponse(kURLUploadCardRequest,
kResponseGetUploadDetailsFailure);
}
void SaveCardBubbleViewsBrowserTestBase::ClickOnView(views::View* view) {
DCHECK(view);
ui::MouseEvent pressed(ui::ET_MOUSE_PRESSED, gfx::Point(), gfx::Point(),
......@@ -425,8 +452,10 @@ views::View* SaveCardBubbleViewsBrowserTestBase::FindViewInBubbleById(
void SaveCardBubbleViewsBrowserTestBase::ClickOnCloseButton() {
SaveCardBubbleViews* save_card_bubble_views = GetSaveCardBubbleViews();
DCHECK(save_card_bubble_views);
ResetEventWaiterForSequence({DialogEvent::BUBBLE_CLOSED});
ClickOnDialogViewAndWait(
save_card_bubble_views->GetBubbleFrameView()->GetCloseButtonForTest());
DCHECK(!GetSaveCardBubbleViews());
}
SaveCardBubbleViews*
......@@ -477,6 +506,10 @@ void SaveCardBubbleViewsBrowserTestBase::WaitForObservedEvent() {
event_waiter_->Wait();
}
void SaveCardBubbleViewsBrowserTestBase::ReturnToInitialPage() {
NavigateTo(test_file_path_);
}
network::TestURLLoaderFactory*
SaveCardBubbleViewsBrowserTestBase::test_url_loader_factory() {
return &test_url_loader_factory_;
......
......@@ -47,7 +47,10 @@ class SaveCardBubbleViewsBrowserTestBase
REQUESTED_UPLOAD_SAVE,
RECEIVED_GET_UPLOAD_DETAILS_RESPONSE,
SENT_UPLOAD_CARD_REQUEST,
BUBBLE_SHOWN
RECEIVED_UPLOAD_CARD_RESPONSE,
STRIKE_CHANGE_COMPLETE,
BUBBLE_SHOWN,
BUBBLE_CLOSED
};
protected:
......@@ -66,9 +69,13 @@ class SaveCardBubbleViewsBrowserTestBase
void OnDecideToRequestUploadSave() override;
void OnReceivedGetUploadDetailsResponse() override;
void OnSentUploadCardRequest() override;
void OnReceivedUploadCardResponse() override;
void OnCCSMStrikeChangeComplete() override;
// SaveCardBubbleControllerImpl::ObserverForTest:
void OnBubbleShown() override;
void OnBubbleClosed() override;
void OnSCBCStrikeChangeComplete() override;
// BrowserTestBase:
void SetUpInProcessBrowserTestFixture() override;
......@@ -95,6 +102,7 @@ class SaveCardBubbleViewsBrowserTestBase
void SetUploadDetailsRpcPaymentsAccepts();
void SetUploadDetailsRpcPaymentsDeclines();
void SetUploadDetailsRpcServerError();
void SetUploadCardRpcPaymentsFails();
// Clicks on the given views::View*.
void ClickOnView(views::View* view);
......@@ -139,6 +147,8 @@ class SaveCardBubbleViewsBrowserTestBase
// Wait for the event(s) passed to ResetEventWaiter*() to occur.
void WaitForObservedEvent();
void ReturnToInitialPage();
network::TestURLLoaderFactory* test_url_loader_factory();
std::unique_ptr<
......
......@@ -12,6 +12,7 @@
#include "components/autofill/core/browser/autofill_experiments.h"
#include "components/autofill/core/browser/credit_card.h"
#include "components/autofill/core/browser/legal_message_line.h"
#include "components/autofill/core/browser/strike_database.h"
#include "components/autofill/core/common/autofill_constants.h"
#include "components/autofill/core/common/autofill_features.h"
#include "components/autofill/core/common/autofill_prefs.h"
......@@ -30,6 +31,7 @@ AutofillSaveCardInfoBarDelegateMobile::AutofillSaveCardInfoBarDelegateMobile(
bool upload,
const CreditCard& card,
std::unique_ptr<base::DictionaryValue> legal_message,
StrikeDatabase* strike_database,
base::OnceCallback<void(const base::string16&)> upload_save_card_callback,
base::OnceClosure local_save_card_callback,
PrefService* pref_service)
......@@ -38,12 +40,13 @@ AutofillSaveCardInfoBarDelegateMobile::AutofillSaveCardInfoBarDelegateMobile(
upload_save_card_callback_(std::move(upload_save_card_callback)),
local_save_card_callback_(std::move(local_save_card_callback)),
pref_service_(pref_service),
strike_database_(strike_database),
had_user_interaction_(false),
issuer_icon_id_(CreditCard::IconResourceId(card.network())),
card_label_(card.NetworkAndLastFourDigits()),
card_sub_label_(card.AbbreviatedExpirationDateForDisplay(
!features::
IsAutofillSaveCardDialogUnlabeledExpirationDateEnabled())) {
!features::IsAutofillSaveCardDialogUnlabeledExpirationDateEnabled())),
card_last_four_digits_(card.LastFourDigits()) {
if (upload) {
DCHECK(!upload_save_card_callback_.is_null());
DCHECK(local_save_card_callback_.is_null());
......@@ -70,8 +73,18 @@ AutofillSaveCardInfoBarDelegateMobile::AutofillSaveCardInfoBarDelegateMobile(
AutofillSaveCardInfoBarDelegateMobile::
~AutofillSaveCardInfoBarDelegateMobile() {
if (!had_user_interaction_)
if (!had_user_interaction_) {
LogUserAction(AutofillMetrics::INFOBAR_IGNORED);
if (base::FeatureList::IsEnabled(
features::kAutofillSaveCreditCardUsesStrikeSystem)) {
// If the infobar was ignored, count that as a strike against offering
// save in the future.
strike_database_->AddStrike(
strike_database_->GetKeyForCreditCardSave(
base::UTF16ToUTF8(card_last_four_digits_)),
base::DoNothing());
}
}
}
void AutofillSaveCardInfoBarDelegateMobile::OnLegalMessageLinkClicked(
......@@ -135,6 +148,14 @@ bool AutofillSaveCardInfoBarDelegateMobile::ShouldExpire(
void AutofillSaveCardInfoBarDelegateMobile::InfoBarDismissed() {
LogUserAction(AutofillMetrics::INFOBAR_DENIED);
if (base::FeatureList::IsEnabled(
features::kAutofillSaveCreditCardUsesStrikeSystem)) {
// If the infobar was explicitly denied, count that as a strike against
// offering save in the future.
strike_database_->AddStrike(strike_database_->GetKeyForCreditCardSave(
base::UTF16ToUTF8(card_last_four_digits_)),
base::DoNothing());
}
}
int AutofillSaveCardInfoBarDelegateMobile::GetButtons() const {
......
......@@ -23,6 +23,7 @@ class DictionaryValue;
namespace autofill {
class CreditCard;
class StrikeDatabase;
// An InfoBarDelegate that enables the user to allow or deny storing credit
// card information gathered from a form submission. Only used on mobile.
......@@ -32,6 +33,7 @@ class AutofillSaveCardInfoBarDelegateMobile : public ConfirmInfoBarDelegate {
bool upload,
const CreditCard& card,
std::unique_ptr<base::DictionaryValue> legal_message,
StrikeDatabase* strike_database,
base::OnceCallback<void(const base::string16&)> upload_save_card_callback,
base::OnceClosure local_save_card_callback,
PrefService* pref_service);
......@@ -85,6 +87,9 @@ class AutofillSaveCardInfoBarDelegateMobile : public ConfirmInfoBarDelegate {
// Weak reference to read & write |kAutofillAcceptSaveCreditCardPromptState|,
PrefService* pref_service_;
// Weak reference to the Autofill StrikeDatabase.
StrikeDatabase* strike_database_;
// Did the user ever explicitly accept or dismiss this infobar?
bool had_user_interaction_;
......@@ -97,6 +102,9 @@ class AutofillSaveCardInfoBarDelegateMobile : public ConfirmInfoBarDelegate {
// The sub-label for the card to show in the content of the infobar.
base::string16 card_sub_label_;
// The last four digits of the card for which save is being offered.
base::string16 card_last_four_digits_;
// The legal messages to show in the content of the infobar.
LegalMessageLines legal_messages_;
......
......@@ -37,6 +37,7 @@
#include "components/autofill/core/browser/strike_database.h"
#include "components/autofill/core/browser/validation.h"
#include "components/autofill/core/common/autofill_clock.h"
#include "components/autofill/core/common/autofill_constants.h"
#include "components/autofill/core/common/autofill_features.h"
#include "components/autofill/core/common/autofill_util.h"
#include "services/identity/public/cpp/identity_manager.h"
......@@ -46,11 +47,6 @@ namespace autofill {
namespace {
// If the Autofill StrikeDatabase returns this many strikes for a given card, it
// will not show the offer-to-save bubble on Desktop or infobar on Android.
// On Desktop, however, the omnibox icon will still be available.
const int kMaxStrikesToPreventPoppingUpOfferToSavePrompt = 3;
// If |name| consists of three whitespace-separated parts and the second of the
// three parts is a single character or a single character followed by a period,
// returns the result of joining the first and third parts with a space.
......@@ -92,6 +88,7 @@ CreditCardSaveManager::~CreditCardSaveManager() {}
void CreditCardSaveManager::AttemptToOfferCardLocalSave(
const CreditCard& card) {
local_card_save_candidate_ = card;
show_save_prompt_ = base::nullopt;
// Query the Autofill StrikeDatabase on if we should pop up the offer-to-save
// prompt for this card.
......@@ -119,6 +116,7 @@ void CreditCardSaveManager::AttemptToOfferCardUploadSave(
upload_request_ = payments::PaymentsClient::UploadRequestDetails();
upload_request_.card = card;
uploading_local_card_ = uploading_local_card;
show_save_prompt_ = base::nullopt;
// In an ideal scenario, when uploading a card, we would have:
// 1) Card number and expiration
......@@ -252,6 +250,9 @@ bool CreditCardSaveManager::IsUploadEnabledForNetwork(
void CreditCardSaveManager::OnDidUploadCard(
AutofillClient::PaymentsRpcResult result,
const std::string& server_id) {
if (observer_for_testing_)
observer_for_testing_->OnReceivedUploadCardResponse();
if (result == AutofillClient::SUCCESS &&
upload_request_.card.HasFirstAndLastName()) {
AutofillMetrics::LogSaveCardWithFirstAndLastNameComplete(
......@@ -287,6 +288,19 @@ void CreditCardSaveManager::OnDidUploadCard(
base::UTF16ToUTF8(upload_request_.card.LastFourDigits())),
base::DoNothing());
}
} else {
if (base::FeatureList::IsEnabled(
features::kAutofillSaveCreditCardUsesStrikeSystem) &&
show_save_prompt_.value()) {
// If the upload failed and the bubble was actually shown (NOT just the
// icon), count that as a strike against offering upload in the future.
StrikeDatabase* strike_database = client_->GetStrikeDatabase();
strike_database->AddStrike(
strike_database->GetKeyForCreditCardSave(
base::UTF16ToUTF8(upload_request_.card.LastFourDigits())),
base::BindRepeating(&CreditCardSaveManager::OnStrikeChangeComplete,
weak_ptr_factory_.GetWeakPtr()));
}
}
}
......@@ -699,6 +713,11 @@ void CreditCardSaveManager::SendUploadCardRequest() {
weak_ptr_factory_.GetWeakPtr()));
}
void CreditCardSaveManager::OnStrikeChangeComplete(const int num_strikes) {
if (observer_for_testing_)
observer_for_testing_->OnCCSMStrikeChangeComplete();
}
AutofillMetrics::CardUploadDecisionMetric
CreditCardSaveManager::GetCVCCardUploadDecisionMetric() const {
// This function assumes a valid CVC was not found.
......
......@@ -76,6 +76,8 @@ class CreditCardSaveManager {
virtual void OnDecideToRequestUploadSave() = 0;
virtual void OnReceivedGetUploadDetailsResponse() = 0;
virtual void OnSentUploadCardRequest() = 0;
virtual void OnReceivedUploadCardResponse() = 0;
virtual void OnCCSMStrikeChangeComplete() = 0;
};
// The parameters should outlive the CreditCardSaveManager.
......@@ -179,6 +181,10 @@ class CreditCardSaveManager {
// Finalizes the upload request and calls PaymentsClient::UploadCard().
void SendUploadCardRequest();
// Used for browsertests. Gives the |observer_for_testing_| a notification
// a strike change has been made.
void OnStrikeChangeComplete(const int num_strikes);
// Returns metric relevant to the CVC field based on values in
// |found_cvc_field_|, |found_value_in_cvc_field_| and
// |found_cvc_value_in_non_cvc_field_|. Only called when a valid CVC was NOT
......
......@@ -134,11 +134,11 @@ void StrikeDatabase::OnAddStrikeComplete(StrikesCallback callback,
std::string key,
bool success) {
if (success) {
callback.Run(num_strikes);
if (GetPrefixFromKey(key) == kKeyPrefixForCreditCardSave) {
base::UmaHistogramCounts1000(
"Autofill.StrikeDatabase.NthStrikeAdded.CreditCardSave", num_strikes);
}
callback.Run(num_strikes);
} else {
callback.Run(0);
}
......
......@@ -76,7 +76,9 @@ class StrikeDatabase : public KeyedService {
const ClearStrikesCallback& outer_callback);
// Returns concatenation of prefix + |card_last_four_digits| to be used as key
// for credit card save.
// for credit card save. Expiration date is not included for privacy reasons,
// as conflicting last-four should be a rare event, and it's not a huge issue
// if we stop showing save bubbles a little earlier than usual in rare cases.
std::string GetKeyForCreditCardSave(const std::string& card_last_four_digits);
protected:
......
......@@ -37,6 +37,12 @@ enum ShowPasswordSuggestionsOptions {
IS_PASSWORD_FIELD = 1 << 1 /* input field is a password field */
};
// Autofill StrikeDatabase: Maximum strikes allowed for the credit card save
// project. If the StrikeDatabase returns this many strikes for a given card, it
// will not show the offer-to-save bubble on Desktop or infobar on Android.
// On Desktop, however, the omnibox icon will still be available.
const int kMaxStrikesToPreventPoppingUpOfferToSavePrompt = 3;
} // namespace autofill
#endif // COMPONENTS_AUTOFILL_CORE_COMMON_AUTOFILL_CONSTANTS_H_
......@@ -22,6 +22,10 @@
- (void)sentUploadCardRequest;
- (void)receivedUploadCardResponse;
- (void)ccsmStrikeChangeComplete;
@end
namespace autofill {
......@@ -41,6 +45,8 @@ class CreditCardSaveManagerTestObserverBridge
void OnDecideToRequestUploadSave() override;
void OnReceivedGetUploadDetailsResponse() override;
void OnSentUploadCardRequest() override;
void OnReceivedUploadCardResponse() override;
void OnCCSMStrikeChangeComplete() override;
private:
__weak id<CreditCardSaveManagerTestObserver> observer_ = nil;
......
......@@ -39,4 +39,12 @@ void CreditCardSaveManagerTestObserverBridge::OnSentUploadCardRequest() {
[observer_ sentUploadCardRequest];
}
void CreditCardSaveManagerTestObserverBridge::OnReceivedUploadCardResponse() {
[observer_ receivedUploadCardResponse];
}
void CreditCardSaveManagerTestObserverBridge::OnCCSMStrikeChangeComplete() {
[observer_ ccsmStrikeChangeComplete];
}
} // namespace autofill
......@@ -174,6 +174,7 @@ void ChromeAutofillClientIOS::ConfirmSaveCreditCardLocally(
infobar_manager_->AddInfoBar(CreateSaveCardInfoBarMobile(
std::make_unique<AutofillSaveCardInfoBarDelegateMobile>(
false, card, std::unique_ptr<base::DictionaryValue>(nullptr),
GetStrikeDatabase(),
/*upload_save_card_callback=*/
base::OnceCallback<void(const base::string16&)>(),
/*local_save_card_callback=*/std::move(callback), GetPrefs())));
......@@ -200,7 +201,7 @@ void ChromeAutofillClientIOS::ConfirmSaveCreditCardToCloud(
DCHECK(show_prompt);
auto save_card_info_bar_delegate_mobile =
std::make_unique<AutofillSaveCardInfoBarDelegateMobile>(
true, card, std::move(legal_message),
true, card, std::move(legal_message), GetStrikeDatabase(),
/*upload_save_card_callback=*/std::move(callback),
/*local_save_card_callback=*/base::Closure(), GetPrefs());
// Allow user to save card only if legal messages are successfully parsed.
......
......@@ -65,6 +65,8 @@ enum InfobarEvent : int {
REQUESTED_UPLOAD_SAVE,
RECEIVED_GET_UPLOAD_DETAILS_RESPONSE,
SENT_UPLOAD_CARD_REQUEST,
RECEIVED_UPLOAD_CARD_RESPONSE,
STRIKE_CHANGE_COMPLETE
};
id<GREYMatcher> closeButtonMatcher() {
......@@ -201,6 +203,14 @@ class SaveCardInfobarEGTestHelper {
[self onEvent:InfobarEvent::SENT_UPLOAD_CARD_REQUEST];
}
- (void)receivedUploadCardResponse {
[self onEvent:InfobarEvent::RECEIVED_UPLOAD_CARD_RESPONSE];
}
- (void)ccsmStrikeChangeComplete {
[self onEvent:InfobarEvent::STRIKE_CHANGE_COMPLETE];
}
#pragma mark - Page interaction helper methods
- (void)fillAndSubmitFormWithCardDetailsOnly {
......
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