Fill on account select in the password manager

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/command_line.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop/message_loop.h"
 #include "base/metrics/histogram.h"
 #include "base/strings/utf_string_conversions.h"
 #include "components/autofill/content/common/autofill_messages.h"
 #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_autofill_util.h"
 #include "components/autofill/core/common/password_form.h"
 #include "components/autofill/core/common/password_form_fill_data.h"
 #include "content/public/renderer/document_state.h"
 #include "content/public/renderer/navigation_state.h"
 #include "content/public/renderer/render_view.h"
 #include "third_party/WebKit/public/platform/WebVector.h"
 #include "third_party/WebKit/public/web/WebAutofillClient.h"
 #include "third_party/WebKit/public/web/WebDocument.h"
 #include "third_party/WebKit/public/web/WebElement.h"
 #include "third_party/WebKit/public/web/WebFormElement.h"
 #include "third_party/WebKit/public/web/WebInputEvent.h"
 #include "third_party/WebKit/public/web/WebLocalFrame.h"
 #include "third_party/WebKit/public/web/WebNode.h"
 #include "third_party/WebKit/public/web/WebNodeList.h"
 #include "third_party/WebKit/public/web/WebSecurityOrigin.h"
 #include "third_party/WebKit/public/web/WebUserGestureIndicator.h"
 #include "third_party/WebKit/public/web/WebView.h"
 #include "ui/base/page_transition_types.h"
 #include "ui/events/keycodes/keyboard_codes.h"
 #include "url/gurl.h"
 
 namespace autofill {
 namespace {
 
+enum FillUserNameAndPasswordOptions {
+  EXACT_USERNAME_MATCH = 1 << 0,
+  SET_SELECTION = 1 << 1,
+  FILL_PREFERRED_USERNAME = 1 << 2
+};
+
 // The size above which we stop triggering autocomplete.
 static const size_t kMaximumTextSizeForAutocomplete = 1000;
 
 // Maps element names to the actual elements to simplify form filling.
 typedef std::map<base::string16, blink::WebInputElement> FormInputElementMap;
 
 // Use the shorter name when referencing SavePasswordProgressLogger::StringID
 // values to spare line breaks. The code provides enough context for that
 // already.
 typedef SavePasswordProgressLogger Logger;
 
 // Utility struct for form lookup and autofill. When we parse the DOM to look up
 // a form, in addition to action and origin URL's we have to compare all
 // necessary form elements. To avoid having to look these up again when we want
 // to fill the form, the FindFormElements function stores the pointers
 // in a FormElements* result, referenced to ensure they are safe to use.
 struct FormElements {
   blink::WebFormElement form_element;
   FormInputElementMap input_elements;
 };
 
 typedef std::vector<FormElements*> FormElementsList;
 
+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.
+
+  return false;
+}
+
 // Helper to search the given form element for the specified input elements
 // in |data|, and add results to |result|.
 static bool FindFormInputElements(blink::WebFormElement* fe,
                                   const FormData& data,
                                   FormElements* result) {
   const bool username_is_present = !data.fields[0].name.empty();
 
   // Loop through the list of elements we need to find on the form in order to
   // autofill it. If we don't find any one of them, abort processing this
   // form; it can't be the right one.
@@ skipping 199 matching lines @@
     }
   }
 
   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|.
+// 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))
     return false;
 
   base::string16 current_username;
   if (!username_element->isNull()) {
     current_username = username_element->value();
   }
 
@@ skipping 74 matching lines @@
 }
 
 // Attempts to fill |username_element| and |password_element| with the
 // |fill_data|. Will use the data corresponding to the preferred username,
 // unless the |username_element| already has a value set. In that case,
 // attempts to fill the password matching the already filled username, if
 // such a password exists. The |password_element| will have the
 // |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,
     base::Callback<void(blink::WebInputElement*)> registration_callback) {
   // Do not fill if the password field is in an iframe.
   DCHECK(password_element.document().frame());
   if (password_element.document().frame()->parent())
     return false;
 
   bool form_contains_username_field = FillDataContainsUsername(fill_data);
   if (!ShouldIgnoreAutocompleteOffForPasswordFields() &&
       form_contains_username_field && !username_element.form().autoComplete())
     return false;
 
   // If we can't modify the password, don't try to set the username
   if (!IsElementAutocompletable(password_element))
     return false;
 
-  // 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);
+  // 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
+  //   (b) The username element isn't prefilled

[Garrett Casto, 2014/11/04 23:35:30]

Is that last line supposed to be (c)?

[jww, 2014/11/06 21:28:56]

Yup :-)

+  //
+  // 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.
+  if (IsElementAutocompletable(username_element)) {

[Garrett Casto, 2014/11/04 23:35:30]

I think that you dropped the form_contains_username_field check. It's possible
that username_element is invalid here.

[jww, 2014/11/06 21:28:56]

Done.

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

Can you add a test so that this would have been caught? 

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

Yup, unit tests are my next step.

+    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
 // true if |key| is not NULL and  |map| contains a non-NULL entry for |key|.
 // Makes sure not to create an entry as a side effect of using the operator [].
 template <class Key, class Value>
 bool ContainsNonNullEntryForNonNullKey(
     const std::map<Key*, linked_ptr<Value>>& map,
     Key* key) {
   if (!key)
@@ skipping 81 matching lines @@
 
   blink::WebInputElement password = password_info.password_field;
   if (!IsElementEditable(password))
     return false;
 
   blink::WebInputElement username = element;  // We need a non-const.
 
   // 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;
   }
   return true;
 }
 
 bool PasswordAutofillAgent::TextDidChangeInTextField(
     const blink::WebInputElement& element) {
   // TODO(vabr): Get a mutable argument instead. http://crbug.com/397083
@@ skipping 139 matching lines @@
     const blink::WebNode& node) {
   blink::WebInputElement username_element;
   PasswordInfo* password_info;
   if (!FindLoginInfo(node, &username_element, &password_info))
     return false;
 
   ClearPreview(&username_element, &password_info->password_field);
   return true;
 }
 
