Move identity_manager into url_lookup_service.

Now that different profiles own their own url_lookup_service,
identity_manager can be a reference inside url_lookup_service. This can
make url_lookup_service more self-contained and reduce the number of
parameters pass into safe_browsing_url_checker.

Bug: 1050859
Change-Id: Ic5886460327d79df245b57e4ced967b14bf3b99a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2071359
Commit-Queue: Xinghui Lu <xinghuilu@chromium.org>
Reviewed-by: Varun Khaneja <vakh@chromium.org>
Reviewed-by: Bo <boliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#746083}

android_webview/browser/aw_content_browser_client.cc

@@ -766,10 +766,10 @@ AwContentBrowserClient::CreateURLLoaderThrottles(
           },
           base::Unretained(this)),
       wc_getter, frame_tree_node_id,
-      // TODO(crbug.com/1033760): identity_manager and rt_lookup_service are
+      // TODO(crbug.com/1033760): rt_lookup_service is
       // used to perform real time URL check, which is gated by UKM opted-in.
       // Since AW currently doesn't support UKM, this feature is not enabled.
-      /* identity_manager */ nullptr, /* rt_lookup_service */ nullptr));
+      /* rt_lookup_service */ nullptr));
 
   if (request.resource_type ==
       static_cast<int>(blink::mojom::ResourceType::kMainFrame)) {

chrome/browser/chrome_content_browser_client.cc

@@ -4226,14 +4226,6 @@ ChromeContentBrowserClient::CreateURLLoaderThrottles(
   bool matches_enterprise_whitelist = safe_browsing::IsURLWhitelistedByPolicy(
       request.url, *profile->GetPrefs());
   if (!matches_enterprise_whitelist) {
-    // |identity_manager| is used when real time url check with token is
-    // enabled.
-    signin::IdentityManager* identity_manager =
-        safe_browsing::RealTimePolicyEngine::CanPerformFullURLLookupWithToken(
-            profile)
-            ? IdentityManagerFactory::GetForProfile(profile)
-            : nullptr;
-
     // |url_lookup_service| is used when real time url check is enabled.
     safe_browsing::RealTimeUrlLookupService* url_lookup_service =
         // |safe_browsing_service_| may be unavailable in tests.
@@ -4249,7 +4241,7 @@ ChromeContentBrowserClient::CreateURLLoaderThrottles(
             &ChromeContentBrowserClient::GetSafeBrowsingUrlCheckerDelegate,
             base::Unretained(this),
             profile->GetPrefs()->GetBoolean(prefs::kSafeBrowsingEnabled)),
-        wc_getter, frame_tree_node_id, identity_manager,
+        wc_getter, frame_tree_node_id,
         url_lookup_service ? url_lookup_service->GetWeakPtr() : nullptr));
   }
 #endif

chrome/browser/safe_browsing/BUILD.gn

@@ -281,6 +281,7 @@ source_set("url_lookup_service_factory") {
   deps = [
     "//components/keyed_service/content",
     "//components/safe_browsing/core/realtime:url_lookup_service",
+    "//components/signin/public/identity_manager",
     "//content/public/browser",
   ]
 }

chrome/browser/safe_browsing/url_lookup_service_factory.cc

@@ -7,6 +7,7 @@
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/safe_browsing/safe_browsing_service.h"
+#include "chrome/browser/signin/identity_manager_factory.h"
 #include "components/keyed_service/content/browser_context_dependency_manager.h"
 #include "components/safe_browsing/core/realtime/url_lookup_service.h"
 #include "components/safe_browsing/core/verdict_cache_manager.h"
@@ -31,7 +32,9 @@ RealTimeUrlLookupServiceFactory::GetInstance() {
 RealTimeUrlLookupServiceFactory::RealTimeUrlLookupServiceFactory()
     : BrowserContextKeyedServiceFactory(
           "RealTimeUrlLookupService",
-          BrowserContextDependencyManager::GetInstance()) {}
+          BrowserContextDependencyManager::GetInstance()) {
+  DependsOn(IdentityManagerFactory::GetInstance());
+}
 
 KeyedService* RealTimeUrlLookupServiceFactory::BuildServiceInstanceFor(
     content::BrowserContext* context) const {
@@ -52,7 +55,7 @@ KeyedService* RealTimeUrlLookupServiceFactory::BuildServiceInstanceFor(
   DCHECK(cache_manager);
   return new RealTimeUrlLookupService(
       network::SharedURLLoaderFactory::Create(std::move(url_loader_factory)),
-      cache_manager);
+      cache_manager, IdentityManagerFactory::GetForProfile(profile));
 }
 
 }  // namespace safe_browsing

