[Autofill] Clear strikes when autofill data wiped out

Clears all strikes in a profile's StrikeDatabase when Autofill
data is wiped out from Chrome Settings (chrome://settings ->
Clear Browsing Data -> Advanced -> Autofill Form Data).

Bug: 884817
Change-Id: I15cab8ab6812b1367d386c7bf321be24b1a0086c
Reviewed-on: https://chromium-review.googlesource.com/c/1277842
Commit-Queue: Anne Lim <annelim@google.com>
Reviewed-by: Martin Šrámek <msramek@chromium.org>
Reviewed-by: Jared Saul <jsaul@google.com>
Reviewed-by: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#600648}

chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.cc

@@ -19,6 +19,7 @@
 #include "build/build_config.h"
 #include "chrome/browser/android/feed/feed_host_service_factory.h"
 #include "chrome/browser/autofill/personal_data_manager_factory.h"
+#include "chrome/browser/autofill/strike_database_factory.h"
 #include "chrome/browser/bookmarks/bookmark_model_factory.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/browsing_data/browsing_data_helper.h"
@@ -62,6 +63,7 @@
 #include "chrome/common/pref_names.h"
 #include "chrome/common/url_constants.h"
 #include "components/autofill/core/browser/personal_data_manager.h"
+#include "components/autofill/core/browser/strike_database.h"
 #include "components/autofill/core/browser/webdata/autofill_webdata_service.h"
 #include "components/bookmarks/browser/bookmark_model.h"
 #include "components/browsing_data/core/features.h"
@@ -849,6 +851,16 @@ void ChromeBrowsingDataRemoverDelegate::RemoveEmbedderData(
           delete_end_);
       web_data_service->RemoveAutofillDataModifiedBetween(
           delete_begin_, delete_end_);
+
+      // Clear out the Autofill StrikeDatabase in its entirety.
+      // TODO(crbug.com/884817): Respect |delete_begin_| and |delete_end_| and
+      // only clear out entries whose last strikes were created in that
+      // timeframe.
+      autofill::StrikeDatabase* strike_database =
+          autofill::StrikeDatabaseFactory::GetForProfile(profile_);
+      if (strike_database)
+        strike_database->ClearAllStrikes(base::DoNothing());
+
       // Ask for a call back when the above calls are finished.
       web_data_service->GetDBTaskRunner()->PostTaskAndReply(
           FROM_HERE, base::DoNothing(), CreatePendingTaskCompletionClosure());

chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc

@@ -25,6 +25,7 @@
 #include "base/time/time.h"
 #include "build/build_config.h"
 #include "chrome/browser/autofill/personal_data_manager_factory.h"
+#include "chrome/browser/autofill/strike_database_factory.h"
 #include "chrome/browser/bookmarks/bookmark_model_factory.h"
 #include "chrome/browser/browsing_data/browsing_data_helper.h"
 #include "chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_factory.h"
@@ -55,6 +56,7 @@
 #include "components/autofill/core/browser/credit_card.h"
 #include "components/autofill/core/browser/personal_data_manager.h"
 #include "components/autofill/core/browser/personal_data_manager_observer.h"
+#include "components/autofill/core/browser/strike_database.h"
 #include "components/autofill/core/browser/test_autofill_clock.h"
 #include "components/autofill/core/common/autofill_constants.h"
 #include "components/bookmarks/browser/bookmark_model.h"
@@ -1041,6 +1043,33 @@ class MockReportingService : public net::ReportingService {
   DISALLOW_COPY_AND_ASSIGN(MockReportingService);
 };
 
