Add password form search using blink::WebNode reference comparison.

After an XHR is completed, PasswordAutofillAgent tries to check if the
form is still visible (i.e. if the user has successfuly logged in or
still sees the form) by searching the form with the same action URL.
This works until the form has action URL. If it doesn't, the search is
performed using field matching. This will fail if the form is modified
by the website right before the XHR has completed.
This CL adds support for the form search using blink::WebFormElement
and blink::WebInputElement (for unowned forms) reference comparison.

BUG=700862
R=dvadym@chromium.org

Review-Url: https://codereview.chromium.org/2746033004
Cr-Commit-Position: refs/heads/master@{#458032}

chrome/renderer/autofill/password_autofill_agent_browsertest.cc

@@ -2175,6 +2175,65 @@ TEST_F(PasswordAutofillAgentTest,
   ASSERT_FALSE(fake_driver_.called_password_form_submitted());
 }
 
+// Tests that no save promt is shown when an unowned form is changed and AJAX
+// completed but the form is still visible.
+TEST_F(PasswordAutofillAgentTest,
+       NoForm_NoPromptForAJAXSubmitWithoutNavigationAndNewElementAppeared) {
+  const char kNoFormHTMLWithHiddenField[] =
+      "<INPUT type='text' id='username'/>"
+      "<INPUT type='password' id='password'/>"
+      "<INPUT type='text' id='captcha' style='display:none'/>";
+  LoadHTML(kNoFormHTMLWithHiddenField);
+
+  UpdateUsernameAndPasswordElements();
+  WebElement captcha_element =
+      GetMainFrame()->document().getElementById(WebString::fromUTF8("captcha"));
+  ASSERT_FALSE(captcha_element.isNull());
+
+  SimulateUsernameChange("Bob");
+  SimulatePasswordChange("mypassword");
+
+  // Simulate captcha element show up right before AJAX completed.
+  captcha_element.setAttribute("style", "display:inline;");
+  password_autofill_agent_->AJAXSucceeded();
+
+  base::RunLoop().RunUntilIdle();
+  EXPECT_FALSE(fake_driver_.called_inpage_navigation());
+  EXPECT_FALSE(fake_driver_.called_password_form_submitted());
+}
+
+// Tests that no save promt is shown when a form with empty action URL is
+// changed and AJAX completed but the form is still visible.
+TEST_F(PasswordAutofillAgentTest,
+       NoAction_NoPromptForAJAXSubmitWithoutNavigationAndNewElementAppeared) {
+  // Form without an action URL.
+  const char kHTMLWithHiddenField[] =
+      "<FORM name='LoginTestForm'>"
+      "  <INPUT type='text' id='username'/>"
+      "  <INPUT type='password' id='password'/>"
+      "  <INPUT type='text' id='captcha' style='display:none'/>"
+      "  <INPUT type='submit' value='Login'/>"
+      "</FORM>";
+  // Set the valid URL so the form action URL can be generated properly.
+  LoadHTMLWithUrlOverride(kHTMLWithHiddenField, "https://www.example.com");
+
+  UpdateUsernameAndPasswordElements();
+  WebElement captcha_element =
+      GetMainFrame()->document().getElementById(WebString::fromUTF8("captcha"));
+  ASSERT_FALSE(captcha_element.isNull());
+
+  SimulateUsernameChange("Bob");
+  SimulatePasswordChange("mypassword");
+
+  // Simulate captcha element show up right before AJAX completed.
+  captcha_element.setAttribute("style", "display:inline;");
+  password_autofill_agent_->AJAXSucceeded();
+
+  base::RunLoop().RunUntilIdle();
+  EXPECT_FALSE(fake_driver_.called_inpage_navigation());
+  EXPECT_FALSE(fake_driver_.called_password_form_submitted());
+}
+
 // Tests that credential suggestions are autofilled on a password (and change
 // password) forms having either ambiguous or empty name.
 TEST_F(PasswordAutofillAgentTest,

components/autofill/content/renderer/BUILD.gn

@@ -21,6 +21,8 @@ static_library("renderer") {
     "password_form_conversion_utils.h",
     "password_generation_agent.cc",
     "password_generation_agent.h",
+    "provisionally_saved_password_form.cc",
+    "provisionally_saved_password_form.h",
     "renderer_save_password_progress_logger.cc",
     "renderer_save_password_progress_logger.h",
   ]

components/autofill/content/renderer/form_autofill_util.cc

@@ -1203,6 +1203,7 @@ bool ExtractFormData(const WebFormElement& form_element, FormData* data) {
 }
 
 bool IsFormVisible(blink::WebFrame* frame,
+                   const blink::WebFormElement& form_element,
                    const GURL& canonical_action,
                    const GURL& canonical_origin,
                    const FormData& form_data) {
@@ -1226,6 +1227,11 @@ bool IsFormVisible(blink::WebFrame* frame,
     if (!AreFormContentsVisible(form))
       continue;
 
+    // Try to match the WebFormElement reference first.
+    if (!form_element.isNull() && form == form_element) {
+      return true;  // Form still exists.
+    }
+
     GURL iter_canonical_action = GetCanonicalActionForForm(form);
     bool form_action_is_empty = iter_canonical_action.is_empty() ||
                                 iter_canonical_action == frame_origin;

components/autofill/content/renderer/form_autofill_util.h

@@ -83,14 +83,16 @@ GURL StripAuthAndParams(const GURL& gurl);
 // successful.
 bool ExtractFormData(const blink::WebFormElement& form_element, FormData* data);
 
-// Helper function to check if there exist any form on |frame| where its action
+// Helper function to check if there exist any visible form on |frame| which
+// equals |form_element|. If |form_element| is null, checks if forms action
 // equals |action|. Returns true if so. For forms with empty or unspecified
 // actions, all form data are used for comparison. Form data comparison is
-// disabled on Mac and Android because the update prompt isn't implemented.
-// It may cause many false password updates.
+// disabled on Mac and Android because the update prompt isn't implemented. It
+// may cause many false password updates.
 // TODO(kolos) Turn on all data comparing when the update prompt will be
 // implemented on Mac and Android.
 bool IsFormVisible(blink::WebFrame* frame,
+                   const blink::WebFormElement& form_element,
                    const GURL& action,
                    const GURL& origin,
                    const FormData& form_data);

components/autofill/content/renderer/password_autofill_agent.cc

@@ -124,10 +124,14 @@ base::string16 FieldName(const FormFieldData& field,
 }
 
 bool IsUnownedPasswordFormVisible(blink::WebFrame* frame,
+                                  const blink::WebInputElement& input_element,
                                   const GURL& action,
                                   const GURL& origin,
                                   const FormData& form_data,
                                   const FormsPredictionsMap& form_predictions) {
+  if (!input_element.isNull() && form_util::IsWebNodeVisible(input_element))
+    return true;
+
   std::unique_ptr<PasswordForm> unowned_password_form(
       CreatePasswordFormFromUnownedInputElements(*frame, nullptr,
                                                  &form_predictions));
@@ -710,7 +714,8 @@ void PasswordAutofillAgent::UpdateStateForTextChange(
     password_form = CreatePasswordFormFromWebForm(
         element.form(), &field_value_and_properties_map_, &form_predictions_);
   }
-  ProvisionallySavePassword(std::move(password_form), RESTRICTION_NONE);
+  ProvisionallySavePassword(std::move(password_form), element.form(), element,
+                            RESTRICTION_NONE);
 
   if (element.isPasswordField()) {
     PasswordToLoginMap::iterator iter = password_to_username_.find(element);
@@ -992,24 +997,26 @@ void PasswordAutofillAgent::AJAXSucceeded() {
 }
 
 void PasswordAutofillAgent::OnSamePageNavigationCompleted() {
-  if (!ProvisionallySavedPasswordIsValid())
+  if (!provisionally_saved_form_.IsPasswordValid())
     return;
 
   // Prompt to save only if the form is now gone, either invisible or
   // removed from the DOM.
   blink::WebFrame* frame = render_frame()->GetWebFrame();
-  if (form_util::IsFormVisible(frame, provisionally_saved_form_->action,
-                               provisionally_saved_form_->origin,
-                               provisionally_saved_form_->form_data) ||
-      IsUnownedPasswordFormVisible(frame, provisionally_saved_form_->action,
-                                   provisionally_saved_form_->origin,
-                                   provisionally_saved_form_->form_data,
-                                   form_predictions_)) {
+  const auto& password_form = provisionally_saved_form_.password_form();
+  if (form_util::IsFormVisible(frame, provisionally_saved_form_.form_element(),
+                               password_form.action, password_form.origin,
+                               password_form.form_data) ||
+      (provisionally_saved_form_.form_element().isNull() &&
+       IsUnownedPasswordFormVisible(
+           frame, provisionally_saved_form_.input_element(),
+           password_form.action, password_form.origin, password_form.form_data,
+           form_predictions_))) {
     return;
   }
 
-  GetPasswordManagerDriver()->InPageNavigation(*provisionally_saved_form_);
-  provisionally_saved_form_.reset();
+  GetPasswordManagerDriver()->InPageNavigation(password_form);
+  provisionally_saved_form_.Reset();
 }
 
 void PasswordAutofillAgent::FirstUserGestureObserved() {
@@ -1158,8 +1165,9 @@ void PasswordAutofillAgent::FrameDetached() {
   // into a password form, try to save the data. See https://crbug.com/450806
   // for examples of sites that perform login using this technique.
   if (render_frame()->GetWebFrame()->parent() &&
-      ProvisionallySavedPasswordIsValid()) {
-    GetPasswordManagerDriver()->InPageNavigation(*provisionally_saved_form_);
+      provisionally_saved_form_.IsPasswordValid()) {
+    GetPasswordManagerDriver()->InPageNavigation(
+        provisionally_saved_form_.password_form());
   }
   FrameClosing();
 }
@@ -1181,7 +1189,8 @@ void PasswordAutofillAgent::WillSendSubmitEvent(
   // already have been updated in TextDidChangeInTextField.
   std::unique_ptr<PasswordForm> password_form = CreatePasswordFormFromWebForm(
       form, &field_value_and_properties_map_, &form_predictions_);
-  ProvisionallySavePassword(std::move(password_form),
+  ProvisionallySavePassword(std::move(password_form), form,
+                            blink::WebInputElement(),
                             RESTRICTION_NON_EMPTY_PASSWORD);
 }
 
@@ -1207,24 +1216,23 @@ void PasswordAutofillAgent::WillSubmitForm(const blink::WebFormElement& form) {
       logger->LogPasswordForm(Logger::STRING_CREATED_PASSWORD_FORM,
                               *submitted_form);
     }
-    if (provisionally_saved_form_ &&
-        submitted_form->action == provisionally_saved_form_->action) {
+    if (provisionally_saved_form_.IsSet() &&
+        submitted_form->action ==
+            provisionally_saved_form_.password_form().action) {
       if (logger)
         logger->LogMessage(Logger::STRING_SUBMITTED_PASSWORD_REPLACED);
-      submitted_form->password_value =
-          provisionally_saved_form_->password_value;
-      submitted_form->new_password_value =
-          provisionally_saved_form_->new_password_value;
-      submitted_form->username_value =
-          provisionally_saved_form_->username_value;
+      const auto& saved_form = provisionally_saved_form_.password_form();
+      submitted_form->password_value = saved_form.password_value;
+      submitted_form->new_password_value = saved_form.new_password_value;
+      submitted_form->username_value = saved_form.username_value;
     }
 
     // Some observers depend on sending this information now instead of when
     // the frame starts loading. If there are redirects that cause a new
     // RenderView to be instantiated (such as redirects to the WebStore)
     // we will never get to finish the load.
     GetPasswordManagerDriver()->PasswordFormSubmitted(*submitted_form);
-    provisionally_saved_form_.reset();
+    provisionally_saved_form_.Reset();
   } else if (logger) {
     logger->LogMessage(Logger::STRING_FORM_IS_NOT_PASSWORD);
   }
@@ -1263,15 +1271,15 @@ void PasswordAutofillAgent::DidStartProvisionalLoad(
       ui::PageTransitionIsNewNavigation(type) &&
       !blink::WebUserGestureIndicator::isProcessingUserGesture()) {
     // If onsubmit has been called, try and save that form.
-    if (provisionally_saved_form_) {
+    if (provisionally_saved_form_.IsSet()) {
       if (logger) {
         logger->LogPasswordForm(
             Logger::STRING_PROVISIONALLY_SAVED_FORM_FOR_FRAME,
-            *provisionally_saved_form_);
+            provisionally_saved_form_.password_form());
       }
       GetPasswordManagerDriver()->PasswordFormSubmitted(
-          *provisionally_saved_form_);
-      provisionally_saved_form_.reset();
+          provisionally_saved_form_.password_form());
+      provisionally_saved_form_.Reset();
     } else {
       std::vector<std::unique_ptr<PasswordForm>> possible_submitted_forms;
       // Loop through the forms on the page looking for one that has been
@@ -1552,7 +1560,7 @@ void PasswordAutofillAgent::FrameClosing() {
     password_to_username_.erase(iter.second.password_field);
   }
   web_input_to_password_info_.clear();
-  provisionally_saved_form_.reset();
+  provisionally_saved_form_.Reset();
   field_value_and_properties_map_.clear();
 }
 
@@ -1573,20 +1581,16 @@ void PasswordAutofillAgent::ClearPreview(
 
 void PasswordAutofillAgent::ProvisionallySavePassword(
     std::unique_ptr<PasswordForm> password_form,
+    const blink::WebFormElement& form,
+    const blink::WebInputElement& input,
     ProvisionallySaveRestriction restriction) {
   if (!password_form || (restriction == RESTRICTION_NON_EMPTY_PASSWORD &&
                          password_form->password_value.empty() &&
                          password_form->new_password_value.empty())) {
     return;
   }
-  provisionally_saved_form_ = std::move(password_form);
-}
-
-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());
+  DCHECK(password_form && (!form.isNull() || !input.isNull()));
+  provisionally_saved_form_.Set(std::move(password_form), form, input);
 }
 
 const mojom::AutofillDriverPtr& PasswordAutofillAgent::GetAutofillDriver() {

components/autofill/content/renderer/password_autofill_agent.h

@@ -14,6 +14,7 @@
 #include "components/autofill/content/common/autofill_driver.mojom.h"
 #include "components/autofill/content/renderer/autofill_agent.h"
 #include "components/autofill/content/renderer/password_form_conversion_utils.h"
+#include "components/autofill/content/renderer/provisionally_saved_password_form.h"
 #include "components/autofill/core/common/form_data_predictions.h"
 #include "components/autofill/core/common/password_form.h"
 #include "components/autofill/core/common/password_form_field_prediction_map.h"
@@ -239,15 +240,17 @@ class PasswordAutofillAgent : public content::RenderFrameObserver,
   void ClearPreview(blink::WebInputElement* username,
                     blink::WebInputElement* password);
 
-  // Saves |password_form| in |provisionally_saved_form_|, as long as it
-  // satisfies |restriction|.
+  // Saves |password_form|, |form| and |input| in |provisionally_saved_form_|,
+  // as long as it satisfies |restriction|. |form| and |input| are the elements
+  // user has just been interacting with before the form save. |form| or |input|
+  // can be null but not both at the same time. For example: if the form is
+  // unowned, |form| will be null; if the user has submitted the form, |input|
+  // will be null.
   void ProvisionallySavePassword(std::unique_ptr<PasswordForm> password_form,
+                                 const blink::WebFormElement& form,
+                                 const blink::WebInputElement& input,
                                  ProvisionallySaveRestriction restriction);
 
-  // Returns true if |provisionally_saved_form_| has enough information that
-  // it is likely filled out.
-  bool ProvisionallySavedPasswordIsValid();
-
   // Helper function called when in-page navigation completed
   void OnSamePageNavigationCompleted();
 
@@ -260,7 +263,7 @@ class PasswordAutofillAgent : public content::RenderFrameObserver,
 
   // Set if the user might be submitting a password form on the current page,
   // but the submit may still fail (i.e. doesn't pass JavaScript validation).
-  std::unique_ptr<PasswordForm> provisionally_saved_form_;
+  ProvisionallySavedPasswordForm provisionally_saved_form_;
 
   // Map WebFormControlElement to the pair of:
   // 1) The most recent text that user typed or PasswordManager autofilled in

components/autofill/content/renderer/provisionally_saved_password_form.cc

@@ -0,0 +1,42 @@
+// Copyright 2017 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/provisionally_saved_password_form.h"
+
+#include <utility>
+
+#include "components/autofill/core/common/password_form.h"
+
+namespace autofill {
+
+ProvisionallySavedPasswordForm::ProvisionallySavedPasswordForm() = default;
+
+ProvisionallySavedPasswordForm::~ProvisionallySavedPasswordForm() = default;
+
+void ProvisionallySavedPasswordForm::Set(
+    std::unique_ptr<PasswordForm> password_form,
+    const blink::WebFormElement& form_element,
+    const blink::WebInputElement& input_element) {
+  password_form_ = std::move(password_form);
+  form_element_ = form_element;
+  input_element_ = input_element;
+}
+
+void ProvisionallySavedPasswordForm::Reset() {
+  password_form_.reset();
+  form_element_.reset();
+  input_element_.reset();
+}
+
+bool ProvisionallySavedPasswordForm::IsSet() const {
+  return static_cast<bool>(password_form_);
+}
+
+bool ProvisionallySavedPasswordForm::IsPasswordValid() const {
+  return IsSet() && !password_form_->username_value.empty() &&
+         !(password_form_->password_value.empty() &&
+           password_form_->new_password_value.empty());
+}
+
+}  // namespace autofill