[Passwords] Fixing password generation outside the form tag.

Forms that do not have a <form> tag are treated by Passwordmanager as
one synthetic form. This might negatively affect websites that allow
navigation via JS scripts, without committing the actual navigation.
Password fields might be added/changed, but the already existing
PasswordFormManager can store outdated server predictions and treat
the form as previously filled.

This CL makes sure that the valid server predictions are used when
the form changes.

Bug: 1140480
Change-Id: I68475dc0c55e8878781351f3f28b3f3f70429271
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2489990
Commit-Queue: Maria Kazinova <kazinova@google.com>
Reviewed-by: Jan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#820379}

components/password_manager/core/browser/form_parsing/form_parser.h

@@ -123,6 +123,8 @@ class FormDataParser {
     predictions_ = std::move(predictions);
   }
 
+  void reset_predictions() { predictions_.reset(); }
+
   const base::Optional<FormPredictions>& predictions() { return predictions_; }
 
   ReadonlyPasswordFields readonly_status() { return readonly_status_; }

components/password_manager/core/browser/password_form_manager.cc

@@ -766,15 +766,9 @@ void PasswordFormManager::ProcessServerPredictions(
     // predictions again.
     return;
   }
-  FormSignature observed_form_signature =
-      CalculateFormSignature(*observed_form());
-  auto it = predictions.find(observed_form_signature);
-  if (it == predictions.end())
-    return;
-
-  ReportTimeBetweenStoreAndServerUMA();
-  parser_.set_predictions(it->second);
-  Fill();
+  UpdatePredictionsForObservedForm(predictions);
+  if (parser_.predictions())
+    Fill();
 }
 
 void PasswordFormManager::Fill() {
@@ -830,15 +824,21 @@ void PasswordFormManager::Fill() {
       form_fetcher_->GetPreferredMatch(), metrics_recorder_.get());
 }
 
-void PasswordFormManager::FillForm(const FormData& observed_form_data) {
+void PasswordFormManager::FillForm(
+    const FormData& observed_form_data,
+    const std::map<FormSignature, FormPredictions>& predictions) {
   uint32_t differences_bitmask =
       FindFormsDifferences(*observed_form(), observed_form_data);
   metrics_recorder_->RecordFormChangeBitmask(differences_bitmask);
 
-  if (differences_bitmask)
-    *mutable_observed_form() = observed_form_data;
-
-  if (!waiting_for_server_predictions_)
+  bool new_predictions_available = false;
+  if (differences_bitmask) {
+    UpdateFormManagerWithFormChanges(observed_form_data, predictions);
+    new_predictions_available = parser_.predictions().has_value();
+  }
+  // Fill the form if relevant form predictions were found or if the
+  // manager is not waiting for new server predictions.
+  if (new_predictions_available || !waiting_for_server_predictions_)
     Fill();
 }
 
@@ -1063,4 +1063,26 @@ bool PasswordFormManager::UsePossibleUsername(
 #endif  // defined(OS_ANDROID)
 }
 
+void PasswordFormManager::UpdatePredictionsForObservedForm(
+    const std::map<FormSignature, FormPredictions>& predictions) {
+  FormSignature observed_form_signature =
+      CalculateFormSignature(*observed_form());
+  auto it = predictions.find(observed_form_signature);
+  if (it == predictions.end())
+    return;
+
+  ReportTimeBetweenStoreAndServerUMA();
+  parser_.set_predictions(it->second);
+}
+
+void PasswordFormManager::UpdateFormManagerWithFormChanges(
+    const FormData& observed_form_data,
+    const std::map<FormSignature, FormPredictions>& predictions) {
+  *mutable_observed_form() = observed_form_data;
+  // If the observed form has changed, it might be autofilled again.
+  autofills_left_ = kMaxTimesAutofill;
+  parser_.reset_predictions();
+  UpdatePredictionsForObservedForm(predictions);
+}
+
 }  // namespace password_manager

components/password_manager/core/browser/password_form_manager.h

@@ -126,8 +126,11 @@ class PasswordFormManager : public PasswordFormManagerForUI,
   // Sends fill data to the renderer.
   void Fill();
 
-  // Sends fill data to the renderer to fill |observed_form_data|.
-  void FillForm(const autofill::FormData& observed_form_data);
+  // Sends fill data to the renderer to fill |observed_form_data| using
+  // new relevant data from |predictions|.
+  void FillForm(
+      const autofill::FormData& observed_form_data,
+      const std::map<autofill::FormSignature, FormPredictions>& predictions);
 
   void UpdateSubmissionIndicatorEvent(
       autofill::mojom::SubmissionIndicatorEvent event);
@@ -317,6 +320,17 @@ class PasswordFormManager : public PasswordFormManagerForUI,
   // looks valid.
   bool UsePossibleUsername(const PossibleUsernameData* possible_username);
 
