Safety check: moved SB check common to desktop, Android, and iOS to components/

For more details: go/safety-check-mobile-design.
Existing unit tests cover all of the affected code.

Bug: 1070620, 1015841
Change-Id: Ie59e53d5f1f5466f5cf5cbd9c5422e2d88dc77f3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2146904
Commit-Queue: Andrey Zaytsev <andzaytsev@google.com>
Reviewed-by: Martin Šrámek <msramek@chromium.org>
Reviewed-by: Rainhard Findling <rainhard@chromium.org>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#760040}

chrome/browser/ui/BUILD.gn

@@ -1475,6 +1475,7 @@ jumbo_static_library("ui") {
       "//components/network_session_configurator/common",
       "//components/page_load_metrics/browser",
       "//components/profile_metrics",
+      "//components/safety_check",
       "//components/search_provider_logos",
       "//components/ui_metrics",
       "//components/url_formatter",

chrome/browser/ui/DEPS

@@ -4,6 +4,7 @@ include_rules = [
   "+components/browser_ui/util/android/url_constants.h",
   "+components/country_codes",
   "+components/page_load_metrics/browser",
+  "+components/safety_check",
   "+mash/public/mojom",
   "+services/content/public",
   "+services/device/public/mojom",

chrome/browser/ui/webui/settings/safety_check_handler.cc

@@ -110,14 +110,17 @@ void SafetyCheckHandler::PerformSafetyCheck() {
                                     base::UTF8ToUTF16(""));
 
   // Run safety check.
+  // Checks common to desktop, Android, and iOS are handled by
+  // safety_check::SafetyCheck.
+  safety_check_.reset(new safety_check::SafetyCheck(this));
+  safety_check_->CheckSafeBrowsing(Profile::FromWebUI(web_ui())->GetPrefs());
+
   if (!version_updater_) {
     version_updater_.reset(VersionUpdater::Create(web_ui()->GetWebContents()));
   }
   DCHECK(version_updater_);
   CheckUpdates();
 
-  CheckSafeBrowsing();
-
   if (!leak_service_) {
     leak_service_ = BulkLeakCheckServiceFactory::GetForProfile(
         Profile::FromWebUI(web_ui()));
@@ -179,26 +182,6 @@ void SafetyCheckHandler::CheckUpdates() {
       VersionUpdater::PromoteCallback());
 }
 
-void SafetyCheckHandler::CheckSafeBrowsing() {
-  PrefService* pref_service = Profile::FromWebUI(web_ui())->GetPrefs();
-  const PrefService::Preference* enabled_pref =
-      pref_service->FindPreference(prefs::kSafeBrowsingEnabled);
-  bool enabled = pref_service->GetBoolean(prefs::kSafeBrowsingEnabled);
-  SafeBrowsingStatus status;
-  if (enabled && pref_service->GetBoolean(prefs::kSafeBrowsingEnhanced)) {
-    status = SafeBrowsingStatus::kEnabledEnhanced;
-  } else if (enabled) {
-    status = SafeBrowsingStatus::kEnabledStandard;
-  } else if (enabled_pref->IsManaged()) {
-    status = SafeBrowsingStatus::kDisabledByAdmin;
-  } else if (enabled_pref->IsExtensionControlled()) {
-    status = SafeBrowsingStatus::kDisabledByExtension;
-  } else {
-    status = SafeBrowsingStatus::kDisabled;
-  }
-  OnSafeBrowsingCheckResult(status);
-}
-
 void SafetyCheckHandler::CheckPasswords() {
   // Remove |this| as an existing observer for BulkLeakCheck if it is
   // registered. This takes care of an edge case when safety check starts twice
@@ -281,19 +264,6 @@ void SafetyCheckHandler::OnUpdateCheckResult(VersionUpdater::Status status,
   CompleteParentIfChildrenCompleted();
 }
 
-void SafetyCheckHandler::OnSafeBrowsingCheckResult(
-    SafetyCheckHandler::SafeBrowsingStatus status) {
-  safe_browsing_status_ = status;
-  if (safe_browsing_status_ != SafeBrowsingStatus::kChecking) {
-    base::UmaHistogramEnumeration("Settings.SafetyCheck.SafeBrowsingResult",
-                                  safe_browsing_status_);
-  }
-  FireBasicSafetyCheckWebUiListener(
-      kSafeBrowsingEvent, static_cast<int>(safe_browsing_status_),
-      GetStringForSafeBrowsing(safe_browsing_status_));
-  CompleteParentIfChildrenCompleted();
-}
-
 void SafetyCheckHandler::OnPasswordsCheckResult(PasswordsStatus status,
                                                 Compromised compromised,
                                                 Done done,
@@ -556,6 +526,19 @@ void SafetyCheckHandler::DetermineIfNoPasswordsOrSafe(
                          Compromised(0), Done(0), Total(0));
 }
 
+void SafetyCheckHandler::OnSafeBrowsingCheckResult(
+    SafetyCheckHandler::SafeBrowsingStatus status) {
+  safe_browsing_status_ = status;
+  if (safe_browsing_status_ != SafeBrowsingStatus::kChecking) {
+    base::UmaHistogramEnumeration("Settings.SafetyCheck.SafeBrowsingResult",
+                                  safe_browsing_status_);
+  }
+  FireBasicSafetyCheckWebUiListener(
+      kSafeBrowsingEvent, static_cast<int>(safe_browsing_status_),
+      GetStringForSafeBrowsing(safe_browsing_status_));
+  CompleteParentIfChildrenCompleted();
+}
+
 void SafetyCheckHandler::OnStateChanged(
     password_manager::BulkLeakCheckService::State state) {
   using password_manager::BulkLeakCheckService;
@@ -631,6 +614,8 @@ void SafetyCheckHandler::OnJavascriptDisallowed() {
   // Destroy the version updater to prevent getting a callback and firing a
   // WebUI event, which would cause a crash.
   version_updater_.reset();
+  // Stop observing safety check events.
+  safety_check_.reset(nullptr);
 }
 
 void SafetyCheckHandler::RegisterMessages() {

chrome/browser/ui/webui/settings/safety_check_handler.h

@@ -20,6 +20,7 @@
 #include "chrome/browser/ui/webui/help/version_updater.h"
 #include "chrome/browser/ui/webui/settings/settings_page_ui_handler.h"
 #include "components/password_manager/core/browser/bulk_leak_check_service.h"
+#include "components/safety_check/safety_check.h"
 #include "extensions/browser/extension_prefs.h"
 #include "extensions/browser/extension_registry.h"
 
@@ -28,7 +29,8 @@
 // software.
 class SafetyCheckHandler
     : public settings::SettingsPageUIHandler,
-      public password_manager::BulkLeakCheckService::Observer {
+      public password_manager::BulkLeakCheckService::Observer,
+      public safety_check::SafetyCheck::SafetyCheckHandlerInterface {
  public:
   // The following enum represent the state of the safety check parent
   // component and  should be kept in sync with the JS frontend
@@ -38,10 +40,12 @@ class SafetyCheckHandler
     kChecking = 1,
     kAfter = 2,
   };
+
   // The following enums represent the state of each component of the safety
   // check and should be kept in sync with the JS frontend
   // (safety_check_browser_proxy.js) and |SafetyCheck*| metrics enums in
   // enums.xml.
+  using SafeBrowsingStatus = safety_check::SafetyCheck::SafeBrowsingStatus;
   enum class UpdateStatus {
     kChecking = 0,
     kUpdated = 1,
@@ -56,18 +60,6 @@ class SafetyCheckHandler
     // New enum values must go above here.
     kMaxValue = kUnknown,
   };
-  enum class SafeBrowsingStatus {
-    kChecking = 0,
-    // Enabled is deprecated; kept not to break old UMA metrics (enums.xml).
-    kEnabled = 1,
-    kDisabled = 2,
-    kDisabledByAdmin = 3,
-    kDisabledByExtension = 4,
-    kEnabledStandard = 5,
-    kEnabledEnhanced = 6,
-    // New enum values must go above here.
-    kMaxValue = kEnabledEnhanced,
-  };
   enum class PasswordsStatus {
     kChecking = 0,
     kSafe = 1,
@@ -141,10 +133,6 @@ class SafetyCheckHandler
   // are available.
   void CheckUpdates();
 
-  // Gets the status of Safe Browsing from the PrefService and invokes
-  // OnSafeBrowsingCheckResult with results.
-  void CheckSafeBrowsing();
-
   // Triggers a bulk password leak check and invokes OnPasswordsCheckResult once
   // results are available.
   void CheckPasswords();
@@ -160,7 +148,6 @@ class SafetyCheckHandler
                            const std::string& version,
                            int64_t update_size,
                            const base::string16& message);
-  void OnSafeBrowsingCheckResult(SafeBrowsingStatus status);
   void OnPasswordsCheckResult(PasswordsStatus status,
                               Compromised compromised,
                               Done done,
@@ -193,6 +180,9 @@ class SafetyCheckHandler
       const std::vector<extensions::api::passwords_private::PasswordUiEntry>&
           passwords);
 
+  // SafetyCheck::SafetyCheckHandlerInterface implementation.
+  void OnSafeBrowsingCheckResult(SafeBrowsingStatus status) override;
+
   // BulkLeakCheckService::Observer implementation.
   void OnStateChanged(
       password_manager::BulkLeakCheckService::State state) override;
@@ -225,6 +215,8 @@ class SafetyCheckHandler
   // System time when safety check completed.
   base::Time safety_check_completion_time_;
 
+  std::unique_ptr<safety_check::SafetyCheck> safety_check_;
+
   std::unique_ptr<VersionUpdater> version_updater_;
   password_manager::BulkLeakCheckService* leak_service_ = nullptr;
   extensions::PasswordsPrivateDelegate* passwords_delegate_ = nullptr;

components/safety_check/BUILD.gn

+# Copyright 2020 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.
+
+source_set("safety_check") {
+  sources = [
+    "safety_check.cc",
+    "safety_check.h",
+  ]
+
+  deps = [
+    "//base",
+    "//components/prefs",
+    "//components/safe_browsing/core/common:safe_browsing_prefs",
+  ]
+}

components/safety_check/DEPS

+include_rules = [
+  "+components/prefs",
+  "+components/safe_browsing/core/common",
+]

components/safety_check/OWNERS

+# Chrome Privacy Team
+andzaytsev@google.com
+harrisonsean@chromium.org
+msramek@chromium.org
+rainhard@chromium.org
+
+# TEAM: chrome-privacy-core@google.com

components/safety_check/README.md

+Safety check
+
+This component contains code common to desktop, Android, and iOS for the
+settings page for checking browser safety. Currently, the only common check is
+Safe Browsing status. In the future, the password leak check will also be moved
+to this component from chrome/browser/ui/webui/settings/.

components/safety_check/safety_check.cc

+// Copyright 2020 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/safety_check/safety_check.h"
+
+#include "components/safe_browsing/core/common/safe_browsing_prefs.h"
+
+namespace safety_check {
+
+SafetyCheck::SafetyCheck(SafetyCheckHandlerInterface* handler)
+    : handler_(handler) {
+  DCHECK(handler_);
+}
+
+SafetyCheck::~SafetyCheck() = default;
+
+void SafetyCheck::CheckSafeBrowsing(PrefService* pref_service) {
+  const PrefService::Preference* enabled_pref =
+      pref_service->FindPreference(prefs::kSafeBrowsingEnabled);
+  bool enabled = pref_service->GetBoolean(prefs::kSafeBrowsingEnabled);
+  SafeBrowsingStatus status;
+  if (enabled && pref_service->GetBoolean(prefs::kSafeBrowsingEnhanced)) {
+    status = SafeBrowsingStatus::kEnabledEnhanced;
+  } else if (enabled) {
+    status = SafeBrowsingStatus::kEnabledStandard;
+  } else if (enabled_pref->IsManaged()) {
+    status = SafeBrowsingStatus::kDisabledByAdmin;
+  } else if (enabled_pref->IsExtensionControlled()) {
+    status = SafeBrowsingStatus::kDisabledByExtension;
+  } else {
+    status = SafeBrowsingStatus::kDisabled;
+  }
+  handler_->OnSafeBrowsingCheckResult(status);
+}
+
+}  // namespace safety_check

components/safety_check/safety_check.h

+// Copyright 2020 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_SAFETY_CHECK_SAFETY_CHECK_H_
+#define COMPONENTS_SAFETY_CHECK_SAFETY_CHECK_H_
+
+#include "base/observer_list.h"
+#include "base/observer_list_types.h"
+#include "components/prefs/pref_service.h"
+
+namespace safety_check {
+
+// Class for performing browser safety checks common to desktop, Android, and
+// iOS. Platform-specific checks, such as updates and extensions, are
+// implemented in handlers.
+class SafetyCheck {
+ public:
+  // The following enums represent the state of each component (common among
+  // desktop, Android, and iOS) of the safety check and should be kept in sync
+  // with the JS frontend (safety_check_browser_proxy.js) and |SafetyCheck*|
+  // metrics enums in enums.xml.
+  enum class SafeBrowsingStatus {
+    kChecking = 0,
+    kEnabled = 1,
+    kDisabled = 2,
+    kDisabledByAdmin = 3,
+    kDisabledByExtension = 4,
+    kEnabledStandard = 5,
+    kEnabledEnhanced = 6,
+    // New enum values must go above here.
+    kMaxValue = kEnabledEnhanced,
+  };
+
+  class SafetyCheckHandlerInterface {
+   public:
+    virtual void OnSafeBrowsingCheckResult(SafeBrowsingStatus status) = 0;
+  };
+
+  explicit SafetyCheck(SafetyCheckHandlerInterface* handler);
+  ~SafetyCheck();
+
+  // Gets the status of Safe Browsing from the PrefService and invokes
+  // OnSafeBrowsingCheckResult on each Observer with results.
+  void CheckSafeBrowsing(PrefService* pref_service);
+
+ private:
+  SafetyCheckHandlerInterface* handler_;
+};
+
+}  // namespace safety_check
+
+#endif  // COMPONENTS_SAFETY_CHECK_SAFETY_CHECK_H_