Teach NewPasswordFormManager gracefully process multiple submissions.

Password form submission detection is based on heuristics. So it's ok,
when submission is found multiple times for the same form. The function
SetSubmittedFormIfIsManaged is called when a submission is detected.
It might be that in call SetSubmittedFormIfIsManaged, the submitted form
is invalid (eg. JavaScript removed all fields), but a previous call
the submitted form was valid. Then NewPasswordFormManager should not
override valid submitted form. This CL implements this.

Bug: 905579, 831123
Change-Id: I5762ac98f951e941a462504aaf62ac9f60203b3e
Reviewed-on: https://chromium-review.googlesource.com/c/1337616Reviewed-by: Vaclav Brozek <vabr@chromium.org>
Commit-Queue: Vadym Doroshenko <dvadym@chromium.org>
Cr-Commit-Position: refs/heads/master@{#608348}

components/password_manager/core/browser/new_password_form_manager.cc

@@ -496,14 +496,19 @@ bool NewPasswordFormManager::SetSubmittedFormIfIsManaged(
     const PasswordManagerDriver* driver) {
   if (!DoesManage(submitted_form, driver))
     return false;
-  parsed_submitted_form_ =
+
+  std::unique_ptr<PasswordForm> parsed_submitted_form =
       ParseFormAndMakeLogging(submitted_form, FormDataParser::Mode::kSaving);
 
-  RecordMetricOnReadonly(parser_.readonly_status(), !!parsed_submitted_form_,
+  RecordMetricOnReadonly(parser_.readonly_status(), !!parsed_submitted_form,
                          FormDataParser::Mode::kSaving);
-  if (!parsed_submitted_form_)
-    return false;
 
+  // This function might be called multiple times. Consider as success if the
+  // submitted form was successfully parsed on a previous call.
+  if (!parsed_submitted_form)
+    return is_submitted_;
+
+  parsed_submitted_form_ = std::move(parsed_submitted_form);
   submitted_form_ = submitted_form;
   is_submitted_ = true;
 

components/password_manager/core/browser/new_password_form_manager_unittest.cc

@@ -416,6 +416,22 @@ TEST_F(NewPasswordFormManagerTest, SetSubmitted) {
   EXPECT_FALSE(form_manager_->is_submitted());
 }
 
+TEST_F(NewPasswordFormManagerTest, SetSubmittedMultipleTimes) {
+  EXPECT_TRUE(
+      form_manager_->SetSubmittedFormIfIsManaged(submitted_form_, &driver_));
+  EXPECT_TRUE(form_manager_->is_submitted());
+
+  // Make the submitted form to be invalid password form.
+  submitted_form_.fields.clear();
+
+  // Expect that |form_manager_| is still in submitted state because the first
+  // time the submited form was valid.
+  EXPECT_TRUE(
+      form_manager_->SetSubmittedFormIfIsManaged(submitted_form_, &driver_));
+  EXPECT_TRUE(form_manager_->is_submitted());
+  EXPECT_TRUE(form_manager_->GetSubmittedForm());
+}
+
 // Tests that when NewPasswordFormManager receives saved matches it waits for
 // server predictions and fills on receving them.
 TEST_F(NewPasswordFormManagerTest, ServerPredictionsWithinDelay) {