Remove DelayCleanObsoleteHttpDataForPasswordStoreAndPrefs.

The code is executed once per profile. As it was introduced one year ago, it's reasonable to assume that most users don't need it anymore.
It's a manual revert of https://codereview.chromium.org/2714543006/

Bug: 687968
Change-Id: Ic9ef109c27ccccadc8b3fd75d156dec818cfc83c
Reviewed-on: https://chromium-review.googlesource.com/955568
Commit-Queue: Vasilii Sukhanov <vasilii@chromium.org>
Reviewed-by: Jan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542084}

chrome/browser/password_manager/password_store_factory.cc

@@ -9,7 +9,6 @@
 
 #include "base/command_line.h"
 #include "base/environment.h"
-#include "base/memory/ref_counted.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/rand_util.h"
 #include "build/build_config.h"
@@ -24,7 +23,6 @@
 #include "components/browser_sync/profile_sync_service.h"
 #include "components/keyed_service/content/browser_context_dependency_manager.h"
 #include "components/os_crypt/os_crypt_switches.h"
-#include "components/password_manager/core/browser/http_data_cleaner.h"
 #include "components/password_manager/core/browser/login_database.h"
 #include "components/password_manager/core/browser/password_manager_util.h"
 #include "components/password_manager/core/browser/password_reuse_defines.h"
@@ -261,9 +259,6 @@ PasswordStoreFactory::BuildServiceInstanceFor(
     return nullptr;
   }
 
