Better filling on suggestion of password fields in 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 <stddef.h>
 #include <utility>
 
 #include "base/bind.h"
@@ skipping 752 matching lines @@
     if (iter != password_to_username_.end()) {
       web_input_to_password_info_[iter->second].password_was_edited_last = true;
       // Note that the suggested value of |mutable_element| was reset when its
       // value changed.
       mutable_element.setAutofilled(false);
     }
   }
 }
 
 bool PasswordAutofillAgent::FillSuggestion(
-    const blink::WebNode& node,
+    const blink::WebFormControlElement& control_element,
     const blink::WebString& username,
     const blink::WebString& password) {
   // The element in context of the suggestion popup.
-  blink::WebInputElement filled_element;
+  const blink::WebInputElement* element = toWebInputElement(&control_element);
+  if (!element)
+    return false;
+
+  blink::WebInputElement username_element;
+  blink::WebInputElement password_element;
   PasswordInfo* password_info;
 
-  if (!FindLoginInfo(node, &filled_element, &password_info) ||
-      !IsElementAutocompletable(filled_element) ||
-      !IsElementAutocompletable(password_info->password_field)) {
+  if (!FindPasswordInfoForElement(*element, &username_element,
+                                  &password_element, &password_info) ||
+      (!username_element.isNull() &&
+       !IsElementAutocompletable(username_element)) ||
+      !IsElementAutocompletable(password_element)) {
     return false;
   }
 
   password_info->password_was_edited_last = false;
-  // Note that in cases where filled_element is the password element, the value
-  // gets overwritten with the correct one below.
-  filled_element.setValue(username, true);
-  filled_element.setAutofilled(true);
-  nonscript_modified_values_[filled_element] = username;
+  if (element->isPasswordField()) {
+    password_info->password_field_suggestion_was_accepted = true;
+    password_info->password_field = password_element;
+  } else if (!username_element.isNull()) {
+    username_element.setValue(username, true);
+    username_element.setAutofilled(true);
+    nonscript_modified_values_[username_element] = username;
+  }
 
-  password_info->password_field.setValue(password, true);
-  password_info->password_field.setAutofilled(true);
-  nonscript_modified_values_[password_info->password_field] = password;
+  password_element.setValue(password, true);
+  password_element.setAutofilled(true);
+  nonscript_modified_values_[password_element] = password;
 
-  filled_element.setSelectionRange(filled_element.value().length(),
-                                   filled_element.value().length());
+  blink::WebInputElement mutable_filled_element = *element;
+  mutable_filled_element.setSelectionRange(element->value().length(),
+                                           element->value().length());
 
   return true;
 }
 
 bool PasswordAutofillAgent::PreviewSuggestion(
-    const blink::WebNode& node,
+    const blink::WebFormControlElement& control_element,
     const blink::WebString& username,
     const blink::WebString& password) {
+  // The element in context of the suggestion popup.
+  const blink::WebInputElement* element = toWebInputElement(&control_element);
+  if (!element)
+    return false;
+
   blink::WebInputElement username_element;
+  blink::WebInputElement password_element;
   PasswordInfo* password_info;
 
-  if (!FindLoginInfo(node, &username_element, &password_info) ||
-      !IsElementAutocompletable(username_element) ||
-      !IsElementAutocompletable(password_info->password_field)) {
+  if (!FindPasswordInfoForElement(*element, &username_element,
+                                  &password_element, &password_info) ||
+      (!username_element.isNull() &&
+       !IsElementAutocompletable(username_element)) ||
+      !IsElementAutocompletable(password_element)) {
     return false;
   }
 
-  if (username_query_prefix_.empty())
-    username_query_prefix_ = username_element.value();
+  if (!element->isPasswordField() && !username_element.isNull()) {
+    if (username_query_prefix_.empty())
+      username_query_prefix_ = username_element.value();
 
-  was_username_autofilled_ = username_element.isAutofilled();
-  username_element.setSuggestedValue(username);
-  username_element.setAutofilled(true);
-  form_util::PreviewSuggestion(username_element.suggestedValue(),
-                               username_query_prefix_, &username_element);
-  was_password_autofilled_ = password_info->password_field.isAutofilled();
-  password_info->password_field.setSuggestedValue(password);
-  password_info->password_field.setAutofilled(true);
+    was_username_autofilled_ = username_element.isAutofilled();
+    username_element.setSuggestedValue(username);
+    username_element.setAutofilled(true);
+    form_util::PreviewSuggestion(username_element.suggestedValue(),
+                                 username_query_prefix_, &username_element);
+  }
+  was_password_autofilled_ = password_element.isAutofilled();
+  password_element.setSuggestedValue(password);
+  password_element.setAutofilled(true);
 
   return true;
 }
 
 bool PasswordAutofillAgent::DidClearAutofillSelection(
-    const blink::WebNode& node) {
-  blink::WebInputElement username_element;
-  PasswordInfo* password_info;
-  if (!FindLoginInfo(node, &username_element, &password_info))
+    const blink::WebFormControlElement& control_element) {
+  const blink::WebInputElement* element = toWebInputElement(&control_element);
+  if (!element)
     return false;
 
-  ClearPreview(&username_element, &password_info->password_field);
+  blink::WebInputElement username_element;
+  blink::WebInputElement password_element;
+  PasswordInfo* password_info;
+
+  if (!FindPasswordInfoForElement(*element, &username_element,
+                                  &password_element, &password_info))
+    return false;
+
+  ClearPreview(&username_element, &password_element);
   return true;
 }
 
 bool PasswordAutofillAgent::FindPasswordInfoForElement(
     const blink::WebInputElement& element,
-    const blink::WebInputElement** username_element,
+    blink::WebInputElement* username_element,
+    blink::WebInputElement* password_element,
     PasswordInfo** password_info) {
-  DCHECK(username_element && password_info);
+  DCHECK(username_element && password_element && password_info);
+  username_element->reset();
+  password_element->reset();
   if (!element.isPasswordField()) {
-    *username_element = &element;
+    *username_element = element;
   } else {
     WebInputToPasswordInfoMap::iterator iter =
         web_input_to_password_info_.find(element);
     if (iter != web_input_to_password_info_.end()) {
       // It's a password field without corresponding username field.
-      *username_element = nullptr;
+      *password_element = element;
       *password_info = &iter->second;
       return true;
     }
     PasswordToLoginMap::const_iterator password_iter =
         password_to_username_.find(element);
-    if (password_iter == password_to_username_.end())
-      return false;
-    *username_element = &password_iter->second;
+    if (password_iter == password_to_username_.end()) {
+      if (web_input_to_password_info_.empty())
+        return false;
+
+      *password_element = element;
+      *password_info = &web_input_to_password_info_.begin()->second;

[vabr (Chromium), 2016/04/01 14:07:37]

nit: Please comment on the choice of the value for |*password_info| here, it
looks rather random.

[dvadym, 2016/04/07 17:30:20]

Done.

+      return true;
+    }
+    *username_element = password_iter->second;
+    *password_element = element;
   }
 
   WebInputToPasswordInfoMap::iterator iter =
-      web_input_to_password_info_.find(**username_element);
+      web_input_to_password_info_.find(*username_element);
 
   if (iter == web_input_to_password_info_.end())
     return false;
 
   *password_info = &iter->second;
+  if (password_element->isNull())
+    *password_element = (*password_info)->password_field;
+
   return true;
 }
 
 bool PasswordAutofillAgent::ShowSuggestions(
     const blink::WebInputElement& element,
     bool show_all,
     bool generation_popup_showing) {
-  const blink::WebInputElement* username_element;
+  blink::WebInputElement username_element;
+  blink::WebInputElement password_element;
   PasswordInfo* password_info;
-  if (!FindPasswordInfoForElement(element, &username_element, &password_info))
+  if (!FindPasswordInfoForElement(element, &username_element, &password_element,
+                                  &password_info))
     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 (!element.isTextField() || !IsElementAutocompletable(element) ||
       !IsElementAutocompletable(password_info->password_field))
     return true;
 
   if (element.nameForAutofill().isEmpty() &&
       !DoesFormContainAmbiguousOrEmptyNames(password_info->fill_data)) {
     return false;  // If the field has no name, then we won't have values.
   }
 
   // Don't attempt to autofill with values that are too large.
   if (element.value().length() > kMaximumTextSizeForAutocomplete)
     return false;
 
-  bool username_is_available = username_element &&
-                               !username_element->isNull() &&
-                               IsElementEditable(*username_element);
+  bool username_is_available =
+      !username_element.isNull() && IsElementEditable(username_element);
   // If the element is a password field, a popup should only be shown if there
   // is no username or the corresponding username element is not editable since
   // it is only in that case that the username element does not have a
   // suggestions popup.

[vabr (Chromium), 2016/04/01 14:07:37]

Please update the comment to also speak about not showing the pop-up if the user
already accepted a suggestion on some other password field.

[dvadym, 2016/04/07 17:30:20]

I've decided to simplify this condition, the only case when not to show when the
user has already accepted it on another password field, that will save us from
cases where the username found incorrectly. Please have a look at changes in
PatchSet 7.

[vabr (Chromium), 2016/04/08 14:47:31]

Acknowledged.

-  if (element.isPasswordField() && username_is_available &&
-      (!password_info->fill_data.is_possible_change_password_form ||
-       password_info->username_was_edited))
+  if (element.isPasswordField() &&
+      ((username_is_available &&
+        (!password_info->fill_data.is_possible_change_password_form ||
+         password_info->username_was_edited)) ||
+       (password_info->password_field_suggestion_was_accepted &&
+        element != password_info->password_field)))
     return true;
 
   UMA_HISTOGRAM_BOOLEAN(
       "PasswordManager.AutocompletePopupSuppressedByGeneration",
       generation_popup_showing);
 
   if (generation_popup_showing)
     return false;
 
   // 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(
-      password_info->fill_data,
-      (!username_element || username_element->isNull()) ? element
-                                                        : *username_element,
+      *password_info, username_element.isNull() ? element : username_element,
       show_all && !element.isPasswordField(), element.isPasswordField());
 }
 
 bool PasswordAutofillAgent::OriginCanAccessPasswordManager(
     const blink::WebSecurityOrigin& origin) {
   return origin.canAccessPasswordManager();
 }
 
 void PasswordAutofillAgent::OnDynamicFormsSeen() {
   SendPasswordForms(false /* only_visible */);
@@ skipping 394 matching lines @@
           form_data,
           username_element,
           password_element,
           &nonscript_modified_values_,
           base::Bind(&PasswordValueGatekeeper::RegisterElement,
                      base::Unretained(&gatekeeper_)));
     }
 
     PasswordInfo password_info;
     password_info.fill_data = form_data;
