Moving CanStartLeakCheck to leak_detection/leak_detection_check.h

This allows ui/bulk_leak_check_service_adapter.cc to stop depending
on leak_detection_delegate.h

Bug: 1479425
Change-Id: If82bb237bed6178fef5c9ebd280f07cdc934dfb1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4936174
Reviewed-by: Ioana Pandele <ioanap@chromium.org>
Commit-Queue: Viktor Semeniuk <vsemeniuk@google.com>
Cr-Commit-Position: refs/heads/main@{#1210140}

components/password_manager/core/browser/leak_detection/BUILD.gn

@@ -55,7 +55,11 @@ source_set("leak_detection") {
   deps = [
     ":proto",
     "//base",
+    "//components/autofill/core/common",
+    "//components/password_manager/core/common",
     "//components/password_manager/core/common:features",
+    "//components/prefs",
+    "//components/safe_browsing/core/common:safe_browsing_prefs",
     "//components/signin/public/identity_manager",
     "//components/version_info:channel",
     "//third_party/private-join-and-compute/src:ec_commutative_cipher",

components/password_manager/core/browser/leak_detection/leak_detection_check.h

@@ -9,6 +9,12 @@
 
 #include "url/gurl.h"
 
+namespace autofill {
+class SavePasswordProgressLogger;
+}  // namespace autofill
+
+class PrefService;
+
 namespace password_manager {
 
 enum class LeakDetectionInitiator;
@@ -33,6 +39,12 @@ class LeakDetectionCheck {
                      const GURL& url,
                      std::u16string username,
                      std::u16string password) = 0;
+
+  // Determines whether the leak check can be started depending on `prefs`. Will
+  // use `logger` for logging if non-null.
+  static bool CanStartLeakCheck(
+      const PrefService& prefs,
+      std::unique_ptr<autofill::SavePasswordProgressLogger> logger);
 };
 
 }  // namespace password_manager

components/password_manager/core/browser/leak_detection/leak_detection_check_impl.cc

@@ -12,9 +12,13 @@
 #include "base/metrics/histogram_functions.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/timer/elapsed_timer.h"
+#include "components/autofill/core/common/save_password_progress_logger.h"
 #include "components/password_manager/core/browser/leak_detection/leak_detection_delegate_interface.h"
 #include "components/password_manager/core/browser/leak_detection/leak_detection_request_utils.h"
 #include "components/password_manager/core/browser/leak_detection/single_lookup_response.h"
+#include "components/password_manager/core/common/password_manager_pref_names.h"
+#include "components/prefs/pref_service.h"
+#include "components/safe_browsing/core/common/safe_browsing_prefs.h"
 #include "components/signin/public/identity_manager/access_token_fetcher.h"
 #include "components/signin/public/identity_manager/access_token_info.h"
 #include "components/signin/public/identity_manager/identity_manager.h"
@@ -219,6 +223,36 @@ void LeakDetectionCheckImpl::Start(LeakDetectionInitiator initiator,
                      weak_ptr_factory_.GetWeakPtr()));
 }
 
