Add testing field trial to NewPasswordFormParsing finch config.

Along the way 2 tests are fixed.

Bug: 831123, 852742
Change-Id: If8466bf72cb81be1edbf5f0b95f7da5d309e6f54
Reviewed-on: https://chromium-review.googlesource.com/1169170
Commit-Queue: Vadym Doroshenko <dvadym@chromium.org>
Reviewed-by: Robert Kaplow (slow) <rkaplow@chromium.org>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583641}

chrome/browser/password_manager/password_manager_browsertest.cc

@@ -42,6 +42,7 @@
 #include "components/password_manager/content/browser/content_password_manager_driver.h"
 #include "components/password_manager/content/browser/content_password_manager_driver_factory.h"
 #include "components/password_manager/core/browser/login_model.h"
+#include "components/password_manager/core/browser/new_password_form_manager.h"
 #include "components/password_manager/core/browser/test_password_store.h"
 #include "components/password_manager/core/common/password_manager_features.h"
 #include "components/version_info/version_info.h"
@@ -78,7 +79,13 @@ class PasswordManagerBrowserTestWithViewsFeature
     : public PasswordManagerBrowserTestBase,
       public ::testing::WithParamInterface<bool> {
  public:
-  PasswordManagerBrowserTestWithViewsFeature() = default;
+  PasswordManagerBrowserTestWithViewsFeature() {
+    // Turn off waiting for server predictions before filing. It makes filling
+    // behaviour more deterministic. Filling with server predictions is tested
+    // in NewPasswordFormManager unit tests.
+    password_manager::NewPasswordFormManager::
+        set_wait_for_server_predictions_for_filling(false);
+  }
   ~PasswordManagerBrowserTestWithViewsFeature() override = default;
 
   void SetUpCommandLine(base::CommandLine* command_line) override {
@@ -1760,7 +1767,7 @@ IN_PROC_BROWSER_TEST_P(PasswordManagerBrowserTestWithViewsFeature,
 }
 
 // Test that if the same dynamic form is created multiple times then all of them
-// are autofilled and no unnecessary PasswordStore requests are fired.
+// are autofilled.
 IN_PROC_BROWSER_TEST_P(PasswordManagerBrowserTestWithViewsFeature,
                        DuplicateFormsGetFilled) {
   // At first let us save a credential to the password store.
@@ -1783,9 +1790,14 @@ IN_PROC_BROWSER_TEST_P(PasswordManagerBrowserTestWithViewsFeature,
   WaitForJsElementValue("document.body.children[0].children[0]", "temp");
   WaitForJsElementValue("document.body.children[0].children[1]", "random");
 
-  // It's a trick. There should be no second request to the password store since
-  // the existing PasswordFormManager will manage the new form.
-  password_store->Clear();
+  if (!base::FeatureList::IsEnabled(
+          password_manager::features::kNewPasswordFormParsing)) {
+    // It's a trick. There should be no second request to the password store
+    // since the existing PasswordFormManager will manage the new form. On other
+    // hand NewPasswordFormManager uses renderer ids for matching forms so a new
+    // NewPasswordFormManager is created for each DOM form object.
+    password_store->Clear();
+  }
   // Add one more form.
   ASSERT_TRUE(content::ExecuteScript(WebContents(), "addForm();"));
   // Wait until the username is filled, to make sure autofill kicked in.

chrome/browser/password_manager/password_manager_interactive_uitest.cc

@@ -13,6 +13,7 @@
 #include "chrome/browser/ui/browser_commands.h"
 #include "chrome/browser/ui/passwords/manage_passwords_ui_controller.h"
 #include "components/autofill/core/browser/autofill_experiments.h"
+#include "components/password_manager/core/browser/new_password_form_manager.h"
 #include "components/password_manager/core/browser/test_password_store.h"
 #include "content/public/test/browser_test_utils.h"
 
@@ -29,7 +30,13 @@ class PasswordManagerBrowserTestWithConditionalPopupViews
     : public PasswordManagerInteractiveTestBase,
       public ::testing::WithParamInterface<bool> {
  public:
-  PasswordManagerBrowserTestWithConditionalPopupViews() = default;
+  PasswordManagerBrowserTestWithConditionalPopupViews() {
+    // Turn off waiting for server predictions before filing. It makes filling
+    // behaviour more deterministic. Filling with server predictions is tested
+    // in NewPasswordFormManager unit tests.
+    password_manager::NewPasswordFormManager::
+        set_wait_for_server_predictions_for_filling(false);
+  }
   ~PasswordManagerBrowserTestWithConditionalPopupViews() override = default;
 
   void SetUp() override {

components/password_manager/core/browser/new_password_form_manager.cc

@@ -29,6 +29,8 @@ using Logger = autofill::SavePasswordProgressLogger;
 
 namespace password_manager {
 
+bool NewPasswordFormManager::wait_for_server_predictions_for_filling_ = true;
+
 namespace {
 
 constexpr TimeDelta kMaxFillingDelayForServerPerdictions =
@@ -209,7 +211,7 @@ void NewPasswordFormManager::ProcessMatches(
 
   autofills_left_ = kMaxTimesAutofill;
 
-  if (predictions_) {
+  if (predictions_ || !wait_for_server_predictions_for_filling_) {
     ReportTimeBetweenStoreAndServerUMA();
     Fill();
   } else {

components/password_manager/core/browser/new_password_form_manager.h

@@ -105,6 +105,12 @@ class NewPasswordFormManager : public PasswordFormManagerForUI,
   void PermanentlyBlacklist() override;
   void OnPasswordsRevealed() override;
 
+#if defined(UNIT_TEST)
+  static void set_wait_for_server_predictions_for_filling(bool value) {
+    wait_for_server_predictions_for_filling_ = value;
+  }
+#endif
+
  protected:
   // FormFetcher::Consumer:
   void ProcessMatches(
@@ -233,6 +239,9 @@ class NewPasswordFormManager : public PasswordFormManagerForUI,
   // loop.
   int autofills_left_ = kMaxTimesAutofill;
 
+  // Controls whether to wait or not server before filling. It is used in tests.
+  static bool wait_for_server_predictions_for_filling_;
+
   // Used for comparison metrics.
   // TODO(https://crbug.com/831123): Remove it when the old form parsing is
   // removed.

testing/variations/fieldtrial_testing_config.json

@@ -2596,6 +2596,26 @@
             ]
         }
     ],
+    "NewPasswordFormParsing": [
+        {
+            "platforms": [
+                "android",
+                "chromeos",
+                "linux",
+                "mac",
+                "windows",
+                "ios"
+            ],
+            "experiments": [
+                {
+                    "name": "Enabled",
+                    "enable_features": [
+                        "new-password-form-parsing"
+                    ]
+                }
+            ]
+        }
+    ],
     "NewPrintPreview": [
         {
             "platforms": [