components/safe_browsing/content/browser/browser_url_loader_throttle.cc

@@ -13,7 +13,6 @@
 #include "components/safe_browsing/core/common/utils.h"
 #include "components/safe_browsing/core/realtime/policy_engine.h"
 #include "components/safe_browsing/core/realtime/url_lookup_service.h"
-#include "components/signin/public/identity_manager/identity_manager.h"
 #include "content/public/browser/browser_task_traits.h"
 #include "content/public/browser/web_contents.h"
 #include "net/log/net_log_event_type.h"
@@ -34,14 +33,12 @@ class BrowserURLLoaderThrottle::CheckerOnIO
       base::RepeatingCallback<content::WebContents*()> web_contents_getter,
       base::WeakPtr<BrowserURLLoaderThrottle> throttle,
       bool real_time_lookup_enabled,
-      signin::IdentityManager* identity_manager,
       base::WeakPtr<RealTimeUrlLookupService> url_lookup_service)
       : delegate_getter_(std::move(delegate_getter)),
         frame_tree_node_id_(frame_tree_node_id),
         web_contents_getter_(web_contents_getter),
         throttle_(std::move(throttle)),
         real_time_lookup_enabled_(real_time_lookup_enabled),
-        identity_manager_(identity_manager),
         url_lookup_service_(url_lookup_service) {}
 
   // Starts the initial safe browsing check. This check and future checks may be
@@ -72,7 +69,7 @@ class BrowserURLLoaderThrottle::CheckerOnIO
     url_checker_ = std::make_unique<SafeBrowsingUrlCheckerImpl>(
         headers, load_flags, resource_type, has_user_gesture,
         url_checker_delegate, web_contents_getter_, real_time_lookup_enabled_,
-        identity_manager_, url_lookup_service_);
+        url_lookup_service_);
 
     CheckUrl(url, method);
   }
@@ -139,7 +136,6 @@ class BrowserURLLoaderThrottle::CheckerOnIO
   bool skip_checks_ = false;
   base::WeakPtr<BrowserURLLoaderThrottle> throttle_;
   bool real_time_lookup_enabled_ = false;
-  signin::IdentityManager* identity_manager_;
   base::WeakPtr<RealTimeUrlLookupService> url_lookup_service_;
 };
 
@@ -148,19 +144,17 @@ std::unique_ptr<BrowserURLLoaderThrottle> BrowserURLLoaderThrottle::Create(
     GetDelegateCallback delegate_getter,
     const base::RepeatingCallback<content::WebContents*()>& web_contents_getter,
     int frame_tree_node_id,
-    signin::IdentityManager* identity_manager,
     base::WeakPtr<RealTimeUrlLookupService> url_lookup_service) {
   return base::WrapUnique<BrowserURLLoaderThrottle>(
       new BrowserURLLoaderThrottle(std::move(delegate_getter),
                                    web_contents_getter, frame_tree_node_id,
-                                   identity_manager, url_lookup_service));
+                                   url_lookup_service));
 }
 
 BrowserURLLoaderThrottle::BrowserURLLoaderThrottle(
     GetDelegateCallback delegate_getter,
     const base::RepeatingCallback<content::WebContents*()>& web_contents_getter,
     int frame_tree_node_id,
-    signin::IdentityManager* identity_manager,
     base::WeakPtr<RealTimeUrlLookupService> url_lookup_service) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
