Persist the choice of AutofillDataModel when using the requestAutocomplete 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 7edf6c17f9bcbbab638d763a0d2a1e225e83a125..6a4a79f213c8d6ed963a04686c6718f9549aad69 100644
--- a/chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc
+++ b/chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc
@@ -18,6 +18,7 @@
 #include "chrome/browser/autofill/personal_data_manager_factory.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/extensions/shell_window_registry.h"
+#include "chrome/browser/prefs/scoped_user_pref_update.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/autofill/autofill_dialog_view.h"
 #include "chrome/browser/ui/autofill/data_model_wrapper.h"
@@ -86,6 +87,11 @@ const char kAddNewItemKey[] = "add-new-item";
 const char kManageItemsKey[] = "manage-items";
 const char kSameAsBillingKey[] = "same-as-billing";
 
+// Keys for the kAutofillDialogAutofillDefault pref dictionary (do not change
+// these values).
+const char kGuidPrefKey[] = "guid";
+const char kVariantPrefKey[] = "variant";
+
 // Returns true if |input| should be shown when |field_type| has been requested.
 bool InputTypeMatchesFieldType(const DetailInput& input,
                                AutofillFieldType field_type) {
@@ -227,6 +233,32 @@ string16 GetValueForType(const DetailOutputMap& output,
   return string16();
 }
 
+// Returns a string descriptor for a DialogSection, for use with prefs (do not
+// change these values).
+std::string SectionToPrefString(DialogSection section) {
+  switch (section) {
+    case SECTION_CC:
+      return "cc";
+
+    case SECTION_BILLING:
+      return "billing";
+
+    case SECTION_CC_BILLING:
+      // The SECTION_CC_BILLING section isn't active when using Autofill.
+      NOTREACHED();
+      return std::string();
+
+    case SECTION_SHIPPING:
+      return "shipping";
+
+    case SECTION_EMAIL:
+      return "email";
+  }
+
+  NOTREACHED();
+  return std::string();
+}
+
 }  // namespace
 
 AutofillDialogController::~AutofillDialogController() {}
@@ -265,6 +297,9 @@ void AutofillDialogControllerImpl::RegisterUserPrefs(
       ::prefs::kAutofillDialogPayWithoutWallet,
       kPayWithoutWalletDefault,
       user_prefs::PrefRegistrySyncable::SYNCABLE_PREF);
+  registry->RegisterDictionaryPref(
+      ::prefs::kAutofillDialogAutofillDefault,
+      user_prefs::PrefRegistrySyncable::SYNCABLE_PREF);
 }
 
 void AutofillDialogControllerImpl::Show() {
@@ -938,18 +973,9 @@ scoped_ptr<DataModelWrapper> AutofillDialogControllerImpl::CreateWrapper(
     return scoped_ptr<DataModelWrapper>(new AutofillCreditCardWrapper(card));
   }
 
-  // Calculate the variant by looking at how many items come from the same
-  // data model.
-  size_t variant = 0;
-  for (int i = model->checked_item() - 1; i >= 0; --i) {
-    if (model->GetItemKeyAt(i) == item_key)
-      variant++;
-    else
-      break;
-  }
-
   AutofillProfile* profile = GetManager()->GetProfileByGUID(item_key);
   DCHECK(profile);
+  size_t variant = GetSelectedVariantForModel(*model);
   return scoped_ptr<DataModelWrapper>(
       new AutofillProfileWrapper(profile, variant));
 }
@@ -1874,12 +1900,18 @@ void AutofillDialogControllerImpl::SuggestionsUpdated() {
       l10n_util::GetStringUTF16(IDS_AUTOFILL_DIALOG_MANAGE_SHIPPING_ADDRESS));
 
   if (!IsPayingWithWallet()) {
-    // When using Autofill, the default option 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);
+      if (!SectionIsActive(section))
+        continue;
+
+      std::string default_guid;
+      int default_variant;
+      SuggestionsMenuModel* model = SuggestionsMenuModelForSection(section);

[Ilya Sherman, 2013/05/21 03:54:35]

nit: Perhaps move this into the if-stmt, since it's not used outside of there?

[Evan Stade, 2013/05/21 17:34:41]

changed this code a bit.

+      GetAutofillChoice(section, &default_guid, &default_variant);
+      if (!default_guid.empty())
+        model->SetCheckedItemNthWithKey(default_guid, default_variant + 1);
+    }
   }
 
   if (view_)