+PasswordAutofillAgent::LoginToPasswordInfoMap::iterator
+PasswordAutofillAgent::FindPasswordInfoForElement(
+    const blink::WebInputElement& element) {
+  const blink::WebInputElement* username_element;

[Garrett Casto, 2014/11/04 23:35:30]

Why does this need to be a pointer?

[jww, 2014/11/06 21:28:56]

I was trying to have only return statement, and since the value username_element
is dependent on whether element is a password field, I made it assignable.
However, I've rewritten the method to get rid of the pointer, so let me know
which version you prefer (or if there's another option I'm not thinking of).

+  if (!element.isPasswordField()) {
+    username_element = &element;
+  } else {
+    PasswordToLoginMap::const_iterator password_iter =
+        password_to_username_.find(element);

[Garrett Casto, 2014/11/04 23:35:30]

Where is this defined and populated?

[jww, 2014/11/06 21:28:57]

It's defined in password_autofill_agent.h and it is populated in
OnFillPasswordForm in password_autofill_agent.cc (at the same time and place
that login_to_password_info_ is populated). In case it's not clear, I did not
add this field.

+    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;
 
   // If autocomplete='off' is set on the form elements, no suggestion dialog
   // should be shown. However, return |true| to indicate that this is a known
   // password form and that the request to show suggestions has been handled (as
   // a no-op).
   if (!IsElementAutocompletable(element) ||
       !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());

[Garrett Casto, 2014/11/04 23:35:30]

Is there any reason why you need the fourth parameter here instead of just
passing |element| as the second param and change behavior in
ShowSuggestionPopup() depending on if element.isPasswordField() is true or not?

[jww, 2014/11/06 21:28:57]

Yes, because we ultimately need the username as the element, not the password
field. Of course, we could pass in the password element and then check and find
the username element, but given that this is a special case, it seems to make
the most sense to abstract out the search for the username element to the one
place it's used. Note the additional search step on line 723 in
FindPasswordInfoForElement; it seems weird to call that again inside
ShowSuggestionPopup (since it's already needed inside ShowSuggestions()).

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

I'm not sure that I understand why we need the username in ShowSuggestionPopup
if we are going to show the popup on the password field. As is, it looks like it
just immediately converts the username element to the password element. Assuming
|user_input| was always the element that you wanted the popup to be shown on,
couldn't you just remove the first new if statement in ShowSuggestionPopup?

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

I believe it's because the browser process can only look up mappings by
username. Thus, we need to pass to the browser the username that we want the
credential for.

 }
 
 bool PasswordAutofillAgent::OriginCanAccessPasswordManager(
     const blink::WebSecurityOrigin& origin) {
   return origin.canAccessPasswordManager();
 }
 
 void PasswordAutofillAgent::OnDynamicFormsSeen(blink::WebFrame* frame) {
   SendPasswordForms(frame, false /* only_visible */);
 }
@@ skipping 328 matching lines @@
       break;
 
     // Get pointer to password element. (We currently only support single
     // password forms).
     password_element =
         form_elements->input_elements[form_data.basic_data.fields[1].name];
 
     // 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;
     }
     // We might have already filled this form if there are two <form> elements
     // with identical markup.
     if (login_to_password_info_.find(username_element) !=
         login_to_password_info_.end())
       continue;
 
@@ skipping 22 matching lines @@
 ////////////////////////////////////////////////////////////////////////////////
 // PasswordAutofillAgent, private:
 
 PasswordAutofillAgent::PasswordInfo::PasswordInfo()
     : backspace_pressed_last(false), password_was_edited_last(false) {
 }
 
 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;
 
   blink::WebView* webview = frame->view();
   if (!webview)
     return false;
 
   FormData form;
   FormFieldData field;
   FindFormAndFieldForFormControlElement(
       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());
 
   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, user_input.value(), show_all, bounding_box_scaled));
+  if (show_on_password_field) {
+    Send(new AutofillHostMsg_ShowPasswordSuggestionsForPasswordField(
+        routing_id(), field, user_input.value(), show_all,
+        bounding_box_scaled));
+  } else {
+    Send(new AutofillHostMsg_ShowPasswordSuggestions(
+        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(
@@ skipping 13 matching lines @@
     return;
   }
 
   // Show the popup with the list of available usernames.
   ShowSuggestionPopup(fill_data, username, 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;
   }
 #endif
 }
 
 void PasswordAutofillAgent::FrameClosing(const blink::WebFrame* frame) {
   for (LoginToPasswordInfoMap::iterator iter = login_to_password_info_.begin();
        iter != login_to_password_info_.end();) {
@@ skipping 20 matching lines @@
                                           blink::WebInputElement* found_input,
                                           PasswordInfo** found_password) {
   if (!node.isElementNode())
     return false;
 
   blink::WebElement element = node.toConst<blink::WebElement>();
   if (!element.hasHTMLTagName("input"))
     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;
 
   *found_input = input;
   *found_password = &iter->second;
   return true;
 }
 
 void PasswordAutofillAgent::ClearPreview(
     blink::WebInputElement* username,
@@ skipping 25 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