@@ -173,8 +167,7 @@ BrowserURLLoaderThrottle::BrowserURLLoaderThrottle(
 
   io_checker_ = std::make_unique<CheckerOnIO>(
       std::move(delegate_getter), frame_tree_node_id, web_contents_getter,
-      weak_factory_.GetWeakPtr(), real_time_lookup_enabled, identity_manager,
-      url_lookup_service);
+      weak_factory_.GetWeakPtr(), real_time_lookup_enabled, url_lookup_service);
 }
 
 BrowserURLLoaderThrottle::~BrowserURLLoaderThrottle() {

components/safe_browsing/content/browser/browser_url_loader_throttle.h

@@ -24,10 +24,6 @@ namespace net {
 class HttpRequestHeaders;
 }
 
-namespace signin {
-class IdentityManager;
-}
-
 namespace safe_browsing {
 
 class UrlCheckerDelegate;
@@ -53,7 +49,6 @@ class BrowserURLLoaderThrottle : public blink::URLLoaderThrottle {
       const base::RepeatingCallback<content::WebContents*()>&
           web_contents_getter,
       int frame_tree_node_id,
-      signin::IdentityManager* identity_manager,
       base::WeakPtr<RealTimeUrlLookupService> url_lookup_service);
 
   ~BrowserURLLoaderThrottle() override;
@@ -89,7 +84,6 @@ class BrowserURLLoaderThrottle : public blink::URLLoaderThrottle {
       const base::RepeatingCallback<content::WebContents*()>&
           web_contents_getter,
       int frame_tree_node_id,
-      signin::IdentityManager* identity_manager,
       base::WeakPtr<RealTimeUrlLookupService> url_lookup_service);
 
   // |slow_check| indicates whether it reports the result of a slow check.

components/safe_browsing/content/browser/mojo_safe_browsing_impl.cc

@@ -150,14 +150,13 @@ void MojoSafeBrowsingImpl::CreateCheckerAndCheck(
   // This is not called for frame resources, and real time URL checks currently
   // only support main frame resources. If we extend real time URL checks to
   // support non-main frames, we will need to provide the user preferences,
-  // identity_manager and url_lookup_service regarding real time lookup here.
+  // url_lookup_service regarding real time lookup here.
   auto checker_impl = std::make_unique<SafeBrowsingUrlCheckerImpl>(
       headers, static_cast<int>(load_flags), resource_type, has_user_gesture,
       delegate_,
       base::BindRepeating(&GetWebContentsFromID, render_process_id_,
                           static_cast<int>(render_frame_id)),
-      /*real_time_lookup_enabled=*/false, /*identity_manager=*/nullptr,
-      /* url_lookup_service */ nullptr);
+      /*real_time_lookup_enabled=*/false, /* url_lookup_service */ nullptr);
 
   checker_impl->CheckUrl(
       url, method,

components/safe_browsing/core/browser/BUILD.gn

@@ -22,7 +22,6 @@ jumbo_source_set("browser") {
     "//components/safe_browsing/core/realtime:url_lookup_service",
     "//components/safe_browsing/core/web_ui:constants",
     "//components/security_interstitials/core:unsafe_resource",
-    "//components/signin/public/identity_manager",
     "//net:extras",
     "//services/network/public/cpp",
   ]

components/safe_browsing/core/browser/safe_browsing_url_checker_impl.cc

@@ -17,7 +17,6 @@
 #include "components/safe_browsing/core/realtime/url_lookup_service.h"
 #include "components/safe_browsing/core/web_ui/constants.h"
 #include "components/security_interstitials/core/unsafe_resource.h"
-#include "components/signin/public/identity_manager/identity_manager.h"
 #include "mojo/public/cpp/bindings/pending_receiver.h"
 #include "net/base/load_flags.h"
 #include "net/http/http_request_headers.h"
@@ -102,7 +101,6 @@ SafeBrowsingUrlCheckerImpl::SafeBrowsingUrlCheckerImpl(
     scoped_refptr<UrlCheckerDelegate> url_checker_delegate,
     const base::RepeatingCallback<content::WebContents*()>& web_contents_getter,
     bool real_time_lookup_enabled,
-    signin::IdentityManager* identity_manager_on_ui,
     base::WeakPtr<RealTimeUrlLookupService> url_lookup_service_on_ui)
     : headers_(headers),
       load_flags_(load_flags),
@@ -112,7 +110,6 @@ SafeBrowsingUrlCheckerImpl::SafeBrowsingUrlCheckerImpl(
       url_checker_delegate_(std::move(url_checker_delegate)),
       database_manager_(url_checker_delegate_->GetDatabaseManager()),
       real_time_lookup_enabled_(real_time_lookup_enabled),
-      identity_manager_on_ui_(identity_manager_on_ui),
       url_lookup_service_on_ui_(url_lookup_service_on_ui) {}
 
 SafeBrowsingUrlCheckerImpl::~SafeBrowsingUrlCheckerImpl() {
@@ -445,7 +442,7 @@ void SafeBrowsingUrlCheckerImpl::OnCheckUrlForHighConfidenceAllowlist(
       FROM_HERE, CreateTaskTraits(ThreadID::UI),
       base::BindOnce(&SafeBrowsingUrlCheckerImpl::StartLookupOnUIThread,
                      weak_factory_.GetWeakPtr(), url, url_lookup_service_on_ui_,
-                     database_manager_, identity_manager_on_ui_));
+                     database_manager_));
 }
 
 void SafeBrowsingUrlCheckerImpl::SetWebUIToken(int token) {
@@ -457,8 +454,7 @@ void SafeBrowsingUrlCheckerImpl::StartLookupOnUIThread(
     base::WeakPtr<SafeBrowsingUrlCheckerImpl> weak_checker_on_io,
     const GURL& url,
     base::WeakPtr<RealTimeUrlLookupService> url_lookup_service_on_ui,
-    scoped_refptr<SafeBrowsingDatabaseManager> database_manager,
-    signin::IdentityManager* identity_manager) {
+    scoped_refptr<SafeBrowsingDatabaseManager> database_manager) {
   DCHECK(CurrentlyOnThread(ThreadID::UI));
   if (!url_lookup_service_on_ui ||
       !url_lookup_service_on_ui->CanCheckUrl(url) ||
@@ -477,8 +473,7 @@ void SafeBrowsingUrlCheckerImpl::StartLookupOnUIThread(
       &SafeBrowsingUrlCheckerImpl::OnRTLookupResponse, weak_checker_on_io);
 
   url_lookup_service_on_ui->StartLookup(url, std::move(request_callback),
-                                        std::move(response_callback),
-                                        identity_manager);
+                                        std::move(response_callback));
 }
 
 void SafeBrowsingUrlCheckerImpl::PerformHashBasedCheck(const GURL& url) {

components/safe_browsing/core/browser/safe_browsing_url_checker_impl.h

@@ -28,10 +28,6 @@ namespace content {
 class WebContents;
 }
 
-namespace signin {
-class IdentityManager;
-}
-
 namespace safe_browsing {
 
 enum class ResourceType;
@@ -88,7 +84,6 @@ class SafeBrowsingUrlCheckerImpl : public mojom::SafeBrowsingUrlChecker,
       const base::RepeatingCallback<content::WebContents*()>&
           web_contents_getter,
       bool real_time_lookup_enabled,
-      signin::IdentityManager* identity_manager_on_ui,
       base::WeakPtr<RealTimeUrlLookupService> url_lookup_service_on_ui);
 
   ~SafeBrowsingUrlCheckerImpl() override;
@@ -175,8 +170,7 @@ class SafeBrowsingUrlCheckerImpl : public mojom::SafeBrowsingUrlChecker,
       base::WeakPtr<SafeBrowsingUrlCheckerImpl> weak_checker_on_io,
       const GURL& url,
       base::WeakPtr<RealTimeUrlLookupService> url_lookup_service_on_ui,
-      scoped_refptr<SafeBrowsingDatabaseManager> database_manager,
-      signin::IdentityManager* identity_manager);
+      scoped_refptr<SafeBrowsingDatabaseManager> database_manager);
 
   // Called when the |request| from the real-time lookup service is sent.
   void OnRTLookupRequest(std::unique_ptr<RTLookupRequest> request);
@@ -245,10 +239,6 @@ class SafeBrowsingUrlCheckerImpl : public mojom::SafeBrowsingUrlChecker,
   // there is at least one check that needs to be cancelled.
   bool browse_url_check_sent_ = false;
 
-  // This object is used to obtain access token when real time url check with
-  // token is enabled. Can only be accessed in UI thread.
-  signin::IdentityManager* identity_manager_on_ui_;
-
   // This object is used to perform real time url check. Can only be accessed in
   // UI thread.
   base::WeakPtr<RealTimeUrlLookupService> url_lookup_service_on_ui_;

components/safe_browsing/core/realtime/BUILD.gn

@@ -58,6 +58,7 @@ if (!is_ios) {
       "//components/safe_browsing/core/common",
       "//components/safe_browsing/core/common:safe_browsing_prefs",
       "//components/safe_browsing/core/common:test_support",
+      "//components/signin/public/identity_manager:test_support",
       "//components/sync_preferences:test_support",
       "//components/unified_consent",
       "//components/user_prefs",

components/safe_browsing/core/realtime/url_lookup_service.cc

@@ -51,14 +51,19 @@ GURL SanitizeURL(const GURL& url) {
 
 RealTimeUrlLookupService::RealTimeUrlLookupService(
     scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
-    VerdictCacheManager* cache_manager)
-    : url_loader_factory_(url_loader_factory), cache_manager_(cache_manager) {}
+    VerdictCacheManager* cache_manager,
+    signin::IdentityManager* identity_manager)
+    : url_loader_factory_(url_loader_factory),
+      cache_manager_(cache_manager),
+      identity_manager_(identity_manager) {
+  DCHECK(cache_manager_);
+  DCHECK(identity_manager_);
+}
 
 void RealTimeUrlLookupService::StartLookup(
     const GURL& url,
     RTLookupRequestCallback request_callback,
-    RTLookupResponseCallback response_callback,
-    signin::IdentityManager* identity_manager) {
+    RTLookupResponseCallback response_callback) {
   DCHECK(CurrentlyOnThread(ThreadID::UI));
   DCHECK(url.is_valid());
 

components/safe_browsing/core/realtime/url_lookup_service.h

@@ -44,7 +44,8 @@ class RealTimeUrlLookupService : public KeyedService {
  public:
   RealTimeUrlLookupService(
       scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
-      VerdictCacheManager* cache_manager);
+      VerdictCacheManager* cache_manager,
+      signin::IdentityManager* identity_manager);
   ~RealTimeUrlLookupService() override;
 
   // Returns true if |url|'s scheme can be checked.
@@ -55,8 +56,6 @@ class RealTimeUrlLookupService : public KeyedService {
   // local hash-based method.
   bool IsInBackoffMode() const;
 
-  // TODO(crbug.com/1041912): |identity_manager_on_ui| is unused. It will
-  // be used to obtain access token in a follow up CL.
   // Start the full URL lookup for |url|, call |request_callback| on the same
   // thread when request is sent, call |response_callback| on the same thread
   // when response is received.
@@ -64,8 +63,7 @@ class RealTimeUrlLookupService : public KeyedService {
   // cache for |url|.
   void StartLookup(const GURL& url,
                    RTLookupRequestCallback request_callback,
-                   RTLookupResponseCallback response_callback,
-                   signin::IdentityManager* identity_manager_on_ui);
+                   RTLookupResponseCallback response_callback);
 
   // KeyedService:
   // Called before the actual deletion of the object.
@@ -148,6 +146,10 @@ class RealTimeUrlLookupService : public KeyedService {
   // Unowned object used for getting and storing real time url check cache.
   VerdictCacheManager* cache_manager_;
 
+  // Unowned object used for getting access token when real time url check with
+  // token is enabled.
+  signin::IdentityManager* identity_manager_;
+
   friend class RealTimeUrlLookupServiceTest;
 
   base::WeakPtrFactory<RealTimeUrlLookupService> weak_factory_{this};

components/safe_browsing/core/realtime/url_lookup_service_unittest.cc

@@ -8,6 +8,7 @@
 #include "components/content_settings/core/browser/host_content_settings_map.h"
 #include "components/safe_browsing/core/common/test_task_environment.h"
 #include "components/safe_browsing/core/verdict_cache_manager.h"
+#include "components/signin/public/identity_manager/identity_test_environment.h"
 #include "components/sync_preferences/testing_pref_service_syncable.h"
 #include "services/network/public/cpp/shared_url_loader_factory.h"
 #include "services/network/public/cpp/weak_wrapper_shared_url_loader_factory.h"
@@ -35,8 +36,10 @@ class RealTimeUrlLookupServiceTest : public PlatformTest {
     cache_manager_ = std::make_unique<VerdictCacheManager>(
         nullptr, content_setting_map_.get());
 
+    signin::IdentityTestEnvironment identity_test_env;
     rt_service_ = std::make_unique<RealTimeUrlLookupService>(
-        test_shared_loader_factory_, cache_manager_.get());
+        test_shared_loader_factory_, cache_manager_.get(),
+        identity_test_env.identity_manager());
   }
 
   void TearDown() override {

weblayer/browser/safe_browsing/safe_browsing_service.cc

@@ -81,10 +81,10 @@ SafeBrowsingService::CreateURLLoaderThrottle(
           },
           base::Unretained(this)),
       wc_getter, frame_tree_node_id,
-      // identity_manager and rt_lookup_service are used to
+      // rt_lookup_service are used to
       // perform real time url check, which is gated by UKM opted in. Since
       // WebLayer currently doesn't support UKM, this feature is not enabled.
-      /*identity_manager*/ nullptr, /*rt_lookup_service*/ nullptr);
+      /*rt_lookup_service*/ nullptr);
 }
 
 std::unique_ptr<content::NavigationThrottle>