@@ -1930,6 +1962,7 @@ void AutofillDialogControllerImpl::FillOutputForSectionWithComparator(
     // billing section as CVC comes from |full_wallet_| in this case.
     if (section == SECTION_CC)
       SetCvcResult(view_->GetCvc());
+

[Ilya Sherman, 2013/05/21 03:54:35]

nit: Spurious diff

[Evan Stade, 2013/05/21 17:34:41]

Done.

   } else {
     // The user manually input data. If using Autofill, save the info as new or
     // edited data. Always fill local data into |form_structure_|.
@@ -2408,6 +2441,21 @@ void AutofillDialogControllerImpl::FinishSubmit() {
     FillOutputForSection(SECTION_SHIPPING);
   }
 
+  if (!IsPayingWithWallet()) {
+    for (size_t i = SECTION_MIN; i <= SECTION_MAX; ++i) {
+      DialogSection section = static_cast<DialogSection>(i);
+      if (!SectionIsActive(section))
+        continue;
+
+      SuggestionsMenuModel* model = SuggestionsMenuModelForSection(section);
+      std::string item_key = model->GetItemKeyForCheckedItem();
+      if (IsASuggestionItemKey(item_key) || item_key == kSameAsBillingKey) {
+        int variant = GetSelectedVariantForModel(*model);
+        PersistAutofillChoice(section, item_key, variant);
+      }
+    }
+  }
+
   callback_.Run(&form_structure_, !wallet_items_ ? std::string() :
       wallet_items_->google_transaction_id());
   callback_ = base::Callback<void(const FormStructure*, const std::string&)>();
@@ -2442,6 +2490,63 @@ void AutofillDialogControllerImpl::FinishSubmit() {
   }
 }
 
+void AutofillDialogControllerImpl::PersistAutofillChoice(
+    DialogSection section,
+    const std::string& guid,
+    int variant) {
+  DCHECK(!IsPayingWithWallet());
+  scoped_ptr<base::DictionaryValue> value(new base::DictionaryValue());
+  value->SetString(kGuidPrefKey, guid);
+  value->SetInteger(kVariantPrefKey, variant);
+
+  DictionaryPrefUpdate updater(profile()->GetPrefs(),
+                               ::prefs::kAutofillDialogAutofillDefault);
+  base::DictionaryValue* autofill_choice = updater.Get();
+  autofill_choice->Set(SectionToPrefString(section), value.release());
+}
+
+void AutofillDialogControllerImpl::GetAutofillChoice(DialogSection section,
+                                                     std::string* guid,
+                                                     int* variant) {
+  DCHECK(!IsPayingWithWallet());
+  // The default choice is the first thing in the menu that is a suggestion
+  // item.
+  *variant = 0;
+  SuggestionsMenuModel* model = SuggestionsMenuModelForSection(section);
+  for (int i = 0; i < model->GetItemCount(); ++i) {
+    if (IsASuggestionItemKey(model->GetItemKeyAt(i))) {
+      *guid = model->GetItemKeyAt(i);
+      break;
+    }
+  }
+
+  const base::DictionaryValue* choices = profile()->GetPrefs()->GetDictionary(
+      ::prefs::kAutofillDialogAutofillDefault);
+  if (!choices)
+    return;
+
+  const base::DictionaryValue* choice = NULL;
+  if (!choices->GetDictionary(SectionToPrefString(section), &choice))
+    return;
+
+  choice->GetString(kGuidPrefKey, guid);
+  choice->GetInteger(kVariantPrefKey, variant);
+}
+
+size_t AutofillDialogControllerImpl::GetSelectedVariantForModel(
+    const SuggestionsMenuModel& model) {
+  size_t variant = 0;
+  // Calculate the variant by looking at how many items come from the same
+  // data model.
+  for (int i = model.checked_item() - 1; i >= 0; --i) {
+    if (model.GetItemKeyAt(i) == model.GetItemKeyForCheckedItem())
+      variant++;

[Ilya Sherman, 2013/05/21 03:54:35]

nit: ++variant

[Evan Stade, 2013/05/21 17:34:41]

this is not a rule that I'm aware of.

"For simple scalar (non-object) values there is no reason to prefer one form and
we allow either."[1]

By my crude count, chrome/browser/ has about 300 preincrements and 500
postincrements of values that are not in for loop headers.

[1] except actually there is, in the words of the style guide: "Some find
post-increment easier to read, since the "subject" (i) precedes the "verb" (++),
just like in English."

[Ilya Sherman, 2013/05/21 23:20:43]

I prefer to always use pre-increment because it's never wrong and hence is an
easy rule to stick to... but ok.

+    else
+      break;
+  }
+  return variant;
+}
+
 void AutofillDialogControllerImpl::LogOnFinishSubmitMetrics() {
   GetMetricLogger().LogDialogUiDuration(
       base::Time::Now() - dialog_shown_timestamp_,