+// static
+bool LeakDetectionCheck::CanStartLeakCheck(
+    const PrefService& prefs,
+    std::unique_ptr<autofill::SavePasswordProgressLogger> logger) {
+  const bool is_leak_protection_on =
+      prefs.GetBoolean(prefs::kPasswordLeakDetectionEnabled);
+  // Leak detection can only start if:
+  // 1. The user has not opted out and Safe Browsing is turned on, or
+  // 2. The user is an enhanced protection user
+  safe_browsing::SafeBrowsingState sb_state =
+      safe_browsing::GetSafeBrowsingState(prefs);
+  switch (sb_state) {
+    case safe_browsing::SafeBrowsingState::NO_SAFE_BROWSING:
+      if (logger) {
+        logger->LogMessage(autofill::SavePasswordProgressLogger::
+                               STRING_LEAK_DETECTION_DISABLED_SAFE_BROWSING);
+      }
+      return false;
+    case safe_browsing::SafeBrowsingState::STANDARD_PROTECTION:
+      if (!is_leak_protection_on && logger) {
+        logger->LogMessage(autofill::SavePasswordProgressLogger::
+                               STRING_LEAK_DETECTION_DISABLED_FEATURE);
+      }
+      return is_leak_protection_on;
+    case safe_browsing::SafeBrowsingState::ENHANCED_PROTECTION:
+      // feature is on.
+      return true;
+  }
+}
+
 void LeakDetectionCheckImpl::OnAccessTokenRequestCompleted(
     GoogleServiceAuthError error,
     signin::AccessTokenInfo access_token_info) {

components/password_manager/core/browser/leak_detection_delegate.cc

@@ -28,11 +28,13 @@ namespace {
 
 using Logger = autofill::SavePasswordProgressLogger;
 
-void LogString(PasswordManagerClient* client, Logger::StringID string_id) {
+std::unique_ptr<autofill::SavePasswordProgressLogger> GetLogger(
+    PasswordManagerClient* client) {
   if (client && password_manager_util::IsLoggingActive(client)) {
-    BrowserSavePasswordProgressLogger logger(client->GetLogManager());
-    logger.LogMessage(string_id);
+    return std::make_unique<BrowserSavePasswordProgressLogger>(
+        client->GetLogManager());
   }
+  return nullptr;
 }
 
 }  // namespace
@@ -48,8 +50,10 @@ void LeakDetectionDelegate::StartLeakCheck(LeakDetectionInitiator initiator,
   if (client_->IsOffTheRecord())
     return;
 
-  if (!CanStartLeakCheck(*client_->GetPrefs(), client_))
+  if (!LeakDetectionCheck::CanStartLeakCheck(*client_->GetPrefs(),
+                                             GetLogger(client_))) {
     return;
+  }
 
   if (credentials.username_value.empty())
     return;
@@ -159,30 +163,4 @@ void LeakDetectionDelegate::OnError(LeakDetectionError error) {
   }
 }
 
-bool CanStartLeakCheck(const PrefService& prefs,
-                       PasswordManagerClient* client) {
-  const bool is_leak_protection_on =
-      prefs.GetBoolean(password_manager::prefs::kPasswordLeakDetectionEnabled);
-
-  // Leak detection can only start if:
-  // 1. The user has not opted out and Safe Browsing is turned on, or
-  // 2. The user is an enhanced protection user
-  safe_browsing::SafeBrowsingState sb_state =
-      safe_browsing::GetSafeBrowsingState(prefs);
-  switch (sb_state) {
-    case safe_browsing::SafeBrowsingState::NO_SAFE_BROWSING:
-      LogString(client, Logger::STRING_LEAK_DETECTION_DISABLED_SAFE_BROWSING);
-      return false;
-    case safe_browsing::SafeBrowsingState::STANDARD_PROTECTION:
-      if (!is_leak_protection_on)
-        LogString(client, Logger::STRING_LEAK_DETECTION_DISABLED_FEATURE);
-      return is_leak_protection_on;
-    case safe_browsing::SafeBrowsingState::ENHANCED_PROTECTION:
-      // feature is on.
-      break;
-  }
-
-  return true;
-}
-
 }  // namespace password_manager

components/password_manager/core/browser/leak_detection_delegate.h

@@ -15,8 +15,6 @@
 #include "components/password_manager/core/browser/leak_detection_dialog_utils.h"
 #include "components/password_manager/core/browser/password_form.h"
 
-class PrefService;
-
 namespace password_manager {
 
 class LeakDetectionCheck;
@@ -84,11 +82,6 @@ class LeakDetectionDelegate : public LeakDetectionDelegateInterface {
   std::unique_ptr<LeakDetectionDelegateHelper> helper_;
 };
 
-// Determines whether the leak check can be started depending on `prefs`. Will
-// use `client` for logging if non-null.
-bool CanStartLeakCheck(const PrefService& prefs,
-                       PasswordManagerClient* client = nullptr);
-
 }  // namespace password_manager
 
 #endif  // COMPONENTS_PASSWORD_MANAGER_CORE_BROWSER_LEAK_DETECTION_DELEGATE_H_

components/password_manager/core/browser/leak_detection_delegate_unittest.cc

@@ -11,6 +11,7 @@
 #include "base/test/scoped_feature_list.h"
 #include "base/test/task_environment.h"
 #include "build/build_config.h"
+#include "components/autofill/core/common/save_password_progress_logger.h"
 #include "components/password_manager/core/browser/form_parsing/form_data_parser.h"
 #include "components/password_manager/core/browser/leak_detection/leak_detection_check.h"
 #include "components/password_manager/core/browser/leak_detection/leak_detection_request_utils.h"
@@ -259,7 +260,7 @@ TEST_F(LeakDetectionDelegateTest, StartCheckWithStandardProtection) {
   delegate().StartLeakCheck(LeakDetectionInitiator::kSignInCheck, form);
 
   EXPECT_TRUE(delegate().leak_check());
-  EXPECT_TRUE(CanStartLeakCheck(*pref_service()));
+  EXPECT_TRUE(LeakDetectionCheck::CanStartLeakCheck(*pref_service(), nullptr));
 }
 
 TEST_F(LeakDetectionDelegateTest, StartCheckWithEnhancedProtection) {
@@ -276,7 +277,7 @@ TEST_F(LeakDetectionDelegateTest, StartCheckWithEnhancedProtection) {
   delegate().StartLeakCheck(LeakDetectionInitiator::kSignInCheck, form);
 
   EXPECT_TRUE(delegate().leak_check());
-  EXPECT_TRUE(CanStartLeakCheck(*pref_service()));
+  EXPECT_TRUE(LeakDetectionCheck::CanStartLeakCheck(*pref_service(), nullptr));
 }
 
 TEST_F(LeakDetectionDelegateTest, DoNotStartCheckWithoutSafeBrowsing) {
@@ -289,7 +290,7 @@ TEST_F(LeakDetectionDelegateTest, DoNotStartCheckWithoutSafeBrowsing) {
   delegate().StartLeakCheck(LeakDetectionInitiator::kSignInCheck, form);
 
   EXPECT_FALSE(delegate().leak_check());
-  EXPECT_FALSE(CanStartLeakCheck(*pref_service()));
+  EXPECT_FALSE(LeakDetectionCheck::CanStartLeakCheck(*pref_service(), nullptr));
 }
 
 TEST_F(LeakDetectionDelegateTest, DoNotStartLeakCheckIfLeakCheckIsOff) {
@@ -302,7 +303,7 @@ TEST_F(LeakDetectionDelegateTest, DoNotStartLeakCheckIfLeakCheckIsOff) {
   delegate().StartLeakCheck(LeakDetectionInitiator::kSignInCheck, form);
 
   EXPECT_FALSE(delegate().leak_check());
-  EXPECT_FALSE(CanStartLeakCheck(*pref_service()));
+  EXPECT_FALSE(LeakDetectionCheck::CanStartLeakCheck(*pref_service(), nullptr));
 }
 
 TEST_F(LeakDetectionDelegateTest, LeakDetectionDoneWithFalseResult) {

components/password_manager/core/browser/ui/bulk_leak_check_service_adapter.cc

@@ -9,10 +9,11 @@
 
 #include "base/check.h"
 #include "base/containers/flat_set.h"
+#include "components/autofill/core/common/save_password_progress_logger.h"
 #include "components/password_manager/core/browser/leak_detection/bulk_leak_check.h"
 #include "components/password_manager/core/browser/leak_detection/encryption_utils.h"
+#include "components/password_manager/core/browser/leak_detection/leak_detection_check.h"
 #include "components/password_manager/core/browser/leak_detection/leak_detection_request_utils.h"
-#include "components/password_manager/core/browser/leak_detection_delegate.h"
 #include "components/password_manager/core/browser/password_form.h"
 #include "components/password_manager/core/browser/ui/credential_utils.h"
 #include "components/password_manager/core/browser/ui/saved_passwords_presenter.h"
@@ -82,7 +83,7 @@ size_t BulkLeakCheckServiceAdapter::GetPendingChecksCount() const {
 
 void BulkLeakCheckServiceAdapter::OnEdited(
     const CredentialUIEntry& credential) {
-  if (CanStartLeakCheck(*prefs_)) {
+  if (LeakDetectionCheck::CanStartLeakCheck(*prefs_, nullptr)) {
     // Here no extra canonicalization is needed, as there are no other
     // credentials we could de-dupe before we pass it on to the service.
     std::vector<LeakCheckCredential> credentials;