-  password_manager::DelayCleanObsoleteHttpDataForPasswordStoreAndPrefs(
-      ps.get(), profile->GetPrefs(),
-      base::WrapRefCounted(profile->GetRequestContext()));
   // TODO(https://crbug.com/817754): remove the code once majority of the users
   // executed it.
   password_manager_util::CleanUserDataInBlacklistedCredentials(

components/password_manager/core/browser/BUILD.gn

@@ -206,8 +206,6 @@ static_library("browser") {
     sources += [
       "hsts_query.cc",
       "hsts_query.h",
-      "http_data_cleaner.cc",
-      "http_data_cleaner.h",
     ]
   }
 
@@ -375,10 +373,7 @@ source_set("unit_tests") {
   if (is_ios) {
     sources += [ "login_database_ios_unittest.cc" ]
   } else {
-    sources += [
-      "hsts_query_unittest.cc",
-      "http_data_cleaner_unittest.cc",
-    ]
+    sources += [ "hsts_query_unittest.cc" ]
   }
   if (password_reuse_detection_support) {
     sources += [

components/password_manager/core/browser/hsts_query_unittest.cc

@@ -19,6 +19,42 @@
 #include "url/gurl.h"
 
 namespace password_manager {
+namespace {
+
+// Auxiliary class to automatically set and reset the HSTS state for a given
+// host.
+class HSTSStateManager {
+ public:
+  HSTSStateManager(net::TransportSecurityState* state,
+                   bool is_hsts,
+                   std::string host);
+  ~HSTSStateManager();
+
+ private:
+  net::TransportSecurityState* state_;
+  const bool is_hsts_;
+  const std::string host_;
+
+  DISALLOW_COPY_AND_ASSIGN(HSTSStateManager);
+};
+
+HSTSStateManager::HSTSStateManager(net::TransportSecurityState* state,
+                                   bool is_hsts,
+                                   std::string host)
+    : state_(state), is_hsts_(is_hsts), host_(std::move(host)) {
+  if (is_hsts_) {
+    base::Time expiry = base::Time::Max();
+    bool include_subdomains = false;
+    state_->AddHSTS(host_, expiry, include_subdomains);
+  }
+}
+
+HSTSStateManager::~HSTSStateManager() {
+  if (is_hsts_)
+    state_->DeleteDynamicDataForHost(host_);
+}
+
+}  // namespace
 
 class HSTSQueryTest : public testing::Test {
  public:

components/password_manager/core/browser/http_data_cleaner.h

-// Copyright 2017 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#ifndef COMPONENTS_PASSWORD_MANAGER_CORE_BROWSER_HTTP_DATA_CLEANER_H_
-#define COMPONENTS_PASSWORD_MANAGER_CORE_BROWSER_HTTP_DATA_CLEANER_H_
-
-#include "base/memory/ref_counted.h"
-#include "net/url_request/url_request_context_getter.h"
-
-class PrefService;
-
-namespace password_manager {
-
-class PasswordStore;
-
-void DelayCleanObsoleteHttpDataForPasswordStoreAndPrefs(
-    PasswordStore* store,
-    PrefService* prefs,
-    const scoped_refptr<net::URLRequestContextGetter>& request_context);
-
-void CleanObsoleteHttpDataForPasswordStoreAndPrefsForTesting(
-    PasswordStore* store,
-    PrefService* prefs,
-    const scoped_refptr<net::URLRequestContextGetter>& request_context);
-
-}  // namespace password_manager
-
-#endif  // COMPONENTS_PASSWORD_MANAGER_CORE_BROWSER_HTTP_DATA_CLEANER_H_

components/password_manager/core/browser/http_data_cleaner_unittest.cc

-// Copyright 2017 The Chromium Authors. All rights reserved.
-// 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_data_cleaner.h"
-
-#include <memory>
-
-#include "base/macros.h"
-#include "base/memory/ref_counted.h"
-#include "base/strings/utf_string_conversions.h"
-#include "base/test/scoped_task_environment.h"
-#include "components/password_manager/core/browser/mock_password_store.h"
-#include "components/password_manager/core/browser/password_manager_test_utils.h"
-#include "components/password_manager/core/common/password_manager_pref_names.h"
-#include "components/prefs/pref_registry_simple.h"
-#include "components/prefs/testing_pref_service.h"
-#include "net/url_request/url_request_context.h"
-#include "net/url_request/url_request_test_util.h"
-#include "testing/gmock/include/gmock/gmock.h"
-#include "testing/gtest/include/gtest/gtest.h"
-
-using autofill::PasswordForm;
-using testing::Invoke;
-using testing::Message;
-using testing::Mock;
-using testing::NiceMock;
-using testing::_;
-
-namespace password_manager {
-
-constexpr char kTestHttpURL[] = "http://example.org/";
-constexpr char kTestHttpsURL[] = "https://example.org/";
-
-PasswordForm CreateTestHTTPForm() {
-  PasswordForm form;
-  form.origin = GURL(kTestHttpURL);
-  form.signon_realm = form.origin.spec();
-  form.action = form.origin;
-  form.username_value = base::ASCIIToUTF16("user");
-  form.password_value = base::ASCIIToUTF16("password");
-  return form;
-}
-
-PasswordForm CreateTestHTTPSForm() {
-  PasswordForm form;
-  form.origin = GURL(kTestHttpsURL);
-  form.signon_realm = form.origin.spec();
-  form.action = form.origin;
-  form.username_value = base::ASCIIToUTF16("user");
-  form.password_value = base::ASCIIToUTF16("password");
-  return form;
-}
-
-InteractionsStats CreateTestHTTPStats() {
-  InteractionsStats stats;
-  stats.origin_domain = GURL(kTestHttpURL);
-  stats.username_value = base::ASCIIToUTF16("user");
-  return stats;
-}
-
-InteractionsStats CreateTestHTTPSStats() {
-  InteractionsStats stats;
-  stats.origin_domain = GURL(kTestHttpsURL);
-  stats.username_value = base::ASCIIToUTF16("user");
-  return stats;
-}
-
-class HTTPDataCleanerTest : public testing::Test {
- public:
-  HTTPDataCleanerTest()
-      : store_(new NiceMock<MockPasswordStore>),
-        prefs_(std::make_unique<TestingPrefServiceSimple>()),
-        request_context_(new net::TestURLRequestContextGetter(
-            base::ThreadTaskRunnerHandle::Get())) {
-    prefs()->registry()->RegisterBooleanPref(prefs::kWasObsoleteHttpDataCleaned,
-                                             false);
-    store_->Init(syncer::SyncableService::StartSyncFlare(), nullptr);
-  }
-
-  ~HTTPDataCleanerTest() override { store_->ShutdownOnUIThread(); }
-
-  MockPasswordStore* store() { return store_.get(); }
-
-  TestingPrefServiceSimple* prefs() { return prefs_.get(); }
-
-  const scoped_refptr<net::TestURLRequestContextGetter>& request_context() {
-    return request_context_;
-  }
-
-  void WaitUntilIdle() { scoped_task_environment_.RunUntilIdle(); }
-
- private:
-  base::test::ScopedTaskEnvironment scoped_task_environment_;
-  scoped_refptr<MockPasswordStore> store_;
-  std::unique_ptr<TestingPrefServiceSimple> prefs_;
-  scoped_refptr<net::TestURLRequestContextGetter> request_context_;
-
-  DISALLOW_COPY_AND_ASSIGN(HTTPDataCleanerTest);
-};
-
-TEST_F(HTTPDataCleanerTest, TestBlacklistDeletion) {
-  for (bool is_http : {false, true}) {
-    for (bool is_hsts : {false, true}) {
-      SCOPED_TRACE(Message() << std::boolalpha << "(is_http, is_hsts): ("
-                             << is_http << ", " << is_hsts << ")");
-
-      prefs()->SetBoolean(prefs::kWasObsoleteHttpDataCleaned, false);
-
-      const bool should_be_deleted = is_http && is_hsts;
-
-      PasswordForm form =
-          is_http ? CreateTestHTTPForm() : CreateTestHTTPSForm();
-      form.blacklisted_by_user = true;
-      form.username_value.clear();
-      form.password_value.clear();
-
-      HSTSStateManager manager(
-          request_context()->GetURLRequestContext()->transport_security_state(),
-          is_hsts, form.origin.host());
-
-      EXPECT_CALL(*store(), FillBlacklistLogins(_))
-          .WillOnce(Invoke(
-              [&form](
-                  std::vector<std::unique_ptr<autofill::PasswordForm>>* forms) {
-                *forms = WrapForms({form});
-                return true;
-              }));
-
-      EXPECT_CALL(*store(), RemoveLogin(form)).Times(should_be_deleted);
-
-      // Initiate clean up and make sure all aync tasks are run until
-      // completion.
-      CleanObsoleteHttpDataForPasswordStoreAndPrefsForTesting(
-          store(), prefs(), request_context());
-      WaitUntilIdle();
-
-      // Verify and clear all expectations as well as the preference.
-      Mock::VerifyAndClearExpectations(store());
-      EXPECT_TRUE(prefs()->GetBoolean(prefs::kWasObsoleteHttpDataCleaned));
-    }
-  }
-}
-
-TEST_F(HTTPDataCleanerTest, TestAutofillableDeletion) {
-  for (bool is_hsts : {false, true}) {
-    for (bool same_host : {false, true}) {
-      for (bool same_user : {false, true}) {
-        for (bool same_pass : {false, true}) {
-          SCOPED_TRACE(Message()
-                       << std::boolalpha
-                       << "(is_hsts, same_host, same_user, same_pass): ("
-                       << is_hsts << ", " << same_host << ", " << same_user
-                       << ", " << same_pass);
-
-          prefs()->SetBoolean(prefs::kWasObsoleteHttpDataCleaned, false);
-
-          const bool should_be_deleted =
-              is_hsts && same_host && same_user && same_pass;
-
-          PasswordForm http_form = CreateTestHTTPForm();
-          PasswordForm https_form = CreateTestHTTPSForm();
-
-          if (!same_host) {
-            GURL::Replacements rep;
-            rep.SetHostStr("a-totally-different-host");
-            http_form.origin = http_form.origin.ReplaceComponents(rep);
-          }
-
-          if (!same_user)
-            http_form.username_value = base::ASCIIToUTF16("different-user");
-
-          if (!same_pass)
-            http_form.password_value = base::ASCIIToUTF16("different-pass");
-
-          HSTSStateManager manager(request_context()
-                                       ->GetURLRequestContext()
-                                       ->transport_security_state(),
-                                   is_hsts, https_form.origin.host());
-
-          EXPECT_CALL(*store(), FillAutofillableLogins(_))
-              .WillOnce(Invoke(
-                  [&http_form, &https_form](
-                      std::vector<std::unique_ptr<autofill::PasswordForm>>*
-                          forms) {
-                    *forms = WrapForms({http_form, https_form});
-                    return true;
-                  }));
-
-          EXPECT_CALL(*store(), RemoveLogin(http_form))
-              .Times(should_be_deleted);
-
-          // Initiate clean up and make sure all aync tasks are run until
-          // completion.
-          CleanObsoleteHttpDataForPasswordStoreAndPrefsForTesting(
-              store(), prefs(), request_context());
-          WaitUntilIdle();
-
-          // Verify and clear all expectations as well as the preference.
-          Mock::VerifyAndClearExpectations(store());
-          EXPECT_TRUE(prefs()->GetBoolean(prefs::kWasObsoleteHttpDataCleaned));
-        }
-      }
-    }
-  }
-}
-
-TEST_F(HTTPDataCleanerTest, TestSiteStatsDeletion) {
-  for (bool is_http : {false, true}) {
-    for (bool is_hsts : {false, true}) {
-      SCOPED_TRACE(Message() << std::boolalpha << "(is_http, is_hsts): ("
-                             << is_http << ", " << is_hsts);
-
-      prefs()->SetBoolean(prefs::kWasObsoleteHttpDataCleaned, false);
-
-      const bool should_be_deleted = is_http && is_hsts;
-
-      InteractionsStats stats =
-          is_http ? CreateTestHTTPStats() : CreateTestHTTPSStats();
-
-      HSTSStateManager manager(
-          request_context()->GetURLRequestContext()->transport_security_state(),
-          is_hsts, stats.origin_domain.host());
-
-      EXPECT_CALL(*store(), GetAllSiteStatsImpl()).WillOnce(Invoke([&stats]() {
-        return std::vector<InteractionsStats>({stats});
-      }));
-      EXPECT_CALL(*store(), RemoveSiteStatsImpl(stats.origin_domain))
-          .Times(should_be_deleted);
-
-      // Initiate clean up and make sure all async tasks are run until
-      // completion.
-      CleanObsoleteHttpDataForPasswordStoreAndPrefsForTesting(
-          store(), prefs(), request_context());
-      WaitUntilIdle();
-
-      // Verify and clear all expectations as well as the preference.
-      Mock::VerifyAndClearExpectations(store());
-      EXPECT_TRUE(prefs()->GetBoolean(prefs::kWasObsoleteHttpDataCleaned));
-    }
-  }
-}
-
-}  // namespace password_manager

components/password_manager/core/browser/password_manager.cc

@@ -249,7 +249,6 @@ void PasswordManager::RegisterProfilePrefs(
   registry->RegisterBooleanPref(
       prefs::kCredentialsEnableAutosignin, true,
       user_prefs::PrefRegistrySyncable::SYNCABLE_PRIORITY_PREF);
-  registry->RegisterBooleanPref(prefs::kWasObsoleteHttpDataCleaned, false);
   registry->RegisterStringPref(prefs::kSyncPasswordHash, std::string(),
                                PrefRegistry::NO_REGISTRATION_FLAGS);
   registry->RegisterStringPref(prefs::kSyncPasswordLengthAndHashSalt,

components/password_manager/core/browser/password_manager_test_utils.cc

@@ -12,7 +12,6 @@
 #include "base/feature_list.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
-#include "base/time/time.h"
 
 using autofill::PasswordForm;
 
@@ -61,17 +60,6 @@ std::unique_ptr<PasswordForm> FillPasswordFormWithData(
   return form;
 }
 
-std::vector<std::unique_ptr<PasswordForm>> WrapForms(
-    std::vector<PasswordForm> forms) {
-  std::vector<std::unique_ptr<PasswordForm>> results;
-  results.reserve(forms.size());
-  std::transform(forms.begin(), forms.end(), std::back_inserter(results),
-                 [](PasswordForm& form) {
-                   return std::make_unique<PasswordForm>(std::move(form));
-                 });
-  return results;
-}
-
 bool ContainsEqualPasswordFormsUnordered(
     const std::vector<std::unique_ptr<PasswordForm>>& expectations,
     const std::vector<std::unique_ptr<PasswordForm>>& actual_values,
@@ -124,19 +112,4 @@ MockPasswordReuseDetectorConsumer::MockPasswordReuseDetectorConsumer() {}
 MockPasswordReuseDetectorConsumer::~MockPasswordReuseDetectorConsumer() {}
 #endif
 
-HSTSStateManager::HSTSStateManager(net::TransportSecurityState* state,
-                                   bool is_hsts,
-                                   const std::string& host)
-    : state_(state), is_hsts_(is_hsts), host_(host) {
-  if (is_hsts_) {
-    base::Time expiry = base::Time::Max();
-    bool include_subdomains = false;
-    state_->AddHSTS(host_, expiry, include_subdomains);
-  }
-}
-
-HSTSStateManager::~HSTSStateManager() {
-  if (is_hsts_)
-    state_->DeleteDynamicDataForHost(host_);
-}
 }  // namespace password_manager

components/password_manager/core/browser/password_manager_test_utils.h

@@ -6,15 +6,12 @@
 #define COMPONENTS_PASSWORD_MANAGER_CORE_BROWSER_PASSWORD_MANAGER_TEST_UTILS_H_
 
 #include <iosfwd>
-#include <string>
 #include <vector>
 
 #include "base/feature_list.h"
-#include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "components/autofill/core/common/password_form.h"
 #include "components/password_manager/core/browser/password_store.h"
-#include "net/http/transport_security_state.h"
 #include "testing/gmock/include/gmock/gmock.h"
 
 namespace password_manager {
@@ -60,11 +57,6 @@ std::unique_ptr<autofill::PasswordForm> FillPasswordFormWithData(
     const PasswordFormData& form_data,
     bool use_federated_login = false);
 
-// Convenience method that wraps the passed in forms in unique ptrs and returns
-// the result.
-std::vector<std::unique_ptr<autofill::PasswordForm>> WrapForms(
-    std::vector<autofill::PasswordForm> forms);
-
 // Checks whether the PasswordForms pointed to in |actual_values| are in some
 // permutation pairwise equal to those in |expectations|. Returns true in case
 // of a perfect match; otherwise returns false and writes details of mismatches
@@ -101,23 +93,6 @@ class MockPasswordReuseDetectorConsumer : public PasswordReuseDetectorConsumer {
 };
 #endif
 
-// Auxiliary class to automatically set and reset the HSTS state for a given
-// host.
-class HSTSStateManager {
- public:
-  HSTSStateManager(net::TransportSecurityState* state,
-                   bool is_hsts,
-                   const std::string& host);
-  ~HSTSStateManager();
-
- private:
-  net::TransportSecurityState* state_;
-  const bool is_hsts_;
-  const std::string host_;
-
-  DISALLOW_COPY_AND_ASSIGN(HSTSStateManager);
-};
-
 }  // namespace password_manager
 
 #endif  // COMPONENTS_PASSWORD_MANAGER_CORE_BROWSER_PASSWORD_MANAGER_TEST_UTILS_H_

components/password_manager/core/browser/password_store_consumer.h

@@ -46,8 +46,6 @@ class PasswordStoreConsumer {
     return weak_ptr_factory_.GetWeakPtr();
   }
 
-  bool HasWeakPtrs() const { return weak_ptr_factory_.HasWeakPtrs(); }
-
   void CancelAllRequests();
 
  protected:

components/password_manager/core/common/password_manager_pref_names.cc

@@ -28,9 +28,6 @@ const char kKeychainMigrationStatus[] = "password_manager.keychain_migration";
 const char kWasAutoSignInFirstRunExperienceShown[] =
     "profile.was_auto_sign_in_first_run_experience_shown";
 
-const char kWasObsoleteHttpDataCleaned[] =
-    "profile.was_obsolete_http_data_cleaned";
-
 const char kWasSignInPasswordPromoClicked[] =
     "profile.was_sign_in_password_promo_clicked";
 

components/password_manager/core/common/password_manager_pref_names.h

@@ -48,9 +48,6 @@ extern const char kKeychainMigrationStatus[];
 // prompt was shown or not.
 extern const char kWasAutoSignInFirstRunExperienceShown[];
 
-// Boolean that indicated if obsolete HTTP data has been cleaned in the past.
-extern const char kWasObsoleteHttpDataCleaned[];
-
 // Boolean that indicated if user interacted with the Chrome Sign in promo.
 extern const char kWasSignInPasswordPromoClicked[];