Handle expired Autofill credit cards in autofill dialog

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 2cf3af3f1319541e9a65db5a543a72ecf92be3b8..8069ad2e280108ba33d576b2d9df4d9274c9753b 100644
--- a/chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc
+++ b/chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc
@@ -374,14 +374,15 @@ void AutofillDialogControllerImpl::Show() {
               arraysize(kShippingInputs),
               &requested_shipping_fields_);
 
-  SuggestionsUpdated();
-
   // TODO(estade): don't show the dialog if the site didn't specify the right
   // fields. First we must figure out what the "right" fields are.
   view_.reset(CreateView());
   view_->Show();
   GetManager()->AddObserver(this);
 
+  // This should be done while |view_| is non-NULL.

[Evan Stade, 2013/04/30 19:03:10]

dislike. The controller should not need to update the view as soon as it's
created. The view should always be able to poll the controller to get all the
current state that's necessary for displaying the UI.

[Dan Beam, 2013/05/01 00:43:09]

Reverted.

+  SuggestionsUpdated();
+
   // Try to see if the user is already signed-in.
   // If signed-in, fetch the user's Wallet data.
   // Otherwise, see if the user could be signed in passively.
@@ -641,11 +642,19 @@ void AutofillDialogControllerImpl::EnsureLegalDocumentsText() {
   legal_documents_text_ = text;
 }
 
+void AutofillDialogControllerImpl::ShowEditingMode(DialogSection section) {
+  scoped_ptr<DataModelWrapper> wrapper = CreateWrapper(section);
+  wrapper->FillInputs(MutableRequestedFieldsForSection(section));
+  section_editing_state_[section] = true;
+  view_->UpdateSection(section);
+}
+
 void AutofillDialogControllerImpl::ResetManualInputForSection(
     DialogSection section) {
   DetailInputs* inputs = MutableRequestedFieldsForSection(section);
   for (size_t i = 0; i < inputs->size(); ++i)
     (*inputs)[i].initial_value.clear();
+  // TODO(dbeam): why is this here rather than in EditCancelledForSection()?
   section_editing_state_[section] = false;
 }
 
