Run new PasswordForm parser even if old one fails

The old FormData -> PasswordForm parser runs in renderer process,
before the data about forms is passed on to the browser process.

The new parser runs in browser, after it receives the data from the
renderer.

For legacy reasons, the FormData (parsing input) is enclosed inside
PasswordForm (output) during the inter-process transport.

As a result, if the old parser fails to parse completely, the new one
has no chance to run at all, because there is no valid PasswordForm to
encapsulate its input FormData during the inter-process transport.

The important case when this matters is if there are password fields
present, but none of them is enabled. The old parser gives up, the
new does not care about enabled vs. disabled, so can parse.

This CL allows the old parser to produce a minimal PasswordForm
if only disabled password fields are present, to allow transporting
the FormData to the new parser. In PasswordManager, all these
minimal PasswordForms are managed by at most one PasswordFormManager
and only used as a fallback, so the functionality of Chrome under
the old parser is not affected.

Bug: 904908
Change-Id: Idaf569d39884375dc5942dea4349aeec41355c76
Reviewed-on: https://chromium-review.googlesource.com/c/1337497
Commit-Queue: Vaclav Brozek <vabr@chromium.org>
Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
Cr-Commit-Position: refs/heads/master@{#609225}

components/autofill/content/renderer/password_form_conversion_utils.cc

@@ -344,11 +344,24 @@ bool StringMatchesCVC(const base::string16& str) {
   return MatchesPattern(str, *kCardCvcReCached);
 }
 
-bool IsEnabledPasswordFieldPresent(const std::vector<FormFieldData>& fields) {
-  return std::find_if(
-             fields.begin(), fields.end(), [](const FormFieldData& field) {
-               return field.is_enabled && field.form_control_type == "password";
-             }) != fields.end();
+// Which types of password fields are present in a form?
+enum class PasswordContents {
+  kEnabled,       // At least one enabled password field.
+  kOnlyDisabled,  // At least one password field, but not enabled.
+  kNone           // No password fields present.
+};
+
+// Returns the PasswordContents reflecting the contents of |fields|.
+PasswordContents GetPasswordContents(const std::vector<FormFieldData>& fields) {
+  PasswordContents result = PasswordContents::kNone;
+  for (const FormFieldData& field : fields) {
+    if (field.form_control_type != "password")
+      continue;
+    result = PasswordContents::kOnlyDisabled;
+    if (field.is_enabled)
+      return PasswordContents::kEnabled;
+  }
+  return result;
 }
 
 // Find the first element in |username_predictions| (i.e. the most reliable
@@ -408,9 +421,21 @@ bool GetPasswordForm(
 
   const FormData& form_data = password_form->form_data;
 
-  // Early exit if no passwords to be typed into.
-  if (!IsEnabledPasswordFieldPresent(form_data.fields))
-    return false;
+  PasswordContents password_contents = GetPasswordContents(form_data.fields);
+  switch (password_contents) {
+    case PasswordContents::kEnabled:
+      // All well, continue parsing.
+      break;
+    case PasswordContents::kOnlyDisabled:
+      // The current parser gives up, but returns a fallback form so that the
+      // newer parser can try parsing as well.
+      password_form->scheme = PasswordForm::SCHEME_HTML;
+      password_form->origin = form_origin;
+      password_form->signon_realm = GetSignOnRealm(password_form->origin);
+      return true;
+    case PasswordContents::kNone:
+      return false;
+  }
 
   // Evaluate the context of the fields.
   if (base::FeatureList::IsEnabled(

components/autofill/content/renderer/password_form_conversion_utils_browsertest.cc

@@ -100,12 +100,12 @@ class PasswordFormBuilder {
 
   // Appends a disabled text-type field at the end of the form.
   void AddDisabledUsernameField() {
-    html_ += "<INPUT type=\"text\" disabled/>";
+    html_ += "<INPUT name=\"disabled field\" type=\"text\" disabled/>";
   }
 
   // Appends a disabled password-type field at the end of the form.
   void AddDisabledPasswordField() {
-    html_ += "<INPUT type=\"password\" disabled/>";
+    html_ += "<INPUT name=\"disabled field\" type=\"password\" disabled/>";
   }
 
   // Appends a hidden field at the end of the form.
@@ -346,6 +346,10 @@ TEST_F(MAYBE_PasswordFormConversionUtilsTest, DisabledFieldsAreIgnored) {
   EXPECT_EQ(base::UTF8ToUTF16("secret"), password_form->password_value);
 }
 
+// When not enough fields are enabled to parse the form, the result should still
+// be not null. It must contain only minimal information, so that it is not used
+// for fill on load, for example. It must contain the full FormData, so that the
+// new parser can be run as well.
 TEST_F(MAYBE_PasswordFormConversionUtilsTest, OnlyDisabledFields) {
   PasswordFormBuilder builder(kTestFormActionURL);
   builder.AddDisabledUsernameField();
@@ -355,7 +359,11 @@ TEST_F(MAYBE_PasswordFormConversionUtilsTest, OnlyDisabledFields) {
 
   std::unique_ptr<PasswordForm> password_form =
       LoadHTMLAndConvertForm(html, nullptr, false);
-  ASSERT_FALSE(password_form);
+  ASSERT_TRUE(password_form);
+  EXPECT_TRUE(password_form->username_element.empty());
+  EXPECT_TRUE(password_form->password_element.empty());
+  EXPECT_TRUE(password_form->new_password_element.empty());
+  EXPECT_EQ(2u, password_form->form_data.fields.size());
 }
 
 TEST_F(MAYBE_PasswordFormConversionUtilsTest,