Fill on account select in the password manager

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 2dc195e133eec5b730a9c3eba62b1d3f07919649..e42edd09cdccdf9ca8c0051a3ea336742b112545 100644
--- a/components/autofill/content/renderer/password_autofill_agent.cc
+++ b/components/autofill/content/renderer/password_autofill_agent.cc
@@ -5,6 +5,7 @@
 #include "components/autofill/content/renderer/password_autofill_agent.h"
 
 #include "base/bind.h"
+#include "base/command_line.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop/message_loop.h"
 #include "base/metrics/histogram.h"
@@ -13,6 +14,7 @@
 #include "components/autofill/content/renderer/form_autofill_util.h"
 #include "components/autofill/content/renderer/password_form_conversion_utils.h"
 #include "components/autofill/content/renderer/renderer_save_password_progress_logger.h"
+#include "components/autofill/core/common/autofill_switches.h"
 #include "components/autofill/core/common/form_field_data.h"
 #include "components/autofill/core/common/password_form.h"
 #include "components/autofill/core/common/password_form_fill_data.h"
@@ -38,6 +40,12 @@
 namespace autofill {
 namespace {
 
+enum FillUserNameAndPasswordOptions {
+  EXACT_USERNAME_MATCH = 1 << 0,
+  SET_SELECTION = 1 << 1,
+  FILL_PREFERRED_USERNAME = 1 << 2

[Garrett Casto, 2014/11/23 07:49:48]

Looks like FILL_PREFERRED_USERNAME isn't used currently? The only place we
handle the preferred username differently is in FillFormOnPasswordRecieved, not
in FillUserNameAndPassword, so I'm also not sure what the intention was here. If
you're not adding an additional here, I'd also leave this as boolean arguments
for FillUserNameAndPassword.

[jww, 2014/11/25 02:46:26]

Done.

+};
+
 // The size above which we stop triggering autocomplete.
 static const size_t kMaximumTextSizeForAutocomplete = 1000;
 
@@ -115,6 +123,23 @@ bool FindFormInputElement(blink::WebFormElement* form_element,
   return true;
 }
 
+bool ShouldFillOnAccountSelect() {
+  if (CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kEnableFillOnAccountSelect)) {
+    return true;
+  }
+
+  // This is made explicit in anticiption of experimental groups being added.
+  if (CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kDisableFillOnAccountSelect)) {
+    return false;
+  }
+
+  // TODO(jww): Add experimental groups check here.

[Garrett Casto, 2014/11/23 07:49:48]

When I said splitting off the CL into two, what I meant was making the changes
in password_autofill_manager and Autofill code in one CL, and adding all the
machinery to this file in another. I think that just holding out just this one
piece doesn't matter much. This change isn't large enough that I think you must
break it up, it was just a suggestion.

[jww, 2014/11/25 02:46:26]

Got it. I'll probably still commit without the experimental groups at first just
to keep it cleaner.

+
+  return false;
+}
+
 // Helper to search the given form element for the specified input elements in
 // |data|, and add results to |result|.
 bool FindFormInputElements(blink::WebFormElement* form_element,
@@ -289,14 +314,16 @@ bool GetSuggestionsStats(const PasswordFormFillData& fill_data,
 // 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|.
+// selected username comes from |fill_data.other_possible_usernames|. |options|
+// should be a bitwise mask of FillUserNameAndPasswordOptions values.
 bool FillUserNameAndPassword(
     blink::WebInputElement* username_element,
     blink::WebInputElement* password_element,
     const PasswordFormFillData& fill_data,
-    bool exact_username_match,
-    bool set_selection,
+    const int options,
     base::Callback<void(blink::WebInputElement*)> registration_callback) {
+  bool exact_username_match = (options & EXACT_USERNAME_MATCH) != 0;
+  bool set_selection = (options & SET_SELECTION) != 0;
   bool other_possible_username_selected = false;
   // Don't fill username if password can't be set.
   if (!IsElementAutocompletable(*password_element))
@@ -391,7 +418,7 @@ bool FillUserNameAndPassword(
 // |suggestedValue| set, and |suggestedValue| will be registered for copying to
 // the real value through |registration_callback|. Returns true when the
 // username gets selected from |other_possible_usernames|, else returns false.
-bool FillFormOnPasswordRecieved(
+bool FillFormOnPasswordReceived(
     const PasswordFormFillData& fill_data,
     blink::WebInputElement username_element,
     blink::WebInputElement password_element,
@@ -406,23 +433,41 @@ bool FillFormOnPasswordRecieved(
     return false;
 
   bool form_contains_username_field = FillDataContainsUsername(fill_data);
-  // Try to set the username to the preferred name, but only if the field
-  // can be set and isn't prefilled.
-  if (form_contains_username_field &&
-      IsElementAutocompletable(username_element) &&
-      username_element.value().isEmpty()) {
-    // TODO(tkent): Check maxlength and pattern.
-    username_element.setValue(fill_data.basic_data.fields[0].value, true);
+  // If the form contains a username field, try to set the username to the
+  // preferred name, but only if:
+  //   (a) The username element is autocompletable, and
+  //   (b) The fill-on-account-select flag is not set, and
+  //   (c) The username element isn't prefilled
+  //
+  // If (a) is true but (b) is false, then just mark the username element as
+  // autofilled and return so the fill step is skipped.
+  //
+  // If (a) is false but (b) is true, then the username element should not be
+  // autofilled, but the user should still be able to select to fill the
+  // password element, so the password element must be marked as autofilled and
+  // the fill step should also be skipped.
+  //
+  // In all other cases, do nothing.
+  if (form_contains_username_field) {
+    if (IsElementAutocompletable(username_element)) {
+      if (ShouldFillOnAccountSelect()) {
+        username_element.setAutofilled(true);
+        return false;
+      } else if (username_element.value().isEmpty()) {
+        // TODO(tkent): Check maxlength and pattern.
+        username_element.setValue(fill_data.basic_data.fields[0].value, true);
+      }
+    } else if (ShouldFillOnAccountSelect()) {
+      password_element.setAutofilled(true);
+      return false;
+    }
   }
 
   // Fill if we have an exact match for the username. Note that this sets
   // username to autofilled.
-  return FillUserNameAndPassword(&username_element,
-                                 &password_element,
-                                 fill_data,
-                                 true /* exact_username_match */,
-                                 false /* set_selection */,
-                                 registration_callback);
+  return FillUserNameAndPassword(
+      &username_element, &password_element, fill_data,
+      EXACT_USERNAME_MATCH | FILL_PREFERRED_USERNAME, registration_callback);
 }
 
 // Takes a |map| with pointers as keys and linked_ptr as values, and returns
@@ -524,11 +569,7 @@ bool PasswordAutofillAgent::TextFieldDidEndEditing(
   // Do not set selection when ending an editing session, otherwise it can
   // mess with focus.
   if (FillUserNameAndPassword(
-          &username,
-          &password,
-          fill_data,
-          true /* exact_username_match */,
-          false /* set_selection */,
+          &username, &password, fill_data, EXACT_USERNAME_MATCH,
           base::Bind(&PasswordValueGatekeeper::RegisterElement,
                      base::Unretained(&gatekeeper_)))) {
     usernames_usage_ = OTHER_POSSIBLE_USERNAME_SELECTED;
@@ -591,7 +632,7 @@ bool PasswordAutofillAgent::TextDidChangeInTextField(
   // But refresh the popup.  Note, since this is ours, return true to signal
   // no further processing is required.
   if (iter->second.backspace_pressed_last) {
-    ShowSuggestionPopup(iter->second.fill_data, element, false);
+    ShowSuggestionPopup(iter->second.fill_data, element, false, false);
     return true;
   }
 
@@ -688,11 +729,28 @@ bool PasswordAutofillAgent::DidClearAutofillSelection(
   return true;
 }
 
+PasswordAutofillAgent::LoginToPasswordInfoMap::iterator
+PasswordAutofillAgent::FindPasswordInfoForElement(
+    const blink::WebInputElement& element) {
+  const blink::WebInputElement* username_element;
+  if (!element.isPasswordField()) {
+    username_element = &element;
+  } else {
+    PasswordToLoginMap::const_iterator password_iter =
+        password_to_username_.find(element);
+    if (password_iter == password_to_username_.end())
+      return login_to_password_info_.end();
+    username_element = &password_iter->second;
+  }
+
+  return login_to_password_info_.find(*username_element);
+}
+
 bool PasswordAutofillAgent::ShowSuggestions(
     const blink::WebInputElement& element,
     bool show_all) {
   LoginToPasswordInfoMap::const_iterator iter =
-      login_to_password_info_.find(element);
+      FindPasswordInfoForElement(element);
   if (iter == login_to_password_info_.end())
     return false;
 
@@ -704,7 +762,13 @@ bool PasswordAutofillAgent::ShowSuggestions(
       !IsElementAutocompletable(iter->second.password_field))
     return true;
 
-  return ShowSuggestionPopup(iter->second.fill_data, element, show_all);
+  // Chrome should never show more than one account for a password element since
+  // this implies that the username element cannot be modified. Thus even if
+  // |show_all| is true, check if the element in question is a password element
+  // for the call to ShowSuggestionPopup.
+  return ShowSuggestionPopup(iter->second.fill_data, iter->first,
+                             show_all && !element.isPasswordField(),
+                             element.isPasswordField());
 }
 
 bool PasswordAutofillAgent::OriginCanAccessPasswordManager(
@@ -1054,10 +1118,8 @@ void PasswordAutofillAgent::OnFillPasswordForm(
     // If wait_for_username is true, we don't want to initially fill the form
     // until the user types in a valid username.
     if (!form_data.wait_for_username &&
-        FillFormOnPasswordRecieved(
-            form_data,
-            username_element,
-            password_element,
+        FillFormOnPasswordReceived(
+            form_data, username_element, password_element,
             base::Bind(&PasswordValueGatekeeper::RegisterElement,
                        base::Unretained(&gatekeeper_)))) {
       usernames_usage_ = OTHER_POSSIBLE_USERNAME_SELECTED;
@@ -1091,7 +1153,8 @@ PasswordAutofillAgent::PasswordInfo::PasswordInfo()
 bool PasswordAutofillAgent::ShowSuggestionPopup(
     const PasswordFormFillData& fill_data,
     const blink::WebInputElement& user_input,
-    bool show_all) {
+    bool show_all,
+    bool show_on_password_field) {
   blink::WebFrame* frame = user_input.document().frame();
   if (!frame)
     return false;
@@ -1106,6 +1169,12 @@ bool PasswordAutofillAgent::ShowSuggestionPopup(
       user_input, &form, &field, REQUIRE_NONE);
 
   blink::WebInputElement selected_element = user_input;
+  if (show_on_password_field) {
+    LoginToPasswordInfoMap::const_iterator iter =
+        login_to_password_info_.find(user_input);
+    DCHECK(iter != login_to_password_info_.end());
+    selected_element = iter->second.password_field;
+  }
   gfx::Rect bounding_box(selected_element.boundsInViewportSpace());
 
   LoginToPasswordInfoKeyMap::const_iterator key_it =
@@ -1117,9 +1186,14 @@ bool PasswordAutofillAgent::ShowSuggestionPopup(
                                  bounding_box.y() * scale,
                                  bounding_box.width() * scale,
                                  bounding_box.height() * scale);
+  int options = 0;
+  if (show_all)
+    options |= ShowPasswordSuggestionsOptions::SHOW_ALL;
+  if (show_on_password_field)
+    options |= ShowPasswordSuggestionsOptions::IS_PASSWORD_FIELD;
   Send(new AutofillHostMsg_ShowPasswordSuggestions(
       routing_id(), key_it->second, field.text_direction, user_input.value(),
-      show_all, bounding_box_scaled));
+      options, bounding_box_scaled));
 
   bool suggestions_present = false;
   if (GetSuggestionsStats(fill_data, user_input.value(), show_all,
@@ -1147,17 +1221,13 @@ void PasswordAutofillAgent::PerformInlineAutocomplete(
   }
 
   // Show the popup with the list of available usernames.
-  ShowSuggestionPopup(fill_data, username, false);
+  ShowSuggestionPopup(fill_data, username, false, false);
 
 #if !defined(OS_ANDROID)
   // Fill the user and password field with the most relevant match. Android
   // only fills in the fields after the user clicks on the suggestion popup.
   if (FillUserNameAndPassword(
-          &username,
-          &password,
-          fill_data,
-          false /* exact_username_match */,
-          true /* set_selection */,
+          &username, &password, fill_data, SET_SELECTION,
           base::Bind(&PasswordValueGatekeeper::RegisterElement,
                      base::Unretained(&gatekeeper_)))) {
     usernames_usage_ = OTHER_POSSIBLE_USERNAME_SELECTED;
@@ -1199,7 +1269,7 @@ bool PasswordAutofillAgent::FindLoginInfo(const blink::WebNode& node,
     return false;
 
   blink::WebInputElement input = element.to<blink::WebInputElement>();
-  LoginToPasswordInfoMap::iterator iter = login_to_password_info_.find(input);
+  LoginToPasswordInfoMap::iterator iter = FindPasswordInfoForElement(input);
   if (iter == login_to_password_info_.end())
     return false;