Blacklisted matches processing during password form filling.

This CL contains:
1.Filtering blacklisted matches in NewPasswordFormManager.
2.Extracting blacklisted matches to |blacklisted_matches_| variable in
NewPasswordFormManager.
3.Implementing functions GetBlacklistedMatches and IsBlacklisted in
NewPasswordFormManager.

Bug: 854197, 831123

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

components/password_manager/core/browser/new_password_form_manager.cc

@@ -18,6 +18,7 @@ using autofill::FormData;
 using autofill::FormFieldData;
 using autofill::FormSignature;
 using autofill::FormStructure;
+using autofill::PasswordForm;
 using base::TimeDelta;
 
 using Logger = autofill::SavePasswordProgressLogger;
@@ -31,11 +32,11 @@ constexpr TimeDelta kMaxFillingDelayForServerPerdictions =
 
 // Helper function for calling form parsing and logging results if logging is
 // active.
-std::unique_ptr<autofill::PasswordForm> ParseFormAndMakeLogging(
+std::unique_ptr<PasswordForm> ParseFormAndMakeLogging(
     PasswordManagerClient* client,
     const FormData& form,
     const base::Optional<FormPredictions>& predictions) {
-  std::unique_ptr<autofill::PasswordForm> password_form =
+  std::unique_ptr<PasswordForm> password_form =
       ParseFormData(form, predictions ? &predictions.value() : nullptr,
                     FormParsingMode::FILLING);
 
@@ -105,16 +106,15 @@ const GURL& NewPasswordFormManager::GetOrigin() const {
   return observed_form_.origin;
 }
 
-const std::map<base::string16, const autofill::PasswordForm*>&
+const std::map<base::string16, const PasswordForm*>&
 NewPasswordFormManager::GetBestMatches() const {
   return best_matches_;
 }
 
-const autofill::PasswordForm& NewPasswordFormManager::GetPendingCredentials()
-    const {
+const PasswordForm& NewPasswordFormManager::GetPendingCredentials() const {
   // TODO(https://crbug.com/831123): Implement.
   DCHECK(false);
-  static autofill::PasswordForm dummy_form;
+  static PasswordForm dummy_form;
   return dummy_form;
 }
 
@@ -128,17 +128,13 @@ PasswordFormMetricsRecorder* NewPasswordFormManager::GetMetricsRecorder() {
   return metrics_recorder_.get();
 }
 
-const std::vector<const autofill::PasswordForm*>&
+const std::vector<const PasswordForm*>&
 NewPasswordFormManager::GetBlacklistedMatches() const {
-  // TODO(https://crbug.com/831123): Implement.
-  DCHECK(false);
-  static std::vector<const autofill::PasswordForm*> dummy_vector;
-  return dummy_vector;
+  return blacklisted_matches_;
 }
 
 bool NewPasswordFormManager::IsBlacklisted() const {
-  // TODO(https://crbug.com/831123): Implement.
-  return false;
+  return !blacklisted_matches_.empty();
 }
 
 bool NewPasswordFormManager::IsPasswordOverridden() const {
@@ -146,16 +142,15 @@ bool NewPasswordFormManager::IsPasswordOverridden() const {
   return false;
 }
 
-const autofill::PasswordForm* NewPasswordFormManager::GetPreferredMatch()
-    const {
+const PasswordForm* NewPasswordFormManager::GetPreferredMatch() const {
   return preferred_match_;
 }
 
 // TODO(https://crbug.com/831123): Implement all methods from
 // PasswordFormManagerForUI.
 void NewPasswordFormManager::Save() {}
-void NewPasswordFormManager::Update(
-    const autofill::PasswordForm& credentials_to_update) {}
+void NewPasswordFormManager::Update(const PasswordForm& credentials_to_update) {
+}
 void NewPasswordFormManager::UpdateUsername(
     const base::string16& new_username) {}
 void NewPasswordFormManager::UpdatePasswordValue(
@@ -167,13 +162,27 @@ void NewPasswordFormManager::PermanentlyBlacklist() {}
 void NewPasswordFormManager::OnPasswordsRevealed() {}
 
 void NewPasswordFormManager::ProcessMatches(
-    const std::vector<const autofill::PasswordForm*>& non_federated,
+    const std::vector<const PasswordForm*>& non_federated,
     size_t filtered_count) {
-  // TODO(https://crbug.com/831123). Implement correct treating of blacklisted
-  // matches.
-  std::vector<const autofill::PasswordForm*> not_best_matches;
-  password_manager_util::FindBestMatches(non_federated, &best_matches_,
+  std::vector<const PasswordForm*> matches;
+  std::copy_if(non_federated.begin(), non_federated.end(),
+               std::back_inserter(matches), [](const PasswordForm* form) {
+                 return !form->blacklisted_by_user &&
+                        form->scheme == PasswordForm::SCHEME_HTML;
+               });
+
+  std::vector<const PasswordForm*> not_best_matches;
+  password_manager_util::FindBestMatches(matches, &best_matches_,
                                          &not_best_matches, &preferred_match_);
+
+  // Copy out blacklisted matches.
+  blacklisted_matches_.clear();
+  std::copy_if(
+      non_federated.begin(), non_federated.end(),
+      std::back_inserter(blacklisted_matches_), [](const PasswordForm* form) {
+        return form->blacklisted_by_user && !form->is_public_suffix_match;
+      });
+
   filled_ = false;
 
   if (predictions_)
@@ -214,7 +223,7 @@ void NewPasswordFormManager::Fill() {
   // 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 =
+  std::unique_ptr<PasswordForm> observed_password_form =
       ParseFormAndMakeLogging(client_, observed_form_, predictions_);
   if (!observed_password_form)
     return;
@@ -223,21 +232,21 @@ void NewPasswordFormManager::Fill() {
 
   // TODO(https://crbug.com/831123). Move this lines to the beginning of the
   // function when the old parsing is removed.
-  if (!driver_ || best_matches_.empty() || filled_)
+  if (!driver_ || filled_)
     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_blacklisted */,
-      *observed_password_form.get(), best_matches_, federated_matches,
-      preferred_match_, metrics_recorder_.get());
+  std::vector<const PasswordForm*> federated_matches;
+  SendFillInformationToRenderer(*client_, driver_.get(), IsBlacklisted(),
+                                *observed_password_form.get(), best_matches_,
+                                federated_matches, preferred_match_,
+                                metrics_recorder_.get());
   filled_ = true;
 }
 
 void NewPasswordFormManager::RecordMetricOnCompareParsingResult(
-    const autofill::PasswordForm& parsed_form) {
+    const PasswordForm& parsed_form) {
   bool same =
       parsed_form.username_element == old_parsing_result_.username_element &&
       parsed_form.password_element == old_parsing_result_.password_element &&

components/password_manager/core/browser/new_password_form_manager.h

@@ -123,6 +123,10 @@ class NewPasswordFormManager : public PasswordFormManagerForUI,
   // by |form_fetcher_|.
   std::map<base::string16, const autofill::PasswordForm*> best_matches_;
 
+  // Set of blacklisted forms from the PasswordStore that best match the current
+  // form. They are owned by |form_fetcher_|.
+  std::vector<const autofill::PasswordForm*> blacklisted_matches_;
+
   // Convenience pointer to entry in best_matches_ that is marked
   // as preferred. This is only allowed to be null if there are no best matches
   // at all, since there will always be one preferred login when there are

components/password_manager/core/browser/new_password_form_manager_unittest.cc

@@ -39,6 +39,7 @@ class MockPasswordManagerDriver : public StubPasswordManagerDriver {
 
   MOCK_METHOD1(FillPasswordForm, void(const PasswordFormFillData&));
   MOCK_METHOD1(AllowPasswordGenerationForForm, void(const PasswordForm&));
+  MOCK_METHOD0(MatchingBlacklistedFormFound, void());
 };
 
 }  // namespace
@@ -76,11 +77,17 @@ class NewPasswordFormManagerTest : public testing::Test {
     saved_match_.preferred = true;
     saved_match_.username_value = ASCIIToUTF16("test@gmail.com");
     saved_match_.password_value = ASCIIToUTF16("test1");
+    saved_match_.is_public_suffix_match = false;
+    saved_match_.scheme = PasswordForm::SCHEME_HTML;
+
+    blacklisted_match_ = saved_match_;
+    blacklisted_match_.blacklisted_by_user = true;
   }
 
  protected:
   FormData observed_form_;
   PasswordForm saved_match_;
+  PasswordForm blacklisted_match_;
   StubPasswordManagerClient client_;
   MockPasswordManagerDriver driver_;
   scoped_refptr<TestMockTimeTaskRunner> task_runner_;
@@ -122,6 +129,7 @@ TEST_F(NewPasswordFormManagerTest, Autofill) {
   FakeFormFetcher fetcher;
   fetcher.Fetch();
   EXPECT_CALL(driver_, AllowPasswordGenerationForForm(_));
+  EXPECT_CALL(driver_, MatchingBlacklistedFormFound()).Times(0);
   PasswordFormFillData fill_data;
   EXPECT_CALL(driver_, FillPasswordForm(_)).WillOnce(SaveArg<0>(&fill_data));
   NewPasswordFormManager form_manager(&client_, driver_.AsWeakPtr(),
@@ -163,6 +171,40 @@ TEST_F(NewPasswordFormManagerTest, AutofillSignUpForm) {
   EXPECT_EQ(saved_match_.password_value, fill_data.password_field.value);
 }
 
+TEST_F(NewPasswordFormManagerTest, AutofillWithBlacklistedMatch) {
+  TestMockTimeTaskRunner::ScopedContext scoped_context(task_runner_.get());
+  FakeFormFetcher fetcher;
+  fetcher.Fetch();
+  EXPECT_CALL(driver_, MatchingBlacklistedFormFound());
+  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_, &blacklisted_match_}, 0u);
+
+  task_runner_->FastForwardUntilNoTasksRemain();
+
+  EXPECT_EQ(observed_form_.origin, fill_data.origin);
+  EXPECT_EQ(saved_match_.username_value, fill_data.username_field.value);
+  EXPECT_EQ(saved_match_.password_value, fill_data.password_field.value);
+}
+
+TEST_F(NewPasswordFormManagerTest, SendIntoToRendererOnlyBlacklistedMatch) {
+  TestMockTimeTaskRunner::ScopedContext scoped_context(task_runner_.get());
+  FakeFormFetcher fetcher;
+  fetcher.Fetch();
+  EXPECT_CALL(driver_, MatchingBlacklistedFormFound());
+  PasswordFormFillData fill_data;
+  EXPECT_CALL(driver_, FillPasswordForm(_)).Times(0);
+
+  NewPasswordFormManager form_manager(&client_, driver_.AsWeakPtr(),
+                                      observed_form_, &fetcher);
+  fetcher.SetNonFederated({&blacklisted_match_}, 0u);
+
+  task_runner_->FastForwardUntilNoTasksRemain();
+}
+
 TEST_F(NewPasswordFormManagerTest, SetSubmitted) {
   FakeFormFetcher fetcher;
   fetcher.Fetch();