Never allow empty preview entries. As some data is always present (we drop completely empty profiles),

display this data in preview if name and first line of address is not available.
TEST=in the bug + unit-test
BUG=50320
Review URL: http://codereview.chromium.org/3806008

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

10 files changed, 74 insertions, 106 deletions

diff --git a/chrome/browser/autofill/autofill_dialog_controller_mac.mm b/chrome/browser/autofill/autofill_dialog_controller_mac.mm
index 46acdab..4d47df1 100644
--- a/chrome/browser/autofill/autofill_dialog_controller_mac.mm
+++ b/chrome/browser/autofill/autofill_dialog_controller_mac.mm
@@ -32,20 +32,6 @@ namespace {
 // dialog.
 typedef std::map<Profile*, AutoFillDialogController*> ProfileControllerMap;
 
-// Update profile labels passed as |input|.  When profile data changes as a
-// result of adding new profiles, edititing existing profiles, or deleting a
-// profile, then the list of profiles need to have their derived labels
-// recomputed.
-void UpdateProfileLabels(std::vector<AutoFillProfile>* input) {
-  DCHECK(input);
-  std::vector<AutoFillProfile*> profiles;
-  profiles.resize(input->size());
-  for (size_t i = 0; i < input->size(); ++i) {
-    profiles[i] = &(*input)[i];
-  }
-  AutoFillProfile::AdjustInferredLabels(&profiles);
-}
-
 }  // namespace
 
 // Delegate protocol that needs to be in place for the AutoFillTableView's
@@ -839,7 +825,6 @@ class PreferenceObserver : public NotificationObserver {
        iter != creditCards.end(); ++iter)
     creditCards_.push_back(**iter);
 
-  UpdateProfileLabels(&profiles_);
   [tableView_ reloadData];
 }
 
diff --git a/chrome/browser/autofill/autofill_dialog_gtk.cc b/chrome/browser/autofill/autofill_dialog_gtk.cc
index bc305ff..de74b94 100644
--- a/chrome/browser/autofill/autofill_dialog_gtk.cc
+++ b/chrome/browser/autofill/autofill_dialog_gtk.cc
@@ -612,7 +612,7 @@ void AutoFillDialog::AddAddressToTree(const AutoFillProfile& profile,
       list_store_, iter,
       COL_WEIGHT, PANGO_WEIGHT_NORMAL,
       COL_WEIGHT_SET, TRUE,
-      COL_TITLE, UTF16ToUTF8(profile.PreviewSummary()).c_str(),
+      COL_TITLE, UTF16ToUTF8(profile.Label()).c_str(),
       -1);
 }
 
diff --git a/chrome/browser/autofill/autofill_profile.cc b/chrome/browser/autofill/autofill_profile.cc
index 7db628e..c26af93 100644
--- a/chrome/browser/autofill/autofill_profile.cc
+++ b/chrome/browser/autofill/autofill_profile.cc
@@ -175,50 +175,6 @@ const string16& AutoFillProfile::Label() const {
   return label_;
 }
 