+namespace autofill {
+// StrikeDatabaseTester is in the autofill namespace since StrikeDatabase
+// declares it as a friend in the autofill namespace.
+class StrikeDatabaseTester {
+ public:
+  explicit StrikeDatabaseTester(Profile* profile)
+      : strike_database_(
+            autofill::StrikeDatabaseFactory::GetForProfile(profile)) {}
+
+  bool IsEmpty() {
+    int num_keys;
+    base::RunLoop run_loop;
+    strike_database_->LoadKeys(base::BindLambdaForTesting(
+        [&](bool success, std::unique_ptr<std::vector<std::string>> keys) {
+          num_keys = keys.get()->size();
+          run_loop.Quit();
+        }));
+    run_loop.Run();
+    return (num_keys == 0);
+  }
+
+ private:
+  autofill::StrikeDatabase* strike_database_;
+};
+
+}  // namespace autofill
+
 class ClearReportingCacheTester {
  public:
   ClearReportingCacheTester(TestingProfile* profile, bool create_service)
@@ -1662,6 +1691,30 @@ TEST_F(ChromeBrowsingDataRemoverDelegateTest, AutofillRemovalEverything) {
   ASSERT_FALSE(tester.HasProfile());
 }
 
+TEST_F(ChromeBrowsingDataRemoverDelegateTest,
+       StrikeDatabaseEmptyOnAutofillRemoveEverything) {
+  GetProfile()->CreateWebDataService();
+  RemoveAutofillTester tester(GetProfile());
+
+  ASSERT_FALSE(tester.HasProfile());
+  tester.AddProfilesAndCards();
+  ASSERT_TRUE(tester.HasProfile());
+
+  autofill::StrikeDatabaseTester strike_database_tester(GetProfile());
+  BlockUntilBrowsingDataRemoved(
+      base::Time(), base::Time::Max(),
+      ChromeBrowsingDataRemoverDelegate::DATA_TYPE_FORM_DATA, false);
+
+  // StrikeDatabase should be empty when DATA_TYPE_FORM_DATA browsing data gets
+  // deleted.
+  ASSERT_TRUE(strike_database_tester.IsEmpty());
+  EXPECT_EQ(ChromeBrowsingDataRemoverDelegate::DATA_TYPE_FORM_DATA,
+            GetRemovalMask());
+  EXPECT_EQ(content::BrowsingDataRemover::ORIGIN_TYPE_UNPROTECTED_WEB,
+            GetOriginTypeMask());
+  ASSERT_FALSE(tester.HasProfile());
+}
+
 // Verify that clearing autofill form data works.
 TEST_F(ChromeBrowsingDataRemoverDelegateTest,
        AutofillOriginsRemovedWithHistory) {

components/autofill/core/browser/strike_database.cc

@@ -50,6 +50,16 @@ void StrikeDatabase::AddStrike(const std::string key,
                                          std::move(outer_callback), key));
 }
 
+void StrikeDatabase::ClearAllStrikes(
+    const ClearStrikesCallback& outer_callback) {
+  // For deleting all, filter method always returns true.
+  db_->UpdateEntriesWithRemoveFilter(
+      std::make_unique<StrikeDataProto::KeyEntryVector>(),
+      base::BindRepeating([](const std::string& key) { return true; }),
+      base::BindRepeating(&StrikeDatabase::OnClearAllStrikes,
+                          base::Unretained(this), outer_callback));
+}
+
 void StrikeDatabase::ClearAllStrikesForKey(
     const std::string& key,
     const ClearStrikesCallback& outer_callback) {
@@ -134,11 +144,20 @@ void StrikeDatabase::OnAddStrikeComplete(StrikesCallback callback,
   }
 }
 
+void StrikeDatabase::OnClearAllStrikes(ClearStrikesCallback callback,
+                                       bool success) {
+  callback.Run(success);
+}
+
 void StrikeDatabase::OnClearAllStrikesForKey(ClearStrikesCallback callback,
                                              bool success) {
   callback.Run(success);
 }
 
