[rAc] Move email address into billing address section.

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 a7406ad0ff8f434b1e73a0779cdfef9cadd1564d..252e46d2e78d6acb044dc8643de8397a96a9e4f1 100644
--- a/chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc
+++ b/chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc
@@ -259,9 +259,6 @@ std::string SectionToPrefString(DialogSection section) {
 
     case SECTION_SHIPPING:
       return "shipping";
-
-    case SECTION_EMAIL:
-      return "email";
   }
 
   NOTREACHED();
@@ -593,8 +590,6 @@ void AutofillDialogControllerImpl::Show() {
     return;
   }
 
-  common::BuildInputsForSection(SECTION_EMAIL,
-                                &requested_email_fields_);
   common::BuildInputsForSection(SECTION_CC,
                                 &requested_cc_fields_);
   common::BuildInputsForSection(SECTION_BILLING,
@@ -1110,8 +1105,6 @@ void AutofillDialogControllerImpl::UpdateForErrors() {
 const DetailInputs& AutofillDialogControllerImpl::RequestedFieldsForSection(
     DialogSection section) const {
   switch (section) {
-    case SECTION_EMAIL:
-      return requested_email_fields_;
     case SECTION_CC:
       return requested_cc_fields_;
     case SECTION_BILLING:
@@ -1203,8 +1196,6 @@ gfx::Image AutofillDialogControllerImpl::ButtonStripImage() const {
 string16 AutofillDialogControllerImpl::LabelForSection(DialogSection section)
     const {
   switch (section) {
-    case SECTION_EMAIL:
-      return l10n_util::GetStringUTF16(IDS_AUTOFILL_DIALOG_SECTION_EMAIL);
     case SECTION_CC:
       return l10n_util::GetStringUTF16(IDS_AUTOFILL_DIALOG_SECTION_CC);
     case SECTION_BILLING:
@@ -1212,10 +1203,9 @@ string16 AutofillDialogControllerImpl::LabelForSection(DialogSection section)
       return l10n_util::GetStringUTF16(IDS_AUTOFILL_DIALOG_SECTION_BILLING);
     case SECTION_SHIPPING:
       return l10n_util::GetStringUTF16(IDS_AUTOFILL_DIALOG_SECTION_SHIPPING);
-    default:
-      NOTREACHED();
-      return string16();
   }
+  NOTREACHED();
+  return string16();
 }
 
 SuggestionState AutofillDialogControllerImpl::SuggestionStateForSection(
@@ -1259,12 +1249,6 @@ bool AutofillDialogControllerImpl::SuggestionTextForSection(
   if (!IsASuggestionItemKey(item_key))
     return false;
 
-  if (section == SECTION_EMAIL) {
-    *vertically_compact = *horizontally_compact =
-        model->GetLabelAt(model->checked_item());
-    return true;
-  }
-
   scoped_ptr<DataModelWrapper> wrapper = CreateWrapper(section);
   return wrapper->GetDisplayText(vertically_compact, horizontally_compact);
 }
@@ -1381,6 +1365,10 @@ scoped_ptr<DataModelWrapper> AutofillDialogControllerImpl::CreateWrapper(
   AutofillProfile* profile = GetManager()->GetProfileByGUID(item_key);
   DCHECK(profile);
   size_t variant = GetSelectedVariantForModel(*model);
+  if (section == SECTION_SHIPPING) {
+    return scoped_ptr<DataModelWrapper>(
+        new AutofillShippingAddressWrapper(profile, variant));
+  }

[Evan Stade, 2013/09/05 16:54:09]

DCHECK_EQ(SECTION_BILLING, section)?

[Ilya Sherman, 2013/09/06 04:16:05]

Done.

   return scoped_ptr<DataModelWrapper>(
       new AutofillProfileWrapper(profile, variant));
 }
@@ -2239,7 +2227,6 @@ AutofillDialogControllerImpl::AutofillDialogControllerImpl(
       account_chooser_model_(this, profile_->GetPrefs(), metric_logger_,
                              dialog_type),
       wallet_client_(profile_->GetRequestContext(), this),
-      suggested_email_(this),
       suggested_cc_(this),
       suggested_billing_(this),
       suggested_cc_billing_(this),
@@ -2359,7 +2346,6 @@ void AutofillDialogControllerImpl::SuggestionsUpdated() {
 
   const DetailOutputMap snapshot = TakeUserInputSnapshot();
 
-  suggested_email_.Reset();
   suggested_cc_.Reset();
   suggested_billing_.Reset();
   suggested_cc_billing_.Reset();
@@ -2371,12 +2357,6 @@ void AutofillDialogControllerImpl::SuggestionsUpdated() {
       l10n_util::GetStringUTF16(IDS_AUTOFILL_DIALOG_USE_BILLING_FOR_SHIPPING));
 
   if (IsPayingWithWallet()) {
-    if (!account_chooser_model_.active_wallet_account_name().empty()) {
-      suggested_email_.AddKeyedItem(
-          base::IntToString(0),
-          account_chooser_model_.active_wallet_account_name());
-    }
-
     const std::vector<wallet::Address*>& addresses =
         wallet_items_->addresses();
     for (size_t i = 0; i < addresses.size(); ++i) {
@@ -2471,22 +2451,13 @@ void AutofillDialogControllerImpl::SuggestionsUpdated() {
     for (size_t i = 0; i < profiles.size(); ++i) {
       if (!HasCompleteAndVerifiedData(*profiles[i],
                                       requested_shipping_fields_) ||
+          profiles[i]->GetRawInfo(EMAIL_ADDRESS).empty() ||

[Evan Stade, 2013/09/05 16:54:09]

I feel like this should be covered in one of the other 2 checks in this
condition.

[Ilya Sherman, 2013/09/06 04:16:05]

Moved to only affect the billing section.

           HasInvalidAddress(*profiles[i])) {
         continue;
       }
 
-      // Add all email addresses.
-      std::vector<string16> values;
-      profiles[i]->GetMultiInfo(
-          AutofillType(EMAIL_ADDRESS), app_locale, &values);
-      for (size_t j = 0; j < values.size(); ++j) {
-        if (IsValidEmailAddress(values[j]))
-          suggested_email_.AddKeyedItem(profiles[i]->guid(), values[j]);
-      }
-
-      // Don't add variants for addresses: the email variants are handled above,
-      // name is part of credit card and we'll just ignore phone number
-      // variants.
+      // Don't add variants for addresses: name is part of credit card and we'll
+      // just ignore email and phone number variants.

[Evan Stade, 2013/09/05 16:54:09]

:/

[Ilya Sherman, 2013/09/06 04:16:05]

I dunno, are email addresses more important than phone numbers when it comes to
variants?  It's a shame to drop them, but it seems like we ought to handle both
equally.

[Evan Stade, 2013/09/06 21:41:00]

I'd rather not drop either, but only dropping one is superior to dropping  both.
I don't have a better idea though.

       suggested_billing_.AddKeyedItem(profiles[i]->guid(),
                                       profiles[i]->Label());
       suggested_shipping_.AddKeyedItem(profiles[i]->guid(),
@@ -2507,15 +2478,6 @@ void AutofillDialogControllerImpl::SuggestionsUpdated() {
         l10n_util::GetStringUTF16(IDS_AUTOFILL_DIALOG_MANAGE_BILLING_ADDRESS));
   }
 
-  suggested_email_.AddKeyedItem(
-      kAddNewItemKey,
-      l10n_util::GetStringUTF16(IDS_AUTOFILL_DIALOG_ADD_EMAIL_ADDRESS));
-  if (!IsPayingWithWallet()) {
-    suggested_email_.AddKeyedItem(
-        kManageItemsKey,
-        l10n_util::GetStringUTF16(IDS_AUTOFILL_DIALOG_MANAGE_EMAIL_ADDRESS));
-  }
-
   suggested_shipping_.AddKeyedItem(
       kAddNewItemKey,
       l10n_util::GetStringUTF16(IDS_AUTOFILL_DIALOG_ADD_SHIPPING_ADDRESS));
@@ -2571,16 +2533,6 @@ void AutofillDialogControllerImpl::FillOutputForSectionWithComparator(
     const InputFieldComparator& compare) {
   const DetailInputs& inputs = RequestedFieldsForSection(section);
 
-  // Email is hidden while using Wallet, special case it.
-  if (section == SECTION_EMAIL && IsPayingWithWallet()) {
-    AutofillProfile profile;
-    profile.SetRawInfo(EMAIL_ADDRESS,
-                       account_chooser_model_.active_wallet_account_name());
-    AutofillProfileWrapper profile_wrapper(&profile, 0);
-    profile_wrapper.FillFormStructure(inputs, compare, &form_structure_);
-    return;
-  }
-
   if (!SectionIsActive(section))
     return;
 
@@ -2594,7 +2546,15 @@ void AutofillDialogControllerImpl::FillOutputForSectionWithComparator(
     // handle them. This isn't necessary when filling the combined CC and
     // billing section as CVC comes from |full_wallet_| in this case.
     if (section == SECTION_CC)
-      SetCvcResult(view_->GetCvc());
+      SetOutputForFieldsOfType(CREDIT_CARD_VERIFICATION_CODE, view_->GetCvc());
+
+    // When filling from Wallet data, use the email address associated with the
+    // account. There is no other email address stored as part of a Wallet
+    // address.
+    if (section == SECTION_CC_BILLING) {
+      SetOutputForFieldsOfType(
+          EMAIL_ADDRESS, account_chooser_model_.active_wallet_account_name());
+    }
   } else {
     // The user manually input data. If using Autofill, save the info as new or
     // edited data. Always fill local data into |form_structure_|.
@@ -2620,18 +2580,14 @@ void AutofillDialogControllerImpl::FillOutputForSectionWithComparator(
       card_wrapper.FillFormStructure(inputs, compare, &form_structure_);
 
       // Again, CVC needs special-casing. Fill it in directly from |output|.
-      SetCvcResult(GetValueForType(output, CREDIT_CARD_VERIFICATION_CODE));
+      SetOutputForFieldsOfType(
+          CREDIT_CARD_VERIFICATION_CODE,
+          GetValueForType(output, CREDIT_CARD_VERIFICATION_CODE));
     } else {
       AutofillProfile profile;
       profile.set_origin(kAutofillDialogOrigin);
       FillFormGroupFromOutputs(output, &profile);
 
-      // For billing, the email address comes from the separate email section.
-      if (section == SECTION_BILLING) {
-        profile.SetRawInfo(EMAIL_ADDRESS,
-                           GetValueFromSection(SECTION_EMAIL, EMAIL_ADDRESS));
-      }
-
       if (ShouldSaveDetailsLocally())
         SaveProfileGleanedFromSection(profile, section);
 
@@ -2656,13 +2612,13 @@ bool AutofillDialogControllerImpl::FormStructureCaresAboutSection(
   return true;
 }
 
-void AutofillDialogControllerImpl::SetCvcResult(const string16& cvc) {
+void AutofillDialogControllerImpl::SetOutputForFieldsOfType(
+    ServerFieldType type,
+    const base::string16& output) {
   for (size_t i = 0; i < form_structure_.field_count(); ++i) {
     AutofillField* field = form_structure_.field(i);
-    if (field->Type().GetStorableType() == CREDIT_CARD_VERIFICATION_CODE) {
-      field->value = cvc;
-      break;
-    }
+    if (field->Type().GetStorableType() == type)
+      field->value = output;
   }
 }
 
@@ -2689,30 +2645,12 @@ string16 AutofillDialogControllerImpl::GetValueFromSection(
 void AutofillDialogControllerImpl::SaveProfileGleanedFromSection(
     const AutofillProfile& profile,
     DialogSection section) {
-  if (section == SECTION_EMAIL) {
-    // Save the email address to the existing (suggested) billing profile. If
-    // there is no existing profile, the newly created one will pick up this
-    // email, so in that case do nothing.
-    scoped_ptr<DataModelWrapper> wrapper = CreateWrapper(SECTION_BILLING);
-    if (wrapper) {
-      std::string item_key = SuggestionsMenuModelForSection(SECTION_BILLING)->
-          GetItemKeyForCheckedItem();
-      AutofillProfile* billing_profile =
-          GetManager()->GetProfileByGUID(item_key);
-      billing_profile->OverwriteWithOrAddTo(
-          profile,
-          g_browser_process->GetApplicationLocale());
-    }
-  } else {
-    GetManager()->SaveImportedProfile(profile);
-  }
+  GetManager()->SaveImportedProfile(profile);
 }
 
 SuggestionsMenuModel* AutofillDialogControllerImpl::
     SuggestionsMenuModelForSection(DialogSection section) {
   switch (section) {
-    case SECTION_EMAIL:
-      return &suggested_email_;
     case SECTION_CC:
       return &suggested_cc_;
     case SECTION_BILLING:
@@ -2735,9 +2673,6 @@ const SuggestionsMenuModel* AutofillDialogControllerImpl::
 
 DialogSection AutofillDialogControllerImpl::SectionForSuggestionsMenuModel(
     const SuggestionsMenuModel& model) {
-  if (&model == &suggested_email_)
-    return SECTION_EMAIL;
-
   if (&model == &suggested_cc_)
     return SECTION_CC;
 
@@ -3059,7 +2994,6 @@ void AutofillDialogControllerImpl::FinishSubmit() {
     }
   }
 
-  FillOutputForSection(SECTION_EMAIL);
   FillOutputForSection(SECTION_CC);
   FillOutputForSection(SECTION_BILLING);
   FillOutputForSection(SECTION_CC_BILLING);
@@ -3304,11 +3238,6 @@ void AutofillDialogControllerImpl::MaybeShowCreditCardBubble() {
       billing_profile.reset(new AutofillProfile(*profile));
     }
 
-    // The bubble also needs the associated email address.
-    billing_profile->SetRawInfo(
-        EMAIL_ADDRESS,
-        GetValueFromSection(SECTION_EMAIL, EMAIL_ADDRESS));
-
     ShowNewCreditCardBubble(newly_saved_card_.Pass(),
                             billing_profile.Pass());
     return;