@@ -751,8 +760,9 @@ string16 AutofillDialogControllerImpl::SuggestionTextForSection(
   if (!action_text.empty())
     return action_text;
 
-  // When the user has clicked 'edit', don't show a suggestion (even though
-  // there is a profile selected in the model).
+  // When the user has clicked 'edit' or a suggestion is somehow invalid (e.g. a
+  // user selects a credit card that has expired), don't show a suggestion (even
+  // though there is a profile selected in the model).
   if (section_editing_state_[section])
     return string16();
 
@@ -902,12 +912,7 @@ bool AutofillDialogControllerImpl::EditEnabledForSection(
 
 void AutofillDialogControllerImpl::EditClickedForSection(
     DialogSection section) {
-  DetailInputs* inputs = MutableRequestedFieldsForSection(section);
-  scoped_ptr<DataModelWrapper> model = CreateWrapper(section);
-  model->FillInputs(inputs);
-  section_editing_state_[section] = true;
-  view_->UpdateSection(section);
-
+  ShowEditingMode(section);
   GetMetricLogger().LogDialogUiEvent(
       dialog_type_, DialogSectionToUiEditEvent(section));
 }
@@ -1395,7 +1400,15 @@ void AutofillDialogControllerImpl::SuggestionItemSelected(
   }
 
   model->SetCheckedIndex(index);
-  EditCancelledForSection(SectionForSuggestionsMenuModel(*model));
+  const DialogSection section = SectionForSuggestionsMenuModel(*model);
+  EditCancelledForSection(section);
+
+  // If a user chooses a real suggestion (not Add/Edit) and the suggestion text
+  // is empty, it's invalid. Show the editing UI and highlight invalid fields.
+  if (IsASuggestionItemKey(model->GetItemKeyForCheckedItem()) &&
+      SuggestionTextForSection(section).empty()) {
+    ShowEditingMode(section);
+  }
 
   LogSuggestionItemSelectedMetric(*model);
 }
@@ -1573,11 +1586,9 @@ void AutofillDialogControllerImpl::OnPersonalDataChanged() {
 
 void AutofillDialogControllerImpl::AccountChoiceChanged() {
   // Whenever the user changes the account, all manual inputs should be reset.
-  ResetManualInputForSection(SECTION_EMAIL);
-  ResetManualInputForSection(SECTION_CC);
-  ResetManualInputForSection(SECTION_BILLING);
-  ResetManualInputForSection(SECTION_CC_BILLING);
-  ResetManualInputForSection(SECTION_SHIPPING);
+  for (size_t section = SECTION_MIN; section <= SECTION_MAX; ++section) {
+    ResetManualInputForSection(static_cast<DialogSection>(section));
+  }
 
   if (is_submitting_)
     GetWalletClient()->CancelRequests();
@@ -1734,7 +1745,6 @@ void AutofillDialogControllerImpl::SuggestionsUpdated() {
           base::IntToString(i),
           addresses[i]->DisplayName(),
           addresses[i]->DisplayNameDetail());
-
     }
 
     if (!IsSubmitPausedOn(wallet::VERIFY_CVV)) {
@@ -1822,12 +1832,25 @@ void AutofillDialogControllerImpl::SuggestionsUpdated() {
       kManageItemsKey,
       l10n_util::GetStringUTF16(IDS_AUTOFILL_DIALOG_MANAGE_SHIPPING_ADDRESS));
 
-  // Default shipping address is the first suggestion, if one exists. Otherwise
-  // it's the "Use shipping for billing" item.
-  const std::string& first_real_suggestion_item_key =
-      suggested_shipping_.GetItemKeyAt(1);
-  if (IsASuggestionItemKey(first_real_suggestion_item_key))
-      suggested_shipping_.SetCheckedItem(first_real_suggestion_item_key);
+  for (size_t i = SECTION_MIN; i <= SECTION_MAX; ++i) {
+    DialogSection section = static_cast<DialogSection>(i);
+    SuggestionsMenuModel* model = SuggestionsMenuModelForSection(section);
+
+    if (model->GetItemCount() == 0)
+      continue;
+
+    size_t default_suggestion_index = 0;
+
+    if (section == SECTION_SHIPPING) {
+      // Default shipping address is the first suggestion, if one exists.
+      // Otherwise it's the "Use shipping for billing" item.
+      const size_t kFirstRealSuggestionIndex = 1;

[Evan Stade, 2013/04/30 19:03:10]

I don't see the benefit to the way you rewrote this. It actually detracts,
because 

IsASuggestionItemKey(model->GetItemKeyAt(kFirstRealSuggestionIndex))

doesn't make much sense --- on the surface, it should always return true

[Dan Beam, 2013/05/01 00:43:09]

Reverted.

+      if (IsASuggestionItemKey(model->GetItemKeyAt(kFirstRealSuggestionIndex)))
+        default_suggestion_index = kFirstRealSuggestionIndex;
+    }
+
+    SuggestionItemSelected(model, default_suggestion_index);

[Evan Stade, 2013/04/30 19:03:10]

this feels wrong to me. For one thing, it screws with the metrics. You should
just fill the inputs and set the editing state for invalid suggestions.

[Dan Beam, 2013/05/01 00:43:09]

Done.

+  }
 
   if (view_)
     view_->ModelChanged();
@@ -2315,7 +2338,6 @@ void AutofillDialogControllerImpl::LogSuggestionItemSelectedMetric(
     dialog_ui_event = DialogSectionToUiItemAddedEvent(section);
   } else if (IsASuggestionItemKey(model.GetItemKeyForCheckedItem())) {
     // Selected an existing item.
-    DCHECK(!section_editing_state_[section]);
     dialog_ui_event = DialogSectionToUiSelectionChangedEvent(section);
   } else {
     // TODO(estade): add logging for "Manage items" or "Use billing for