Handle failed signin when setting kAutofillDialogPayWithoutWallet pref.

chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/autofill/autofill_dialog_controller_impl.h"
 
 #include <algorithm>
 #include <string>
 
 #include "base/base64.h"
@@ skipping 672 matching lines @@
 void AutofillDialogControllerImpl::HideSignIn() {
   signin_registrar_.RemoveAll();
   view_->HideSignIn();
   view_->UpdateAccountChooser();
 }
 
 void AutofillDialogControllerImpl::SignedInStateUpdated() {
   switch (SignedInState()) {
     case SIGNED_IN:
       // Start fetching the user name if we don't know it yet.
       if (account_chooser_model_.active_wallet_account_name().empty()) {

[Dan Beam, 2013/05/31 22:22:53]

nit: and here (!account_chooser_model_.HasAccountsToChoose())

[Evan Stade, 2013/05/31 22:41:10]

The current code is logical and easy to make sense of. Changing the code to what
you suggest requires the reader to have knowledge of the implementation of
HasAccountsToChoose.

         signin_helper_.reset(new wallet::WalletSigninHelper(
             this, profile_->GetRequestContext()));
         signin_helper_->StartUserNameFetch();
       } else {
         LogDialogLatencyToShow();
       }
       break;
 
     case REQUIRES_SIGN_IN:
     case SIGN_IN_DISABLED:
@@ skipping 22 matching lines @@
   UpdateAccountChooserView();
 
   if (view_)
     view_->UpdateButtonStrip();
 
   // On the first successful response, compute the initial user state metric.
   if (initial_user_state_ == AutofillMetrics::DIALOG_USER_STATE_UNKNOWN)
     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;
 
   // The text has already been constructed, no need to recompute.
   if (!legal_documents_text_.empty())
     return;
 
   const std::vector<wallet::WalletItems::LegalDocument*>& documents =
       wallet_items_->legal_documents();
@@ skipping 1206 matching lines @@
       chrome::FindBrowserWithWebContents(web_contents()),
       url,
       content::PAGE_TRANSITION_AUTO_BOOKMARK);
   params.disposition = NEW_FOREGROUND_TAB;
   chrome::Navigate(&params);
 #else
   // TODO(estade): use TabModelList?
 #endif
 }
 
+void AutofillDialogControllerImpl::OnWalletSigninError() {
+  signin_helper_.reset();
+  account_chooser_model_.SetHadWalletSigninError();
+  GetWalletClient()->CancelRequests();
+  LogDialogLatencyToShow();
+}
+
 void AutofillDialogControllerImpl::DisableWallet() {
   signin_helper_.reset();
   wallet_items_.reset();
   GetWalletClient()->CancelRequests();
   SetIsSubmitting(false);
   account_chooser_model_.SetHadWalletError();
 }
 
 void AutofillDialogControllerImpl::SuggestionsUpdated() {
   suggested_email_.Reset();
@@ skipping 703 matching lines @@
     }
   }
 
   callback_.Run(&form_structure_, !wallet_items_ ? std::string() :
       wallet_items_->google_transaction_id());
   callback_ = base::Callback<void(const FormStructure*, const std::string&)>();
 
   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_.active_wallet_account_name().empty()) {

[Dan Beam, 2013/05/31 22:22:53]

nit: account_chooser_model_.HasAccountsToChoose()

[Evan Stade, 2013/05/31 22:41:10]

Done.

+    profile_->GetPrefs()->SetBoolean(
+        ::prefs::kAutofillDialogPayWithoutWallet,
+        !account_chooser_model_.WalletIsSelected());
+  }
 
   switch (GetDialogType()) {
     case DIALOG_TYPE_AUTOCHECKOUT:
       // Stop observing PersonalDataManager to avoid the dialog redrawing while
       // in an Autocheckout flow.
       GetManager()->RemoveObserver(this);
       autocheckout_started_timestamp_ = base::Time::Now();
       DCHECK_EQ(AUTOCHECKOUT_NOT_STARTED, autocheckout_state_);
       autocheckout_state_ = AUTOCHECKOUT_IN_PROGRESS;
       view_->UpdateButtonStrip();
@@ skipping 168 matching lines @@
         AutofillMetrics::DIALOG_USER_SIGNED_IN_NO_WALLET_NO_AUTOFILL;
   }
 
   // Has Wallet items.
   return has_autofill_profiles ?
       AutofillMetrics::DIALOG_USER_SIGNED_IN_HAS_WALLET_HAS_AUTOFILL :
       AutofillMetrics::DIALOG_USER_SIGNED_IN_HAS_WALLET_NO_AUTOFILL;
 }
 
 }  // namespace autofill