+  // Updates the predictions stored in |parser_| with predictions relevant for
+  // |observed_form_or_digest_|.
+  void UpdatePredictionsForObservedForm(
+      const std::map<autofill::FormSignature, FormPredictions>& predictions);
+
+  // Updates |observed_form_or_digest_| and form predictions stored in
+  // |parser_| and resets the amount of autofills left.
+  void UpdateFormManagerWithFormChanges(
+      const autofill::FormData& observed_form_data,
+      const std::map<autofill::FormSignature, FormPredictions>& predictions);
+
   // The client which implements embedder-specific PasswordManager operations.
   PasswordManagerClient* client_;
 

components/password_manager/core/browser/password_form_manager_unittest.cc

@@ -1712,7 +1712,7 @@ TEST_P(PasswordFormManagerTest, FillForm) {
 
     PasswordFormFillData fill_data;
     EXPECT_CALL(driver_, FillPasswordForm(_)).WillOnce(SaveArg<0>(&fill_data));
-    form_manager_->FillForm(form);
+    form_manager_->FillForm(form, {});
 
     EXPECT_EQ(form.fields[kUsernameFieldIndex].name,
               fill_data.username_field.name);
@@ -1746,8 +1746,7 @@ TEST_P(PasswordFormManagerTest, FillFormWaitForServerPredictions) {
   // Check that no filling until server predicions or filling timeout
   // expiration.
   EXPECT_CALL(driver_, FillPasswordForm(_)).Times(0);
-  form_manager_->FillForm(changed_form);
-  Mock::VerifyAndClearExpectations(&driver_);
+  form_manager_->FillForm(changed_form, {});
 
   // Check that the changed form is filled after the filling timeout expires.
 

components/password_manager/core/browser/password_manager.cc

@@ -575,7 +575,7 @@ void PasswordManager::CreateFormManagers(
       // filled values.
       // TODO(https://crbug.com/831123): Implement more robust filling and
       // remove the next line.
-      manager->FillForm(form_data);
+      manager->FillForm(form_data, predictions_);
     } else {
       new_forms_data.push_back(&form_data);
     }

components/password_manager/core/browser/password_manager_unittest.cc

@@ -3668,6 +3668,56 @@ TEST_P(PasswordManagerTest,
   }
 }
 
+TEST_P(PasswordManagerTest, GenerationOnChangedForm) {
+  EXPECT_CALL(client_, IsSavingAndFillingEnabled(_))
+      .WillRepeatedly(Return(true));
+  EXPECT_CALL(*store_, GetLogins(_, _))
+      .WillRepeatedly(WithArg<1>(InvokeEmptyConsumerWithForms(store_.get())));
+
+  // Create FormdData for a form with 1 password field and process it.
+  FormData form_data;
+  form_data.is_form_tag = false;
+  form_data.url = GURL("http://www.testwebsite.com");
+
+  FormFieldData old_password_field;
+  old_password_field.form_control_type = "password";
+  old_password_field.unique_renderer_id = FieldRendererId(0);
+  old_password_field.name = ASCIIToUTF16("oldpass");
+  form_data.fields.push_back(old_password_field);
+
+  manager()->OnPasswordFormsParsed(&driver_, {form_data});
+
+  // Form changes: new and confirmation password fields are added by the
+  // website's scripts.
+  FormFieldData new_password_field;
+  new_password_field.form_control_type = "password";
+  new_password_field.unique_renderer_id = FieldRendererId(1);
+  new_password_field.name = ASCIIToUTF16("newpass");
+  form_data.fields.push_back(new_password_field);
+
+  FormFieldData confirm_password_field;
+  confirm_password_field.form_control_type = "password";
+  confirm_password_field.unique_renderer_id = FieldRendererId(2);
+  confirm_password_field.name = ASCIIToUTF16("confpass");
+  form_data.fields.push_back(confirm_password_field);
+
+  // Server predictions may arrive before the form is parsed by PasswordManager.
+  FormStructure form_structure(form_data);
+  form_structure.field(1)->set_server_type(autofill::ACCOUNT_CREATION_PASSWORD);
+  form_structure.field(2)->set_server_type(autofill::CONFIRMATION_PASSWORD);
+  manager()->ProcessAutofillPredictions(&driver_, {&form_structure});
+
+  autofill::PasswordFormGenerationData form_generation_data;
+  EXPECT_CALL(driver_, FormEligibleForGenerationFound(_))
+      .WillOnce(SaveArg<0>(&form_generation_data));
+  // The change is discovered by PasswordManager.
+  manager()->OnPasswordFormsParsed(&driver_, {form_data});
+#if !defined(OS_IOS)
+  EXPECT_EQ(new_password_field.unique_renderer_id,
+            form_generation_data.new_password_renderer_id);
+#endif
+}
+
 INSTANTIATE_TEST_SUITE_P(, PasswordManagerTest, testing::Bool());
 
 }  // namespace password_manager