Revert "Fixing race condition in webState:didSubmitDocumentWithFormNamed..."

This reverts commit dbdd4ac1.

Reason for revert: This CL causes the Save/Update Password Infobar to stop presenting itself.

Original change's description:
> Fixing race condition in webState:didSubmitDocumentWithFormNamed...
> 
> The FormData extraction code was racing against new page load even
> though FormData was already passed into a function in string format.
> Now the FormData is extracted from a passed in string.
> 
> Bug: 418827
> Change-Id: I845b539974b57d1f430016f4500af5a4da5fcbf2
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2022771
> Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
> Commit-Queue: Maria Kazinova <kazinova@google.com>
> Cr-Commit-Position: refs/heads/master@{#735936}

TBR=dvadym@chromium.org,kazinova@google.com

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 418827
Change-Id: Id35a4cd9b09cd370281f705a903bcbb887485131
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2033580Reviewed-by: Sergio Collazos <sczs@chromium.org>
Commit-Queue: Sergio Collazos <sczs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#737497}

components/password_manager/ios/password_form_helper.mm

@@ -56,6 +56,14 @@ constexpr char kCommandPrefix[] = "passwordForm";
 // Handler for injected JavaScript callbacks.
 - (BOOL)handleScriptCommand:(const base::DictionaryValue&)JSONCommand;
 
+// Finds the currently submitted password form named |formName| and calls
+// |completionHandler| with the populated data structure. |found| is YES if the
+// current form was found successfully, NO otherwise.
+- (void)extractSubmittedPasswordForm:(const std::string&)formName
+                   completionHandler:
+                       (void (^)(BOOL found,
+                                 const FormData& form))completionHandler;
+
 // Parses the |jsonString| which contatins the password forms found on a web
 // page to populate the |forms| vector.
 - (void)getPasswordFormsFromJSON:(NSString*)jsonString
@@ -173,16 +181,24 @@ constexpr char kCommandPrefix[] = "passwordForm";
     // origin.
     return;
   }
-  if (!self.delegate || formData.empty())
-    return;
-  FormData form;
-  NSString* nsFormData = [NSString stringWithUTF8String:formData.c_str()];
-  std::unique_ptr<base::Value> formValue = autofill::ParseJson(nsFormData);
-  autofill::ExtractFormData(*formValue, false, base::string16(), pageURL,
-                            pageURL.GetOrigin(), &form);
-  [self.delegate formHelper:self
-              didSubmitForm:form
-                inMainFrame:formInMainFrame];
+  __weak PasswordFormHelper* weakSelf = self;
+  // This code is racing against the new page loading and will not get the
+  // password form data if the page has changed. In most cases this code wins
+  // the race.
+  // TODO(crbug.com/418827): Fix this by passing in more data from the JS side.
+  id completionHandler = ^(BOOL found, const FormData& form) {
+    PasswordFormHelper* strongSelf = weakSelf;
+    id<PasswordFormHelperDelegate> strongDelegate = strongSelf.delegate;
+    if (!strongSelf || !strongSelf->_webState || !strongDelegate) {
+      return;
+    }
+    [strongDelegate formHelper:strongSelf
+                 didSubmitForm:form
+                   inMainFrame:formInMainFrame];
+  };
+  // TODO(crbug.com/418827): Use |formData| instead of extracting form again.
+  [self extractSubmittedPasswordForm:formName
+                   completionHandler:completionHandler];
 }
 
 #pragma mark - Private methods
@@ -216,6 +232,43 @@ constexpr char kCommandPrefix[] = "passwordForm";
   return NO;
 }
 
