Move HttpMetricsMigrationReporter into a separate class

This CL move the class that reports metrics about HTTP to HTTPS
migration in separate files. The class is renamed in
HttpCredentialCleaner since it will be extended in the next CLs to
remove obsolete HTTP credentials as well.

Bug: 871140
Change-Id: I52784893278a54876d68922cb2478a61974c049a
Reviewed-on: https://chromium-review.googlesource.com/1249122
Commit-Queue: Narcis Gemene <gemene@google.com>
Reviewed-by: Vaclav Brozek <vabr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#594695}

components/password_manager/core/browser/BUILD.gn

@@ -223,6 +223,8 @@ jumbo_static_library("browser") {
     sources += [
       "hsts_query.cc",
       "hsts_query.h",
+      "http_credentials_cleaner.cc",
+      "http_credentials_cleaner.h",
     ]
     deps += [ "//components/safe_browsing/common:safe_browsing_prefs" ]
   }
@@ -420,7 +422,10 @@ source_set("unit_tests") {
   if (is_ios) {
     sources += [ "login_database_ios_unittest.cc" ]
   } else {
-    sources += [ "hsts_query_unittest.cc" ]
+    sources += [
+      "hsts_query_unittest.cc",
+      "http_credentials_cleaner_unittest.cc",
+    ]
   }
   if (password_reuse_detection_support) {
     sources += [

components/password_manager/core/browser/DEPS

@@ -41,7 +41,7 @@ specific_include_rules = {
   "password_manager_unittest\.cc": [
     "+components/ukm",
   ],
-  "password_manager_util_unittest\.cc": [
+  "http_credentials_cleaner_unittest\.cc": [
     "+services/network",
   ],
 }

components/password_manager/core/browser/http_credentials_cleaner.cc

+// Copyright 2018 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_credentials_cleaner.h"
+
+#include "base/metrics/histogram_functions.h"
+#include "base/strings/string_piece.h"
+#include "components/password_manager/core/browser/password_manager_util.h"
+#include "url/gurl.h"
+
+namespace password_manager {
+
+HttpCredentialCleaner::HttpCredentialCleaner(
+    scoped_refptr<PasswordStore> store,
+    base::RepeatingCallback<network::mojom::NetworkContext*()>
+        network_context_getter)
+    : store_(std::move(store)),
+      network_context_getter_(network_context_getter) {
+  store_->GetAutofillableLogins(this);
+}
+
+HttpCredentialCleaner::~HttpCredentialCleaner() = default;
+
+void HttpCredentialCleaner::OnGetPasswordStoreResults(
+    std::vector<std::unique_ptr<autofill::PasswordForm>> results) {
+  // Non HTTP or HTTPS credentials are ignored.
+  base::EraseIf(results, [](const auto& form) {
+    return !form->origin.SchemeIsHTTPOrHTTPS();
+  });
+
+  for (auto& form : results) {
+    // The next signon-realm has the protocol excluded. For example if original
+    // signon_realm is "https://google.com/". After excluding protocol it
+    // becomes "google.com/".
+    FormKey form_key({GURL(form->signon_realm).GetContent(), form->scheme,
+                      form->username_value});
+    if (form->origin.SchemeIs(url::kHttpScheme)) {
+      PostHSTSQueryForHostAndNetworkContext(
+          form->origin, network_context_getter_.Run(),
+          base::Bind(&HttpCredentialCleaner::OnHSTSQueryResult,
+                     base::Unretained(this), form_key, form->password_value));
+      ++total_http_credentials_;
+    } else {  // HTTPS
+      https_credentials_map_[form_key].insert(form->password_value);
+    }
+  }
+
+  // This is needed in case of empty |results|.
+  ReportMetrics();
+}
+
+// |key| and |password_value| was created from the same form.
+void HttpCredentialCleaner::OnHSTSQueryResult(
+    FormKey key,
+    base::string16 password_value,
+    password_manager::HSTSResult hsts_result) {
+  ++processed_results_;
+  base::ScopedClosureRunner report(base::BindOnce(
+      &HttpCredentialCleaner::ReportMetrics, base::Unretained(this)));
+
+  if (hsts_result == HSTSResult::kError)
+    return;
+
+  bool is_hsts = (hsts_result == HSTSResult::kYes);
+
+  auto user_it = https_credentials_map_.find(key);
+  if (user_it == https_credentials_map_.end()) {
+    // Credentials are not migrated yet.
+    ++https_credential_not_found_[is_hsts];
+    return;
+  }
+
+  if (base::ContainsKey(user_it->second, password_value)) {
+    // The password store contains the same credentials (signon_realm, scheme,
+    // username and password) on HTTP version of the form.
+    ++same_password_[is_hsts];
+  } else {
+    ++different_password_[is_hsts];
+  }
+}
+
+void HttpCredentialCleaner::ReportMetrics() {
+  // The metrics have to be recorded after all requests are done.
+  if (processed_results_ != total_http_credentials_)
+    return;
+
+  for (bool is_hsts_enabled : {false, true}) {
+    std::string suffix = (is_hsts_enabled ? std::string("WithHSTSEnabled")
+                                          : std::string("HSTSNotEnabled"));
+
+    base::UmaHistogramCounts1000(
+        "PasswordManager.HttpCredentialsWithEquivalentHttpsCredential." +
+            suffix,
+        same_password_[is_hsts_enabled]);
+
+    base::UmaHistogramCounts1000(
+        "PasswordManager.HttpCredentialsWithConflictingHttpsCredential." +
+            suffix,
+        different_password_[is_hsts_enabled]);
+
+    base::UmaHistogramCounts1000(
+        "PasswordManager.HttpCredentialsWithoutMatchingHttpsCredential." +
+            suffix,
+        https_credential_not_found_[is_hsts_enabled]);
+  }
+
+  delete this;
+}
+
+}  // namespace password_manager
\ No newline at end of file

components/password_manager/core/browser/http_credentials_cleaner.h

+// Copyright 2018 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_CREDENTIALS_CLEANER_H_
+#define COMPONENTS_PASSWORD_MANAGER_CORE_BROWSER_HTTP_CREDENTIALS_CLEANER_H_
+
+#include <map>
+#include <memory>
+#include <string>
+#include <tuple>
+#include <utility>
+#include <vector>
+
+#include "base/containers/flat_set.h"
+#include "base/memory/ref_counted.h"
+#include "components/autofill/core/common/password_form.h"
+#include "components/password_manager/core/browser/hsts_query.h"
+#include "components/password_manager/core/browser/password_store_consumer.h"
+
+namespace network {
+namespace mojom {
+class NetworkContext;
+}  // namespace mojom
+}  // namespace network
+
+namespace password_manager {
+
+class PasswordStore;
+
+// This class is responsible for reporting metrics about HTTP to HTTPS
+// migration. Important note: The object will delete itself once metrics are
+// reported. Having a private destructor enforces this.
+class HttpCredentialCleaner : public PasswordStoreConsumer {
+ public:
+  HttpCredentialCleaner(
+      scoped_refptr<PasswordStore> store,
+      base::RepeatingCallback<network::mojom::NetworkContext*()>
+          network_context_getter);
+
+ private:
+  // This type define a subset of PasswordForm where first argument is the
+  // signon-realm excluding the protocol, the second argument is
+  // the PasswordForm::scheme (i.e. HTML, BASIC, etc.) and the third argument is
+  // the username of the form.
+  using FormKey =
+      std::tuple<std::string, autofill::PasswordForm::Scheme, base::string16>;
+
+  ~HttpCredentialCleaner() override;
+
+  // PasswordStoreConsumer:
+  void OnGetPasswordStoreResults(
+      std::vector<std::unique_ptr<autofill::PasswordForm>> results) override;
+
+  void OnHSTSQueryResult(FormKey key,
+                         base::string16 password_value,
+                         HSTSResult hsts_result);
+
+  void ReportMetrics();
+
+  scoped_refptr<PasswordStore> store_;
+
+  // Needed to create HSTS request.
+  base::RepeatingCallback<network::mojom::NetworkContext*()>
+      network_context_getter_;
+
+  // Map from (signon-realm excluding the protocol, Password::Scheme, username)
+  // tuples of HTTPS forms to a list of passwords for that pair.
+  std::map<FormKey, base::flat_set<base::string16>> https_credentials_map_;
+
+  // The number of HTTP credentials processed after HSTS query results are
+  // received.
+  size_t processed_results_ = 0;
+
+  // The next three counters are in pairs where [0] component means that HSTS is
+  // not enabled and [1] component means that HSTS is enabled for that HTTP type
+  // of credentials.
+
+  // Number of HTTP credentials for which no HTTPS credential for the same
+  // signon_realm excluding protocol, PasswordForm::scheme and username exists.
+  size_t https_credential_not_found_[2] = {0, 0};
+
+  // Number of HTTP credentials for which an equivalent (i.e. same signon_realm
+  // excluding protocol, PasswordForm::scheme (i.e. HTML, BASIC, etc.), username
+  // and password) HTTPS credential exists.
+  size_t same_password_[2] = {0, 0};
+
+  // Number of HTTP credentials for which a conflicting (i.e. same signon-realm
+  // excluding the protocol, PasswordForm::Scheme and username but different
+  // password) HTTPS credential exists.
+  size_t different_password_[2] = {0, 0};
+
+  // Number of HTTP credentials from the password store. Used to know when all
+  // credentials were processed.
+  size_t total_http_credentials_ = 0;
+
+  DISALLOW_COPY_AND_ASSIGN(HttpCredentialCleaner);
+};
+
+}  // namespace password_manager
+
+#endif  // COMPONENTS_PASSWORD_MANAGER_CORE_BROWSER_HTTP_CREDENTIALS_CLEANER_H_
\ No newline at end of file

components/password_manager/core/browser/http_credentials_cleaner_unittest.cc

+// Copyright 2018 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 "base/macros.h"
+#include "base/stl_util.h"
+#include "base/strings/utf_string_conversions.h"
+#include "base/test/bind_test_util.h"
+#include "base/test/metrics/histogram_tester.h"
+#include "base/test/scoped_task_environment.h"
+#include "components/password_manager/core/browser/password_manager_util.h"
+#include "components/password_manager/core/browser/test_password_store.h"
+#include "net/url_request/url_request_test_util.h"
+#include "services/network/network_context.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "url/gurl.h"
+
+namespace password_manager {
+
+TEST(HttpCredentialCleaner, ReportHttpMigrationMetrics) {
+  enum class HttpCredentialType { kNoMatching, kEquivalent, kConflicting };
+
+  struct TestCase {
+    bool is_hsts_enabled;
+    autofill::PasswordForm::Scheme http_form_scheme;
+    bool same_signon_realm;
+    bool same_scheme;
+    bool same_username;
+    bool same_password;
+    HttpCredentialType expected;
+  };
+
+  struct Histogram {
+    bool test_hsts_enabled;
+    HttpCredentialType test_type;
+    std::string histogram_name;
+  };
+
+  constexpr static TestCase cases[] = {
+
+      {true, autofill::PasswordForm::Scheme::SCHEME_HTML, false, true, true,
+       true, HttpCredentialType::kNoMatching},
+      {true, autofill::PasswordForm::Scheme::SCHEME_HTML, true, false, true,
+       true, HttpCredentialType::kNoMatching},
+      {true, autofill::PasswordForm::Scheme::SCHEME_HTML, true, true, false,
+       true, HttpCredentialType::kNoMatching},
+      {true, autofill::PasswordForm::Scheme::SCHEME_HTML, true, true, true,
+       false, HttpCredentialType::kConflicting},
+      {true, autofill::PasswordForm::Scheme::SCHEME_HTML, true, true, true,
+       true, HttpCredentialType::kEquivalent},
+
+      {false, autofill::PasswordForm::Scheme::SCHEME_HTML, false, true, true,
+       true, HttpCredentialType::kNoMatching},
+      {false, autofill::PasswordForm::Scheme::SCHEME_HTML, true, false, true,
+       true, HttpCredentialType::kNoMatching},
+      {false, autofill::PasswordForm::Scheme::SCHEME_HTML, true, true, false,
+       true, HttpCredentialType::kNoMatching},
+      {false, autofill::PasswordForm::Scheme::SCHEME_HTML, true, true, true,
+       false, HttpCredentialType::kConflicting},
+      {false, autofill::PasswordForm::Scheme::SCHEME_HTML, true, true, true,
+       true, HttpCredentialType::kEquivalent},
+
+      {true, autofill::PasswordForm::Scheme::SCHEME_BASIC, false, true, true,
+       true, HttpCredentialType::kNoMatching},
+      {true, autofill::PasswordForm::Scheme::SCHEME_BASIC, true, false, true,
+       true, HttpCredentialType::kNoMatching},
+      {true, autofill::PasswordForm::Scheme::SCHEME_BASIC, true, true, false,
+       true, HttpCredentialType::kNoMatching},
+      {true, autofill::PasswordForm::Scheme::SCHEME_BASIC, true, true, true,
+       false, HttpCredentialType::kConflicting},
+      {true, autofill::PasswordForm::Scheme::SCHEME_BASIC, true, true, true,
+       true, HttpCredentialType::kEquivalent},
+
+      {false, autofill::PasswordForm::Scheme::SCHEME_BASIC, false, true, true,
+       true, HttpCredentialType::kNoMatching},
+      {false, autofill::PasswordForm::Scheme::SCHEME_BASIC, true, false, true,
+       true, HttpCredentialType::kNoMatching},
+      {false, autofill::PasswordForm::Scheme::SCHEME_BASIC, true, true, false,
+       true, HttpCredentialType::kNoMatching},
+      {false, autofill::PasswordForm::Scheme::SCHEME_BASIC, true, true, true,
+       false, HttpCredentialType::kConflicting},
+      {false, autofill::PasswordForm::Scheme::SCHEME_BASIC, true, true, true,
+       true, HttpCredentialType::kEquivalent}
+
+  };
+
+  const base::string16 username[2] = {base::ASCIIToUTF16("user0"),
+                                      base::ASCIIToUTF16("user1")};
+  const base::string16 password[2] = {base::ASCIIToUTF16("pass0"),
+                                      base::ASCIIToUTF16("pass1")};
+
+  std::vector<Histogram> histograms_to_test;
+  for (bool test_hsts_enabled : {true, false}) {
+    std::string suffix =
+        (test_hsts_enabled ? "WithHSTSEnabled" : "HSTSNotEnabled");
+    histograms_to_test.push_back(
+        {test_hsts_enabled, HttpCredentialType::kNoMatching,
+         "PasswordManager.HttpCredentialsWithoutMatchingHttpsCredential." +
+             suffix});
+    histograms_to_test.push_back(
+        {test_hsts_enabled, HttpCredentialType::kEquivalent,
+         "PasswordManager.HttpCredentialsWithEquivalentHttpsCredential." +
+             suffix});
+    histograms_to_test.push_back(
+        {test_hsts_enabled, HttpCredentialType::kConflicting,
+         "PasswordManager.HttpCredentialsWithConflictingHttpsCredential." +
+             suffix});
+  }
+  for (const auto& test : cases) {
+    SCOPED_TRACE(testing::Message()
+                 << "is_hsts_enabled=" << test.is_hsts_enabled
+                 << ", http_form_scheme="
+                 << static_cast<int>(test.http_form_scheme)
+                 << ", same_signon_realm=" << test.same_signon_realm
+                 << ", same_scheme=" << test.same_scheme
+                 << ", same_username=" << test.same_username
+                 << ", same_password=" << test.same_password);
+
+    base::test::ScopedTaskEnvironment scoped_task_environment;
+    auto request_context =
+        base::MakeRefCounted<net::TestURLRequestContextGetter>(
+            base::ThreadTaskRunnerHandle::Get());
+    network::mojom::NetworkContextPtr network_context_pipe;
+    auto network_context = std::make_unique<network::NetworkContext>(
+        nullptr, mojo::MakeRequest(&network_context_pipe),
+        request_context->GetURLRequestContext());
+
+    auto password_store =
+        base::MakeRefCounted<password_manager::TestPasswordStore>();
+    ASSERT_TRUE(password_store->Init(syncer::SyncableService::StartSyncFlare(),
+                                     nullptr));
+
+    autofill::PasswordForm http_form;
+    http_form.origin = GURL("http://example.org/");
+    http_form.signon_realm = http_form.origin.GetOrigin().spec();
+    http_form.scheme = test.http_form_scheme;
+    http_form.username_value = username[1];
+    http_form.password_value = password[1];
+    password_store->AddLogin(http_form);
+
+    autofill::PasswordForm https_form;
+    https_form.origin = GURL("https://example.org/");
+    https_form.scheme = test.http_form_scheme;
+    if (!test.same_scheme) {
+      if (https_form.scheme == autofill::PasswordForm::Scheme::SCHEME_BASIC)
+        https_form.scheme = autofill::PasswordForm::Scheme::SCHEME_HTML;
+      else
+        https_form.scheme = autofill::PasswordForm::Scheme::SCHEME_BASIC;
+    }
+
+    https_form.signon_realm = https_form.origin.GetOrigin().spec();
+    if (!test.same_signon_realm)
+      https_form.signon_realm += "different/";
+
+    https_form.username_value = username[test.same_username];
+    https_form.password_value = password[test.same_password];
+    password_store->AddLogin(https_form);
+
+    if (test.is_hsts_enabled) {
+      network_context->AddHSTSForTesting(
+          http_form.origin.host(), base::Time::Max(),
+          false /*include_subdomains*/, base::DoNothing());
+    }
+    scoped_task_environment.RunUntilIdle();
+
+    base::HistogramTester histogram_tester;
+    password_manager_util::ReportHttpMigrationMetrics(
+        password_store,
+        base::BindLambdaForTesting([&]() -> network::mojom::NetworkContext* {
+          // This needs to be network_context_pipe.get() and
+          // not network_context.get() to make HSTS queries asynchronous, which
+          // is what the progress tracking logic in HttpMetricsMigrationReporter
+          // assumes.  This also matches reality, since
+          // StoragePartition::GetNetworkContext will return a mojo pipe
+          // even in the in-process case.
+          return network_context_pipe.get();
+        }));
+    scoped_task_environment.RunUntilIdle();
+
+    for (const auto& histogram : histograms_to_test) {
+      int sample =
+          static_cast<int>(histogram.test_type == test.expected &&
+                           histogram.test_hsts_enabled == test.is_hsts_enabled);
+      histogram_tester.ExpectUniqueSample(histogram.histogram_name, sample, 1);
+    }
+
+    password_store->ShutdownOnUIThread();
+    scoped_task_environment.RunUntilIdle();
+  }
+}
+
+}  // namespace password_manager
\ No newline at end of file

components/password_manager/core/browser/password_manager_util.cc

@@ -8,7 +8,6 @@
 #include <string>
 #include <utility>
 
-#include "base/containers/flat_set.h"
 #include "base/metrics/histogram_functions.h"
 #include "base/stl_util.h"
 #include "build/build_config.h"
@@ -19,7 +18,7 @@
 #include "components/password_manager/core/browser/blacklisted_duplicates_cleaner.h"
 #include "components/password_manager/core/browser/credentials_cleaner.h"
 #include "components/password_manager/core/browser/credentials_cleaner_runner.h"
-#include "components/password_manager/core/browser/hsts_query.h"
+#include "components/password_manager/core/browser/http_credentials_cleaner.h"
 #include "components/password_manager/core/browser/invalid_realm_credential_cleaner.h"
 #include "components/password_manager/core/browser/log_manager.h"
 #include "components/password_manager/core/browser/password_generation_manager.h"
@@ -32,7 +31,6 @@
 #include "components/password_manager/core/common/password_manager_pref_names.h"
 #include "components/prefs/pref_service.h"
 #include "components/sync/driver/sync_service.h"
-#include "url/gurl.h"
 
 using autofill::PasswordForm;
 
@@ -48,146 +46,6 @@ bool IsBetterMatch(const PasswordForm* lhs, const PasswordForm* rhs) {
          std::make_pair(!rhs->is_public_suffix_match, rhs->preferred);
 }
 
-// This class is responsible for reporting metrics about HTTP to HTTPS
-// migration.
-class HttpMetricsMigrationReporter
-    : public password_manager::PasswordStoreConsumer {
- public:
-  HttpMetricsMigrationReporter(
-      password_manager::PasswordStore* store,
-      base::RepeatingCallback<network::mojom::NetworkContext*()>
-          network_context_getter)
-      : network_context_getter_(network_context_getter) {
-    store->GetAutofillableLogins(this);
-  }
-
- private:
-  // This type define a subset of PasswordForm where first argument is the
-  // signon-realm excluding the protocol, the second argument is
-  // PasswordForm::scheme (i.e. HTML, BASIC, etc.) and the third argument is the
-  // username of the form.
-  using FormKey = std::tuple<std::string, PasswordForm::Scheme, base::string16>;
-
-  // This overrides the PasswordStoreConsumer method.
-  void OnGetPasswordStoreResults(
-      std::vector<std::unique_ptr<autofill::PasswordForm>> results) override;
-
-  void OnHSTSQueryResult(FormKey key,
-                         base::string16 password_value,
-                         password_manager::HSTSResult is_hsts);
-
-  void ReportMetrics();
-
-  base::RepeatingCallback<network::mojom::NetworkContext*()>
-      network_context_getter_;
-
-  std::map<FormKey, base::flat_set<base::string16>> https_credentials_map_;
-  size_t processed_results_ = 0;
-
-  // The next three counters are in pairs where [0] component means that HSTS is
-  // not enabled and [1] component means that HSTS is enabled for that HTTP type
-  // of credentials.
-
-  // Number of HTTP credentials for which no HTTPS credential for the same
-  // username exists.
-  size_t https_credential_not_found_[2] = {0, 0};
-
-  // Number of HTTP credentials for which an equivalent (i.e. same host,
-  // username and password) HTTPS credential exists.
-  size_t same_password_[2] = {0, 0};
-
-  // Number of HTTP credentials for which a conflicting (i.e. same host and
-  // username, but different password) HTTPS credential exists.
-  size_t different_password_[2] = {0, 0};
-
-  // Number of HTTP credentials from the Password Store.
-  size_t total_http_credentials_ = 0;
-
-  DISALLOW_COPY_AND_ASSIGN(HttpMetricsMigrationReporter);
-};
-
-void HttpMetricsMigrationReporter::OnGetPasswordStoreResults(
-    std::vector<std::unique_ptr<autofill::PasswordForm>> results) {
-  // Non HTTP or HTTPS credentials are ignored.
-  base::EraseIf(results, [](const std::unique_ptr<PasswordForm>& form) {
-    return !form->origin.SchemeIsHTTPOrHTTPS();
-  });
-
-  for (auto& form : results) {
-    // The next signon-realm has the protocol excluded. For example if original
-    // signon_realm is "https://google.com/". After excluding protocol it
-    // becomes "google.com/".
-    FormKey form_key({GURL(form->signon_realm).GetContent(), form->scheme,
-                      form->username_value});
-    if (form->origin.SchemeIs(url::kHttpScheme)) {
-      password_manager::PostHSTSQueryForHostAndNetworkContext(
-          form->origin, network_context_getter_.Run(),
-          base::Bind(&HttpMetricsMigrationReporter::OnHSTSQueryResult,
-                     base::Unretained(this), form_key, form->password_value));
-      ++total_http_credentials_;
-    } else {  // Https
-      https_credentials_map_[form_key].insert(form->password_value);
-    }
-  }
-  ReportMetrics();
-}
-
-// |key| and |password_value| was created from the same form.
-void HttpMetricsMigrationReporter::OnHSTSQueryResult(
-    FormKey key,
-    base::string16 password_value,
-    password_manager::HSTSResult hsts_result) {
-  ++processed_results_;
-  base::ScopedClosureRunner report(base::BindOnce(
-      &HttpMetricsMigrationReporter::ReportMetrics, base::Unretained(this)));
-
-  if (hsts_result == password_manager::HSTSResult::kError)
-    return;
-
-  bool is_hsts = (hsts_result == password_manager::HSTSResult::kYes);
-
-  auto user_it = https_credentials_map_.find(key);
-  if (user_it == https_credentials_map_.end()) {
-    // Credentials are not migrated yet.
-    ++https_credential_not_found_[is_hsts];
-    return;
-  }
-  if (base::ContainsKey(user_it->second, password_value)) {
-    // The password store contains the same credentials (username and
-    // password) on HTTP version of the form.
-    ++same_password_[is_hsts];
-  } else {
-    ++different_password_[is_hsts];
-  }
-}
-
-void HttpMetricsMigrationReporter::ReportMetrics() {
-  // The metrics have to be recorded after all requests are done.
-  if (processed_results_ != total_http_credentials_)
-    return;
-
-  for (bool is_hsts_enabled : {false, true}) {
-    std::string suffix = (is_hsts_enabled ? std::string("WithHSTSEnabled")
-                                          : std::string("HSTSNotEnabled"));
-
-    base::UmaHistogramCounts1000(
-        "PasswordManager.HttpCredentialsWithEquivalentHttpsCredential." +
-            suffix,
-        same_password_[is_hsts_enabled]);
-
-    base::UmaHistogramCounts1000(
-        "PasswordManager.HttpCredentialsWithConflictingHttpsCredential." +
-            suffix,
-        different_password_[is_hsts_enabled]);
-
-    base::UmaHistogramCounts1000(
-        "PasswordManager.HttpCredentialsWithoutMatchingHttpsCredential." +
-            suffix,
-        https_credential_not_found_[is_hsts_enabled]);
-  }
-  delete this;
-}
-
 }  // namespace
 
 #if !defined(OS_IOS)
@@ -196,7 +54,8 @@ void ReportHttpMigrationMetrics(
     base::RepeatingCallback<network::mojom::NetworkContext*()>
         network_context_getter) {
   // The object will delete itself once the metrics are recorded.
-  new HttpMetricsMigrationReporter(store.get(), network_context_getter);
+  new password_manager::HttpCredentialCleaner(std::move(store),
+                                              network_context_getter);
 }
 #endif  // !defined(OS_IOS)
 

components/password_manager/core/browser/password_manager_util_unittest.cc

@@ -12,19 +12,13 @@
 #include "base/macros.h"
 #include "base/stl_util.h"
 #include "base/strings/utf_string_conversions.h"
-#include "base/test/bind_test_util.h"
-#include "base/test/metrics/histogram_tester.h"
 #include "base/test/scoped_task_environment.h"
-#include "build/build_config.h"
 #include "components/autofill/core/common/password_form.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/browser/test_password_store.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_test_util.h"
-#include "services/network/network_context.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -95,180 +89,6 @@ TEST(PasswordManagerUtil, TrimUsernameOnlyCredentials) {
   EXPECT_THAT(forms, UnorderedPasswordFormElementsAre(&expected_forms));
 }
 
-#if !defined(OS_IOS)
-TEST(PasswordManagerUtil, ReportHttpMigrationMetrics) {
-  enum class HttpCredentialType { kNoMatching, kEquivalent, kConflicting };
-
-  struct TestCase {
-    bool is_hsts_enabled;
-    autofill::PasswordForm::Scheme http_form_scheme;
-    bool same_signon_realm;
-    bool same_scheme;
-    bool same_username;
-    bool same_password;
-    HttpCredentialType expected;
-  };
-
-  struct Histogram {
-    bool test_hsts_enabled;
-    HttpCredentialType test_type;
-    std::string histogram_name;
-  };
-
-  constexpr static TestCase cases[] = {
-
-      {true, autofill::PasswordForm::Scheme::SCHEME_HTML, false, true, true,
-       true, HttpCredentialType::kNoMatching},
-      {true, autofill::PasswordForm::Scheme::SCHEME_HTML, true, false, true,
-       true, HttpCredentialType::kNoMatching},
-      {true, autofill::PasswordForm::Scheme::SCHEME_HTML, true, true, false,
-       true, HttpCredentialType::kNoMatching},
-      {true, autofill::PasswordForm::Scheme::SCHEME_HTML, true, true, true,
-       false, HttpCredentialType::kConflicting},
-      {true, autofill::PasswordForm::Scheme::SCHEME_HTML, true, true, true,
-       true, HttpCredentialType::kEquivalent},
-
-      {false, autofill::PasswordForm::Scheme::SCHEME_HTML, false, true, true,
-       true, HttpCredentialType::kNoMatching},
-      {false, autofill::PasswordForm::Scheme::SCHEME_HTML, true, false, true,
-       true, HttpCredentialType::kNoMatching},
-      {false, autofill::PasswordForm::Scheme::SCHEME_HTML, true, true, false,
-       true, HttpCredentialType::kNoMatching},
-      {false, autofill::PasswordForm::Scheme::SCHEME_HTML, true, true, true,
-       false, HttpCredentialType::kConflicting},
-      {false, autofill::PasswordForm::Scheme::SCHEME_HTML, true, true, true,
-       true, HttpCredentialType::kEquivalent},
-
-      {true, autofill::PasswordForm::Scheme::SCHEME_BASIC, false, true, true,
-       true, HttpCredentialType::kNoMatching},
-      {true, autofill::PasswordForm::Scheme::SCHEME_BASIC, true, false, true,
-       true, HttpCredentialType::kNoMatching},
-      {true, autofill::PasswordForm::Scheme::SCHEME_BASIC, true, true, false,
-       true, HttpCredentialType::kNoMatching},
-      {true, autofill::PasswordForm::Scheme::SCHEME_BASIC, true, true, true,
-       false, HttpCredentialType::kConflicting},
-      {true, autofill::PasswordForm::Scheme::SCHEME_BASIC, true, true, true,
-       true, HttpCredentialType::kEquivalent},
-
-      {false, autofill::PasswordForm::Scheme::SCHEME_BASIC, false, true, true,
-       true, HttpCredentialType::kNoMatching},
-      {false, autofill::PasswordForm::Scheme::SCHEME_BASIC, true, false, true,
-       true, HttpCredentialType::kNoMatching},
-      {false, autofill::PasswordForm::Scheme::SCHEME_BASIC, true, true, false,
-       true, HttpCredentialType::kNoMatching},
-      {false, autofill::PasswordForm::Scheme::SCHEME_BASIC, true, true, true,
-       false, HttpCredentialType::kConflicting},
-      {false, autofill::PasswordForm::Scheme::SCHEME_BASIC, true, true, true,
-       true, HttpCredentialType::kEquivalent}
-
-  };
-
-  const base::string16 username[2] = {base::ASCIIToUTF16("user0"),
-                                      base::ASCIIToUTF16("user1")};
-  const base::string16 password[2] = {base::ASCIIToUTF16("pass0"),
-                                      base::ASCIIToUTF16("pass1")};
-
-  std::vector<Histogram> histograms_to_test;
-  for (bool test_hsts_enabled : {true, false}) {
-    std::string suffix =
-        (test_hsts_enabled ? "WithHSTSEnabled" : "HSTSNotEnabled");
-    histograms_to_test.push_back(
-        {test_hsts_enabled, HttpCredentialType::kNoMatching,
-         "PasswordManager.HttpCredentialsWithoutMatchingHttpsCredential." +
-             suffix});
-    histograms_to_test.push_back(
-        {test_hsts_enabled, HttpCredentialType::kEquivalent,
-         "PasswordManager.HttpCredentialsWithEquivalentHttpsCredential." +
-             suffix});
-    histograms_to_test.push_back(
-        {test_hsts_enabled, HttpCredentialType::kConflicting,
-         "PasswordManager.HttpCredentialsWithConflictingHttpsCredential." +
-             suffix});
-  }
-  for (const auto& test : cases) {
-    SCOPED_TRACE(testing::Message()
-                 << "is_hsts_enabled=" << test.is_hsts_enabled
-                 << ", http_form_scheme="
-                 << static_cast<int>(test.http_form_scheme)
-                 << ", same_signon_realm=" << test.same_signon_realm
-                 << ", same_scheme=" << test.same_scheme
-                 << ", same_username=" << test.same_username
-                 << ", same_password=" << test.same_password);
-
-    base::test::ScopedTaskEnvironment scoped_task_environment;
-    auto request_context =
-        base::MakeRefCounted<net::TestURLRequestContextGetter>(
-            base::ThreadTaskRunnerHandle::Get());
-    network::mojom::NetworkContextPtr network_context_pipe;
-    auto network_context = std::make_unique<network::NetworkContext>(
-        nullptr, mojo::MakeRequest(&network_context_pipe),
-        request_context->GetURLRequestContext());
-
-    auto password_store =
-        base::MakeRefCounted<password_manager::TestPasswordStore>();
-    ASSERT_TRUE(password_store->Init(syncer::SyncableService::StartSyncFlare(),
-                                     nullptr));
-
-    autofill::PasswordForm http_form;
-    http_form.origin = GURL("http://example.org/");
-    http_form.signon_realm = http_form.origin.GetOrigin().spec();
-    http_form.scheme = test.http_form_scheme;
-    http_form.username_value = username[1];
-    http_form.password_value = password[1];
-    password_store->AddLogin(http_form);
-
-    autofill::PasswordForm https_form;
-    https_form.origin = GURL("https://example.org/");
-    https_form.scheme = test.http_form_scheme;
-    if (!test.same_scheme) {
-      if (https_form.scheme == autofill::PasswordForm::Scheme::SCHEME_BASIC)
-        https_form.scheme = autofill::PasswordForm::Scheme::SCHEME_HTML;
-      else
-        https_form.scheme = autofill::PasswordForm::Scheme::SCHEME_BASIC;
-    }
-
-    https_form.signon_realm = https_form.origin.GetOrigin().spec();
-    if (!test.same_signon_realm)
-      https_form.signon_realm += "different/";
-
-    https_form.username_value = username[test.same_username];
-    https_form.password_value = password[test.same_password];
-    password_store->AddLogin(https_form);
-
-    if (test.is_hsts_enabled) {
-      network_context->AddHSTSForTesting(
-          http_form.origin.host(), base::Time::Max(),
-          false /*include_subdomains*/, base::DoNothing());
-    }
-    scoped_task_environment.RunUntilIdle();
-
-    base::HistogramTester histogram_tester;
-    ReportHttpMigrationMetrics(
-        password_store,
-        base::BindLambdaForTesting([&]() -> network::mojom::NetworkContext* {
-          // This needs to be network_context_pipe.get() and
-          // not network_context.get() to make HSTS queries asynchronous, which
-          // is what the progress tracking logic in HttpMetricsMigrationReporter
-          // assumes.  This also matches reality, since
-          // StoragePartition::GetNetworkContext will return a mojo pipe
-          // even in the in-process case.
-          return network_context_pipe.get();
-        }));
-    scoped_task_environment.RunUntilIdle();
-
-    for (const auto& histogram : histograms_to_test) {
-      int sample =
-          static_cast<int>(histogram.test_type == test.expected &&
-                           histogram.test_hsts_enabled == test.is_hsts_enabled);
-      histogram_tester.ExpectUniqueSample(histogram.histogram_name, sample, 1);
-    }
-
-    password_store->ShutdownOnUIThread();
-    scoped_task_environment.RunUntilIdle();
-  }
-}
-#endif  // !defined(OS_IOS)
-
 // This test is supposed to check the behavior when:
 // 1. User blacklisted on http site two forms (they being considered
 // duplicated because they have the same signon_realm).