diff options
4 files changed, 36 insertions, 41 deletions
diff --git a/chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc b/chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc index 1b1c4b0a..caa7fcc 100644 --- a/chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc +++ b/chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc @@ -1680,7 +1680,9 @@ void AutofillDialogControllerImpl::OnAccept() { UTF16ToUTF8(view_->GetCvc()), wallet_items_->obfuscated_gaia_id()); } else if (IsPayingWithWallet()) { - SubmitWithWallet(); + // TODO(dbeam): disallow interacting with the dialog while submitting. + // http://crbug.com/230932 + AcceptLegalDocuments(); } else { FinishSubmit(); } @@ -1801,12 +1803,12 @@ DialogType AutofillDialogControllerImpl::GetDialogType() const { } std::string AutofillDialogControllerImpl::GetRiskData() const { - return risk_data_.empty() ? "no pagers" : risk_data_; + DCHECK(!risk_data_.empty()); + return risk_data_; } void AutofillDialogControllerImpl::OnDidAcceptLegalDocuments() { DCHECK(is_submitting_ && IsPayingWithWallet()); - has_accepted_legal_documents_ = true; LoadRiskFingerprintData(); } @@ -2080,10 +2082,6 @@ bool AutofillDialogControllerImpl::IsPayingWithWallet() const { } void AutofillDialogControllerImpl::LoadRiskFingerprintData() { - DCHECK(AreLegalDocumentsCurrent()); - - // Clear potential stale data to ensure |GetFullWalletIfReady()| triggers only - // when a new fingerprint is loaded. risk_data_.clear(); uint64 obfuscated_gaia_id = 0; @@ -2123,7 +2121,7 @@ void AutofillDialogControllerImpl::OnDidLoadRiskFingerprintData( bool success = base::Base64Encode(proto_data, &risk_data_); DCHECK(success); - GetFullWalletIfReady(); + SubmitWithWallet(); } void AutofillDialogControllerImpl::OpenTabWithUrl(const GURL& url) { @@ -2671,17 +2669,10 @@ bool AutofillDialogControllerImpl::AreLegalDocumentsCurrent() const { (wallet_items_ && wallet_items_->legal_documents().empty()); } -void AutofillDialogControllerImpl::SubmitWithWallet() { - // TODO(dbeam): disallow interacting with the dialog while submitting. - // http://crbug.com/230932 - - active_instrument_id_.clear(); - active_address_id_.clear(); - full_wallet_.reset(); - +void AutofillDialogControllerImpl::AcceptLegalDocuments() { content::BrowserThread::PostTask( - content::BrowserThread::IO, FROM_HERE, - base::Bind(&UserDidOptIntoLocationServices)); + content::BrowserThread::IO, FROM_HERE, + base::Bind(&UserDidOptIntoLocationServices)); GetWalletClient()->AcceptLegalDocuments( wallet_items_->legal_documents(), @@ -2690,6 +2681,12 @@ void AutofillDialogControllerImpl::SubmitWithWallet() { if (AreLegalDocumentsCurrent()) LoadRiskFingerprintData(); +} + +void AutofillDialogControllerImpl::SubmitWithWallet() { + active_instrument_id_.clear(); + active_address_id_.clear(); + full_wallet_.reset(); const wallet::WalletItems::MaskedInstrument* active_instrument = ActiveInstrument(); @@ -2740,8 +2737,10 @@ void AutofillDialogControllerImpl::SubmitWithWallet() { // If there's neither an address nor instrument to save, |GetFullWallet()| // is called when the risk fingerprint is loaded. - if (!active_instrument_id_.empty() && !active_address_id_.empty()) + if (!active_instrument_id_.empty() && !active_address_id_.empty()) { + GetFullWallet(); return; + } // If instrument and address aren't based off of any existing data, save both. if (inputted_instrument && inputted_address && !update_request && @@ -2844,10 +2843,8 @@ void AutofillDialogControllerImpl::GetFullWalletIfReady() { DCHECK(is_submitting_); DCHECK(IsPayingWithWallet()); - if (!active_instrument_id_.empty() && !active_address_id_.empty() && - !risk_data_.empty()) { + if (!active_instrument_id_.empty() && !active_address_id_.empty()) GetFullWallet(); - } } void AutofillDialogControllerImpl::HandleSaveOrUpdateRequiredActions( diff --git a/chrome/browser/ui/autofill/autofill_dialog_controller_impl.h b/chrome/browser/ui/autofill/autofill_dialog_controller_impl.h index cf4587a..2748ec3 100644 --- a/chrome/browser/ui/autofill/autofill_dialog_controller_impl.h +++ b/chrome/browser/ui/autofill/autofill_dialog_controller_impl.h @@ -446,6 +446,9 @@ class AutofillDialogControllerImpl : public AutofillDialogController, // Whether the user has accepted all the current legal documents' terms. bool AreLegalDocumentsCurrent() const; + // Accepts any pending legal documents now that the user has pressed Submit. + void AcceptLegalDocuments(); + // Start the submit proccess to interact with Online Wallet (might do various // things like accept documents, save details, update details, respond to // required actions, etc.). diff --git a/chrome/browser/ui/autofill/autofill_dialog_controller_unittest.cc b/chrome/browser/ui/autofill/autofill_dialog_controller_unittest.cc index ab4557d..01ec950 100644 --- a/chrome/browser/ui/autofill/autofill_dialog_controller_unittest.cc +++ b/chrome/browser/ui/autofill/autofill_dialog_controller_unittest.cc @@ -1064,6 +1064,7 @@ TEST_F(AutofillDialogControllerTest, SaveAddress) { controller()->MenuModelForSection(SECTION_SHIPPING); shipping_model->ActivatedAt(shipping_model->GetItemCount() - 1); controller()->OnAccept(); + controller()->OnDidLoadRiskFingerprintData(GetFakeFingerprint().Pass()); } TEST_F(AutofillDialogControllerTest, SaveInstrument) { @@ -1075,6 +1076,7 @@ TEST_F(AutofillDialogControllerTest, SaveInstrument) { wallet_items->AddAddress(wallet::GetTestShippingAddress()); controller()->OnDidGetWalletItems(wallet_items.Pass()); controller()->OnAccept(); + controller()->OnDidLoadRiskFingerprintData(GetFakeFingerprint().Pass()); } TEST_F(AutofillDialogControllerTest, SaveInstrumentWithInvalidInstruments) { @@ -1087,6 +1089,7 @@ TEST_F(AutofillDialogControllerTest, SaveInstrumentWithInvalidInstruments) { wallet_items->AddInstrument(wallet::GetTestMaskedInstrumentInvalid()); controller()->OnDidGetWalletItems(wallet_items.Pass()); controller()->OnAccept(); + controller()->OnDidLoadRiskFingerprintData(GetFakeFingerprint().Pass()); } TEST_F(AutofillDialogControllerTest, SaveInstrumentAndAddress) { @@ -1095,6 +1098,7 @@ TEST_F(AutofillDialogControllerTest, SaveInstrumentAndAddress) { controller()->OnDidGetWalletItems(wallet::GetTestWalletItems()); controller()->OnAccept(); + controller()->OnDidLoadRiskFingerprintData(GetFakeFingerprint().Pass()); } // Tests that editing an address (in wallet mode0 and submitting the dialog @@ -1110,6 +1114,7 @@ TEST_F(AutofillDialogControllerTest, UpdateAddress) { controller()->EditClickedForSection(SECTION_SHIPPING); controller()->OnAccept(); + controller()->OnDidLoadRiskFingerprintData(GetFakeFingerprint().Pass()); } // Tests that editing an instrument (CC + address) in wallet mode updates an @@ -1122,6 +1127,7 @@ TEST_F(AutofillDialogControllerTest, UpdateInstrument) { controller()->EditClickedForSection(SECTION_CC_BILLING); controller()->OnAccept(); + controller()->OnDidLoadRiskFingerprintData(GetFakeFingerprint().Pass()); EXPECT_TRUE( controller()->GetTestingWalletClient()->updated_billing_address()); @@ -1139,6 +1145,7 @@ TEST_F(AutofillDialogControllerTest, UpdateInstrumentSaveAddress) { controller()->EditClickedForSection(SECTION_CC_BILLING); controller()->OnAccept(); + controller()->OnDidLoadRiskFingerprintData(GetFakeFingerprint().Pass()); EXPECT_TRUE( controller()->GetTestingWalletClient()->updated_billing_address()); @@ -1158,6 +1165,7 @@ TEST_F(AutofillDialogControllerTest, SaveInstrumentUpdateAddress) { controller()->EditClickedForSection(SECTION_SHIPPING); controller()->OnAccept(); + controller()->OnDidLoadRiskFingerprintData(GetFakeFingerprint().Pass()); } MATCHER(UsesLocalBillingAddress, "uses the local billing address") { @@ -1179,6 +1187,7 @@ TEST_F(AutofillDialogControllerTest, BillingForShipping) { UseBillingForShipping(); controller()->OnAccept(); + controller()->OnDidLoadRiskFingerprintData(GetFakeFingerprint().Pass()); } // Tests that when using billing address for shipping, and there is an exact @@ -1203,6 +1212,7 @@ TEST_F(AutofillDialogControllerTest, BillingForShippingHasMatch) { UseBillingForShipping(); controller()->OnAccept(); + controller()->OnDidLoadRiskFingerprintData(GetFakeFingerprint().Pass()); } // Tests that adding new instrument and also using billing address for shipping, @@ -1219,6 +1229,7 @@ TEST_F(AutofillDialogControllerTest, BillingForShippingNewInstrument) { UseBillingForShipping(); controller()->OnAccept(); + controller()->OnDidLoadRiskFingerprintData(GetFakeFingerprint().Pass()); } // Test that the local view contents is used when saving a new instrument and @@ -1244,6 +1255,7 @@ TEST_F(AutofillDialogControllerTest, SaveInstrumentSameAsBilling) { EXPECT_CALL(*controller()->GetTestingWalletClient(), SaveAddress(UsesLocalBillingAddress(), _)).Times(1); controller()->OnAccept(); + controller()->OnDidLoadRiskFingerprintData(GetFakeFingerprint().Pass()); EXPECT_TRUE( controller()->GetTestingWalletClient()->updated_billing_address()); @@ -1919,23 +1931,6 @@ TEST_F(AutofillDialogControllerTest, RiskNeverLoadsWithPendingLegalDocuments) { wallet_items->AddLegalDocument(wallet::GetTestLegalDocument()); controller()->OnDidGetWalletItems(wallet_items.Pass()); controller()->OnAccept(); - - EXPECT_EQ("no pagers", controller()->GetRiskData()); -} - -TEST_F(AutofillDialogControllerTest, RiskLoadsWithoutPendingLegalDocuments) { - EXPECT_CALL(*controller(), LoadRiskFingerprintData()).Times(1); - - scoped_ptr<wallet::WalletItems> wallet_items = wallet::GetTestWalletItems(); - wallet_items->AddInstrument(wallet::GetTestMaskedInstrument()); - wallet_items->AddAddress(wallet::GetTestShippingAddress()); - controller()->OnDidGetWalletItems(wallet_items.Pass()); - controller()->OnAccept(); - - EXPECT_EQ("no pagers", controller()->GetRiskData()); - - controller()->OnDidLoadRiskFingerprintData(GetFakeFingerprint().Pass()); - EXPECT_EQ(kFakeFingerprintEncoded, controller()->GetRiskData()); } TEST_F(AutofillDialogControllerTest, RiskLoadsAfterAcceptingLegalDocuments) { @@ -1949,7 +1944,6 @@ TEST_F(AutofillDialogControllerTest, RiskLoadsAfterAcceptingLegalDocuments) { EXPECT_CALL(*controller(), LoadRiskFingerprintData()).Times(1); controller()->OnAccept(); - EXPECT_EQ("no pagers", controller()->GetRiskData()); // Simulate a risk load and verify |GetRiskData()| matches the encoded value. controller()->OnDidAcceptLegalDocuments(); diff --git a/components/autofill/content/browser/wallet/wallet_client.h b/components/autofill/content/browser/wallet/wallet_client.h index d024225..cd06a03 100644 --- a/components/autofill/content/browser/wallet/wallet_client.h +++ b/components/autofill/content/browser/wallet/wallet_client.h @@ -171,7 +171,8 @@ class WalletClient // The GetWalletItems call to the Online Wallet backend may require the user // to accept various legal documents before a FullWallet can be generated. // The |google_transaction_id| is provided in the response to the - // GetWalletItems call. + // GetWalletItems call. If |documents| are empty, |delegate_| will not receive + // a corresponding |OnDidAcceptLegalDocuments()| call. virtual void AcceptLegalDocuments( const std::vector<WalletItems::LegalDocument*>& documents, const std::string& google_transaction_id, |