Remove Obsolete HTTP SiteStatistics From PasswordStore during Migration

components/password_manager/core/browser/http_password_store_migrator_unittest.cc

Index: components/password_manager/core/browser/http_password_store_migrator_unittest.cc
diff --git a/components/password_manager/core/browser/http_password_migrator_unittest.cc b/components/password_manager/core/browser/http_password_store_migrator_unittest.cc
similarity index 62%
rename from components/password_manager/core/browser/http_password_migrator_unittest.cc
rename to components/password_manager/core/browser/http_password_store_migrator_unittest.cc
index 084daa70a1a4547aeab0c9c5d95dc0adc00cfe88..6aa5d2b74fcc48439d53b9600d0845072de72a81 100644
--- a/components/password_manager/core/browser/http_password_migrator_unittest.cc
+++ b/components/password_manager/core/browser/http_password_store_migrator_unittest.cc
@@ -2,10 +2,11 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "components/password_manager/core/browser/http_password_migrator.h"
+#include "components/password_manager/core/browser/http_password_store_migrator.h"
 
 #include "base/memory/ptr_util.h"
 #include "base/message_loop/message_loop.h"
+#include "base/run_loop.h"
 #include "base/strings/utf_string_conversions.h"
 #include "components/password_manager/core/browser/mock_password_store.h"
 #include "components/password_manager/core/browser/stub_password_manager_client.h"
