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 715 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 1047 matching lines @@
 
   SuggestionsUpdated();
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // AccountChooserModelDelegate implementation.
 
 void AutofillDialogControllerImpl::AccountChoiceChanged() {
   if (is_submitting_)
     GetWalletClient()->CancelRequests();
 

[Dan Beam, 2013/05/31 00:21:55]

why not just detect the change here?

   SetIsSubmitting(false);
 
   SuggestionsUpdated();
   UpdateAccountChooserView();
 }
 
 void AutofillDialogControllerImpl::UpdateAccountChooserView() {
   if (view_) {
     view_->UpdateAccountChooser();
     view_->UpdateNotificationArea();
@@ skipping 138 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);

[Dan Beam, 2013/05/31 00:21:55]

don't we use this to determine if it's first run?

+  // 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()) {
+    profile_->GetPrefs()->SetBoolean(
+        ::prefs::kAutofillDialogPayWithoutWallet,
+        !account_chooser_model_.WalletIsSelected());
+  }

[Dan Beam, 2013/05/31 00:21:55]

and ensure this pref is set to its current value here?

[Evan Stade, 2013/05/31 02:47:33]

it seems better to stop making this pref pull double duty, i.e. use this code as
written in this CL but create a new pref for the number of runs of the dialog.
I'd just as soon leave IsFirstRun as is for now and I'll start a discussion for
determining the correct behavior of the various promo-ish banners.

[Evan Stade, 2013/05/31 03:12:29]

filed https://code.google.com/p/chromium/issues/detail?id=245542 for this.

 
   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