-string16 AutoFillProfile::PreviewSummary() const {
-  // Fetch the components of the summary string.  Any or all of these
-  // may be an empty string.
-  string16 first_name = GetFieldText(AutoFillType(NAME_FIRST));
-  string16 last_name = GetFieldText(AutoFillType(NAME_LAST));
-  string16 address = GetFieldText(AutoFillType(ADDRESS_HOME_LINE1));
-
-  // String separators depend (below) on the existence of the various fields.
-  bool have_first_name = first_name.length() > 0;
-  bool have_last_name = last_name.length() > 0;
-  bool have_address = address.length() > 0;
-
-  // Name separator defaults to "". Space if we have first and last name.
-  string16 name_separator;
-
-  if (have_first_name && have_last_name) {
-    name_separator = l10n_util::GetStringUTF16(
-        IDS_AUTOFILL_DIALOG_ADDRESS_NAME_SEPARATOR);
-  }
-
-  // E.g. "John Smith", or "John", or "Smith", or "".
-  string16 name_format = l10n_util::GetStringFUTF16(
-      IDS_AUTOFILL_DIALOG_ADDRESS_SUMMARY_NAME_FORMAT,
-      first_name,
-      name_separator,
-      last_name);
-
-  // Summary separator defaults to "".  ", " if we have name and address.
-  string16 summary_separator;
-  if ((have_first_name || have_last_name) && have_address) {
-    summary_separator = l10n_util::GetStringUTF16(
-        IDS_AUTOFILL_DIALOG_ADDRESS_SUMMARY_SEPARATOR);
-  }
-
-  // E.g. "John Smith, 123 Main Street".
-  string16 summary_format = l10n_util::GetStringFUTF16(
-      IDS_AUTOFILL_DIALOG_ADDRESS_SUMMARY_FORMAT,
-      name_format,
-      summary_separator,
-      address);
-
-  return summary_format;
-}
-
 // static
 bool AutoFillProfile::AdjustInferredLabels(
     std::vector<AutoFillProfile*>* profiles) {
diff --git a/chrome/browser/autofill/autofill_profile.h b/chrome/browser/autofill/autofill_profile.h
index 4336139b..cff668c 100644
--- a/chrome/browser/autofill/autofill_profile.h
+++ b/chrome/browser/autofill/autofill_profile.h
@@ -46,6 +46,9 @@ class AutoFillProfile : public FormGroup {
   // Returns a copy of the profile it is called on. The caller is responsible
   // for deleting profile when they are done with it.
   virtual FormGroup* Clone() const;
+  // The user-visible label of the profile, generated in relation to other
+  // profiles. Shows at least 2 fields that differentiate profile from other
+  // profiles. See AdjustInferredLabels() further down for more description.
   virtual const string16& Label() const;
 
   int unique_id() const { return unique_id_; }
@@ -55,16 +58,6 @@ class AutoFillProfile : public FormGroup {
   const std::string guid() const { return guid_; }
   void set_guid(const std::string& guid) { guid_ = guid; }
 
-  // Profile summary string for UI.
-  // Constructs a summary string based on NAME_FIRST, NAME_LAST, and
-  // ADDRESS_HOME_LINE1 fields of the profile.  The summary string is of the
-  // form:
-  //     L"<first_name> <last_name>, <address_line_1>"
-  // but may omit any or all of the fields if they are not present in the
-  // profile.
-  // The form of the string is governed by generated resources.
-  string16 PreviewSummary() const;
-
   // Adjusts the labels according to profile data.
   // Labels contain minimal different combination of:
   // 1. Full name.
diff --git a/chrome/browser/autofill/autofill_profile_unittest.cc b/chrome/browser/autofill/autofill_profile_unittest.cc
index 5a1f27b..a18bbe9 100644
--- a/chrome/browser/autofill/autofill_profile_unittest.cc
+++ b/chrome/browser/autofill/autofill_profile_unittest.cc
@@ -15,77 +15,114 @@
 
 namespace {
 
+bool UpdateProfileLabel(AutoFillProfile *profile) {
+  std::vector<AutoFillProfile*> profiles;
+  profiles.push_back(profile);
+  return AutoFillProfile::AdjustInferredLabels(&profiles);
+}
+
 // Tests different possibilities for summary string generation.
 // Based on existence of first name, last name, and address line 1.
 TEST(AutoFillProfileTest, PreviewSummaryString) {
   // Case 0/null: ""
   AutoFillProfile profile0(string16(), 0);
-  string16 summary0 = profile0.PreviewSummary();
+  // Empty profile - nothing to update.
+  EXPECT_FALSE(UpdateProfileLabel(&profile0));
+  string16 summary0 = profile0.Label();
   EXPECT_EQ(string16(), summary0);
 
-  // Case 0/empty: ""
+  // Case 0a/empty name and address, so the first two fields of the rest of the
+  // data is used: "Hollywood, CA"
   AutoFillProfile profile00(string16(), 0);
   autofill_test::SetProfileInfo(&profile00, "Billing", "", "Mitchell", "",
       "johnwayne@me.xyz", "Fox", "", "unit 5", "Hollywood", "CA", "91601", "US",
       "12345678910", "01987654321");
-  string16 summary00 = profile00.PreviewSummary();
-  EXPECT_EQ(string16(), summary00);
+  EXPECT_TRUE(UpdateProfileLabel(&profile00));
+  string16 summary00 = profile00.Label();
+  EXPECT_EQ(string16(ASCIIToUTF16("Hollywood, CA")), summary00);
 
   // Case 1: "<address>"
   AutoFillProfile profile1(string16(), 0);
   autofill_test::SetProfileInfo(&profile1, "Billing", "", "Mitchell", "",
       "johnwayne@me.xyz", "Fox", "123 Zoo St.", "unit 5", "Hollywood", "CA",
       "91601", "US", "12345678910", "01987654321");
-  string16 summary1 = profile1.PreviewSummary();
-  EXPECT_EQ(string16(ASCIIToUTF16("123 Zoo St.")), summary1);
+  EXPECT_TRUE(UpdateProfileLabel(&profile1));
+  string16 summary1 = profile1.Label();
+  EXPECT_EQ(string16(ASCIIToUTF16("123 Zoo St., Hollywood")), summary1);
 
   // Case 2: "<lastname>"
   AutoFillProfile profile2(string16(), 0);
   autofill_test::SetProfileInfo(&profile2, "Billing", "", "Mitchell",
       "Morrison", "johnwayne@me.xyz", "Fox", "", "unit 5", "Hollywood", "CA",
       "91601", "US", "12345678910", "01987654321");
-  string16 summary2 = profile2.PreviewSummary();
-  EXPECT_EQ(string16(ASCIIToUTF16("Morrison")), summary2);
+  EXPECT_TRUE(UpdateProfileLabel(&profile2));
+  string16 summary2 = profile2.Label();
+  // Summary does include full name which is empty if the first name is empty.
+  EXPECT_EQ(string16(ASCIIToUTF16("Hollywood, CA")), summary2);
 
   // Case 3: "<lastname>, <address>"
   AutoFillProfile profile3(string16(), 0);
   autofill_test::SetProfileInfo(&profile3, "Billing", "", "Mitchell",
       "Morrison", "johnwayne@me.xyz", "Fox", "123 Zoo St.", "unit 5",
       "Hollywood", "CA", "91601", "US", "12345678910", "01987654321");
-  string16 summary3 = profile3.PreviewSummary();
-  EXPECT_EQ(string16(ASCIIToUTF16("Morrison, 123 Zoo St.")), summary3);
+  EXPECT_TRUE(UpdateProfileLabel(&profile3));
+  string16 summary3 = profile3.Label();
+  EXPECT_EQ(string16(ASCIIToUTF16("123 Zoo St., Hollywood")), summary3);
 
   // Case 4: "<firstname>"
   AutoFillProfile profile4(string16(), 0);
   autofill_test::SetProfileInfo(&profile4, "Billing", "Marion", "Mitchell", "",
       "johnwayne@me.xyz", "Fox", "", "unit 5", "Hollywood", "CA", "91601", "US",
       "12345678910", "01987654321");
-  string16 summary4 = profile4.PreviewSummary();
-  EXPECT_EQ(string16(ASCIIToUTF16("Marion")), summary4);
+  EXPECT_TRUE(UpdateProfileLabel(&profile4));
+  string16 summary4 = profile4.Label();
+  EXPECT_EQ(string16(ASCIIToUTF16("Marion Mitchell, Hollywood")), summary4);
 
   // Case 5: "<firstname>, <address>"
   AutoFillProfile profile5(string16(), 0);
   autofill_test::SetProfileInfo(&profile5, "Billing", "Marion", "Mitchell", "",
       "johnwayne@me.xyz", "Fox", "123 Zoo St.", "unit 5", "Hollywood", "CA",
       "91601", "US", "12345678910", "01987654321");
-  string16 summary5 = profile5.PreviewSummary();
-  EXPECT_EQ(string16(ASCIIToUTF16("Marion, 123 Zoo St.")), summary5);
+  EXPECT_TRUE(UpdateProfileLabel(&profile5));
+  string16 summary5 = profile5.Label();
+  EXPECT_EQ(string16(ASCIIToUTF16("Marion Mitchell, 123 Zoo St.")), summary5);
 
   // Case 6: "<firstname> <lastname>"
   AutoFillProfile profile6(string16(), 0);
   autofill_test::SetProfileInfo(&profile6, "Billing", "Marion", "Mitchell",
       "Morrison", "johnwayne@me.xyz", "Fox", "", "unit 5", "Hollywood", "CA",
       "91601", "US", "12345678910", "01987654321");
-  string16 summary6 = profile6.PreviewSummary();
-  EXPECT_EQ(string16(ASCIIToUTF16("Marion Morrison")), summary6);
+  EXPECT_TRUE(UpdateProfileLabel(&profile6));
+  string16 summary6 = profile6.Label();
+  EXPECT_EQ(string16(ASCIIToUTF16("Marion Mitchell Morrison, Hollywood")),
+            summary6);
 
   // Case 7: "<firstname> <lastname>, <address>"
   AutoFillProfile profile7(string16(), 0);
   autofill_test::SetProfileInfo(&profile7, "Billing", "Marion", "Mitchell",
       "Morrison", "johnwayne@me.xyz", "Fox", "123 Zoo St.", "unit 5",
       "Hollywood", "CA", "91601", "US", "12345678910", "01987654321");
-  string16 summary7 = profile7.PreviewSummary();
-  EXPECT_EQ(string16(ASCIIToUTF16("Marion Morrison, 123 Zoo St.")), summary7);
+  EXPECT_TRUE(UpdateProfileLabel(&profile7));
+  string16 summary7 = profile7.Label();
+  EXPECT_EQ(string16(ASCIIToUTF16("Marion Mitchell Morrison, 123 Zoo St.")),
+            summary7);
+
+  // Case 7a: "<firstname> <lastname>, <address>" - same as #7, except for
+  // e-mail.
+  AutoFillProfile profile7a(string16(), 0);
+  autofill_test::SetProfileInfo(&profile7a, "Billing", "Marion", "Mitchell",
+    "Morrison", "marion@me.xyz", "Fox", "123 Zoo St.", "unit 5",
+    "Hollywood", "CA", "91601", "US", "12345678910", "01987654321");
+  std::vector<AutoFillProfile*> profiles;
+  profiles.push_back(&profile7);
+  profiles.push_back(&profile7a);
+  EXPECT_TRUE(AutoFillProfile::AdjustInferredLabels(&profiles));
+  summary7 = profile7.Label();
+  string16 summary7a = profile7a.Label();
+  EXPECT_EQ(string16(ASCIIToUTF16(
+      "Marion Mitchell Morrison, 123 Zoo St., johnwayne@me.xyz")), summary7);
+  EXPECT_EQ(string16(ASCIIToUTF16(
+      "Marion Mitchell Morrison, 123 Zoo St., marion@me.xyz")), summary7a);
 }
 
 TEST(AutoFillProfileTest, AdjustInferredLabels) {
diff --git a/chrome/browser/autofill/personal_data_manager.cc b/chrome/browser/autofill/personal_data_manager.cc
index 9b4f146..4600cad 100644
--- a/chrome/browser/autofill/personal_data_manager.cc
+++ b/chrome/browser/autofill/personal_data_manager.cc
@@ -87,6 +87,10 @@ void PersonalDataManager::OnWebDataServiceRequestDone(
   // If both requests have responded, then all personal data is loaded.
   if (pending_profiles_query_ == 0 && pending_creditcards_query_ == 0) {
     is_data_loaded_ = true;
+    std::vector<AutoFillProfile*> profile_pointers(web_profiles_.size());
+    std::copy(web_profiles_.begin(), web_profiles_.end(),
+              profile_pointers.begin());
+    AutoFillProfile::AdjustInferredLabels(&profile_pointers);
     FOR_EACH_OBSERVER(Observer, observers_, OnPersonalDataLoaded());
   }
 }
diff --git a/chrome/browser/autofill/personal_data_manager_unittest.cc b/chrome/browser/autofill/personal_data_manager_unittest.cc
index 267933f..ef4f4a5 100644
--- a/chrome/browser/autofill/personal_data_manager_unittest.cc
+++ b/chrome/browser/autofill/personal_data_manager_unittest.cc
@@ -501,8 +501,11 @@ TEST_F(PersonalDataManagerTest, Refresh) {
 
   profile0.set_unique_id(update[0].unique_id());
   profile1.set_unique_id(update[1].unique_id());
-  profile0.set_label(update[0].Label());
-  profile1.set_label(update[1].Label());
+  // Labels depend on other profiles in the list - update labels manually/
+  std::vector<AutoFillProfile *> profile_pointers;
+  profile_pointers.push_back(&profile0);
+  profile_pointers.push_back(&profile1);
+  AutoFillProfile::AdjustInferredLabels(&profile_pointers);
 
   // Wait for the refresh.
   EXPECT_CALL(personal_data_observer_,
@@ -521,6 +524,10 @@ TEST_F(PersonalDataManagerTest, Refresh) {
       "joewayne@me.xyz", "Fox", "1212 Center.", "Bld. 5", "Orlando", "FL",
       "32801", "US", "19482937549", "13502849239");
 
+  // Adjust all labels.
+  profile_pointers.push_back(profile2.get());
+  AutoFillProfile::AdjustInferredLabels(&profile_pointers);
+
   WebDataService* wds = profile_->GetWebDataService(Profile::EXPLICIT_ACCESS);
   ASSERT_TRUE(wds);
   wds->AddAutoFillProfile(*profile2.get());
diff --git a/chrome/browser/dom_ui/options/autofill_options_handler.cc b/chrome/browser/dom_ui/options/autofill_options_handler.cc
index 62cc760..82867cd 100644
--- a/chrome/browser/dom_ui/options/autofill_options_handler.cc
+++ b/chrome/browser/dom_ui/options/autofill_options_handler.cc
@@ -157,7 +157,7 @@ void AutoFillOptionsHandler::LoadAutoFillData() {
            personal_data_->profiles().begin();
        i != personal_data_->profiles().end(); ++i) {
     DictionaryValue* address = new DictionaryValue();
-    address->SetString("label", (*i)->PreviewSummary());
+    address->SetString("label", (*i)->Label());
     address->SetInteger("uniqueID", (*i)->unique_id());
     addresses.Append(address);
   }
diff --git a/chrome/browser/views/autofill_profiles_view_win.cc b/chrome/browser/views/autofill_profiles_view_win.cc
index 7315953..488ec72 100644
--- a/chrome/browser/views/autofill_profiles_view_win.cc
+++ b/chrome/browser/views/autofill_profiles_view_win.cc
@@ -258,15 +258,6 @@ void AutoFillProfilesView::UpdateWidgetState() {
                              autofill_enabled);
 }
 
-void AutoFillProfilesView::UpdateProfileLabels() {
-  std::vector<AutoFillProfile*> profiles;
-  profiles.resize(profiles_set_.size());
-  for (size_t i = 0; i < profiles_set_.size(); ++i) {
-    profiles[i] = &(profiles_set_[i].address);
-  }
-  AutoFillProfile::AdjustInferredLabels(&profiles);
-}
-
 void AutoFillProfilesView::ChildWindowOpened() {
   child_dialog_opened_ = true;
   UpdateWidgetState();
@@ -446,7 +437,6 @@ void AutoFillProfilesView::OnPersonalDataChanged() {
        ++address_it) {
     profiles_set_.push_back(EditableSetInfo(*address_it));
   }
-  UpdateProfileLabels();
 
   credit_card_set_.clear();
   for (std::vector<CreditCard*>::const_iterator cc_it =
@@ -563,7 +553,6 @@ void AutoFillProfilesView::GetData() {
       profiles_set_.push_back(EditableSetInfo(*address_it));
     }
   }
-  UpdateProfileLabels();
 
   if (!imported_data_present) {
     credit_card_set_.reserve(personal_data_manager_->credit_cards().size());
@@ -1313,7 +1302,7 @@ std::wstring AutoFillProfilesView::ContentListTableModel::GetText(
     int row, int column_id) {
   DCHECK(row < static_cast<int>(profiles_->size() + credit_cards_->size()));
   if (row < static_cast<int>(profiles_->size())) {
-    return profiles_->at(row).address.PreviewSummary();
+    return profiles_->at(row).address.Label();
   } else {
     row -= profiles_->size();
     return credit_cards_->at(row).credit_card.PreviewSummary();
diff --git a/chrome/browser/views/autofill_profiles_view_win.h b/chrome/browser/views/autofill_profiles_view_win.h
index e1a6c25..7ccfa63 100644
--- a/chrome/browser/views/autofill_profiles_view_win.h
+++ b/chrome/browser/views/autofill_profiles_view_win.h
@@ -89,9 +89,6 @@ class AutoFillProfilesView : public views::View,
   // Updates state of the buttons.
   void UpdateWidgetState();
 
-  // Updates inferred labels.
-  void UpdateProfileLabels();
-
   // Following two functions are called from opened child dialog to
   // disable/enable buttons.
   void ChildWindowOpened();