@@ -17,8 +18,10 @@ namespace {
 
 using autofill::PasswordForm;
 using testing::ElementsAre;
+using testing::Invoke;
 using testing::Pointee;
 using testing::SaveArg;
+using testing::Unused;
 using testing::_;
 
 constexpr char kTestHttpsURL[] = "https://example.org/";
@@ -60,7 +63,7 @@ PasswordForm CreateAndroidCredential() {
   return form;
 }
 
-class MockConsumer : public HttpPasswordMigrator::Consumer {
+class MockConsumer : public HttpPasswordStoreMigrator::Consumer {
  public:
   MOCK_METHOD1(ProcessForms,
                void(const std::vector<autofill::PasswordForm*>& forms));
@@ -92,13 +95,15 @@ class MockPasswordManagerClient : public StubPasswordManagerClient {
 
 }  // namespace
 
-class HttpPasswordMigratorTest : public testing::Test {
+class HttpPasswordStoreMigratorTest : public testing::Test {
  public:
-  HttpPasswordMigratorTest()
+  HttpPasswordStoreMigratorTest()
       : mock_store_(new testing::StrictMock<MockPasswordStore>),
         client_(mock_store_.get()) {}
 
-  ~HttpPasswordMigratorTest() override { mock_store_->ShutdownOnUIThread(); }
+  ~HttpPasswordStoreMigratorTest() override {
+    mock_store_->ShutdownOnUIThread();
+  }
 
   MockConsumer& consumer() { return consumer_; }
   MockPasswordStore& store() { return *mock_store_; }
@@ -107,6 +112,7 @@ class HttpPasswordMigratorTest : public testing::Test {
  protected:
   void TestEmptyStore(bool is_hsts);
   void TestFullStore(bool is_hsts);
+  void TestMigratorDeletionByConsumer(bool is_hsts);
 
  private:
   base::MessageLoop message_loop_;  // Used by mock_store_.
@@ -114,33 +120,45 @@ class HttpPasswordMigratorTest : public testing::Test {
   scoped_refptr<MockPasswordStore> mock_store_;
   MockPasswordManagerClient client_;
 
-  DISALLOW_COPY_AND_ASSIGN(HttpPasswordMigratorTest);
+  DISALLOW_COPY_AND_ASSIGN(HttpPasswordStoreMigratorTest);
 };
 
-void HttpPasswordMigratorTest::TestEmptyStore(bool is_hsts) {
+void HttpPasswordStoreMigratorTest::TestEmptyStore(bool is_hsts) {
   PasswordStore::FormDigest form(autofill::PasswordForm::SCHEME_HTML,
                                  kTestHttpURL, GURL(kTestHttpURL));
   EXPECT_CALL(store(), GetLogins(form, _));
   PasswordManagerClient::HSTSCallback callback;
   EXPECT_CALL(client(), PostHSTSQueryForHost(GURL(kTestHttpsURL), _))
       .WillOnce(SaveArg<1>(&callback));
-  HttpPasswordMigrator migrator(GURL(kTestHttpsURL), &client(), &consumer());
+  HttpPasswordStoreMigrator migrator(GURL(kTestHttpsURL), &client(),
+                                     &consumer());
   callback.Run(is_hsts);
+  // We expect a potential call to |RemoveSiteStatsImpl| which is a async task
+  // posted from |PasswordStore::RemoveSiteStats|. Hence the following lines are
+  // necessary to ensure |RemoveSiteStatsImpl| gets called when expected.
+  EXPECT_CALL(store(), RemoveSiteStatsImpl(GURL(kTestHttpURL))).Times(is_hsts);
+  base::RunLoop().RunUntilIdle();
 
   EXPECT_CALL(consumer(), ProcessForms(std::vector<autofill::PasswordForm*>()));
   migrator.OnGetPasswordStoreResults(
       std::vector<std::unique_ptr<autofill::PasswordForm>>());
 }
 
-void HttpPasswordMigratorTest::TestFullStore(bool is_hsts) {
+void HttpPasswordStoreMigratorTest::TestFullStore(bool is_hsts) {
   PasswordStore::FormDigest form_digest(autofill::PasswordForm::SCHEME_HTML,
                                         kTestHttpURL, GURL(kTestHttpURL));
   EXPECT_CALL(store(), GetLogins(form_digest, _));
   PasswordManagerClient::HSTSCallback callback;
   EXPECT_CALL(client(), PostHSTSQueryForHost(GURL(kTestHttpsURL), _))
       .WillOnce(SaveArg<1>(&callback));
-  HttpPasswordMigrator migrator(GURL(kTestHttpsURL), &client(), &consumer());
+  HttpPasswordStoreMigrator migrator(GURL(kTestHttpsURL), &client(),
+                                     &consumer());
   callback.Run(is_hsts);
+  // We expect a potential call to |RemoveSiteStatsImpl| which is a async task
+  // posted from |PasswordStore::RemoveSiteStats|. Hence the following lines are
+  // necessary to ensure |RemoveSiteStatsImpl| gets called when expected.
+  EXPECT_CALL(store(), RemoveSiteStatsImpl(GURL(kTestHttpURL))).Times(is_hsts);
+  base::RunLoop().RunUntilIdle();
 
   PasswordForm form = CreateTestForm();
   PasswordForm psl_form = CreateTestPSLForm();
@@ -159,20 +177,57 @@ void HttpPasswordMigratorTest::TestFullStore(bool is_hsts) {
   migrator.OnGetPasswordStoreResults(std::move(results));
 }
 
-TEST_F(HttpPasswordMigratorTest, EmptyStoreWithHSTS) {
+// This test checks whether the migration successfully completes even if the
+// migrator gets explicitly deleted by its consumer. This test will crash if
+// this is not the case.
+void HttpPasswordStoreMigratorTest::TestMigratorDeletionByConsumer(
+    bool is_hsts) {
+  // Setup expectations on store and client.
+  EXPECT_CALL(store(), GetLogins(_, _));
+  PasswordManagerClient::HSTSCallback callback;
+  EXPECT_CALL(client(), PostHSTSQueryForHost(GURL(kTestHttpsURL), _))
+      .WillOnce(SaveArg<1>(&callback));
+
+  // Construct the migrator, call |OnGetPasswordStoreResults| explicitly and
+  // manually delete it.
+  auto migrator = base::MakeUnique<HttpPasswordStoreMigrator>(
+      GURL(kTestHttpsURL), &client(), &consumer());
+  migrator->OnGetPasswordStoreResults(
+      std::vector<std::unique_ptr<autofill::PasswordForm>>());
+  EXPECT_CALL(consumer(), ProcessForms(_)).WillOnce(Invoke([&migrator](Unused) {
+    migrator.reset();
+  }));
+
+  callback.Run(is_hsts);
+  // We expect a potential call to |RemoveSiteStatsImpl| which is a async task
+  // posted from |PasswordStore::RemoveSiteStats|. Hence the following lines are
+  // necessary to ensure |RemoveSiteStatsImpl| gets called when expected.
+  EXPECT_CALL(store(), RemoveSiteStatsImpl(GURL(kTestHttpURL))).Times(is_hsts);
+  base::RunLoop().RunUntilIdle();
+}
+
+TEST_F(HttpPasswordStoreMigratorTest, EmptyStoreWithHSTS) {
   TestEmptyStore(true);
 }
 
-TEST_F(HttpPasswordMigratorTest, EmptyStoreWithoutHSTS) {
+TEST_F(HttpPasswordStoreMigratorTest, EmptyStoreWithoutHSTS) {
   TestEmptyStore(false);
 }
 
-TEST_F(HttpPasswordMigratorTest, FullStoreWithHSTS) {
+TEST_F(HttpPasswordStoreMigratorTest, FullStoreWithHSTS) {
   TestFullStore(true);
 }
 
-TEST_F(HttpPasswordMigratorTest, FullStoreWithoutHSTS) {
+TEST_F(HttpPasswordStoreMigratorTest, FullStoreWithoutHSTS) {
   TestFullStore(false);
 }
 
+TEST_F(HttpPasswordStoreMigratorTest, MigratorDeletionByConsumerWithHSTS) {
+  TestMigratorDeletionByConsumer(true);
+}
+
+TEST_F(HttpPasswordStoreMigratorTest, MigratorDeletionByConsumerWithoutHSTS) {
+  TestMigratorDeletionByConsumer(false);
+}
+
 }  // namespace password_manager