Do not haul suggestions back to browser in AutofillHostMsg_ShowPasswordSuggestions

components/autofill/content/renderer/password_autofill_agent.cc

Index: components/autofill/content/renderer/password_autofill_agent.cc
diff --git a/components/autofill/content/renderer/password_autofill_agent.cc b/components/autofill/content/renderer/password_autofill_agent.cc
index c5986925655c495ce14568681a46695e53a3f80d..0e432c7964c229cfd9a48d9437e303ff5be18c6c 100644
--- a/components/autofill/content/renderer/password_autofill_agent.cc
+++ b/components/autofill/content/renderer/password_autofill_agent.cc
@@ -240,45 +240,42 @@ bool FillDataContainsUsername(const PasswordFormFillData& fill_data) {
   return !fill_data.basic_data.fields[0].name.empty();
 }
 
-// This function attempts to fill |suggestions| and |realms| form |fill_data|
-// based on |current_username|. Returns true when |suggestions| gets filled
-// from |fill_data.other_possible_usernames|, else returns false.
-bool GetSuggestions(const PasswordFormFillData& fill_data,
-                    const base::string16& current_username,
-                    std::vector<base::string16>* suggestions,
-                    std::vector<base::string16>* realms,
-                    bool show_all) {
-  bool other_possible_username_shown = false;
-  if (show_all ||
-      StartsWith(
-          fill_data.basic_data.fields[0].value, current_username, false)) {
-    suggestions->push_back(fill_data.basic_data.fields[0].value);
-    realms->push_back(base::UTF8ToUTF16(fill_data.preferred_realm));
+// Sets |suggestions_present| to true if there are any suggestions to be derived
+// from |fill_data|. Unless |show_all| is true, only considers suggestions with
+// usernames having |current_username| as a prefix. Returns true if a username
+// from the |fill_data.other_possible_usernames| would be included in the
+// suggestions.
+bool GetSuggestionsStats(const PasswordFormFillData& fill_data,
+                         const base::string16& current_username,
+                         bool show_all,
+                         bool* suggestions_present) {
+  *suggestions_present = false;
+  // First, check the other possible usernames.
+  for (const auto& usernames : fill_data.other_possible_usernames) {
+    for (size_t i = 0; i < usernames.second.size(); ++i) {
+      if (show_all ||
+          StartsWith(usernames.second[i], current_username, false)) {
+        *suggestions_present = true;
+        return true;
+      }
+    }
   }
 
-  for (PasswordFormFillData::LoginCollection::const_iterator iter =
-           fill_data.additional_logins.begin();
-       iter != fill_data.additional_logins.end();
-       ++iter) {
-    if (show_all || StartsWith(iter->first, current_username, false)) {
-      suggestions->push_back(iter->first);
-      realms->push_back(base::UTF8ToUTF16(iter->second.realm));
-    }
+  // No other possible usernames, but perhaps at least some suggestion?

[Garrett Casto, 2014/10/30 22:33:29]

Nit: Personally I don't think that these comments are necessary given the
simplicity of the code and the function comment, but if you want to keep it I
would change it to be a statement not a question.

[vabr (Chromium), 2014/10/31 08:54:12]

Done, by removing, also the one on line 253.
I'm sometimes prone to over-commenting, so it's good to have another person
assessing the real need for the comment. :)

+  if (show_all || StartsWith(fill_data.basic_data.fields[0].value,
+                             current_username, false)) {
+    *suggestions_present = true;
+    return false;
   }
 
-  for (PasswordFormFillData::UsernamesCollection::const_iterator iter =
-           fill_data.other_possible_usernames.begin();
-       iter != fill_data.other_possible_usernames.end();
-       ++iter) {
-    for (size_t i = 0; i < iter->second.size(); ++i) {
-      if (show_all || StartsWith(iter->second[i], current_username, false)) {
-        other_possible_username_shown = true;
-        suggestions->push_back(iter->second[i]);
-        realms->push_back(base::UTF8ToUTF16(iter->first.realm));
-      }
+  for (const auto& login : fill_data.additional_logins) {
+    if (show_all || StartsWith(login.first, current_username, false)) {
+      *suggestions_present = true;
+      return false;
     }
   }
-  return other_possible_username_shown;
+
+  return false;
 }
 
 // This function attempts to fill |username_element| and |password_element|
@@ -1108,15 +1105,6 @@ bool PasswordAutofillAgent::ShowSuggestionPopup(
   if (!webview)
     return false;
 
-  std::vector<base::string16> suggestions;
-  std::vector<base::string16> realms;
-  if (GetSuggestions(
-          fill_data, user_input.value(), &suggestions, &realms, show_all)) {
-    usernames_usage_ = OTHER_POSSIBLE_USERNAME_SHOWN;
-  }
-
-  DCHECK_EQ(suggestions.size(), realms.size());
-
   FormData form;
   FormFieldData field;
   FindFormAndFieldForFormControlElement(
@@ -1131,8 +1119,14 @@ bool PasswordAutofillAgent::ShowSuggestionPopup(
                                  bounding_box.width() * scale,
                                  bounding_box.height() * scale);
   Send(new AutofillHostMsg_ShowPasswordSuggestions(
-      routing_id(), field, bounding_box_scaled, suggestions, realms));
-  return !suggestions.empty();
+      routing_id(), field, user_input.value(), show_all, bounding_box_scaled));
+
+  bool suggestions_present = false;
+  if (GetSuggestionsStats(fill_data, user_input.value(), show_all,
+                          &suggestions_present)) {
+    usernames_usage_ = OTHER_POSSIBLE_USERNAME_SHOWN;
+  }
+  return suggestions_present;
 }
 
 void PasswordAutofillAgent::PerformInlineAutocomplete(