Do not haul suggestions back to browser in AutofillHostMsg_ShowPasswordSuggestions

components/autofill/content/renderer/password_autofill_agent.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/autofill/content/renderer/password_autofill_agent.h"
 
 #include "base/bind.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop/message_loop.h"
 #include "base/metrics/histogram.h"
@@ skipping 222 matching lines @@
                  const blink::WebFormElement& form) {
   logger->LogHTMLForm(message_id,
                       form.name().utf8(),
                       GURL(form.action().utf8()));
 }
 
 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));
-  }
-
-  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));
+// 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::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));
-      }
+  // 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 (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|
 // with values from |fill_data|. The |password_element| will only have the
 // |suggestedValue| set, and will be registered for copying that to the real
 // value through |registration_callback|. The function returns true when
 // selected username comes from |fill_data.other_possible_usernames|.
 bool FillUserNameAndPassword(
     blink::WebInputElement* username_element,
     blink::WebInputElement* password_element,
@@ skipping 809 matching lines @@
     const blink::WebInputElement& user_input,
     bool show_all) {
   blink::WebFrame* frame = user_input.document().frame();
   if (!frame)
     return false;
 
   blink::WebView* webview = frame->view();
   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(
       user_input, &form, &field, REQUIRE_NONE);
 
   blink::WebInputElement selected_element = user_input;
   gfx::Rect bounding_box(selected_element.boundsInViewportSpace());
 
   float scale = web_view_->pageScaleFactor();
   gfx::RectF bounding_box_scaled(bounding_box.x() * scale,
                                  bounding_box.y() * scale,
                                  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(
     const blink::WebInputElement& username_input,
     const blink::WebInputElement& password_input,
     const PasswordFormFillData& fill_data) {
   DCHECK(!fill_data.wait_for_username);
 
   // We need non-const versions of the username and password inputs.
   blink::WebInputElement username = username_input;
@@ skipping 97 matching lines @@
   scoped_ptr<PasswordForm> password_form(CreatePasswordForm(form));
   if (!password_form || (restriction == RESTRICTION_NON_EMPTY_PASSWORD &&
                          password_form->password_value.empty() &&
                          password_form->new_password_value.empty())) {
     return;
   }
   provisionally_saved_forms_[frame].reset(password_form.release());
 }
 
 }  // namespace autofill