Pass along risk params on save/update Online Wallet calls

BUG=247830
R=ahutter@chromium.org
TEST=unit_tests and Online Wallet submit flow still works.

Review URL: https://chromiumcodereview.appspot.com/17253004

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@207222 0039d316-1c4b-4281-b951-d872f2087c98

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,