Interactive autofill: Handle Online Wallet being unavailable.

chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc

Index: chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc
diff --git a/chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc b/chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc
index 123704ec7aba365ae787d035aad3804121a4b06e..61fbcf237a2fbaf5e96dcc7bdba1308544aae1a1 100644
--- a/chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc
+++ b/chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc
@@ -133,6 +133,9 @@ AutofillDialogControllerImpl::AutofillDialogControllerImpl(
       ssl_status_(ssl_status),
       callback_(callback),
       wallet_client_(profile_->GetRequestContext()),
+      refresh_wallet_items_in_progress_(false),
+      refresh_wallet_items_queued_(false),
+      had_wallet_error_(false),
       ALLOW_THIS_IN_INITIALIZER_LIST(suggested_email_(this)),
       ALLOW_THIS_IN_INITIALIZER_LIST(suggested_cc_(this)),
       ALLOW_THIS_IN_INITIALIZER_LIST(suggested_billing_(this)),
@@ -223,7 +226,7 @@ void AutofillDialogControllerImpl::Show() {
   view_->Show();
 
   // Request sugar info after the view is showing to simplify code for now.
-  wallet_client_.GetWalletItems(this);
+  ScheduleRefreshWalletItems();
 }
 
 void AutofillDialogControllerImpl::Hide() {
@@ -238,6 +241,10 @@ string16 AutofillDialogControllerImpl::DialogTitle() const {
   return l10n_util::GetStringUTF16(IDS_AUTOFILL_DIALOG_TITLE);
 }
 
+string16 AutofillDialogControllerImpl::PayWithoutWallet() const {
+  return l10n_util::GetStringUTF16(IDS_AUTOFILL_DIALOG_PAY_WITHOUT_WALLET);
+}
+
 string16 AutofillDialogControllerImpl::EditSuggestionText() const {
   return l10n_util::GetStringUTF16(IDS_AUTOFILL_DIALOG_EDIT);
 }
@@ -262,12 +269,27 @@ string16 AutofillDialogControllerImpl::ConfirmButtonText() const {
 
 string16 AutofillDialogControllerImpl::SignInText() const {
   // TODO(abodenha): real strings and l10n.
-  return string16(ASCIIToUTF16("Sign in to use Google Wallet"));
+  return ASCIIToUTF16("Sign in to use Google Wallet");
+}
+
+string16 AutofillDialogControllerImpl::SignedInUser() const {
+  // TODO(dbeam): get the real signed in user.
+  DCHECK_EQ(SIGNED_IN, SignedInState());
+  return ASCIIToUTF16("user@example.com");
 }
 
 string16 AutofillDialogControllerImpl::CancelSignInText() const {
   // TODO(abodenha): real strings and l10n.
-  return string16(ASCIIToUTF16("Don't sign in."));
+  return ASCIIToUTF16("Don't sign in.");
+}
+
+string16 AutofillDialogControllerImpl::SaveLocallyText() const {
+  return l10n_util::GetStringUTF16(IDS_AUTOFILL_DIALOG_SAVE_LOCALLY_CHECKBOX);
+}
+
+string16 AutofillDialogControllerImpl::ProgressBarText() const {
+  return l10n_util::GetStringUTF16(
+      IDS_AUTOFILL_DIALOG_AUTOCHECKOUT_PROGRESS_BAR);
 }
 
 DialogSignedInState AutofillDialogControllerImpl::SignedInState() const {
@@ -283,13 +305,8 @@ DialogSignedInState AutofillDialogControllerImpl::SignedInState() const {
   return SIGNED_IN;
 }
 
-string16 AutofillDialogControllerImpl::SaveLocallyText() const {
-  return l10n_util::GetStringUTF16(IDS_AUTOFILL_DIALOG_SAVE_LOCALLY_CHECKBOX);
-}
-
-string16 AutofillDialogControllerImpl::ProgressBarText() const {
-  return l10n_util::GetStringUTF16(
-      IDS_AUTOFILL_DIALOG_AUTOCHECKOUT_PROGRESS_BAR);
+bool AutofillDialogControllerImpl::IsWalletAvailable() const {
+  return !had_wallet_error_;
 }
 
 const DetailInputs& AutofillDialogControllerImpl::RequestedFieldsForSection(
@@ -554,14 +571,22 @@ DialogNotification AutofillDialogControllerImpl::CurrentNotification() const {
             IDS_AUTOFILL_DIALOG_SITE_WARNING, UTF8ToUTF16(source_url_.host())));
   }
 
+  if (had_wallet_error_) {

[Evan Stade, 2013/02/06 21:57:49]

is the plan to have multiple notifications at once (I hope so). Can you add a
TODO for that and/or file a bug.

[Dan Beam, 2013/02/07 01:22:40]

I assume we'll never be *showing* multiple notifications at once, no.  I don't
see this in the mocks at least.

Assuming you mean return multiple DialogNotifications from this method - I hoped
to simply return the most relevant notification to the view at that point so and
call view_->UpdateNotificationArea() when anything changes.  That way we're not
putting logic in each platform UI (which I think will happen if we pass back
multiple notifications).

[Evan Stade, 2013/02/07 01:54:42]

I think we will want to show multiple notifications at once. For example, we
might show both the insecure warning and the wallet error notification at the
same time. Which one of these is OK not to show? Right now, your code seems to
indicate that it's fine not to show any indication that wallet had an error. I
disagree that this is ok.

[Dan Beam, 2013/02/07 03:26:53]

https://crbug.com/174814

+    // TODO(dbeam): pass along the Wallet error or remove from the translation.
+    return DialogNotification(
+        DialogNotification::WALLET_ERROR,
+        l10n_util::GetStringFUTF16(
+            IDS_AUTOFILL_DIALOG_COMPLETE_WITHOUT_WALLET,
+            ASCIIToUTF16("[Wallet-Error].")));
+  }
+
   return DialogNotification();
 }
 
 void AutofillDialogControllerImpl::StartSignInFlow() {
   DCHECK(registrar_.IsEmpty());
 
-  content::Source<content::NavigationController> source(
-      &view_->ShowSignIn());
+  content::Source<content::NavigationController> source(&view_->ShowSignIn());
   registrar_.Add(this, content::NOTIFICATION_NAV_ENTRY_COMMITTED, source);
 }
 
@@ -633,10 +658,7 @@ void AutofillDialogControllerImpl::Observe(
       content::Details<content::LoadCommittedDetails>(details).ptr();
   if (wallet::IsSignInContinueUrl(load_details->entry->GetVirtualURL())) {
     EndSignInFlow();
-    // TODO(dbeam): the fetcher can't handle being called multiple times.
-    // Address this soon as we will be re-fetching wallet items after every
-    // required action is resolved.
-    wallet_client_.GetWalletItems(this);
+    ScheduleRefreshWalletItems();
   }
 }
 
@@ -671,48 +693,55 @@ void AutofillDialogControllerImpl::OnDidEscrowSensitiveInformation(
 void AutofillDialogControllerImpl::OnDidGetFullWallet(
     scoped_ptr<wallet::FullWallet> full_wallet) {
   NOTIMPLEMENTED();
+  WalletRequestCompleted(true);
 }
 
 void AutofillDialogControllerImpl::OnDidGetWalletItems(
     scoped_ptr<wallet::WalletItems> wallet_items) {
+  bool items_changed = !wallet_items_ || *wallet_items != *wallet_items_;
   wallet_items_ = wallet_items.Pass();
-  view_->UpdateAccountChooser();
-  view_->UpdateNotificationArea();
+  WalletRequestCompleted(true);
+
+  if (items_changed) {
+    view_->UpdateAccountChooser();
+    view_->UpdateNotificationArea();
+  }
 }
 
 void AutofillDialogControllerImpl::OnDidSaveAddress(
     const std::string& address_id) {
   NOTIMPLEMENTED() << " address_id=" << address_id;
+  WalletRequestCompleted(true);
 }
 
 void AutofillDialogControllerImpl::OnDidSaveInstrument(
     const std::string& instrument_id) {
   NOTIMPLEMENTED() << " instrument_id=" << instrument_id;
+  WalletRequestCompleted(true);
 }
 
 void AutofillDialogControllerImpl::OnDidSaveInstrumentAndAddress(
     const std::string& instrument_id, const std::string& address_id) {
   NOTIMPLEMENTED() << " instrument_id=" << instrument_id
                    << ", address_id=" << address_id;
+  WalletRequestCompleted(true);
 }
 
 void AutofillDialogControllerImpl::OnDidSendAutocheckoutStatus() {
   NOTIMPLEMENTED();
+  WalletRequestCompleted(true);
 }
 
 void AutofillDialogControllerImpl::OnWalletError() {
-  NOTIMPLEMENTED();
-  wallet_items_.reset();
+  WalletRequestCompleted(false);
 }
 
 void AutofillDialogControllerImpl::OnMalformedResponse() {
-  NOTIMPLEMENTED();
-  wallet_items_.reset();
+  WalletRequestCompleted(false);
 }
 
 void AutofillDialogControllerImpl::OnNetworkError(int response_code) {
-  NOTIMPLEMENTED() << " response_code=" << response_code;
-  wallet_items_.reset();
+  WalletRequestCompleted(false);
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -754,6 +783,40 @@ bool AutofillDialogControllerImpl::HasRequiredAction(
   return std::find(actions.begin(), actions.end(), action) != actions.end();
 }
 
+void AutofillDialogControllerImpl::ScheduleRefreshWalletItems() {
+  if (had_wallet_error_ || refresh_wallet_items_queued_)

[Evan Stade, 2013/02/06 21:57:49]

the second half of this check seems unnecessary/undesirable

[Dan Beam, 2013/02/07 01:22:40]

Done.

+    return;
+
+  if (refresh_wallet_items_in_progress_) {
+    refresh_wallet_items_queued_ = true;
+    return;
+  }
+
+  wallet_client_.GetWalletItems(this);

[Evan Stade, 2013/02/06 21:57:49]

you sohuld set refresh_wallet_items_queued_ to false here rather than below.

[Dan Beam, 2013/02/07 01:22:40]

Done.

+  refresh_wallet_items_in_progress_ = true;
+}
+
+void AutofillDialogControllerImpl::WalletRequestCompleted(bool success) {
+  // |wallet_client_| only handles 1 request at a time, so this can be safely
+  // set to false every time regardless of which type of request completed.
+  refresh_wallet_items_in_progress_ = false;

[Evan Stade, 2013/02/06 21:57:49]

this can be done here, but why should it be? Can't you put it in just
OnDidGetWalletItems?

[Dan Beam, 2013/02/07 01:22:40]

if I move this to OnDidGetWalletItems(), what happens when a request ends via
OnNetworkError()/OnWalletError()/OnMalformedResponse()?  this flag would seem to
get stuck as true, no?

[Evan Stade, 2013/02/07 01:54:42]

fair enough.

+
+  if (!success) {
+    if (!had_wallet_error_) {

[Evan Stade, 2013/02/06 21:57:49]

this check seems wrong, the wallet error text may have changed.

[Dan Beam, 2013/02/07 01:22:40]

Right now we stop at the first wallet error, but sure, this might not be always
the case -- removed.

+      had_wallet_error_ = true;

[Evan Stade, 2013/02/06 21:57:49]

when do you set had_wallet_error_ to false

[Dan Beam, 2013/02/07 01:22:40]

never, by design -- at the first error we stop interacting with wallet

+      wallet_items_.reset();
+      view_->UpdateAccountChooser();
+      view_->UpdateNotificationArea();
+    }
+    return;
+  }
+
+  if (refresh_wallet_items_queued_) {
+    refresh_wallet_items_queued_ = false;
+    ScheduleRefreshWalletItems();
+  }
+}
+
 void AutofillDialogControllerImpl::GenerateSuggestionsModels() {
   PersonalDataManager* manager = GetManager();
   const std::vector<CreditCard*>& cards = manager->credit_cards();