Handle failed signin when setting kAutofillDialogPayWithoutWallet pref.

BUG=244902
R=dbeam@chromium.org

Review URL: https://codereview.chromium.org/15871004

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

4 files changed, 95 insertions, 41 deletions

diff --git a/chrome/browser/ui/autofill/account_chooser_model.cc b/chrome/browser/ui/autofill/account_chooser_model.cc
index daf73d2..299ad40 100644
--- a/chrome/browser/ui/autofill/account_chooser_model.cc
+++ b/chrome/browser/ui/autofill/account_chooser_model.cc
@@ -91,20 +91,20 @@ void AccountChooserModel::ExecuteCommand(int command_id, int event_flags) {
     return;
 
   // Log metrics.
-   AutofillMetrics::DialogUiEvent chooser_event;
-   if (command_id == kAutofillItemId) {
-     chooser_event =
-         AutofillMetrics::DIALOG_UI_ACCOUNT_CHOOSER_SWITCHED_TO_AUTOFILL;
-   } else if (checked_item_ == kAutofillItemId) {
-     chooser_event =
-         AutofillMetrics::DIALOG_UI_ACCOUNT_CHOOSER_SWITCHED_TO_WALLET;
-   } else {
-     chooser_event =
-         AutofillMetrics::DIALOG_UI_ACCOUNT_CHOOSER_SWITCHED_WALLET_ACCOUNT;
-   }
-   metric_logger_.LogDialogUiEvent(dialog_type_, chooser_event);
-
-   checked_item_ = command_id;
+  AutofillMetrics::DialogUiEvent chooser_event;
+  if (command_id == kAutofillItemId) {
+    chooser_event =
+        AutofillMetrics::DIALOG_UI_ACCOUNT_CHOOSER_SWITCHED_TO_AUTOFILL;
+  } else if (checked_item_ == kAutofillItemId) {
+    chooser_event =
+        AutofillMetrics::DIALOG_UI_ACCOUNT_CHOOSER_SWITCHED_TO_WALLET;
+  } else {
+    chooser_event =
+        AutofillMetrics::DIALOG_UI_ACCOUNT_CHOOSER_SWITCHED_WALLET_ACCOUNT;
+  }
+  metric_logger_.LogDialogUiEvent(dialog_type_, chooser_event);
+
+  checked_item_ = command_id;
   ReconstructMenuItems();
   delegate_->AccountChoiceChanged();
 }
diff --git a/chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc b/chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc
index d6ee51e..ba4dcde 100644
--- a/chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc
+++ b/chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc
@@ -743,13 +743,6 @@ void AutofillDialogControllerImpl::OnWalletOrSigninUpdate() {
     initial_user_state_ = GetInitialUserState();
 }
 