+    password_info.key = key;
     password_info.password_field = password_element;
     web_input_to_password_info_[main_element] = password_info;
     password_to_username_[password_element] = username_element;
-    web_element_to_password_info_key_[main_element] = key;
   }
 }
 
 void PasswordAutofillAgent::OnSetLoggingState(bool active) {
   logging_state_active_ = active;
 }
 
 void PasswordAutofillAgent::OnAutofillUsernameAndPasswordDataReceived(
     const FormsPredictionsMap& predictions) {
   form_predictions_.insert(predictions.begin(), predictions.end());
@@ skipping 31 matching lines @@
 
 ////////////////////////////////////////////////////////////////////////////////
 // PasswordAutofillAgent, private:
 
 PasswordAutofillAgent::PasswordInfo::PasswordInfo()
     : password_was_edited_last(false),
       username_was_edited(false) {
 }
 
 bool PasswordAutofillAgent::ShowSuggestionPopup(
-    const PasswordFormFillData& fill_data,
+    const PasswordInfo& password_info,
     const blink::WebInputElement& user_input,
     bool show_all,
     bool show_on_password_field) {
   DCHECK(!user_input.isNull());
   blink::WebFrame* frame = user_input.document().frame();
   if (!frame)
     return false;
 
   blink::WebView* webview = frame->view();
   if (!webview)
@@ skipping 14 matching lines @@
     WebInputToPasswordInfoMap::const_iterator iter =
         web_input_to_password_info_.find(user_input);
     DCHECK(iter != web_input_to_password_info_.end());
     selected_element = iter->second.password_field;
   }
 
   blink::WebInputElement username;
   if (!show_on_password_field || !user_input.isPasswordField()) {
     username = user_input;
   }
-  WebElementToPasswordInfoKeyMap::const_iterator key_it =
-      web_element_to_password_info_key_.find(user_input);
-  DCHECK(key_it != web_element_to_password_info_key_.end());
 
   int options = 0;
   if (show_all)
     options |= SHOW_ALL;
   if (show_on_password_field)
     options |= IS_PASSWORD_FIELD;
   base::string16 username_string(
       username.isNull() ? base::string16()
                         : static_cast<base::string16>(user_input.value()));
 
   Send(new AutofillHostMsg_ShowPasswordSuggestions(
-           routing_id(),
-           key_it->second,
-           field.text_direction,
-           username_string,
-           options,
-           render_frame()->GetRenderView()->ElementBoundsInWindow(
-               selected_element)));
+      routing_id(), password_info.key, field.text_direction, username_string,
+      options, render_frame()->GetRenderView()->ElementBoundsInWindow(
+                   selected_element)));
   username_query_prefix_ = username_string;
