Implement filling with NewPasswordFormManager.

This CL contains:
1.Making NewPasswordFormManager (NPFM) to be FormFetcher::Consumer
and creating FormFetcher in it.
2.Implementing processing of matches from FormFetcher and sending fill
message to the renderer.

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

components/password_manager/core/browser/new_password_form_manager.cc

@@ -5,8 +5,12 @@
 #include "components/password_manager/core/browser/new_password_form_manager.h"
 
 #include "components/password_manager/core/browser/browser_save_password_progress_logger.h"
+#include "components/password_manager/core/browser/form_fetcher_impl.h"
 #include "components/password_manager/core/browser/form_parsing/ios_form_parser.h"
+#include "components/password_manager/core/browser/password_form_filling.h"
+#include "components/password_manager/core/browser/password_form_metrics_recorder.h"
 #include "components/password_manager/core/browser/password_manager_client.h"
+#include "components/password_manager/core/browser/password_manager_driver.h"
 #include "components/password_manager/core/browser/password_manager_util.h"
 
 using autofill::FormData;
@@ -15,27 +19,72 @@ using Logger = autofill::SavePasswordProgressLogger;
 
 namespace password_manager {
 
-NewPasswordFormManager::NewPasswordFormManager(
+namespace {
+
+// On iOS, id are used for field identification, whereas on other platforms
+// name field is used. Set id equal to name.
+// TODO(https://crbug.com/831123): Remove this method when the browser side form
+// parsing is ready.
+FormData PreprocessFormData(const FormData& form) {
+  FormData result_form = form;
+  for (auto& field : result_form.fields)
+    field.id = field.name;
+  return result_form;
+}
+
+// Helper function for calling form parsing and logging results if logging is
+// active.
+std::unique_ptr<autofill::PasswordForm> ParseFormAndMakeLogging(
     PasswordManagerClient* client,
-    const autofill::FormData& observed_form)
-    : client_(client), observed_form_(observed_form) {
-  if (password_manager_util::IsLoggingActive(client_)) {
-    BrowserSavePasswordProgressLogger logger(client_->GetLogManager());
-    // On iOS, id are used for field identification, whereas on other platforms
-    // name. Set id equal to name.
-    auto mutable_observed_form = observed_form_;
-    for (auto& field : mutable_observed_form.fields)
-      field.id = field.name;
-    logger.LogFormData(Logger::STRING_FORM_PARSING_INPUT,
-                       mutable_observed_form);
-    // iOS form parsing is a prototype for parsing on all platforms. Call it for
-    // developing and experimentation purposes.
-    // TODO(https://crbug.com/831123): Call general form parsing instead of iOS
-    // one when it is ready.
-    std::unique_ptr<autofill::PasswordForm> password_form =
-        ParseFormData(mutable_observed_form, FormParsingMode::FILLING);
-    logger.LogPasswordForm(Logger::STRING_FORM_PARSING_OUTPUT, *password_form);
+    const FormData& form) {
+  // iOS form parsing is a prototype for parsing on all platforms. Call it for
+  // developing and experimentation purposes.
+  // TODO(https://crbug.com/831123): Call general form parsing instead of iOS
+  // one when it is ready.
+  std::unique_ptr<autofill::PasswordForm> password_form =
+      ParseFormData(PreprocessFormData(form), FormParsingMode::FILLING);
+
+  if (password_manager_util::IsLoggingActive(client)) {
+    BrowserSavePasswordProgressLogger logger(client->GetLogManager());
+    logger.LogFormData(Logger::STRING_FORM_PARSING_INPUT, form);
+    if (password_form)
+      logger.LogPasswordForm(Logger::STRING_FORM_PARSING_OUTPUT,
+                             *password_form);
   }
+  return password_form;
+}
+
+}  // namespace
+
+NewPasswordFormManager::NewPasswordFormManager(
+    PasswordManagerClient* client,
+    const base::WeakPtr<PasswordManagerDriver>& driver,
+    const FormData& observed_form,
+    FormFetcher* form_fetcher)
+    : client_(client),
+      driver_(driver),
+      observed_form_(observed_form),
+      owned_form_fetcher_(
+          form_fetcher ? nullptr
+                       : std::make_unique<FormFetcherImpl>(
+                             PasswordStore::FormDigest(observed_form),
+                             client_,
+                             true /* should_migrate_http_passwords */,
+                             true /* should_query_suppressed_https_forms */)),
+      form_fetcher_(form_fetcher ? form_fetcher : owned_form_fetcher_.get()) {
+  metrics_recorder_ = base::MakeRefCounted<PasswordFormMetricsRecorder>(
+      client_->IsMainFrameSecure(), client_->GetUkmSourceId());
+  metrics_recorder_->RecordFormSignature(CalculateFormSignature(observed_form));
+
+  if (owned_form_fetcher_)
+    owned_form_fetcher_->Fetch();
+  form_fetcher_->AddConsumer(this);
+
+  // The folloing code is for development and debugging purposes.
+  // TODO(https://crbug.com/831123): remove it when NewPasswordFormManager will
+  // be production ready.
+  if (password_manager_util::IsLoggingActive(client_))
+    ParseFormAndMakeLogging(client_, observed_form_);
 }
 NewPasswordFormManager::~NewPasswordFormManager() = default;
 
@@ -43,4 +92,37 @@ bool NewPasswordFormManager::DoesManage(const autofill::FormData& form) const {
   return observed_form_.SameFormAs(form);
 }
 
+void NewPasswordFormManager::ProcessMatches(
+    const std::vector<const autofill::PasswordForm*>& non_federated,
+    size_t filtered_count) {
+  if (!driver_)
+    return;
+
+  // There are additional signals (server-side data) and parse results in
+  // filling and saving mode might be different so it is better not to cache
+  // parse result, but to parse each time again.
+  std::unique_ptr<autofill::PasswordForm> observed_password_form =
+      ParseFormAndMakeLogging(client_, observed_form_);
+  if (!observed_password_form)
+    return;
+
+  // TODO(https://crbug.com/831123). Implement correct treating of blacklisted
+  // matches.
+  std::map<base::string16, const autofill::PasswordForm*> best_matches;
+  std::vector<const autofill::PasswordForm*> not_best_matches;
+  const autofill::PasswordForm* preferred_match = nullptr;
+  password_manager_util::FindBestMatches(non_federated, &best_matches,
+                                         &not_best_matches, &preferred_match);
+  if (best_matches.empty())
+    return;
+
+  // TODO(https://crbug.com/831123). Implement correct treating of federated
+  // matches.
+  std::vector<const autofill::PasswordForm*> federated_matches;
+  SendFillInformationToRenderer(
+      *client_, driver_.get(), false /* is_blaclisted */,
+      *observed_password_form.get(), best_matches, federated_matches,
+      preferred_match, metrics_recorder_.get());
+}
+
 }  // namespace password_manager

components/password_manager/core/browser/new_password_form_manager.h

@@ -6,33 +6,61 @@
 #define COMPONENTS_PASSWORD_MANAGER_CORE_BROWSER_NEW_PASSWORD_FORM_MANAGER_H_
 
 #include "base/macros.h"
+#include "base/memory/scoped_refptr.h"
+#include "base/memory/weak_ptr.h"
 #include "components/autofill/core/common/form_data.h"
+#include "components/password_manager/core/browser/form_fetcher.h"
 
 namespace password_manager {
 
+class PasswordFormMetricsRecorder;
 class PasswordManagerClient;
+class PasswordManagerDriver;
 
 // This class helps with filling the observed form and with saving/updating the
 // stored information about it. It is aimed to replace PasswordFormManager and
 // to be renamed in new Password Manager design. Details
 // go/new-cpm-design-refactoring.
-class NewPasswordFormManager {
+class NewPasswordFormManager : public FormFetcher::Consumer {
  public:
+  // TODO(crbug.com/621355): So far, |form_fetcher| can be null. In that case
+  // |this| creates an instance of it itself (meant for production code). Once
+  // the fetcher is shared between PasswordFormManager instances, it will be
+  // required that |form_fetcher| is not null.
   NewPasswordFormManager(PasswordManagerClient* client,
-                         const autofill::FormData& observed_form);
+                         const base::WeakPtr<PasswordManagerDriver>& driver,
+                         const autofill::FormData& observed_form,
+                         FormFetcher* form_fetcher);
 
-  ~NewPasswordFormManager();
+  ~NewPasswordFormManager() override;
 
   // Compares |observed_form_| with |form| and returns true if they are the
   // same.
   bool DoesManage(const autofill::FormData& form) const;
 
+ protected:
+  // FormFetcher::Consumer:
+  void ProcessMatches(
+      const std::vector<const autofill::PasswordForm*>& non_federated,
+      size_t filtered_count) override;
+
  private:
   // The client which implements embedder-specific PasswordManager operations.
   PasswordManagerClient* client_;
 
+  base::WeakPtr<PasswordManagerDriver> driver_;
+
   const autofill::FormData observed_form_;
 
+  // Takes care of recording metrics and events for |*this|.
+  scoped_refptr<PasswordFormMetricsRecorder> metrics_recorder_;
+
+  // When not null, then this is the object which |form_fetcher_| points to.
+  std::unique_ptr<FormFetcher> owned_form_fetcher_;
+
+  // FormFetcher instance which owns the login data from PasswordStore.
+  FormFetcher* form_fetcher_;
+
   DISALLOW_COPY_AND_ASSIGN(NewPasswordFormManager);
 };
 

components/password_manager/core/browser/new_password_form_manager_unittest.cc

@@ -6,49 +6,105 @@
 
 #include "base/strings/utf_string_conversions.h"
 #include "components/autofill/core/common/form_data.h"
+#include "components/autofill/core/common/password_form.h"
+#include "components/autofill/core/common/password_form_fill_data.h"
+#include "components/password_manager/core/browser/fake_form_fetcher.h"
 #include "components/password_manager/core/browser/stub_password_manager_client.h"
+#include "components/password_manager/core/browser/stub_password_manager_driver.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 using autofill::FormData;
 using autofill::FormFieldData;
+using autofill::PasswordForm;
+using autofill::PasswordFormFillData;
 using base::ASCIIToUTF16;
+using testing::_;
+using testing::SaveArg;
 
 namespace password_manager {
+
+namespace {
+class MockPasswordManagerDriver : public StubPasswordManagerDriver {
+ public:
+  MockPasswordManagerDriver() {}
+
+  ~MockPasswordManagerDriver() override {}
+
+  MOCK_METHOD1(FillPasswordForm, void(const PasswordFormFillData&));
+  MOCK_METHOD1(AllowPasswordGenerationForForm, void(const PasswordForm&));
+};
+
+}  // namespace
+
 class NewPasswordFormManagerTest : public testing::Test {
  public:
   NewPasswordFormManagerTest() {
-    form_.name = ASCIIToUTF16("sign-in");
+    GURL origin = GURL("http://accounts.google.com/a/ServiceLoginAuth");
+    GURL action = GURL("http://accounts.google.com/a/ServiceLogin");
+
+    observed_form_.origin = origin;
+    observed_form_.action = action;
+    observed_form_.name = ASCIIToUTF16("sign-in");
 
     FormFieldData field;
     field.name = ASCIIToUTF16("firstname");
     field.form_control_type = "text";
-    form_.fields.push_back(field);
+    observed_form_.fields.push_back(field);
 
     field.name = ASCIIToUTF16("username");
     field.form_control_type = "text";
-    form_.fields.push_back(field);
+    observed_form_.fields.push_back(field);
 
     field.name = ASCIIToUTF16("password");
     field.form_control_type = "password";
-    form_.fields.push_back(field);
+    observed_form_.fields.push_back(field);
+
+    saved_match_.origin = origin;
+    saved_match_.action = action;
+    saved_match_.preferred = true;
+    saved_match_.username_value = ASCIIToUTF16("test@gmail.com");
+    saved_match_.password_value = ASCIIToUTF16("test1");
   }
 
  protected:
-  FormData form_;
+  FormData observed_form_;
+  PasswordForm saved_match_;
   StubPasswordManagerClient client_;
+  MockPasswordManagerDriver driver_;
 };
 
 TEST_F(NewPasswordFormManagerTest, DoesManage) {
-  NewPasswordFormManager form_manager(&client_, form_);
-  EXPECT_TRUE(form_manager.DoesManage(form_));
-  FormData another_form = form_;
+  FakeFormFetcher fetcher;
+  fetcher.Fetch();
+  NewPasswordFormManager form_manager(&client_, driver_.AsWeakPtr(),
+                                      observed_form_, &fetcher);
+  EXPECT_TRUE(form_manager.DoesManage(observed_form_));
+  FormData another_form = observed_form_;
   another_form.name += ASCIIToUTF16("1");
   EXPECT_FALSE(form_manager.DoesManage(another_form));
 
-  another_form = form_;
+  another_form = observed_form_;
   another_form.fields[0].name += ASCIIToUTF16("1");
   EXPECT_FALSE(form_manager.DoesManage(another_form));
 }
 
+TEST_F(NewPasswordFormManagerTest, Autofill) {
+  FakeFormFetcher fetcher;
+  fetcher.Fetch();
+  EXPECT_CALL(driver_, AllowPasswordGenerationForForm(_));
+  PasswordFormFillData fill_data;
+  EXPECT_CALL(driver_, FillPasswordForm(_)).WillOnce(SaveArg<0>(&fill_data));
+  NewPasswordFormManager form_manager(&client_, driver_.AsWeakPtr(),
+                                      observed_form_, &fetcher);
+  fetcher.SetNonFederated({&saved_match_}, 0u);
+
+  EXPECT_EQ(observed_form_.origin, fill_data.origin);
+  EXPECT_FALSE(fill_data.wait_for_username);
+  EXPECT_EQ(observed_form_.fields[1].name, fill_data.username_field.name);
+  EXPECT_EQ(saved_match_.username_value, fill_data.username_field.value);
+  EXPECT_EQ(observed_form_.fields[2].name, fill_data.password_field.name);
+  EXPECT_EQ(saved_match_.password_value, fill_data.password_field.value);
+}
+
 }  // namespace  password_manager

components/password_manager/core/browser/password_form_manager.cc

@@ -627,6 +627,9 @@ void PasswordFormManager::ProcessFrame(
 
 void PasswordFormManager::ProcessFrameInternal(
     const base::WeakPtr<PasswordManagerDriver>& driver) {
+  if (base::FeatureList::IsEnabled(
+          password_manager::features::kNewPasswordFormParsing))
+    return;
   if (!driver)
     return;
   SendFillInformationToRenderer(*client_, driver.get(), IsBlacklisted(),

components/password_manager/core/browser/password_manager.cc

@@ -558,7 +558,7 @@ void PasswordManager::CreatePendingLoginManagers(
 
   if (base::FeatureList::IsEnabled(
           password_manager::features::kNewPasswordFormParsing)) {
-    CreateFormManagers(forms);
+    CreateFormManagers(driver, forms);
   }
 
   const PasswordForm::Scheme effective_form_scheme =
@@ -661,6 +661,7 @@ void PasswordManager::CreatePendingLoginManagers(
 }
 
 void PasswordManager::CreateFormManagers(
+    password_manager::PasswordManagerDriver* driver,
     const std::vector<autofill::PasswordForm>& forms) {
   // Find new forms.
   std::vector<const autofill::FormData*> new_forms;
@@ -676,8 +677,10 @@ void PasswordManager::CreateFormManagers(
 
   // Create form manager for new forms.
   for (const autofill::FormData* new_form : new_forms) {
-    form_managers_.push_back(
-        std::make_unique<NewPasswordFormManager>(client_, *new_form));
+    form_managers_.push_back(std::make_unique<NewPasswordFormManager>(
+        client_,
+        driver ? driver->AsWeakPtr() : base::WeakPtr<PasswordManagerDriver>(),
+        *new_form, nullptr));
   }
 }
 

components/password_manager/core/browser/password_manager.h

@@ -259,7 +259,8 @@ class PasswordManager : public LoginModel {
 
   // Checks for every form in |forms| whether |form_managers_| already contain a
   // manager for that form. If not, adds a manager for each such form.
-  void CreateFormManagers(const std::vector<autofill::PasswordForm>& forms);
+  void CreateFormManagers(password_manager::PasswordManagerDriver* driver,
+                          const std::vector<autofill::PasswordForm>& forms);
 
   // Returns the best match in |pending_login_managers_| for |form|. May return
   // nullptr if no match exists.

components/password_manager/core/browser/password_store.cc

@@ -17,6 +17,7 @@
 #include "base/task_scheduler/post_task.h"
 #include "base/threading/sequenced_task_runner_handle.h"
 #include "build/build_config.h"
+#include "components/autofill/core/common/form_data.h"
 #include "components/autofill/core/common/password_form.h"
 #include "components/password_manager/core/browser/android_affiliation/affiliated_match_helper.h"
 #include "components/password_manager/core/browser/password_manager_metrics_util.h"
@@ -97,6 +98,11 @@ PasswordStore::FormDigest::FormDigest(const PasswordForm& form)
       signon_realm(form.signon_realm),
       origin(form.origin) {}
 
+PasswordStore::FormDigest::FormDigest(const autofill::FormData& form)
+    : scheme(PasswordForm::SCHEME_HTML),
+      signon_realm(form.origin.GetOrigin().spec()),
+      origin(form.origin) {}
+
 PasswordStore::FormDigest::FormDigest(const FormDigest& other) = default;
 
 PasswordStore::FormDigest::FormDigest(FormDigest&& other) = default;

components/password_manager/core/browser/password_store.h

@@ -34,6 +34,7 @@
 class PrefService;
 
 namespace autofill {
+struct FormData;
 struct PasswordForm;
 }
 
@@ -82,6 +83,7 @@ class PasswordStore : protected PasswordStoreSync,
                const std::string& signon_realm,
                const GURL& origin);
     explicit FormDigest(const autofill::PasswordForm& form);
+    explicit FormDigest(const autofill::FormData& form);
     FormDigest(const FormDigest& other);
     FormDigest(FormDigest&& other);
     FormDigest& operator=(const FormDigest& other);