Add FormData::is_action_empty.

In case of a form action is absent, FormData::action is set to origin.
That makes it impossible to check whether FormData has an empty action.
And it's used for detection of submission success in Password Manager.
This CL fixed that.

Bug: 1008798
Change-Id: Ic44aaf6ef94df6766fe7eddccbe14a2b030974a7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1832818Reviewed-by: Emily Stark <estark@chromium.org>
Reviewed-by: Dominic Battré <battre@chromium.org>
Commit-Queue: Vadym Doroshenko <dvadym@chromium.org>
Cr-Commit-Position: refs/heads/master@{#705890}

components/autofill/content/renderer/form_autofill_util.cc

@@ -1768,6 +1768,7 @@ bool WebFormElementToFormData(
   form->unique_renderer_id = form_element.UniqueRendererFormId();
   form->url = GetCanonicalOriginForDocument(frame->GetDocument());
   form->action = GetCanonicalActionForForm(form_element);
+  form->is_action_empty = form_element.Action().IsNull();
   if (IsAutofillFieldMetadataEnabled()) {
     SCOPED_UMA_HISTOGRAM_TIMER(
         "PasswordManager.ButtonTitlePerformance.HasFormTag");

components/autofill/content/renderer/form_autofill_util_browsertest.cc

@@ -21,18 +21,23 @@
 #include "third_party/blink/public/web/web_local_frame.h"
 #include "third_party/blink/public/web/web_select_element.h"
 
-using autofill::FormFieldData;
 using autofill::mojom::ButtonTitleType;
 using blink::WebDocument;
 using blink::WebElement;
+using blink::WebElementCollection;
 using blink::WebFormControlElement;
 using blink::WebFormElement;
 using blink::WebInputElement;
 using blink::WebLocalFrame;
+using blink::WebNode;
 using blink::WebSelectElement;
 using blink::WebString;
 using blink::WebVector;
 
+namespace autofill {
+namespace form_util {
+namespace {
+
 struct AutofillFieldLabelSourceCase {
   const char* html;
   const FormFieldData::LabelSource label_source;
@@ -160,7 +165,7 @@ TEST_F(FormAutofillUtilsTest, FindChildTextTest) {
     WebElement target = web_frame->GetDocument().GetElementById("target");
     ASSERT_FALSE(target.IsNull());
     EXPECT_EQ(base::UTF8ToUTF16(test_case.expected_label),
-              autofill::form_util::FindChildText(target));
+              FindChildText(target));
   }
 }
 
@@ -177,14 +182,13 @@ TEST_F(FormAutofillUtilsTest, FindChildTextSkipElementTest) {
     ASSERT_FALSE(target.IsNull());
     WebVector<WebElement> web_to_skip =
         web_frame->GetDocument().QuerySelectorAll("div[class='skip']");
-    std::set<blink::WebNode> to_skip;
+    std::set<WebNode> to_skip;
     for (size_t i = 0; i < web_to_skip.size(); ++i) {
       to_skip.insert(web_to_skip[i]);
     }
 
     EXPECT_EQ(base::UTF8ToUTF16(test_case.expected_label),
-              autofill::form_util::FindChildTextWithIgnoreListForTesting(
-                  target, to_skip));
+              FindChildTextWithIgnoreListForTesting(target, to_skip));
   }
 }
 
@@ -213,8 +217,8 @@ TEST_F(FormAutofillUtilsTest, InferLabelForElementTest) {
     FormFieldData::LabelSource label_source =
         FormFieldData::LabelSource::kUnknown;
     base::string16 label;
-    autofill::form_util::InferLabelForElementForTesting(form_target, stop_words,
-                                                        &label, &label_source);
+    InferLabelForElementForTesting(form_target, stop_words, &label,
+                                   &label_source);
     EXPECT_EQ(base::UTF8ToUTF16(test_case.expected_label), label);
   }
 }
@@ -290,8 +294,7 @@ TEST_F(FormAutofillUtilsTest, InferButtonTitleForFormTest) {
   const WebFormElement& form_target = target.ToConst<WebFormElement>();
   ASSERT_FALSE(form_target.IsNull());
 
-  autofill::ButtonTitleList actual =
-      autofill::form_util::InferButtonTitlesForTesting(form_target);
+  autofill::ButtonTitleList actual = InferButtonTitlesForTesting(form_target);
   autofill::ButtonTitleList expected = {
       {base::UTF8ToUTF16("Clear field"),
        ButtonTitleType::INPUT_ELEMENT_BUTTON_TYPE},
@@ -327,8 +330,7 @@ TEST_F(FormAutofillUtilsTest, InferButtonTitleForFormTest_TooLongTitle) {
   const WebFormElement& form_target = target.ToConst<WebFormElement>();
   ASSERT_FALSE(form_target.IsNull());
 
-  autofill::ButtonTitleList actual =
-      autofill::form_util::InferButtonTitlesForTesting(form_target);
+  autofill::ButtonTitleList actual = InferButtonTitlesForTesting(form_target);
 
   int total_length = 0;
   for (auto title : actual) {
@@ -358,8 +360,7 @@ TEST_F(FormAutofillUtilsTest, InferButtonTitle_Formless) {
   const WebElement& body = web_frame->GetDocument().Body();
   ASSERT_FALSE(body.IsNull());
 
-  autofill::ButtonTitleList actual =
-      autofill::form_util::InferButtonTitlesForTesting(body);
+  autofill::ButtonTitleList actual = InferButtonTitlesForTesting(body);
   autofill::ButtonTitleList expected = {
       {base::UTF8ToUTF16("Show password"),
        ButtonTitleType::INPUT_ELEMENT_BUTTON_TYPE},
@@ -377,23 +378,22 @@ TEST_F(FormAutofillUtilsTest, IsEnabled) {
       "<input type='password' id='name3'>"
       "<input type='text' id='name4' disabled>");
 
-  const std::vector<blink::WebElement> dummy_fieldsets;
+  const std::vector<WebElement> dummy_fieldsets;
 
   WebLocalFrame* web_frame = GetMainFrame();
   ASSERT_TRUE(web_frame);
-  std::vector<blink::WebFormControlElement> control_elements;
-  blink::WebElementCollection inputs =
+  std::vector<WebFormControlElement> control_elements;
+  WebElementCollection inputs =
       web_frame->GetDocument().GetElementsByHTMLTagName("input");
-  for (blink::WebElement element = inputs.FirstItem(); !element.IsNull();
+  for (WebElement element = inputs.FirstItem(); !element.IsNull();
        element = inputs.NextItem()) {
-    control_elements.push_back(element.To<blink::WebFormControlElement>());
+    control_elements.push_back(element.To<WebFormControlElement>());
   }
 
   autofill::FormData target;
-  EXPECT_TRUE(
-      autofill::form_util::UnownedPasswordFormElementsAndFieldSetsToFormData(
-          dummy_fieldsets, control_elements, nullptr, web_frame->GetDocument(),
-          nullptr, autofill::form_util::EXTRACT_NONE, &target, nullptr));
+  EXPECT_TRUE(UnownedPasswordFormElementsAndFieldSetsToFormData(
+      dummy_fieldsets, control_elements, nullptr, web_frame->GetDocument(),
+      nullptr, EXTRACT_NONE, &target, nullptr));
   const struct {
     const char* const name;
     bool enabled;
@@ -416,23 +416,22 @@ TEST_F(FormAutofillUtilsTest, IsReadonly) {
       "<input type='password' id='name3'>"
       "<input type='text' id='name4' readonly>");
 
-  const std::vector<blink::WebElement> dummy_fieldsets;
+  const std::vector<WebElement> dummy_fieldsets;
 
   WebLocalFrame* web_frame = GetMainFrame();
   ASSERT_TRUE(web_frame);
-  std::vector<blink::WebFormControlElement> control_elements;
-  blink::WebElementCollection inputs =
+  std::vector<WebFormControlElement> control_elements;
+  WebElementCollection inputs =
       web_frame->GetDocument().GetElementsByHTMLTagName("input");
-  for (blink::WebElement element = inputs.FirstItem(); !element.IsNull();
+  for (WebElement element = inputs.FirstItem(); !element.IsNull();
        element = inputs.NextItem()) {
-    control_elements.push_back(element.To<blink::WebFormControlElement>());
+    control_elements.push_back(element.To<WebFormControlElement>());
   }
 
   autofill::FormData target;
-  EXPECT_TRUE(
-      autofill::form_util::UnownedPasswordFormElementsAndFieldSetsToFormData(
-          dummy_fieldsets, control_elements, nullptr, web_frame->GetDocument(),
-          nullptr, autofill::form_util::EXTRACT_NONE, &target, nullptr));
+  EXPECT_TRUE(UnownedPasswordFormElementsAndFieldSetsToFormData(
+      dummy_fieldsets, control_elements, nullptr, web_frame->GetDocument(),
+      nullptr, EXTRACT_NONE, &target, nullptr));
   const struct {
     const char* const name;
     bool readonly;
@@ -453,27 +452,26 @@ TEST_F(FormAutofillUtilsTest, IsFocusable) {
       "<input type='text' id='name1' value='123'>"
       "<input type='text' id='name2' style='display:none'>");
 
-  const std::vector<blink::WebElement> dummy_fieldsets;
+  const std::vector<WebElement> dummy_fieldsets;
 
   WebLocalFrame* web_frame = GetMainFrame();
   ASSERT_TRUE(web_frame);
 
-  std::vector<blink::WebFormControlElement> control_elements;
+  std::vector<WebFormControlElement> control_elements;
   control_elements.push_back(web_frame->GetDocument()
                                  .GetElementById("name1")
-                                 .To<blink::WebFormControlElement>());
+                                 .To<WebFormControlElement>());
   control_elements.push_back(web_frame->GetDocument()
                                  .GetElementById("name2")
-                                 .To<blink::WebFormControlElement>());
+                                 .To<WebFormControlElement>());
 
   EXPECT_TRUE(autofill::form_util::IsWebElementVisible(control_elements[0]));
   EXPECT_FALSE(autofill::form_util::IsWebElementVisible(control_elements[1]));
 
   autofill::FormData target;
-  EXPECT_TRUE(
-      autofill::form_util::UnownedPasswordFormElementsAndFieldSetsToFormData(
-          dummy_fieldsets, control_elements, nullptr, web_frame->GetDocument(),
-          nullptr, autofill::form_util::EXTRACT_NONE, &target, nullptr));
+  EXPECT_TRUE(UnownedPasswordFormElementsAndFieldSetsToFormData(
+      dummy_fieldsets, control_elements, nullptr, web_frame->GetDocument(),
+      nullptr, EXTRACT_NONE, &target, nullptr));
   ASSERT_EQ(2u, target.fields.size());
   EXPECT_EQ(base::UTF8ToUTF16("name1"), target.fields[0].name);
   EXPECT_TRUE(target.fields[0].is_focusable);
@@ -484,19 +482,17 @@ TEST_F(FormAutofillUtilsTest, IsFocusable) {
 TEST_F(FormAutofillUtilsTest, FindFormByUniqueId) {
   LoadHTML("<body><form id='form1'></form><form id='form2'></form></body>");
   WebDocument doc = GetMainFrame()->GetDocument();
-  blink::WebVector<WebFormElement> forms;
+  WebVector<WebFormElement> forms;
   doc.Forms(forms);
 
   for (const auto& form : forms) {
-    EXPECT_EQ(form, autofill::form_util::FindFormByUniqueRendererId(
-                        doc, form.UniqueRendererFormId()));
+    EXPECT_EQ(form,
+              FindFormByUniqueRendererId(doc, form.UniqueRendererFormId()));
   }
 
   // Expect null form element for non-existing form id.
   uint32_t non_existing_id = forms[0].UniqueRendererFormId() + 1000;
-  EXPECT_TRUE(
-      autofill::form_util::FindFormByUniqueRendererId(doc, non_existing_id)
-          .IsNull());
+  EXPECT_TRUE(FindFormByUniqueRendererId(doc, non_existing_id).IsNull());
 }
 
 TEST_F(FormAutofillUtilsTest, FindFormControlByUniqueId) {
@@ -506,7 +502,6 @@ TEST_F(FormAutofillUtilsTest, FindFormControlByUniqueId) {
   auto input1 = doc.GetElementById("i1").To<WebInputElement>();
   auto input2 = doc.GetElementById("i2").To<WebInputElement>();
   uint32_t non_existing_id = input2.UniqueRendererFormControlId() + 1000;
-  using autofill::form_util::FindFormControlElementsByUniqueRendererId;
 
   EXPECT_EQ(input1, FindFormControlElementsByUniqueRendererId(
                         doc, input1.UniqueRendererFormControlId()));
@@ -527,9 +522,7 @@ TEST_F(FormAutofillUtilsTest, FindFormControlElementsByUniqueIdNoForm) {
                                         non_existing_id,
                                         input1.UniqueRendererFormControlId()};
 
-  auto elements =
-      autofill::form_util::FindFormControlElementsByUniqueRendererId(
-          doc, renderer_ids);
+  auto elements = FindFormControlElementsByUniqueRendererId(doc, renderer_ids);
 
   ASSERT_EQ(3u, elements.size());
   EXPECT_EQ(input3, elements[0]);
@@ -551,9 +544,8 @@ TEST_F(FormAutofillUtilsTest, FindFormControlElementsByUniqueIdWithForm) {
                                         non_existing_id,
                                         input1.UniqueRendererFormControlId()};
 
-  auto elements =
-      autofill::form_util::FindFormControlElementsByUniqueRendererId(
-          doc, form.UniqueRendererFormId(), renderer_ids);
+  auto elements = FindFormControlElementsByUniqueRendererId(
+      doc, form.UniqueRendererFormId(), renderer_ids);
 
   // |input3| is not in the form, so it shouldn't be returned.
   ASSERT_EQ(3u, elements.size());
@@ -563,7 +555,7 @@ TEST_F(FormAutofillUtilsTest, FindFormControlElementsByUniqueIdWithForm) {
 
   // Expect that no elements are retured for non existing form id.
   uint32_t non_existing_form_id = form.UniqueRendererFormId() + 1000;
-  elements = autofill::form_util::FindFormControlElementsByUniqueRendererId(
+  elements = FindFormControlElementsByUniqueRendererId(
       doc, non_existing_form_id, renderer_ids);
   ASSERT_EQ(3u, elements.size());
   EXPECT_TRUE(elements[0].IsNull());
@@ -713,10 +705,41 @@ TEST_F(FormAutofillUtilsTest, IsFormControlVisible) {
   auto input = doc.GetElementById("input1").To<WebFormControlElement>();
   uint32_t input_id = input.UniqueRendererFormControlId();
 
-  EXPECT_TRUE(
-      autofill::form_util::IsFormControlVisible(GetMainFrame(), input_id));
+  EXPECT_TRUE(IsFormControlVisible(GetMainFrame(), input_id));
 
   // Hide a field.
   input.SetAttribute("style", "display:none");
   EXPECT_FALSE(autofill::form_util::IsFormVisible(GetMainFrame(), input_id));
 }
+
+TEST_F(FormAutofillUtilsTest, IsActionEmptyFalse) {
+  LoadHTML(
+      "<body><form id='form1' action='done.html'><input "
+      "id='i1'></form></body>");
+  WebDocument doc = GetMainFrame()->GetDocument();
+  auto web_form = doc.GetElementById("form1").To<WebFormElement>();
+
+  FormData form_data;
+  ASSERT_TRUE(WebFormElementToFormData(
+      web_form, WebFormControlElement(), nullptr /*field_data_manager*/,
+      EXTRACT_VALUE, &form_data, nullptr /* FormFieldData */));
+
+  EXPECT_FALSE(form_data.is_action_empty);
+}
+
+TEST_F(FormAutofillUtilsTest, IsActionEmptyTrue) {
+  LoadHTML("<body><form id='form1'><input id='i1'></form></body>");
+  WebDocument doc = GetMainFrame()->GetDocument();
+  auto web_form = doc.GetElementById("form1").To<WebFormElement>();
+
+  FormData form_data;
+  ASSERT_TRUE(WebFormElementToFormData(
+      web_form, WebFormControlElement(), nullptr /*field_data_manager*/,
+      EXTRACT_VALUE, &form_data, nullptr /* FormFieldData */));
+
+  EXPECT_TRUE(form_data.is_action_empty);
+}
+
+}  // namespace
+}  // namespace form_util
+}  // namespace autofill

components/autofill/core/browser/autofill_test_utils.cc

@@ -120,6 +120,7 @@ void CreateTestAddressFormData(FormData* form,
                      mojom::ButtonTitleType::BUTTON_ELEMENT_SUBMIT_TYPE)};
   form->url = GURL("http://myform.com/form.html");
   form->action = GURL("http://myform.com/submit.html");
+  form->is_action_empty = true;
   form->main_frame_origin =
       url::Origin::Create(GURL("https://myform_root.com/form.html"));
   types->clear();

components/autofill/core/common/form_data.cc

@@ -98,10 +98,12 @@ bool FormData::SameFormAs(const FormData& form) const {
 bool FormData::SimilarFormAs(const FormData& form) const {
   if (name != form.name || id_attribute != form.id_attribute ||
       name_attribute != form.name_attribute || url != form.url ||
-      action != form.action || is_form_tag != form.is_form_tag ||
+      action != form.action || is_action_empty != form.is_action_empty ||
+      is_form_tag != form.is_form_tag ||
       is_formless_checkout != form.is_formless_checkout ||
-      fields.size() != form.fields.size())
+      fields.size() != form.fields.size()) {
     return false;
+  }
   for (size_t i = 0; i < fields.size(); ++i) {
     if (!fields[i].SimilarFieldAs(form.fields[i]))
       return false;
@@ -124,7 +126,7 @@ bool FormData::DynamicallySameFormAs(const FormData& form) const {
 bool FormData::operator==(const FormData& form) const {
   return name == form.name && id_attribute == form.id_attribute &&
          name_attribute == form.name_attribute && url == form.url &&
-         action == form.action &&
+         action == form.action && is_action_empty == form.is_action_empty &&
          unique_renderer_id == form.unique_renderer_id &&
          submission_event == form.submission_event &&
          is_form_tag == form.is_form_tag &&
@@ -237,6 +239,7 @@ LogBuffer& operator<<(LogBuffer& buffer, const FormData& form) {
   buffer << Tr{} << "Unique renderer Id:" << form.unique_renderer_id;
   buffer << Tr{} << "URL:" << form.url;
   buffer << Tr{} << "Action:" << form.action;
+  buffer << Tr{} << "Is action empty:" << form.is_action_empty;
   buffer << Tr{} << "Is <form> tag:" << form.is_form_tag;
   for (size_t i = 0; i < form.fields.size(); ++i) {
     buffer << Tag{"tr"};

components/autofill/core/common/form_data.h

@@ -78,6 +78,10 @@ struct FormData {
   GURL url;
   // The action target of the form.
   GURL action;
+  // If the form in the DOM has an empty action attribute, the |action| field in
+  // the FormData is set to the frame URL of the embedding document. This field
+  // indicates whether the action attribute is empty in the form in the DOM.
+  bool is_action_empty = false;
   // The URL of main frame containing this form.
   url::Origin main_frame_origin;
   // True if this form is a form tag.

components/autofill/core/common/mojom/autofill_types.mojom

@@ -153,6 +153,7 @@ struct FormData {
   array<ButtonTitleInfo> button_titles;
   url.mojom.Url url;
   url.mojom.Url action;
+  bool is_action_empty;
   url.mojom.Origin main_frame_origin;
   bool is_form_tag;
   bool is_formless_checkout;

components/autofill/core/common/mojom/autofill_types_mojom_traits.cc

@@ -107,6 +107,7 @@ bool StructTraits<autofill::mojom::FormDataDataView, autofill::FormData>::Read(
     return false;
   if (!data.ReadAction(&out->action))
     return false;
+  out->is_action_empty = data.is_action_empty();
   if (!data.ReadMainFrameOrigin(&out->main_frame_origin))
     return false;
 

components/autofill/core/common/mojom/autofill_types_mojom_traits.h

@@ -196,6 +196,10 @@ struct StructTraits<autofill::mojom::FormDataDataView, autofill::FormData> {
 
   static const GURL& action(const autofill::FormData& r) { return r.action; }
 
+  static bool is_action_empty(const autofill::FormData& r) {
+    return r.is_action_empty;
+  }
+
   static const url::Origin& main_frame_origin(const autofill::FormData& r) {
     return r.main_frame_origin;
   }