-  return CanShowSuggestion(fill_data, username_string, show_all);
+  return CanShowSuggestion(password_info.fill_data, username_string, show_all);
 }
 
 void PasswordAutofillAgent::FrameClosing() {
   for (auto const& iter : web_input_to_password_info_) {
-    web_element_to_password_info_key_.erase(iter.first);
     password_to_username_.erase(iter.second.password_field);
   }
   web_input_to_password_info_.clear();
   provisionally_saved_form_.reset();
   nonscript_modified_values_.clear();
 }
 
-bool PasswordAutofillAgent::FindLoginInfo(const blink::WebNode& node,
-                                          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;
-
-  *found_input = element.to<blink::WebInputElement>();
-  const blink::WebInputElement* username_element;  // ignored
-  return FindPasswordInfoForElement(*found_input, &username_element,
-                                    found_password);
-}
-
 void PasswordAutofillAgent::ClearPreview(
     blink::WebInputElement* username,
     blink::WebInputElement* password) {
-  if (!username->suggestedValue().isEmpty()) {
+  if (!username->isNull() && !username->suggestedValue().isEmpty()) {
     username->setSuggestedValue(blink::WebString());
     username->setAutofilled(was_username_autofilled_);
     username->setSelectionRange(username_query_prefix_.length(),
                                 username->value().length());
   }
   if (!password->suggestedValue().isEmpty()) {
       password->setSuggestedValue(blink::WebString());
       password->setAutofilled(was_password_autofilled_);
   }
 }
@@ skipping 10 matching lines @@
 }
 
 bool PasswordAutofillAgent::ProvisionallySavedPasswordIsValid() {
   return provisionally_saved_form_ &&
          !provisionally_saved_form_->username_value.empty() &&
          !(provisionally_saved_form_->password_value.empty() &&
          provisionally_saved_form_->new_password_value.empty());
 }
 
 }  // namespace autofill