rAc: fill in "shipping name" when the user has selected "same as billing"

BUG=245493

Review URL: https://chromiumcodereview.appspot.com/16325023

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@204482 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 140 insertions, 81 deletions

diff --git a/chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc b/chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc
index acda09b..9602e00 100644
--- a/chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc
+++ b/chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc
@@ -472,11 +472,14 @@ void AutofillDialogControllerImpl::Show() {
               arraysize(kShippingInputs),
               &requested_shipping_fields_);
 
-  cares_about_shipping_ =
-      FillFormStructureForSection(NULL,
-                                  0,
-                                  SECTION_SHIPPING,
-                                  base::Bind(DetailInputMatchesField));
+  // Test whether we need to show the shipping section. If filling that section
+  // would be a no-op, don't show it.
+  const DetailInputs& inputs = RequestedFieldsForSection(SECTION_SHIPPING);
+  EmptyDataModelWrapper empty_wrapper;
+  cares_about_shipping_ = empty_wrapper.FillFormStructure(
+      inputs,
+      base::Bind(DetailInputMatchesField),
+      &form_structure_);
 
   SuggestionsUpdated();
 
@@ -2198,12 +2201,15 @@ void AutofillDialogControllerImpl::SuggestionsUpdated() {
 void AutofillDialogControllerImpl::FillOutputForSectionWithComparator(
     DialogSection section,
     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());
-    FillFormStructureForSection(&profile, 0, section, compare);
+    AutofillProfileWrapper profile_wrapper(&profile, 0);
+    profile_wrapper.FillFormStructure(inputs, compare, &form_structure_);
     return;
   }
 
@@ -2235,7 +2241,8 @@ void AutofillDialogControllerImpl::FillOutputForSectionWithComparator(
       if (ShouldSaveDetailsLocally())
         GetManager()->SaveImportedCreditCard(card);
 
-      FillFormStructureForSection(&card, 0, section, compare);
+      AutofillCreditCardWrapper card_wrapper(&card);
+      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));
@@ -2255,7 +2262,8 @@ void AutofillDialogControllerImpl::FillOutputForSectionWithComparator(
       if (ShouldSaveDetailsLocally())
         SaveProfileGleanedFromSection(profile, section);
 
-      FillFormStructureForSection(&profile, 0, section, compare);
+      AutofillProfileWrapper profile_wrapper(&profile, 0);
+      profile_wrapper.FillFormStructure(inputs, compare, &form_structure_);
     }
   }
 }
@@ -2265,30 +2273,6 @@ void AutofillDialogControllerImpl::FillOutputForSection(DialogSection section) {
                                      base::Bind(DetailInputMatchesField));
 }
 