+- (void)extractSubmittedPasswordForm:(const std::string&)formName
+                   completionHandler:
+                       (void (^)(BOOL found,
+                                 const FormData& form))completionHandler {
+  DCHECK(completionHandler);
+
+  if (!_webState) {
+    return;
+  }
+
+  GURL pageURL;
+  if (!GetPageURLAndCheckTrustLevel(_webState, &pageURL)) {
+    completionHandler(NO, FormData());
+    return;
+  }
+
+  id extractSubmittedFormCompletionHandler = ^(NSString* jsonString) {
+    std::unique_ptr<base::Value> formValue = autofill::ParseJson(jsonString);
+    if (!formValue) {
+      completionHandler(NO, FormData());
+      return;
+    }
+
+    FormData form;
+    if (!autofill::ExtractFormData(*formValue, false, base::string16(), pageURL,
+                                   pageURL.GetOrigin(), &form)) {
+      completionHandler(NO, FormData());
+      return;
+    }
+
+    completionHandler(YES, form);
+  };
+
+  [self.jsPasswordManager extractForm:base::SysUTF8ToNSString(formName)
+                    completionHandler:extractSubmittedFormCompletionHandler];
+}
+
 - (void)getPasswordFormsFromJSON:(NSString*)jsonString
                          pageURL:(const GURL&)pageURL
                            forms:(std::vector<FormData>*)forms {

components/password_manager/ios/password_form_helper_unittest.mm

@@ -42,6 +42,12 @@ using test_helpers::SetFillData;
 
 @interface PasswordFormHelper (Testing)
 
+// Provides access to the method below for testing with mocks.
+- (void)extractSubmittedPasswordForm:(const std::string&)formName
+                   completionHandler:
+                       (void (^)(BOOL found,
+                                 const PasswordForm& form))completionHandler;
+
 // Provides access to replace |jsPasswordManager| with Mock one for test.
 - (void)setJsPasswordManager:(JsPasswordManager*)jsPasswordManager;
 
@@ -157,6 +163,97 @@ class PasswordFormHelperTest : public web::WebTestWithWebState {
   DISALLOW_COPY_AND_ASSIGN(PasswordFormHelperTest);
 };
 
+struct GetSubmittedPasswordFormTestData {
+  // HTML String of the form.
+  NSString* html_string;
+  // Javascript to submit the form.
+  NSString* java_script;
+  // 0 based index of the form on the page to submit.
+  const int index_of_the_form_to_submit;
+  // Expected number of fields in found form.
+  const size_t expected_number_of_fields;
+  // Expected form name.
+  const char* expected_form_name;
+};
+
+// Check that HTML forms are captured and converted correctly into
+// PasswordForms on submission.
+TEST_F(PasswordFormHelperTest, GetSubmittedPasswordForm) {
+  // clang-format off
+  const GetSubmittedPasswordFormTestData test_data[] = {
+    // Two forms with no explicit names.
+    {
+      @"<form action='javascript:;'>"
+      "<input type='text' name='user1' value='user1'>"
+      "<input type='password' name='pass1' value='pw1'>"
+      "</form>"
+      "<form action='javascript:;'>"
+      "<input type='text' name='user2' value='user2'>"
+      "<input type='password' name='pass2' value='pw2'>"
+      "<input type='submit' id='s2'>"
+      "</form>",
+      @"document.getElementById('s2').click()",
+      1, 2, "gChrome~form~1"
+    },
+    // Two forms with explicit names.
+    {
+      @"<form name='test2a' action='javascript:;'>"
+      "<input type='text' name='user1' value='user1'>"
+      "<input type='password' name='pass1' value='pw1'>"
+      "<input type='submit' id='s1'>"
+      "</form>"
+      "<form name='test2b' action='javascript:;' value='user2'>"
+      "<input type='text' name='user2'>"
+      "<input type='password' name='pass2' value='pw2'>"
+      "</form>",
+      @"document.getElementById('s1').click()",
+      0, 2, "test2a"
+    },
+    // Not-password form.
+    {
+      @"<form action='javascript:;' name='form1'>"
+      "<input type='text' name='user1' value='user1'>"
+      "<input type='text' name='not_pass1' value='text1'>"
+      "<input type='submit' id='s1'>"
+      "</form>",
+      @"document.getElementById('s1').click()",
+      0, 2, "form1"
+    },
+    // Form with quotes in the form and field names.
+    {
+      @"<form name=\"foo'\" action='javascript:;'>"
+      "<input type='text' name=\"user1'\" value='user1'>"
+      "<input type='password' id='s1' name=\"pass1'\" value='pw2'>"
+      "</form>",
+      @"document.getElementById('s1').click()",
+      0, 2, "foo'"
+    },
+  };
+  // clang-format on
+
+  for (const GetSubmittedPasswordFormTestData& data : test_data) {
+    SCOPED_TRACE(testing::Message() << "for html_string=" << data.html_string
+                                    << " and java_script=" << data.java_script
+                                    << " and index_of_the_form_to_submit="
+                                    << data.index_of_the_form_to_submit);
+    LoadHtml(data.html_string);
+    ExecuteJavaScript(data.java_script);
+    __block BOOL block_was_called = NO;
+    id completion_handler = ^(BOOL found, const FormData& form) {
+      block_was_called = YES;
+      EXPECT_EQ(data.expected_number_of_fields, form.fields.size());
+      EXPECT_EQ(data.expected_form_name, base::UTF16ToUTF8(form.name));
+    };
+    [helper_
+        extractSubmittedPasswordForm:GetFormId(data.index_of_the_form_to_submit)
+                   completionHandler:completion_handler];
+    EXPECT_TRUE(
+        WaitUntilConditionOrTimeout(kWaitForJSCompletionTimeout, ^bool() {
+          return block_was_called;
+        }));
+  }
+}
+
 struct FindPasswordFormTestData {
   // HTML String of the form.
   NSString* html_string;

ios/chrome/browser/passwords/password_controller_unittest.mm

@@ -174,6 +174,11 @@ ACTION(InvokeEmptyConsumerWithForms) {
 // Provides access to JavaScript Manager for testing with mocks.
 @property(readonly) JsPasswordManager* jsPasswordManager;
 
+- (void)extractSubmittedPasswordForm:(const std::string&)formName
+                   completionHandler:
+                       (void (^)(BOOL found,
+                                 const PasswordForm& form))completionHandler;
+
 - (void)findPasswordFormsWithCompletionHandler:
     (void (^)(const std::vector<PasswordForm>&))completionHandler;
 
@@ -387,6 +392,99 @@ TEST_F(PasswordControllerTest, FLAKY_FindPasswordFormsInView) {
   }
 }
 
+struct GetSubmittedPasswordFormTestData {
+  // HTML String of the form.
+  NSString* html_string;
+  // Javascript to submit the form.
+  NSString* java_script;
+  // 0 based index of the form on the page to submit.
+  const int index_of_the_form_to_submit;
+  // Expected number of fields in found form.
+  const size_t expected_number_of_fields;
+  // Expected form name.
+  const char* expected_form_name;
+};
+
+// TODO(crbug.com/403705) This test is flaky.
+// Check that HTML forms are captured and converted correctly into
+// PasswordForms on submission.
+TEST_F(PasswordControllerTest, FLAKY_GetSubmittedPasswordForm) {
+  // clang-format off
+  GetSubmittedPasswordFormTestData test_data[] = {
+    // Two forms with no explicit names.
+    {
+      @"<form action='javascript:;'>"
+          "<input type='text' name='user1' value='user1'>"
+          "<input type='password' name='pass1' value='pw1'>"
+          "</form>"
+          "<form action='javascript:;'>"
+          "<input type='text' name='user2' value='user2'>"
+          "<input type='password' name='pass2' value='pw2'>"
+          "<input type='submit' id='s2'>"
+          "</form>",
+      @"document.getElementById('s2').click()",
+      1, 2, "gChrome~form~1"
+    },
+    // Two forms with explicit names.
+    {
+      @"<form name='test2a' action='javascript:;'>"
+          "<input type='text' name='user1' value='user1'>"
+          "<input type='password' name='pass1' value='pw1'>"
+          "<input type='submit' id='s1'>"
+          "</form>"
+          "<form name='test2b' action='javascript:;' value='user2'>"
+          "<input type='text' name='user2'>"
+          "<input type='password' name='pass2' value='pw2'>"
+          "</form>",
+      @"document.getElementById('s1').click()",
+      0, 2, "test2a"
+    },
+    // No password forms.
+    {
+      @"<form action='javascript:;'>"
+          "<input type='text' name='user1' value='user1'>"
+          "<input type='text' name='pass1' value='text1'>"
+          "<input type='submit' id='s1'>"
+          "</form>",
+      @"document.getElementById('s1').click()",
+      0, 2, "gChrome~form~0"
+    },
+    // Form with quotes in the form and field names.
+    {
+      @"<form name=\"foo'\" action='javascript:;'>"
+          "<input type='text' name=\"user1'\" value='user1'>"
+          "<input type='password' id='s1' name=\"pass1'\" value='pw2'>"
+          "</form>",
+      @"document.getElementById('s1').click()",
+      0, 2, "foo'"
+    },
+  };
+  // clang-format on
+
+  for (const GetSubmittedPasswordFormTestData& data : test_data) {
+    SCOPED_TRACE(testing::Message() << "for html_string="
+                                    << base::SysNSStringToUTF8(data.html_string)
+                                    << " and java_script=" << data.java_script
+                                    << " and index_of_the_form_to_submit="
+                                    << data.index_of_the_form_to_submit);
+    LoadHtml(data.html_string);
+    ExecuteJavaScript(data.java_script);
+    __block BOOL block_was_called = NO;
+    id completion_handler = ^(BOOL found, const FormData& form) {
+      block_was_called = YES;
+      EXPECT_EQ(data.expected_number_of_fields, form.fields.size());
+      EXPECT_EQ(data.expected_form_name, base::UTF16ToUTF8(form.name));
+    };
+    [passwordController_.formHelper
+        extractSubmittedPasswordForm:FormName(data.index_of_the_form_to_submit)
+                   completionHandler:completion_handler];
+    EXPECT_TRUE(
+        WaitUntilConditionOrTimeout(kWaitForJSCompletionTimeout, ^bool() {
+          return block_was_called;
+        }));
+  }
+}
+
 // Test HTML page.  It contains several password forms.  Tests autofill
 // them and verify that the right ones are autofilled.
 static NSString* kHtmlWithMultiplePasswordForms =