+void StrikeDatabase::LoadKeys(const LoadKeysCallback& callback) {
+  db_->LoadKeys(callback);
+}
+
 std::string StrikeDatabase::CreateKey(const std::string& type_prefix,
                                       const std::string& identifier_suffix) {
   return type_prefix + kKeyDeliminator + identifier_suffix;

components/autofill/core/browser/strike_database.h

@@ -7,6 +7,7 @@
 
 #include <memory>
 #include <string>
+#include <vector>
 
 #include "base/callback_forward.h"
 #include "base/memory/weak_ptr.h"
@@ -43,6 +44,10 @@ class StrikeDatabase : public KeyedService {
 
   using SetValueCallback = base::RepeatingCallback<void(bool success)>;
 
+  using LoadKeysCallback =
+      base::RepeatingCallback<void(bool success,
+                                   std::unique_ptr<std::vector<std::string>>)>;
+
   using StrikeDataProto = leveldb_proto::ProtoDatabase<StrikeData>;
 
   explicit StrikeDatabase(const base::FilePath& database_dir);
@@ -61,6 +66,10 @@ class StrikeDatabase : public KeyedService {
   virtual void AddStrike(const std::string key,
                          const StrikesCallback& outer_callback);
 
+  // Removes all database entries, which implicitly resets all strike counts to
+  // 0.
+  virtual void ClearAllStrikes(const ClearStrikesCallback& outer_callback);
+
   // Removes database entry for |key|, which implicitly sets strike count to 0.
   virtual void ClearAllStrikesForKey(
       const std::string& key,
@@ -78,7 +87,10 @@ class StrikeDatabase : public KeyedService {
 
  private:
   FRIEND_TEST_ALL_PREFIXES(StrikeDatabaseTest, GetPrefixFromKey);
+  FRIEND_TEST_ALL_PREFIXES(ChromeBrowsingDataRemoverDelegateTest,
+                           StrikeDatabaseEmptyOnAutofillRemoveEverything);
   friend class StrikeDatabaseTest;
+  friend class StrikeDatabaseTester;
 
   void OnDatabaseInit(bool success);
 
@@ -109,9 +121,14 @@ class StrikeDatabase : public KeyedService {
                            std::string key,
                            bool success);
 
+  void OnClearAllStrikes(ClearStrikesCallback outer_callback, bool success);
+
   void OnClearAllStrikesForKey(ClearStrikesCallback outer_callback,
                                bool success);
 
+  // Exposed for testing purposes.
+  void LoadKeys(const LoadKeysCallback& callback);
+
   // Concatenates type prefix and identifier suffix to create a key.
   std::string CreateKey(const std::string& type_prefix,
                         const std::string& identifier_suffix);

components/autofill/core/browser/strike_database_unittest.cc

@@ -110,6 +110,19 @@ class StrikeDatabaseTest : public ::testing::Test {
     run_loop.Run();
   }
 
+  void OnClearAllStrikes(base::RepeatingClosure run_loop_closure,
+                         bool success) {
+    run_loop_closure.Run();
+  }
+
+  void ClearAllStrikes() {
+    base::RunLoop run_loop;
+    strike_database_.ClearAllStrikes(
+        base::BindRepeating(&StrikeDatabaseTest::OnClearAllStrikesForKey,
+                            base::Unretained(this), run_loop.QuitClosure()));
+    run_loop.Run();
+  }
+
  protected:
   base::HistogramTester* GetHistogramTester() { return &histogram_tester_; }
   base::test::ScopedTaskEnvironment scoped_task_environment_;
@@ -204,6 +217,27 @@ TEST_F(StrikeDatabaseTest, ClearStrikesForMultipleNonZeroStrikesEntriesTest) {
   EXPECT_EQ(5, strikes);
 }
 
+TEST_F(StrikeDatabaseTest, ClearAllStrikesTest) {
+  // Set up database with 3 pre-existing strikes at |key1|, and 5 pre-existing
+  // strikes at |key2|.
+  const std::string key1 = "12345";
+  const std::string key2 = "13579";
+  std::vector<std::pair<std::string, StrikeData>> entries;
+  StrikeData data1;
+  data1.set_num_strikes(3);
+  entries.push_back(std::make_pair(key1, data1));
+  StrikeData data2;
+  data2.set_num_strikes(5);
+  entries.push_back(std::make_pair(key2, data2));
+  AddEntries(entries);
+
+  EXPECT_EQ(3, GetStrikes(key1));
+  EXPECT_EQ(5, GetStrikes(key2));
+  ClearAllStrikes();
+  EXPECT_EQ(0, GetStrikes(key1));
+  EXPECT_EQ(0, GetStrikes(key2));
+}
+
 TEST_F(StrikeDatabaseTest, GetKeyForCreditCardSave) {
   const std::string last_four = "1234";
   EXPECT_EQ("creditCardSave__1234",

components/autofill/core/browser/test_strike_database.cc

@@ -31,6 +31,12 @@ void TestStrikeDatabase::AddStrike(const std::string key,
   outer_callback.Run(strike_data.num_strikes());
 }
 
+void TestStrikeDatabase::ClearAllStrikes(
+    const ClearStrikesCallback& outer_callback) {
+  db_.clear();
+  outer_callback.Run(/*success=*/true);
+}
+
 void TestStrikeDatabase::ClearAllStrikesForKey(
     const std::string& key,
     const ClearStrikesCallback& outer_callback) {

components/autofill/core/browser/test_strike_database.h

@@ -26,6 +26,7 @@ class TestStrikeDatabase : public StrikeDatabase {
                   const StrikesCallback& outer_callback) override;
   void AddStrike(const std::string key,
                  const StrikesCallback& outer_callback) override;
+  void ClearAllStrikes(const ClearStrikesCallback& outer_callback) override;
   void ClearAllStrikesForKey(
       const std::string& key,
       const ClearStrikesCallback& outer_callback) override;