-bool AutofillDialogControllerImpl::FillFormStructureForSection(
-    const AutofillDataModel* data_model,
-    size_t variant,
-    DialogSection section,
-    const InputFieldComparator& compare) {
-  bool found_match = false;
-  std::string app_locale = g_browser_process->GetApplicationLocale();
-  for (size_t i = 0; i < form_structure_.field_count(); ++i) {
-    AutofillField* field = form_structure_.field(i);
-    // Only fill in data that is associated with this section.
-    const DetailInputs& inputs = RequestedFieldsForSection(section);
-    for (size_t j = 0; j < inputs.size(); ++j) {
-      if (compare.Run(inputs[j], *field)) {
-        found_match = true;
-        if (data_model)
-          data_model->FillFormField(*field, variant, app_locale, field);
-        break;
-      }
-    }
-  }
-
-  return found_match;
-}
-
 bool AutofillDialogControllerImpl::FormStructureCaresAboutSection(
     DialogSection section) const {
   // For now, only SECTION_SHIPPING may be omitted due to a site not asking for
diff --git a/chrome/browser/ui/autofill/autofill_dialog_controller_impl.h b/chrome/browser/ui/autofill/autofill_dialog_controller_impl.h
index 03113f6..791ecea 100644
--- a/chrome/browser/ui/autofill/autofill_dialog_controller_impl.h
+++ b/chrome/browser/ui/autofill/autofill_dialog_controller_impl.h
@@ -333,15 +333,6 @@ class AutofillDialogControllerImpl : public AutofillDialogController,
   void FillOutputForSectionWithComparator(DialogSection section,
                                           const InputFieldComparator& compare);
 
-  // Fills in |form_structure_| using |data_model|. Returns whether any matches
-  // were found in the form structure for the data in |section|. |data_model|
-  // may be NULL, in which case it's just a dry run (where the return value
-  // is still valid).
-  bool FillFormStructureForSection(const AutofillDataModel* data_model,
-                                   size_t variant,
-                                   DialogSection section,
-                                   const InputFieldComparator& compare);
-
   // Returns whether |form_structure|_| has any fields that match the fieldset
   // represented by |section|.
   bool FormStructureCaresAboutSection(DialogSection section) const;
diff --git a/chrome/browser/ui/autofill/autofill_dialog_controller_unittest.cc b/chrome/browser/ui/autofill/autofill_dialog_controller_unittest.cc
index e117790..53d410f 100644
--- a/chrome/browser/ui/autofill/autofill_dialog_controller_unittest.cc
+++ b/chrome/browser/ui/autofill/autofill_dialog_controller_unittest.cc
@@ -46,7 +46,27 @@ const char kFakeEmail[] = "user@example.com";
 const char kFakeFingerprintEncoded[] = "CgVaAwiACA==";
 const char kEditedBillingAddress[] = "123 edited billing address";
 const char* kFieldsFromPage[] =
-    { "email", "cc-number", "billing region", "shipping region" };
+    { "email",
+      "cc-name",
+      "cc-number",
+      "cc-exp-month",
+      "cc-exp-year",
+      "cc-csc",
+      "billing name",
+      "billing address-line1",
+      "billing locality",
+      "billing region",
+      "billing postal-code",
+      "billing country",
+      "billing tel",
+      "shipping name",
+      "shipping address-line1",
+      "shipping locality",
+      "shipping region",
+      "shipping postal-code",
+      "shipping country",
+      "shipping tel",
+    };
 const char kSettingsOrigin[] = "Chrome settings";
 
 using content::BrowserThread;
@@ -777,11 +797,29 @@ TEST_F(AutofillDialogControllerTest, DontUseBillingAsShipping) {
   shipping_model->ActivatedAt(2);
 
   controller()->OnAccept();
-  ASSERT_EQ(4U, form_structure()->field_count());
-  EXPECT_EQ("CA", UTF16ToUTF8(form_structure()->field(2)->value));
-  EXPECT_EQ("MI", UTF16ToUTF8(form_structure()->field(3)->value));
-  EXPECT_EQ(ADDRESS_BILLING_STATE, form_structure()->field(2)->type());
-  EXPECT_EQ(ADDRESS_HOME_STATE, form_structure()->field(3)->type());
+  ASSERT_EQ(20U, form_structure()->field_count());
+  EXPECT_EQ(ADDRESS_BILLING_STATE, form_structure()->field(9)->type());
+  EXPECT_EQ(ADDRESS_HOME_STATE, form_structure()->field(16)->type());
+  string16 billing_state = form_structure()->field(9)->value;
+  string16 shipping_state = form_structure()->field(16)->value;
+  EXPECT_FALSE(billing_state.empty());
+  EXPECT_FALSE(shipping_state.empty());
+  EXPECT_NE(billing_state, shipping_state);
+
+  EXPECT_EQ(CREDIT_CARD_NAME, form_structure()->field(1)->type());
+  string16 cc_name = form_structure()->field(1)->value;
+  EXPECT_EQ(NAME_FULL, form_structure()->field(6)->type());
+  string16 billing_name = form_structure()->field(6)->value;
+  EXPECT_EQ(NAME_FULL, form_structure()->field(13)->type());
+  string16 shipping_name = form_structure()->field(13)->value;
+
+  EXPECT_FALSE(cc_name.empty());
+  EXPECT_FALSE(billing_name.empty());
+  EXPECT_FALSE(shipping_name.empty());
+  // Billing name should always be the same as cardholder name.
+  // TODO(estade): this currently fails. http://crbug.com/246417
+  // EXPECT_EQ(cc_name, billing_name);
+  EXPECT_NE(cc_name, shipping_name);
 }
 
 // Test selecting UseBillingForShipping.
@@ -797,11 +835,27 @@ TEST_F(AutofillDialogControllerTest, UseBillingAsShipping) {
   UseBillingForShipping();
 
   controller()->OnAccept();
-  ASSERT_EQ(4U, form_structure()->field_count());
-  EXPECT_EQ("CA", UTF16ToUTF8(form_structure()->field(2)->value));
-  EXPECT_EQ("CA", UTF16ToUTF8(form_structure()->field(3)->value));
-  EXPECT_EQ(ADDRESS_BILLING_STATE, form_structure()->field(2)->type());
-  EXPECT_EQ(ADDRESS_HOME_STATE, form_structure()->field(3)->type());
+  ASSERT_EQ(20U, form_structure()->field_count());
+  EXPECT_EQ(ADDRESS_BILLING_STATE, form_structure()->field(9)->type());
+  EXPECT_EQ(ADDRESS_HOME_STATE, form_structure()->field(16)->type());
+  string16 billing_state = form_structure()->field(9)->value;
+  string16 shipping_state = form_structure()->field(16)->value;
+  EXPECT_FALSE(billing_state.empty());
+  EXPECT_FALSE(shipping_state.empty());
+  EXPECT_EQ(billing_state, shipping_state);
+
+  EXPECT_EQ(CREDIT_CARD_NAME, form_structure()->field(1)->type());
+  string16 cc_name = form_structure()->field(1)->value;
+  EXPECT_EQ(NAME_FULL, form_structure()->field(6)->type());
+  string16 billing_name = form_structure()->field(6)->value;
+  EXPECT_EQ(NAME_FULL, form_structure()->field(13)->type());
+  string16 shipping_name = form_structure()->field(13)->value;
+
+  EXPECT_FALSE(cc_name.empty());
+  EXPECT_FALSE(billing_name.empty());
+  EXPECT_FALSE(shipping_name.empty());
+  EXPECT_EQ(cc_name, billing_name);
+  EXPECT_EQ(cc_name, shipping_name);
 }
 
 // Tests that shipping and billing telephone fields are supported, and filled
@@ -1723,11 +1777,11 @@ TEST_F(AutofillDialogControllerTest, AutofillTypes) {
   controller()->OnDidGetWalletItems(wallet::GetTestWalletItems());
   controller()->OnAccept();
   controller()->OnDidGetFullWallet(wallet::GetTestFullWallet());
-  ASSERT_EQ(4U, form_structure()->field_count());
+  ASSERT_EQ(20U, form_structure()->field_count());
   EXPECT_EQ(EMAIL_ADDRESS, form_structure()->field(0)->type());
-  EXPECT_EQ(CREDIT_CARD_NUMBER, form_structure()->field(1)->type());
-  EXPECT_EQ(ADDRESS_BILLING_STATE, form_structure()->field(2)->type());
-  EXPECT_EQ(ADDRESS_HOME_STATE, form_structure()->field(3)->type());
+  EXPECT_EQ(CREDIT_CARD_NUMBER, form_structure()->field(2)->type());
+  EXPECT_EQ(ADDRESS_BILLING_STATE, form_structure()->field(9)->type());
+  EXPECT_EQ(ADDRESS_HOME_STATE, form_structure()->field(16)->type());
 }
 
 TEST_F(AutofillDialogControllerTest, SaveDetailsInChrome) {
diff --git a/chrome/browser/ui/autofill/data_model_wrapper.cc b/chrome/browser/ui/autofill/data_model_wrapper.cc
index cbed133..76674b5 100644
--- a/chrome/browser/ui/autofill/data_model_wrapper.cc
+++ b/chrome/browser/ui/autofill/data_model_wrapper.cc
@@ -37,19 +37,22 @@ string16 DataModelWrapper::GetDisplayText() {
   return label;
 }
 
-void DataModelWrapper::FillFormStructure(
+bool DataModelWrapper::FillFormStructure(
     const DetailInputs& inputs,
     const InputFieldComparator& compare,
-    FormStructure* form_structure) {
+    FormStructure* form_structure) const {
+  bool filled_something = false;
   for (size_t i = 0; i < form_structure->field_count(); ++i) {
     AutofillField* field = form_structure->field(i);
     for (size_t j = 0; j < inputs.size(); ++j) {
       if (compare.Run(inputs[j], *field)) {
         FillFormField(field);
+        filled_something = true;
         break;
       }
     }
   }
+  return filled_something;
 }
 
 void DataModelWrapper::FillInputs(DetailInputs* inputs) {
@@ -58,7 +61,7 @@ void DataModelWrapper::FillInputs(DetailInputs* inputs) {
   }
 }
 
-void DataModelWrapper::FillFormField(AutofillField* field) {
+void DataModelWrapper::FillFormField(AutofillField* field) const {
   field->value = GetInfo(field->type());
 }
 
@@ -68,6 +71,17 @@ gfx::Image DataModelWrapper::GetIcon() {
   return gfx::Image();
 }
 
+// EmptyDataModelWrapper
+
+EmptyDataModelWrapper::EmptyDataModelWrapper() {}
+EmptyDataModelWrapper::~EmptyDataModelWrapper() {}
+
+string16 EmptyDataModelWrapper::GetInfo(AutofillFieldType type) const {
+  return string16();
+}
+
+void EmptyDataModelWrapper::FillFormField(AutofillField* field) const {}
+
 // AutofillDataModelWrapper
 
 AutofillDataModelWrapper::AutofillDataModelWrapper(
@@ -78,11 +92,11 @@ AutofillDataModelWrapper::AutofillDataModelWrapper(
 
 AutofillDataModelWrapper::~AutofillDataModelWrapper() {}
 
-string16 AutofillDataModelWrapper::GetInfo(AutofillFieldType type) {
+string16 AutofillDataModelWrapper::GetInfo(AutofillFieldType type) const {
   return data_model_->GetInfo(type, g_browser_process->GetApplicationLocale());
 }
 
-void AutofillDataModelWrapper::FillFormField(AutofillField* field) {
+void AutofillDataModelWrapper::FillFormField(AutofillField* field) const {
   data_model_->FillFormField(
       *field, variant_, g_browser_process->GetApplicationLocale(), field);
 }
@@ -113,7 +127,7 @@ AutofillCreditCardWrapper::AutofillCreditCardWrapper(const CreditCard* card)
 
 AutofillCreditCardWrapper::~AutofillCreditCardWrapper() {}
 
-string16 AutofillCreditCardWrapper::GetInfo(AutofillFieldType type) {
+string16 AutofillCreditCardWrapper::GetInfo(AutofillFieldType type) const {
   if (type == CREDIT_CARD_EXP_MONTH)
     return MonthComboboxModel::FormatMonth(card_->expiration_month());
 
@@ -136,7 +150,7 @@ string16 AutofillCreditCardWrapper::GetDisplayText() {
   return card_->TypeAndLastFourDigits();
 }
 
-void AutofillCreditCardWrapper::FillFormField(AutofillField* field) {
+void AutofillCreditCardWrapper::FillFormField(AutofillField* field) const {
   AutofillFieldType field_type = field->type();
 
   if (field_type == NAME_FULL) {
@@ -158,7 +172,7 @@ WalletAddressWrapper::WalletAddressWrapper(
 
 WalletAddressWrapper::~WalletAddressWrapper() {}
 
-string16 WalletAddressWrapper::GetInfo(AutofillFieldType type) {
+string16 WalletAddressWrapper::GetInfo(AutofillFieldType type) const {
   return address_->GetInfo(type, g_browser_process->GetApplicationLocale());
 }
 
@@ -179,7 +193,7 @@ WalletInstrumentWrapper::WalletInstrumentWrapper(
 
 WalletInstrumentWrapper::~WalletInstrumentWrapper() {}
 
-string16 WalletInstrumentWrapper::GetInfo(AutofillFieldType type) {
+string16 WalletInstrumentWrapper::GetInfo(AutofillFieldType type) const {
   if (type == CREDIT_CARD_EXP_MONTH)
     return MonthComboboxModel::FormatMonth(instrument_->expiration_month());
 
@@ -214,7 +228,7 @@ FullWalletBillingWrapper::FullWalletBillingWrapper(
 
 FullWalletBillingWrapper::~FullWalletBillingWrapper() {}
 
-string16 FullWalletBillingWrapper::GetInfo(AutofillFieldType type) {
+string16 FullWalletBillingWrapper::GetInfo(AutofillFieldType type) const {
   if (AutofillType(type).group() == AutofillType::CREDIT_CARD)
     return full_wallet_->GetInfo(type);
 
@@ -240,7 +254,7 @@ FullWalletShippingWrapper::FullWalletShippingWrapper(
 
 FullWalletShippingWrapper::~FullWalletShippingWrapper() {}
 
-string16 FullWalletShippingWrapper::GetInfo(AutofillFieldType type) {
+string16 FullWalletShippingWrapper::GetInfo(AutofillFieldType type) const {
   return full_wallet_->shipping_address()->GetInfo(
       type, g_browser_process->GetApplicationLocale());
 }
diff --git a/chrome/browser/ui/autofill/data_model_wrapper.h b/chrome/browser/ui/autofill/data_model_wrapper.h
index 958de00..c81d80f 100644
--- a/chrome/browser/ui/autofill/data_model_wrapper.h
+++ b/chrome/browser/ui/autofill/data_model_wrapper.h
@@ -35,7 +35,7 @@ class DataModelWrapper {
   virtual ~DataModelWrapper();
 
   // Returns the data for a specific autocomplete type.
-  virtual string16 GetInfo(AutofillFieldType type) = 0;
+  virtual string16 GetInfo(AutofillFieldType type) const = 0;
 
   // Returns the icon, if any, that represents this model.
   virtual gfx::Image GetIcon();
@@ -50,32 +50,48 @@ class DataModelWrapper {
 
   // Fills in |form_structure| with the data that this model contains. |inputs|
   // and |comparator| are used to determine whether each field in the
-  // FormStructure should be filled in or left alone.
-  void FillFormStructure(
+  // FormStructure should be filled in or left alone. Returns whether any fields
+  // in |form_structure| were found to be matching.
+  bool FillFormStructure(
       const DetailInputs& inputs,
       const InputFieldComparator& compare,
-      FormStructure* form_structure);
+      FormStructure* form_structure) const;
 
  protected:
   DataModelWrapper();
 
   // Fills in |field| with data from the model.
-  virtual void FillFormField(AutofillField* field);
+  virtual void FillFormField(AutofillField* field) const;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(DataModelWrapper);
 };
 
+// A DataModelWrapper that does not hold data and does nothing when told to
+// fill in a form.
+class EmptyDataModelWrapper : public DataModelWrapper {
+ public:
+  EmptyDataModelWrapper();
+  virtual ~EmptyDataModelWrapper();
+
+  virtual string16 GetInfo(AutofillFieldType type) const OVERRIDE;
+
+ protected:
+  virtual void FillFormField(AutofillField* field) const OVERRIDE;
+
+  DISALLOW_COPY_AND_ASSIGN(EmptyDataModelWrapper);
+};
+
 // A DataModelWrapper for Autofill data.
 class AutofillDataModelWrapper : public DataModelWrapper {
  public:
   AutofillDataModelWrapper(const AutofillDataModel* data_model, size_t variant);
   virtual ~AutofillDataModelWrapper();
 
-  virtual string16 GetInfo(AutofillFieldType type) OVERRIDE;
+  virtual string16 GetInfo(AutofillFieldType type) const OVERRIDE;
 
  protected:
-  virtual void FillFormField(AutofillField* field) OVERRIDE;
+  virtual void FillFormField(AutofillField* field) const OVERRIDE;
 
   size_t variant() const { return variant_; }
 
@@ -106,12 +122,12 @@ class AutofillCreditCardWrapper : public AutofillDataModelWrapper {
   explicit AutofillCreditCardWrapper(const CreditCard* card);
   virtual ~AutofillCreditCardWrapper();
 
-  virtual string16 GetInfo(AutofillFieldType type) OVERRIDE;
+  virtual string16 GetInfo(AutofillFieldType type) const OVERRIDE;
   virtual gfx::Image GetIcon() OVERRIDE;
   virtual string16 GetDisplayText() OVERRIDE;
 
  protected:
-  virtual void FillFormField(AutofillField* field) OVERRIDE;
+  virtual void FillFormField(AutofillField* field) const OVERRIDE;
 
  private:
   const CreditCard* card_;
@@ -125,7 +141,7 @@ class WalletAddressWrapper : public DataModelWrapper {
   explicit WalletAddressWrapper(const wallet::Address* address);
   virtual ~WalletAddressWrapper();
 
-  virtual string16 GetInfo(AutofillFieldType type) OVERRIDE;
+  virtual string16 GetInfo(AutofillFieldType type) const OVERRIDE;
   virtual string16 GetDisplayText() OVERRIDE;
 
  private:
@@ -141,7 +157,7 @@ class WalletInstrumentWrapper : public DataModelWrapper {
       const wallet::WalletItems::MaskedInstrument* instrument);
   virtual ~WalletInstrumentWrapper();
 
-  virtual string16 GetInfo(AutofillFieldType type) OVERRIDE;
+  virtual string16 GetInfo(AutofillFieldType type) const OVERRIDE;
   virtual gfx::Image GetIcon() OVERRIDE;
   virtual string16 GetDisplayText() OVERRIDE;
 
@@ -157,7 +173,7 @@ class FullWalletBillingWrapper : public DataModelWrapper {
   explicit FullWalletBillingWrapper(wallet::FullWallet* full_wallet);
   virtual ~FullWalletBillingWrapper();
 
-  virtual string16 GetInfo(AutofillFieldType type) OVERRIDE;
+  virtual string16 GetInfo(AutofillFieldType type) const OVERRIDE;
   virtual string16 GetDisplayText() OVERRIDE;
 
  private:
@@ -172,7 +188,7 @@ class FullWalletShippingWrapper : public DataModelWrapper {
   explicit FullWalletShippingWrapper(wallet::FullWallet* full_wallet);
   virtual ~FullWalletShippingWrapper();
 
-  virtual string16 GetInfo(AutofillFieldType type) OVERRIDE;
+  virtual string16 GetInfo(AutofillFieldType type) const OVERRIDE;
 
  private:
   wallet::FullWallet* full_wallet_;
diff --git a/components/autofill/browser/credit_card.cc b/components/autofill/browser/credit_card.cc
index 056a79c..92bd9b0 100644
--- a/components/autofill/browser/credit_card.cc
+++ b/components/autofill/browser/credit_card.cc
@@ -341,7 +341,7 @@ void CreditCard::SetRawInfo(AutofillFieldType type,
 }
 
 base::string16 CreditCard::GetInfo(AutofillFieldType type,
-                             const std::string& app_locale) const {
+                                   const std::string& app_locale) const {
   if (type == CREDIT_CARD_NUMBER)
     return StripSeparators(number_);