-void AutofillDialogControllerImpl::OnWalletSigninError() {
-  signin_helper_.reset();
-  account_chooser_model_.SetHadWalletSigninError();
-  GetWalletClient()->CancelRequests();
-  LogDialogLatencyToShow();
-}
-
 void AutofillDialogControllerImpl::EnsureLegalDocumentsText() {
   if (!wallet_items_ || wallet_items_->legal_documents().empty())
     return;
@@ -1977,6 +1970,13 @@ void AutofillDialogControllerImpl::OpenTabWithUrl(const GURL& url) {
 #endif
 }
 
+void AutofillDialogControllerImpl::OnWalletSigninError() {
+  signin_helper_.reset();
+  account_chooser_model_.SetHadWalletSigninError();
+  GetWalletClient()->CancelRequests();
+  LogDialogLatencyToShow();
+}
+
 void AutofillDialogControllerImpl::DisableWallet() {
   signin_helper_.reset();
   wallet_items_.reset();
@@ -2725,12 +2725,15 @@ void AutofillDialogControllerImpl::FinishSubmit() {
   LogOnFinishSubmitMetrics();
 
   // On a successful submit, if the user manually selected "pay without wallet",
-  // stop trying to pay with Wallet on future runs of the dialog.
-  bool manually_selected_pay_without_wallet =
-      !account_chooser_model_.WalletIsSelected() &&
-      !account_chooser_model_.had_wallet_error();
-  profile_->GetPrefs()->SetBoolean(::prefs::kAutofillDialogPayWithoutWallet,
-                                   manually_selected_pay_without_wallet);
+  // stop trying to pay with Wallet on future runs of the dialog. On the other
+  // hand, if there was an error that prevented the user from having the choice
+  // of using Wallet, leave the pref alone.
+  if (!account_chooser_model_.had_wallet_error() &&
+      account_chooser_model_.HasAccountsToChoose()) {
+    profile_->GetPrefs()->SetBoolean(
+        ::prefs::kAutofillDialogPayWithoutWallet,
+        !account_chooser_model_.WalletIsSelected());
+  }
 
   switch (GetDialogType()) {
     case DIALOG_TYPE_AUTOCHECKOUT:
diff --git a/chrome/browser/ui/autofill/autofill_dialog_controller_impl.h b/chrome/browser/ui/autofill/autofill_dialog_controller_impl.h
index 5f03962..bcb35ab 100644
--- a/chrome/browser/ui/autofill/autofill_dialog_controller_impl.h
+++ b/chrome/browser/ui/autofill/autofill_dialog_controller_impl.h
@@ -278,6 +278,9 @@ class AutofillDialogControllerImpl : public AutofillDialogController,
   // happens when a user clicks "Edit" or a suggestion is invalid.
   virtual bool IsEditingExistingData(DialogSection section) const;
 
+  // Should be called on the Wallet sign-in error.
+  virtual void OnWalletSigninError();
+
  private:
   // Whether or not the current request wants credit info back.
   bool RequestingCreditCardInfo() const;
@@ -305,9 +308,6 @@ class AutofillDialogControllerImpl : public AutofillDialogController,
   // Refreshes the model on Wallet or sign-in state update.
   void OnWalletOrSigninUpdate();
 
-  // Should be called on the Wallet sign-in error.
-  void OnWalletSigninError();
-
   // Calculates |legal_documents_text_| and |legal_document_link_ranges_| if
   // they have not already been calculated.
   void EnsureLegalDocumentsText();
diff --git a/chrome/browser/ui/autofill/autofill_dialog_controller_unittest.cc b/chrome/browser/ui/autofill/autofill_dialog_controller_unittest.cc
index ff31fff..3eea31d 100644
--- a/chrome/browser/ui/autofill/autofill_dialog_controller_unittest.cc
+++ b/chrome/browser/ui/autofill/autofill_dialog_controller_unittest.cc
@@ -242,9 +242,12 @@ class TestAutofillDialogController
 
   void set_dialog_type(DialogType dialog_type) { dialog_type_ = dialog_type; }
 
+  void SimulateSigninError() {
+    OnWalletSigninError();
+  }
+
   MOCK_METHOD0(LoadRiskFingerprintData, void());
   using AutofillDialogControllerImpl::OnDidLoadRiskFingerprintData;
-
   using AutofillDialogControllerImpl::IsEditingExistingData;
 
  protected:
@@ -297,18 +300,11 @@ class AutofillDialogControllerTest : public testing::Test {
 
   // testing::Test implementation:
   virtual void SetUp() OVERRIDE {
-    FormData form_data;
-    for (size_t i = 0; i < arraysize(kFieldsFromPage); ++i) {
-      FormFieldData field;
-      field.autocomplete_attribute = kFieldsFromPage[i];
-      form_data.fields.push_back(field);
-    }
-
     profile()->CreateRequestContext();
     test_web_contents_.reset(
         content::WebContentsTester::CreateTestWebContents(profile(), NULL));
 
-    SetUpControllerWithFormData(form_data);
+    SetUpControllerWithFormData(DefaultFormData());
   }
 
   virtual void TearDown() OVERRIDE {
@@ -317,6 +313,16 @@ class AutofillDialogControllerTest : public testing::Test {
   }
 
  protected:
+  FormData DefaultFormData() {
+    FormData form_data;
+    for (size_t i = 0; i < arraysize(kFieldsFromPage); ++i) {
+      FormFieldData field;
+      field.autocomplete_attribute = kFieldsFromPage[i];
+      form_data.fields.push_back(field);
+    }
+    return form_data;
+  }
+
   void SetUpControllerWithFormData(const FormData& form_data) {
     if (controller_)
       controller_->ViewClosed();
@@ -368,6 +374,10 @@ class AutofillDialogControllerTest : public testing::Test {
         TestAccountChooserModel::kActiveWalletItemId);
   }
 
+  void SimulateSigninError() {
+    controller_->SimulateSigninError();
+  }
+
   void UseBillingForShipping() {
     controller()->MenuModelForSection(SECTION_SHIPPING)->ActivatedAt(0);
   }
@@ -1637,20 +1647,61 @@ TEST_F(AutofillDialogControllerTest, ViewCancelDoesntSetPref) {
       ::prefs::kAutofillDialogPayWithoutWallet));
 }
 
+TEST_F(AutofillDialogControllerTest, SubmitWithSigninErrorDoesntSetPref) {
+  ASSERT_FALSE(profile()->GetPrefs()->HasPrefPath(
+      ::prefs::kAutofillDialogPayWithoutWallet));
+
+  SimulateSigninError();
+  FillCreditCardInputs();
+  controller()->OnAccept();
+
+  EXPECT_FALSE(profile()->GetPrefs()->HasPrefPath(
+      ::prefs::kAutofillDialogPayWithoutWallet));
+}
+
 TEST_F(AutofillDialogControllerTest, ViewSubmitSetsPref) {
   ASSERT_FALSE(profile()->GetPrefs()->HasPrefPath(
       ::prefs::kAutofillDialogPayWithoutWallet));
 
   SwitchToAutofill();
+  FillCreditCardInputs();
+  controller()->OnAccept();
 
-  // We also have to simulate CC inputs to keep the controller happy.
+  EXPECT_TRUE(profile()->GetPrefs()->HasPrefPath(
+      ::prefs::kAutofillDialogPayWithoutWallet));
+  EXPECT_TRUE(profile()->GetPrefs()->GetBoolean(
+      ::prefs::kAutofillDialogPayWithoutWallet));
+
+  // Try again with a signin error (just leaves the pref alone).
+  SetUpControllerWithFormData(DefaultFormData());
+
+  // Setting up the controller again should not change the pref.
+  EXPECT_TRUE(profile()->GetPrefs()->HasPrefPath(
+      ::prefs::kAutofillDialogPayWithoutWallet));
+  EXPECT_TRUE(profile()->GetPrefs()->GetBoolean(
+      ::prefs::kAutofillDialogPayWithoutWallet));
+
+  SimulateSigninError();
   FillCreditCardInputs();
+  controller()->OnAccept();
+  EXPECT_TRUE(profile()->GetPrefs()->HasPrefPath(
+      ::prefs::kAutofillDialogPayWithoutWallet));
+  EXPECT_TRUE(profile()->GetPrefs()->GetBoolean(
+      ::prefs::kAutofillDialogPayWithoutWallet));
 
+  // Succesfully choosing wallet does set the pref.
+  SetUpControllerWithFormData(DefaultFormData());
+
+  SwitchToWallet();
+  scoped_ptr<wallet::WalletItems> wallet_items = wallet::GetTestWalletItems();
+  wallet_items->AddInstrument(wallet::GetTestMaskedInstrument());
+  controller()->OnDidGetWalletItems(wallet_items.Pass());
   controller()->OnAccept();
+  controller()->OnDidGetFullWallet(wallet::GetTestFullWallet());
 
   EXPECT_TRUE(profile()->GetPrefs()->HasPrefPath(
       ::prefs::kAutofillDialogPayWithoutWallet));
-  EXPECT_TRUE(profile()->GetPrefs()->GetBoolean(
+  EXPECT_FALSE(profile()->GetPrefs()->GetBoolean(
       ::prefs::kAutofillDialogPayWithoutWallet));
 }