Implement IsFormVisible with render ids.

This CL is preparation for cleaning PasswordAutofillAgent.

Bug: 949519

Change-Id: I0917c681f707f5b77230b73a34f0bbf5981f3f81
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1821477
Commit-Queue: Vadym Doroshenko <dvadym@chromium.org>
Reviewed-by: Mohamed Amir Yosef <mamir@chromium.org>
Cr-Commit-Position: refs/heads/master@{#699773}

components/autofill/content/renderer/form_autofill_util.cc

@@ -1486,57 +1486,22 @@ bool ExtractFormData(const WebFormElement& form_element, FormData* data) {
       data, nullptr);
 }
 
-bool IsFormVisible(blink::WebLocalFrame* frame,
-                   const blink::WebFormElement& form_element,
-                   const GURL& canonical_action,
-                   const GURL& canonical_origin,
-                   const FormData& form_data) {
-  const GURL frame_origin = GetCanonicalOriginForDocument(frame->GetDocument());
-  blink::WebVector<WebFormElement> forms;
-  frame->GetDocument().Forms(forms);
-
-  // Omitting the action attribute would result in |canonical_origin| for
-  // hierarchical schemes like http:, and in an empty URL for non-hierarchical
-  // schemes like about: or data: etc.
-  const bool action_is_empty = canonical_action.is_empty()
-                               || canonical_action == canonical_origin;
-
-  // Since empty or unspecified action fields are automatically set to page URL,
-  // action field for forms cannot be used for comparing (all forms with
-  // empty/unspecified actions have the same value). If an action field is set
-  // to the page URL, this method checks ALL fields of the form instead (using
-  // FormData.SameFormAs). This is also true if the action was set to the page
-  // URL on purpose.
-  for (const WebFormElement& form : forms) {
-    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;
-    if (action_is_empty != form_action_is_empty)
-      continue;
-
-    if (action_is_empty) {  // Both actions are empty, compare all fields.
-      FormData extracted_form_data;
-      WebFormElementToFormData(form, WebFormControlElement(), nullptr,
-                               EXTRACT_NONE, &extracted_form_data, nullptr);
-      if (form_data.SameFormAs(extracted_form_data)) {
-        return true;  // Form still exists.
-      }
-    } else {  // Both actions are non-empty, compare actions only.
-      if (canonical_action == iter_canonical_action) {
-        return true;  // Form still exists.
-      }
-    }
-  }
+bool IsFormVisible(blink::WebLocalFrame* frame, uint32_t form_renderer_id) {
+  WebDocument doc = frame->GetDocument();
+  if (doc.IsNull())
+    return false;
+  WebFormElement form = FindFormByUniqueRendererId(doc, form_renderer_id);
+  return form.IsNull() ? false : AreFormContentsVisible(form);
+}
 
-  return false;
+bool IsFormControlVisible(blink::WebLocalFrame* frame,
+                          uint32_t field_renderer_id) {
+  WebDocument doc = frame->GetDocument();
+  if (doc.IsNull())
+    return false;
+  WebFormControlElement field =
+      FindFormControlElementsByUniqueRendererId(doc, field_renderer_id);
+  return field.IsNull() ? false : IsWebElementVisible(field);
 }
 
 bool IsSomeControlElementVisible(

components/autofill/content/renderer/form_autofill_util.h

@@ -71,15 +71,14 @@ GURL StripAuthAndParams(const GURL& gurl);
 // successful.
 bool ExtractFormData(const blink::WebFormElement& form_element, FormData* data);
 
-// 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.
-bool IsFormVisible(blink::WebLocalFrame* frame,
-                   const blink::WebFormElement& form_element,
-                   const GURL& action,
-                   const GURL& origin,
-                   const FormData& form_data);
+// Helper function to check if a form with renderer id |form_renderer_id| exists
+// in |frame| and is visible.
+bool IsFormVisible(blink::WebLocalFrame* frame, uint32_t form_renderer_id);
+
+// Helper function to check if a field with renderer id |field_renderer_id|
+// exists in |frame| and is visible.
+bool IsFormControlVisible(blink::WebLocalFrame* frame,
+                          uint32_t field_renderer_id);
 
 // Returns true if at least one element from |control_elements| is visible.
 bool IsSomeControlElementVisible(

components/autofill/content/renderer/form_autofill_util_browsertest.cc

@@ -692,3 +692,30 @@ TEST_F(FormAutofillUtilsTest, GetAriaDescribedByInvalid) {
   EXPECT_EQ(autofill::form_util::GetAriaDescription(doc, element),
             base::UTF8ToUTF16(""));
 }
+
+TEST_F(FormAutofillUtilsTest, IsFormVisible) {
+  LoadHTML("<body><form id='form1'><input id='i1'></form></body>");
+  WebDocument doc = GetMainFrame()->GetDocument();
+  auto form = doc.GetElementById("form1").To<WebFormElement>();
+  uint32_t form_id = form.UniqueRendererFormId();
+
+  EXPECT_TRUE(autofill::form_util::IsFormVisible(GetMainFrame(), form_id));
+
+  // Hide a form.
+  form.SetAttribute("style", "display:none");
+  EXPECT_FALSE(autofill::form_util::IsFormVisible(GetMainFrame(), form_id));
+}
+
+TEST_F(FormAutofillUtilsTest, IsFormControlVisible) {
+  LoadHTML("<body><input id='input1'></body>");
+  WebDocument doc = GetMainFrame()->GetDocument();
+  auto input = doc.GetElementById("input1").To<WebFormControlElement>();
+  uint32_t input_id = input.UniqueRendererFormControlId();
+
+  EXPECT_TRUE(
+      autofill::form_util::IsFormControlVisible(GetMainFrame(), input_id));
+
+  // Hide a field.
+  input.SetAttribute("style", "display:none");
+  EXPECT_FALSE(autofill::form_util::IsFormVisible(GetMainFrame(), input_id));
+}

components/autofill/content/renderer/password_autofill_agent.cc

@@ -72,6 +72,9 @@ using blink::WebView;
 
 namespace autofill {
 
+using form_util::IsFormControlVisible;
+using form_util::IsFormVisible;
+
 using mojom::FocusedFieldType;
 using mojom::SubmissionIndicatorEvent;
 using mojom::SubmissionSource;
@@ -117,11 +120,6 @@ bool DoesFormContainAmbiguousOrEmptyNames(
                base::ASCIIToUTF16(kDummyUsernameField)));
 }
 
-bool IsUnownedPasswordFormVisible(const WebInputElement& input_element) {
-  return !input_element.IsNull() &&
-         form_util::IsWebElementVisible(input_element);
-}
-
 bool IsElementEditable(const WebInputElement& element) {
   return element.IsEnabled() && !element.IsReadOnly();
 }
@@ -898,18 +896,22 @@ void PasswordAutofillAgent::FireSubmissionIfFormDisappear(
   // Prompt to save only if the form is now gone, either invisible or
   // removed from the DOM.
   WebLocalFrame* frame = render_frame()->GetWebFrame();
-  const auto& password_form = provisionally_saved_form_.password_form();
   // TODO(crbug.com/720347): This method could be called often and checking form
-  // visibility could be expesive. Add performance metrics for this.
+  // visibility could be expensive. Add performance metrics for this.
   if (event != SubmissionIndicatorEvent::DOM_MUTATION_AFTER_XHR) {
-    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(
-             provisionally_saved_form_.input_element()))) {
-      return;
+    bool is_last_updated_field_in_form =
+        last_updated_form_renderer_id_ != FormData::kNotSetFormRendererId;
+    // Check whether the form which is the candidate for submission disappeared.
+    // If yes this form is considered to be successfully submitted.
+    if (is_last_updated_field_in_form) {
+      // A form is inside <form> tag. Check the visibility of the whole form.
+      if (IsFormVisible(frame, last_updated_form_renderer_id_))
+        return;
+    } else {
+      // A form is without <form> tag. Check the visibility of the last updated
+      // field.
+      if (IsFormControlVisible(frame, last_updated_field_renderer_id_))
+        return;
     }
   }
 
@@ -1401,6 +1403,8 @@ void PasswordAutofillAgent::CleanupOnDocumentShutdown() {
   username_detector_cache_.clear();
   forms_structure_cache_.clear();
   autofilled_elements_cache_.clear();
+  last_updated_field_renderer_id_ = FormData::kNotSetFormRendererId;
+  last_updated_form_renderer_id_ = FormData::kNotSetFormRendererId;
 #if !defined(OS_ANDROID) && !defined(OS_IOS)
   page_passwords_analyser_.Reset();
 #endif
@@ -1425,6 +1429,7 @@ void PasswordAutofillAgent::ProvisionallySavePassword(
     ProvisionallySaveRestriction restriction) {
   DCHECK(!form.IsNull() || !element.IsNull());
 
+  SetLastUpdatedFormAndField(form, element);
   std::unique_ptr<PasswordForm> password_form;
   if (form.IsNull()) {
     password_form = GetPasswordFormFromUnownedInputElements();
@@ -1827,4 +1832,17 @@ void PasswordAutofillAgent::AutofillField(const base::string16& value,
                                      WebString::FromUTF16(value));
 }
 
+void SetLastUpdatedFormAndField(const blink::WebFormElement& Form,
+                                const blink::WebFormControlElement& input);
+void PasswordAutofillAgent::SetLastUpdatedFormAndField(
+    const WebFormElement& form,
+    const WebFormControlElement& input) {
+  last_updated_form_renderer_id_ = form.IsNull()
+                                       ? FormData::kNotSetFormRendererId
+                                       : form.UniqueRendererFormId();
+  last_updated_field_renderer_id_ = input.IsNull()
+                                        ? FormData::kNotSetFormRendererId
+                                        : input.UniqueRendererFormControlId();
+}
+
 }  // namespace autofill

components/autofill/content/renderer/password_autofill_agent.h

@@ -453,6 +453,9 @@ class PasswordAutofillAgent : public content::RenderFrameObserver,
   // non-null.
   void AutofillField(const base::string16& value, blink::WebInputElement field);
 
+  void SetLastUpdatedFormAndField(const blink::WebFormElement& form,
+                                  const blink::WebFormControlElement& input);
+
   // The logins we have filled so far with their associated info.
   WebInputToPasswordInfoMap web_input_to_password_info_;
   // A (sort-of) reverse map to |web_input_to_password_info_|.
@@ -537,6 +540,11 @@ class PasswordAutofillAgent : public content::RenderFrameObserver,
   // DidCommitProvisionalLoad() but only for non-same-document-navigations.
   bool recorded_first_filling_result_ = false;
 
+  // Contains renderer id of last updated input element.
+  uint32_t last_updated_field_renderer_id_ = FormData::kNotSetFormRendererId;
+  // Contains renderer id of the form of the last updated input element.
+  uint32_t last_updated_form_renderer_id_ = FormData::kNotSetFormRendererId;
+
   DISALLOW_COPY_AND_ASSIGN(PasswordAutofillAgent);
 };
 

components/autofill/core/common/form_data.h

@@ -29,6 +29,8 @@ using ButtonTitleList = std::vector<ButtonTitleInfo>;
 
 // Holds information about a form to be filled and/or submitted.
 struct FormData {
+  // TODO(https://crbug.com/875768): Rename this const to kNotSetRendererId, and
+  // use it also for not set renderer ids in FormFieldData.
   static constexpr uint32_t kNotSetFormRendererId =
       std::numeric_limits<uint32_t>::max();