requestAutocomplete - more elaborate suggestion labels

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 05b45203c0b53f9cd72844de1d529b136edbef63..de63349b5c878d7dec9664d5e3a5b46fb6f1df1e 100644
--- a/chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc
+++ b/chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc
@@ -30,6 +30,7 @@
 #include "grit/generated_resources.h"
 #include "net/base/cert_status_flags.h"
 #include "ui/base/l10n/l10n_util.h"
+#include "ui/base/resource/resource_bundle.h"
 
 namespace autofill {
 
@@ -329,14 +330,27 @@ string16 AutofillDialogControllerImpl::SuggestionTextForSection(
 
   if (section == SECTION_CC) {
     CreditCard* card = GetManager()->GetCreditCardByGUID(item_key);
-    if (card)
-      return card->Label();
+    if (!card)
+      return string16();

[Ilya Sherman, 2013/01/24 04:27:27]

When would card be null?  The TODO on line 356 makes it sound like this
shouldn't happen, but there's no similar TODO associated with this early return.

[Evan Stade, 2013/01/24 23:34:25]

same reason. I just get tired of repeating the same TODO over and over (it
already exists 5 times in this file).

+
+    return card->TypeAndLastFourDigits();
   } else {
-    // TODO(estade): This doesn't display as much info as it should (for
-    // example, it should show full addresses rather than just a summary).
     AutofillProfile* profile = GetManager()->GetProfileByGUID(item_key);
-    if (profile)
-      return profile->Label();
+    if (!profile)
+      return string16();
+
+    const std::string app_locale = AutofillCountry::ApplicationLocale();
+    string16 comma = ASCIIToUTF16(", ");
+    string16 label = profile->GetInfo(NAME_FULL, app_locale) +
+        comma + profile->GetInfo(ADDRESS_HOME_LINE1, app_locale);
+    string16 address2 = profile->GetInfo(ADDRESS_HOME_LINE2, app_locale);
+    if (!address2.empty())
+      label += comma + address2;
+    label += ASCIIToUTF16("\n") +
+        profile->GetInfo(ADDRESS_HOME_CITY, app_locale) + comma +
+        profile->GetInfo(ADDRESS_HOME_STATE, app_locale) + ASCIIToUTF16(" ") +
+        profile->GetInfo(ADDRESS_HOME_ZIP, app_locale);

[Ilya Sherman, 2013/01/24 04:27:27]

This code is very US-centric.  That's fine for now, but please add a TODO and/or
file a bug to internationalize this code.

[Evan Stade, 2013/01/24 23:34:25]

the entire dialog needs localization, so this fits under http://crbug.com/163656

+    return label;
   }
 
   // TODO(estade): The FormGroup was likely deleted while menu was showing. We
@@ -344,6 +358,21 @@ string16 AutofillDialogControllerImpl::SuggestionTextForSection(
   return string16();

[Ilya Sherman, 2013/01/24 04:27:27]

This code doesn't seem to be reachable anymore...

[Evan Stade, 2013/01/24 23:34:25]

good catch, wonder why the compiler doesn't warn.

 }
 
+gfx::Image AutofillDialogControllerImpl::SuggestionIconForSection(
+    DialogSection section) {
+  if (section != SECTION_CC)
+    return gfx::Image();
+
+  std::string item_key =
+      suggested_cc_.GetItemKeyAt(suggested_cc_.checked_item());
+  CreditCard* card = GetManager()->GetCreditCardByGUID(item_key);
+  if (!card)

[Ilya Sherman, 2013/01/24 04:27:27]

When is this case reachable?  Can it be a DCHECK instead?

[Evan Stade, 2013/01/24 23:34:25]

it's potentially reachable if the credit card is deleted while the dialog is
showing. I don't think it can be a DCHECK, but it should go away when I fix the
TODO about listening for PDM changes.

+    return gfx::Image();
+
+  ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
+  return rb.GetImageNamed(card->IconResourceId());
+}
+
 void AutofillDialogControllerImpl::EditClickedForSection(
     DialogSection section) {
   SuggestionsMenuModel* model = SuggestionsMenuModelForSection(section);