Revert "Move ChromePasswordProtectionService to be a KeyedService."

This reverts commit 596bbbfe.

Reason for revert: Check failed: checker.CalledOnValidSequence() during destruction.

TBR=drubery@chromium.org

Original change's description:
> Move ChromePasswordProtectionService to be a KeyedService.
>
> Bug: 1058431
> Change-Id: Id831530b726e63cfcba71dbcb86af8bf00e11dd0
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2198536
> Reviewed-by: Daniel Rubery <drubery@chromium.org>
> Commit-Queue: Xinghui Lu <xinghuilu@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#769913}

TBR=drubery@chromium.org,xinghuilu@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 1058431,1085536
Change-Id: I1047ec0f415a4504d13edb52f8809d6c946073a3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2212817
Commit-Queue: Xinghui Lu <xinghuilu@chromium.org>
Reviewed-by: Xinghui Lu <xinghuilu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#771236}

chrome/browser/safe_browsing/BUILD.gn

@@ -74,8 +74,6 @@ static_library("safe_browsing") {
       "certificate_reporting_service_factory.h",
       "chrome_password_protection_service.cc",
       "chrome_password_protection_service.h",
-      "chrome_password_protection_service_factory.cc",
-      "chrome_password_protection_service_factory.h",
       "delayed_warning_navigation_throttle.cc",
       "delayed_warning_navigation_throttle.h",
       "safe_browsing_blocking_page.cc",

chrome/browser/safe_browsing/chrome_password_protection_service.cc

@@ -294,13 +294,11 @@ void ChromePasswordProtectionService::Init() {
 #endif
 }
 
-void ChromePasswordProtectionService::Shutdown() {
+ChromePasswordProtectionService::~ChromePasswordProtectionService() {
   if (pref_change_registrar_)
     pref_change_registrar_->RemoveAll();
 }
 
-ChromePasswordProtectionService::~ChromePasswordProtectionService() = default;
-
 // static
 ChromePasswordProtectionService*
 ChromePasswordProtectionService::GetPasswordProtectionService(

chrome/browser/safe_browsing/chrome_password_protection_service.h

@@ -15,7 +15,6 @@
 #include "chrome/browser/password_manager/password_store_factory.h"
 #include "chrome/browser/security_events/security_event_recorder.h"
 #include "chrome/browser/security_events/security_event_recorder_factory.h"
-#include "components/keyed_service/core/keyed_service.h"
 #include "components/password_manager/core/browser/hash_password_manager.h"
 #include "components/password_manager/core/browser/password_manager_metrics_util.h"
 #include "components/password_manager/core/browser/password_reuse_detector.h"
@@ -74,8 +73,7 @@ MaybeCreateNavigationThrottle(content::NavigationHandle* navigation_handle);
 
 // ChromePasswordProtectionService extends PasswordProtectionService by adding
 // access to SafeBrowsingNaivigationObserverManager and Profile.
-class ChromePasswordProtectionService : public PasswordProtectionService,
-                                        public KeyedService {
+class ChromePasswordProtectionService : public PasswordProtectionService {
  public:
   // Observer is used to coordinate password protection UIs (e.g. modal warning,
   // change password card, etc) in reaction to user events.
@@ -285,10 +283,6 @@ class ChromePasswordProtectionService : public PasswordProtectionService,
   // Gets |account_info_| based on |profile_|.
   AccountInfo GetAccountInfo() const override;
 
-  // KeyedService:
-  // Called before the actual deletion of the object.
-  void Shutdown() override;
-
  protected:
   // PasswordProtectionService overrides.
   const policy::BrowserPolicyConnector* GetBrowserPolicyConnector()

chrome/browser/safe_browsing/chrome_password_protection_service_browsertest.cc

@@ -737,7 +737,6 @@ IN_PROC_BROWSER_TEST_F(ChromePasswordProtectionServiceBrowserTest,
 
 IN_PROC_BROWSER_TEST_F(ChromePasswordProtectionServiceBrowserTest,
                        OnEnterpriseTriggerOffGSuite) {
-  GetService(/*is_incognito=*/false);  // Create a service to listen to events.
   ConfigureEnterprisePasswordProtection(
       /*is_gsuite=*/true, PasswordProtectionTrigger::PHISHING_REUSE);
   Profile* profile = browser()->profile();
@@ -763,7 +762,6 @@ IN_PROC_BROWSER_TEST_F(ChromePasswordProtectionServiceBrowserTest,
 
 IN_PROC_BROWSER_TEST_F(ChromePasswordProtectionServiceBrowserTest,
                        OnEnterpriseTriggerOff) {
-  GetService(/*is_incognito=*/false);  // Create a service to listen to events.
   ConfigureEnterprisePasswordProtection(
       /*is_gsuite=*/false, PasswordProtectionTrigger::PHISHING_REUSE);
   Profile* profile = browser()->profile();

chrome/browser/safe_browsing/chrome_password_protection_service_factory.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 "chrome/browser/safe_browsing/chrome_password_protection_service_factory.h"
-
-#include "chrome/browser/browser_process.h"
-#include "chrome/browser/profiles/incognito_helpers.h"
-#include "chrome/browser/profiles/profile.h"
-#include "chrome/browser/safe_browsing/safe_browsing_service.h"
-#include "components/keyed_service/content/browser_context_dependency_manager.h"
-#include "content/public/browser/browser_context.h"
-
-namespace safe_browsing {
-
-// static
-ChromePasswordProtectionService*
-ChromePasswordProtectionServiceFactory::GetForProfile(Profile* profile) {
-  return static_cast<ChromePasswordProtectionService*>(
-      GetInstance()->GetServiceForBrowserContext(profile, /* create= */ true));
-}
-
-// static
-ChromePasswordProtectionServiceFactory*
-ChromePasswordProtectionServiceFactory::GetInstance() {
-  return base::Singleton<ChromePasswordProtectionServiceFactory>::get();
-}
-
-ChromePasswordProtectionServiceFactory::ChromePasswordProtectionServiceFactory()
-    : BrowserContextKeyedServiceFactory(
-          "ChromePasswordProtectionService",
-          BrowserContextDependencyManager::GetInstance()) {}
-
-KeyedService* ChromePasswordProtectionServiceFactory::BuildServiceInstanceFor(
-    content::BrowserContext* context) const {
-  if (!g_browser_process->safe_browsing_service()) {
-    return nullptr;
-  }
-  Profile* profile = Profile::FromBrowserContext(context);
-  if (!IsSafeBrowsingEnabled(*profile->GetPrefs())) {
-    return nullptr;
-  }
-  return new ChromePasswordProtectionService(
-      g_browser_process->safe_browsing_service(), profile);
-}
-
-content::BrowserContext*
-ChromePasswordProtectionServiceFactory::GetBrowserContextToUse(
-    content::BrowserContext* context) const {
-  return chrome::GetBrowserContextOwnInstanceInIncognito(context);
-}
-
-}  // namespace safe_browsing

chrome/browser/safe_browsing/chrome_password_protection_service_factory.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 CHROME_BROWSER_SAFE_BROWSING_CHROME_PASSWORD_PROTECTION_SERVICE_FACTORY_H_
-#define CHROME_BROWSER_SAFE_BROWSING_CHROME_PASSWORD_PROTECTION_SERVICE_FACTORY_H_
-
-#include "base/memory/singleton.h"
-#include "components/keyed_service/content/browser_context_keyed_service_factory.h"
-
-class KeyedService;
-class Profile;
-
-namespace content {
-class BrowserContext;
-}
-
-namespace safe_browsing {
-
-class ChromePasswordProtectionService;
-
-// Singleton that owns ChromePasswordProtectionService objects, one for each
-// active Profile. It listens to profile destroy events and destroy its
-// associated service. It returns a separate instance if the profile is in the
-// Incognito mode.
-class ChromePasswordProtectionServiceFactory
-    : public BrowserContextKeyedServiceFactory {
- public:
-  // Creates the service if it doesn't exist already for the given |profile|.
-  // If the service already exists, return its pointer.
-  static ChromePasswordProtectionService* GetForProfile(Profile* profile);
-
-  // Get the singleton instance.
-  static ChromePasswordProtectionServiceFactory* GetInstance();
-
- private:
-  friend struct base::DefaultSingletonTraits<
-      ChromePasswordProtectionServiceFactory>;
-
-  ChromePasswordProtectionServiceFactory();
-  ~ChromePasswordProtectionServiceFactory() override = default;
-
-  // BrowserContextKeyedServiceFactory:
-  KeyedService* BuildServiceInstanceFor(
-      content::BrowserContext* context) const override;
-  content::BrowserContext* GetBrowserContextToUse(
-      content::BrowserContext* context) const override;
-
-  DISALLOW_COPY_AND_ASSIGN(ChromePasswordProtectionServiceFactory);
-};
-
-}  // namespace safe_browsing
-#endif  // CHROME_BROWSER_SAFE_BROWSING_CHROME_PASSWORD_PROTECTION_SERVICE_FACTORY_H_

chrome/browser/safe_browsing/safe_browsing_service.cc

@@ -28,7 +28,6 @@
 #include "chrome/browser/net/system_network_context_manager.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
-#include "chrome/browser/safe_browsing/chrome_password_protection_service_factory.h"
 #include "chrome/browser/safe_browsing/safe_browsing_navigation_observer_manager.h"
 #include "chrome/browser/safe_browsing/services_delegate.h"
 #include "chrome/browser/safe_browsing/ui_manager.h"
@@ -223,7 +222,9 @@ TriggerManager* SafeBrowsingService::trigger_manager() const {
 
 PasswordProtectionService* SafeBrowsingService::GetPasswordProtectionService(
     Profile* profile) const {
-  return ChromePasswordProtectionServiceFactory::GetForProfile(profile);
+  if (IsSafeBrowsingEnabled(*profile->GetPrefs()))
+    return services_delegate_->GetPasswordProtectionService(profile);
+  return nullptr;
 }
 
 std::unique_ptr<prefs::mojom::TrackedPreferenceValidationDelegate>
@@ -375,11 +376,13 @@ void SafeBrowsingService::OnOffTheRecordProfileCreated(
 
 void SafeBrowsingService::OnProfileWillBeDestroyed(Profile* profile) {
   observed_profiles_.Remove(profile);
+  services_delegate_->RemovePasswordProtectionService(profile);
   services_delegate_->RemoveTelemetryService(profile);
   services_delegate_->RemoveSafeBrowsingNetworkContext(profile);
 }
 
 void SafeBrowsingService::CreateServicesForProfile(Profile* profile) {
+  services_delegate_->CreatePasswordProtectionService(profile);
   services_delegate_->CreateTelemetryService(profile);
   services_delegate_->CreateSafeBrowsingNetworkContext(profile);
   observed_profiles_.Add(profile);

chrome/browser/safe_browsing/services_delegate.cc

@@ -41,7 +41,36 @@ ServicesDelegate::~ServicesDelegate() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 }
 
+void ServicesDelegate::CreatePasswordProtectionService(Profile* profile) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  DCHECK(profile);
+  auto it = password_protection_service_map_.find(profile);
+  DCHECK(it == password_protection_service_map_.end());
+  auto service = std::make_unique<ChromePasswordProtectionService>(
+      safe_browsing_service_, profile);
+  password_protection_service_map_[profile] = std::move(service);
+}
+
+void ServicesDelegate::RemovePasswordProtectionService(Profile* profile) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  DCHECK(profile);
+  auto it = password_protection_service_map_.find(profile);
+  if (it != password_protection_service_map_.end())
+    password_protection_service_map_.erase(it);
+}
+
+PasswordProtectionService* ServicesDelegate::GetPasswordProtectionService(
+    Profile* profile) const {
+  DCHECK(profile);
+  auto it = password_protection_service_map_.find(profile);
+  return it != password_protection_service_map_.end() ? it->second.get()
+                                                      : nullptr;
+}
+
 void ServicesDelegate::ShutdownServices() {
+  // Delete the ChromePasswordProtectionService instances.
+  password_protection_service_map_.clear();
+
   // Delete the NetworkContexts and associated ProxyConfigMonitors
   network_context_map_.clear();
   proxy_config_monitor_map_.clear();

chrome/browser/safe_browsing/services_delegate.h

@@ -12,6 +12,7 @@
 #include "build/build_config.h"
 #include "chrome/browser/safe_browsing/chrome_password_protection_service.h"
 #include "chrome/browser/safe_browsing/incident_reporting/delayed_analysis_callback.h"
+#include "components/safe_browsing/content/password_protection/password_protection_service.h"
 #include "services/network/public/mojom/network_context.mojom.h"
 
 class Profile;
@@ -37,6 +38,7 @@ namespace safe_browsing {
 class DownloadProtectionService;
 #endif
 class IncidentReportingService;
+class PasswordProtectionService;
 class ResourceRequestDetector;
 struct ResourceRequestInfo;
 class SafeBrowsingService;
@@ -121,6 +123,11 @@ class ServicesDelegate {
       const V4ProtocolConfig& v4_config) = 0;
   virtual void StopOnIOThread(bool shutdown) = 0;
 
+  void CreatePasswordProtectionService(Profile* profile);
+  void RemovePasswordProtectionService(Profile* profile);
+  PasswordProtectionService* GetPasswordProtectionService(
+      Profile* profile) const;
+
   virtual void CreateTelemetryService(Profile* profile) {}
   virtual void RemoveTelemetryService(Profile* profile) {}
 
@@ -141,6 +148,12 @@ class ServicesDelegate {
 
   std::unique_ptr<ProxyConfigMonitor> proxy_config_monitor_;
 
+  // Tracks existing Profiles, and their corresponding
+  // ChromePasswordProtectionService instances.
+  // Accessed on UI thread.
+  base::flat_map<Profile*, std::unique_ptr<ChromePasswordProtectionService>>
+      password_protection_service_map_;
+
   // Tracks existing Profiles, and their corresponding
   // SafeBrowsingNetworkContexts. Accessed on UI thread.
   base::flat_map<Profile*, std::unique_ptr<SafeBrowsingNetworkContext>>