[Safe Browsing] Make SafeBrowsingTokenFetcher an interface

This CL prepares for the upcoming usage of Gaia-keyed URL lookups in
Weblayer by turning SafeBrowsingTokenFetcher into an interface and its
implementation into a SafeBrowsingPrimaryAccountTokenFetcher subclass.
This layer of abstraction is needed as WebLayer will not fetch access
tokens via IdentityManager as the current implementation does.

A followup CL will abstract the remaining dependences on
//components/signin from safe_browsing_token_fetcher.h.

Bug: 1080748
Change-Id: Ib9c0cb63a3e1364611c21a3cc069bab7bdba0e4f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2613968
Commit-Queue: Colin Blundell <blundell@chromium.org>
Reviewed-by: Xinghui Lu <xinghuilu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#841435}

chrome/browser/safe_browsing/download_protection/check_client_download_request_base.cc

@@ -170,8 +170,8 @@ CheckClientDownloadRequestBase::CheckClientDownloadRequestBase(
     signin::IdentityManager* identity_manager =
         IdentityManagerFactory::GetForProfile(profile);
     if (!profile->IsOffTheRecord() && identity_manager) {
-      token_fetcher_ =
-          std::make_unique<SafeBrowsingTokenFetcher>(identity_manager);
+      token_fetcher_ = std::make_unique<SafeBrowsingPrimaryAccountTokenFetcher>(
+          identity_manager);
     }
   }
 }

chrome/browser/safe_browsing/download_protection/check_client_download_request_base.h

@@ -26,7 +26,7 @@
 #include "chrome/services/file_util/public/cpp/sandboxed_rar_analyzer.h"
 #include "chrome/services/file_util/public/cpp/sandboxed_zip_analyzer.h"
 #include "components/history/core/browser/history_service.h"
-#include "components/safe_browsing/core/browser/safe_browsing_token_fetcher.h"
+#include "components/safe_browsing/core/browser/sync/safe_browsing_primary_account_token_fetcher.h"
 #include "components/safe_browsing/core/db/database_manager.h"
 #include "content/public/browser/browser_thread.h"
 #include "url/gurl.h"

chrome/test/BUILD.gn

@@ -5761,7 +5761,7 @@ test("unit_tests") {
       "//components/safe_browsing/content/triggers:ad_redirect_trigger",
       "//components/safe_browsing/core:ping_manager_unittest",
       "//components/safe_browsing/core/browser:safe_browsing_url_checker_unittest",
-      "//components/safe_browsing/core/browser:token_fetcher_unittest",
+      "//components/safe_browsing/core/browser/sync:unittests",
       "//components/safe_browsing/core/db:v4_test_util",
     ]
   } else if (safe_browsing_mode == 2 && is_android) {

components/safe_browsing/core/browser/BUILD.gn

@@ -77,28 +77,10 @@ source_set("referrer_chain_provider") {
 }
 
 source_set("token_fetcher") {
-  sources = [
-    "safe_browsing_token_fetcher.cc",
-    "safe_browsing_token_fetcher.h",
-  ]
+  sources = [ "safe_browsing_token_fetcher.h" ]
 
   deps = [
     "//base",
-    "//components/safe_browsing/core/common:thread_utils",
     "//components/signin/public/identity_manager",
-    "//google_apis",
-  ]
-}
-
-source_set("token_fetcher_unittest") {
-  testonly = true
-  sources = [ "safe_browsing_token_fetcher_unittest.cc" ]
-
-  deps = [
-    ":token_fetcher",
-    "//base/test:test_support",
-    "//components/safe_browsing/core/common:test_support",
-    "//components/signin/public/identity_manager:test_support",
-    "//testing/gtest",
   ]
 }

components/safe_browsing/core/browser/safe_browsing_token_fetcher.h

@@ -8,62 +8,26 @@
 #include <memory>
 
 #include "base/callback.h"
-#include "base/containers/flat_map.h"
-#include "base/memory/weak_ptr.h"
 #include "base/optional.h"
 #include "components/signin/public/identity_manager/access_token_info.h"
 #include "components/signin/public/identity_manager/consent_level.h"
-#include "google_apis/gaia/google_service_auth_error.h"
-
-namespace signin {
-class IdentityManager;
-class AccessTokenFetcher;
-}  // namespace signin
 
 namespace safe_browsing {
 
-// This class is used to fetch access tokens for communcations with Safe
-// Browsing. It asynchronously returns the access token for the current
-// primary account, or nullopt if an error occurred. This must be
-// run on the UI thread.
+// This interface is used to fetch access tokens for communcations with Safe
+// Browsing. It asynchronously returns an access token for the current account
+// (as determined in concrete implementations), or nullopt if an error occurred.
+// This must be run on the UI thread.
 class SafeBrowsingTokenFetcher {
  public:
   using Callback =
       base::OnceCallback<void(base::Optional<signin::AccessTokenInfo>)>;
 
-  // Create a SafeBrowsingTokenFetcher for the primary account of
-  // |identity_manager|. |identity_manager| is unowned, and must outlive this
-  // object.
-  explicit SafeBrowsingTokenFetcher(signin::IdentityManager* identity_manager);
-
-  ~SafeBrowsingTokenFetcher();
+  virtual ~SafeBrowsingTokenFetcher() = default;
 
   // Begin fetching a token for the account with the given |consent_level|. The
   // result will be returned in |callback|. Must be called on the UI thread.
-  void Start(signin::ConsentLevel consent_level, Callback callback);
-
- private:
-  void OnTokenFetched(int request_id,
-                      GoogleServiceAuthError error,
-                      signin::AccessTokenInfo access_token_info);
-  void OnTokenTimeout(int request_id);
-  void Finish(int request_id,
-              base::Optional<signin::AccessTokenInfo> token_info);
-
-  // Reference to the identity manager to fetch from.
-  signin::IdentityManager* identity_manager_;
-
-  // The count of requests sent. This is used as an ID for requests.
-  int requests_sent_;
-
-  // Active fetchers, keyed by ID.
-  base::flat_map<int, std::unique_ptr<signin::AccessTokenFetcher>>
-      token_fetchers_;
-
-  // Active callbacks, keyed by ID.
-  base::flat_map<int, Callback> callbacks_;
-
-  base::WeakPtrFactory<SafeBrowsingTokenFetcher> weak_ptr_factory_;
+  virtual void Start(signin::ConsentLevel consent_level, Callback callback) = 0;
 };
 
 }  // namespace safe_browsing

components/safe_browsing/core/browser/sync/BUILD.gn

+# Copyright 2021 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.
+
+import("//build/config/features.gni")
+
+source_set("sync") {
+  sources = [
+    "safe_browsing_primary_account_token_fetcher.cc",
+    "safe_browsing_primary_account_token_fetcher.h",
+  ]
+
+  deps = [
+    "//base",
+    "//components/safe_browsing/core/browser:token_fetcher",
+    "//components/safe_browsing/core/common:thread_utils",
+    "//components/signin/public/identity_manager",
+    "//google_apis",
+  ]
+}
+
+source_set("unittests") {
+  testonly = true
+  sources = [ "safe_browsing_primary_account_token_fetcher_unittest.cc" ]
+
+  deps = [
+    ":sync",
+    "//base/test:test_support",
+    "//components/safe_browsing/core/common:test_support",
+    "//components/signin/public/identity_manager:test_support",
+    "//testing/gtest",
+  ]
+}

components/safe_browsing/core/browser/sync/README.md

+Holds touchpoints for safe_browsing's integration with signin and sync.

components/safe_browsing/core/browser/safe_browsing_token_fetcher.cc → components/safe_browsing/core/browser/sync/safe_browsing_primary_account_token_fetcher.cc

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "components/safe_browsing/core/browser/safe_browsing_token_fetcher.h"
+#include "components/safe_browsing/core/browser/sync/safe_browsing_primary_account_token_fetcher.h"
 
 #include "base/bind.h"
 #include "base/memory/weak_ptr.h"
@@ -33,7 +33,7 @@ const int kTimeoutDelayFromMilliseconds = 1000;
 
 }  // namespace
 
-SafeBrowsingTokenFetcher::SafeBrowsingTokenFetcher(
+SafeBrowsingPrimaryAccountTokenFetcher::SafeBrowsingPrimaryAccountTokenFetcher(
     signin::IdentityManager* identity_manager)
     : identity_manager_(identity_manager),
       requests_sent_(0),
@@ -41,14 +41,16 @@ SafeBrowsingTokenFetcher::SafeBrowsingTokenFetcher(
   DCHECK(CurrentlyOnThread(ThreadID::UI));
 }
 
-SafeBrowsingTokenFetcher::~SafeBrowsingTokenFetcher() {
+SafeBrowsingPrimaryAccountTokenFetcher::
+    ~SafeBrowsingPrimaryAccountTokenFetcher() {
   for (auto& id_and_callback : callbacks_) {
     std::move(id_and_callback.second).Run(base::nullopt);
   }
 }
 
-void SafeBrowsingTokenFetcher::Start(signin::ConsentLevel consent_level,
-                                     Callback callback) {
+void SafeBrowsingPrimaryAccountTokenFetcher::Start(
+    signin::ConsentLevel consent_level,
+    Callback callback) {
   DCHECK(CurrentlyOnThread(ThreadID::UI));
   const int request_id = requests_sent_;
   requests_sent_++;
@@ -58,17 +60,18 @@ void SafeBrowsingTokenFetcher::Start(signin::ConsentLevel consent_level,
   token_fetchers_[request_id] =
       identity_manager_->CreateAccessTokenFetcherForAccount(
           account_id, "safe_browsing_service", {kAPIScope},
-          base::BindOnce(&SafeBrowsingTokenFetcher::OnTokenFetched,
-                         weak_ptr_factory_.GetWeakPtr(), request_id),
+          base::BindOnce(
+              &SafeBrowsingPrimaryAccountTokenFetcher::OnTokenFetched,
+              weak_ptr_factory_.GetWeakPtr(), request_id),
           signin::AccessTokenFetcher::Mode::kImmediate);
   base::PostDelayedTask(
       FROM_HERE, CreateTaskTraits(ThreadID::UI),
-      base::BindOnce(&SafeBrowsingTokenFetcher::OnTokenTimeout,
+      base::BindOnce(&SafeBrowsingPrimaryAccountTokenFetcher::OnTokenTimeout,
                      weak_ptr_factory_.GetWeakPtr(), request_id),
       base::TimeDelta::FromMilliseconds(kTimeoutDelayFromMilliseconds));
 }
 
-void SafeBrowsingTokenFetcher::OnTokenFetched(
+void SafeBrowsingPrimaryAccountTokenFetcher::OnTokenFetched(
     int request_id,
     GoogleServiceAuthError error,
     signin::AccessTokenInfo access_token_info) {
@@ -80,11 +83,11 @@ void SafeBrowsingTokenFetcher::OnTokenFetched(
     Finish(request_id, base::nullopt);
 }
 
-void SafeBrowsingTokenFetcher::OnTokenTimeout(int request_id) {
+void SafeBrowsingPrimaryAccountTokenFetcher::OnTokenTimeout(int request_id) {
   Finish(request_id, base::nullopt);
 }
 
-void SafeBrowsingTokenFetcher::Finish(
+void SafeBrowsingPrimaryAccountTokenFetcher::Finish(
     int request_id,
     base::Optional<signin::AccessTokenInfo> token_info) {
   if (callbacks_.contains(request_id)) {

components/safe_browsing/core/browser/sync/safe_browsing_primary_account_token_fetcher.h

+// Copyright 2021 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_SAFE_BROWSING_CORE_BROWSER_SYNC_SAFE_BROWSING_PRIMARY_ACCOUNT_TOKEN_FETCHER_H_
+#define COMPONENTS_SAFE_BROWSING_CORE_BROWSER_SYNC_SAFE_BROWSING_PRIMARY_ACCOUNT_TOKEN_FETCHER_H_
+
+#include <memory>
+
+#include "base/containers/flat_map.h"
+#include "base/memory/weak_ptr.h"
+#include "components/safe_browsing/core/browser/safe_browsing_token_fetcher.h"
+#include "google_apis/gaia/google_service_auth_error.h"
+
+namespace signin {
+class AccessTokenFetcher;
+class IdentityManager;
+}  // namespace signin
+
+namespace safe_browsing {
+
+// This class fetches access tokens for Safe Browsing for the current
+// primary account.
+class SafeBrowsingPrimaryAccountTokenFetcher : public SafeBrowsingTokenFetcher {
+ public:
+  // Create a SafeBrowsingPrimaryAccountTokenFetcher for the primary account of
+  // |identity_manager|. |identity_manager| is unowned, and must outlive this
+  // object.
+  explicit SafeBrowsingPrimaryAccountTokenFetcher(
+      signin::IdentityManager* identity_manager);
+
+  ~SafeBrowsingPrimaryAccountTokenFetcher() override;
+
+  // SafeBrowsingTokenFetcher:
+  void Start(signin::ConsentLevel consent_level, Callback callback) override;
+
+ private:
+  void OnTokenFetched(int request_id,
+                      GoogleServiceAuthError error,
+                      signin::AccessTokenInfo access_token_info);
+  void OnTokenTimeout(int request_id);
+  void Finish(int request_id,
+              base::Optional<signin::AccessTokenInfo> token_info);
+
+  // Reference to the identity manager to fetch from.
+  signin::IdentityManager* identity_manager_;
+
+  // The count of requests sent. This is used as an ID for requests.
+  int requests_sent_;
+
+  // Active fetchers, keyed by ID.
+  base::flat_map<int, std::unique_ptr<signin::AccessTokenFetcher>>
+      token_fetchers_;
+
+  // Active callbacks, keyed by ID.
+  base::flat_map<int, Callback> callbacks_;
+
+  base::WeakPtrFactory<SafeBrowsingPrimaryAccountTokenFetcher>
+      weak_ptr_factory_;
+};
+
+}  // namespace safe_browsing
+
+#endif  // COMPONENTS_SAFE_BROWSING_CORE_BROWSER_SYNC_SAFE_BROWSING_PRIMARY_ACCOUNT_TOKEN_FETCHER_H_

components/safe_browsing/core/browser/safe_browsing_token_fetcher_unittest.cc → components/safe_browsing/core/browser/sync/safe_browsing_primary_account_token_fetcher_unittest.cc

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "components/safe_browsing/core/browser/safe_browsing_token_fetcher.h"
+#include "components/safe_browsing/core/browser/sync/safe_browsing_primary_account_token_fetcher.h"
 #include <memory>
 
 #include "base/run_loop.h"
@@ -13,9 +13,9 @@
 
 namespace safe_browsing {
 
-class SafeBrowsingTokenFetcherTest : public ::testing::Test {
+class SafeBrowsingPrimaryAccountTokenFetcherTest : public ::testing::Test {
  public:
-  SafeBrowsingTokenFetcherTest()
+  SafeBrowsingPrimaryAccountTokenFetcherTest()
       : task_environment_(CreateTestTaskEnvironment()) {}
 
  protected:
@@ -23,11 +23,11 @@ class SafeBrowsingTokenFetcherTest : public ::testing::Test {
   signin::IdentityTestEnvironment identity_test_environment_;
 };
 
-TEST_F(SafeBrowsingTokenFetcherTest, Success) {
+TEST_F(SafeBrowsingPrimaryAccountTokenFetcherTest, Success) {
   identity_test_environment_.MakeUnconsentedPrimaryAccountAvailable(
       "test@example.com");
   base::Optional<signin::AccessTokenInfo> maybe_account_info;
-  SafeBrowsingTokenFetcher fetcher(
+  SafeBrowsingPrimaryAccountTokenFetcher fetcher(
       identity_test_environment_.identity_manager());
   fetcher.Start(signin::ConsentLevel::kNotRequired,
                 base::BindOnce(
@@ -43,11 +43,11 @@ TEST_F(SafeBrowsingTokenFetcherTest, Success) {
   EXPECT_EQ(maybe_account_info.value().token, "token");
 }
 
-TEST_F(SafeBrowsingTokenFetcherTest, Failure) {
+TEST_F(SafeBrowsingPrimaryAccountTokenFetcherTest, Failure) {
   identity_test_environment_.MakeUnconsentedPrimaryAccountAvailable(
       "test@example.com");
   base::Optional<signin::AccessTokenInfo> maybe_account_info;
-  SafeBrowsingTokenFetcher fetcher(
+  SafeBrowsingPrimaryAccountTokenFetcher fetcher(
       identity_test_environment_.identity_manager());
   fetcher.Start(signin::ConsentLevel::kNotRequired,
                 base::BindOnce(
@@ -62,11 +62,11 @@ TEST_F(SafeBrowsingTokenFetcherTest, Failure) {
   ASSERT_FALSE(maybe_account_info.has_value());
 }
 
-TEST_F(SafeBrowsingTokenFetcherTest, NoSyncingAccount) {
+TEST_F(SafeBrowsingPrimaryAccountTokenFetcherTest, NoSyncingAccount) {
   identity_test_environment_.MakeUnconsentedPrimaryAccountAvailable(
       "test@example.com");
   base::Optional<signin::AccessTokenInfo> maybe_account_info;
-  SafeBrowsingTokenFetcher fetcher(
+  SafeBrowsingPrimaryAccountTokenFetcher fetcher(
       identity_test_environment_.identity_manager());
   fetcher.Start(signin::ConsentLevel::kSync,
                 base::BindOnce(
@@ -81,10 +81,10 @@ TEST_F(SafeBrowsingTokenFetcherTest, NoSyncingAccount) {
   ASSERT_FALSE(maybe_account_info.has_value());
 }
 
-TEST_F(SafeBrowsingTokenFetcherTest, SyncSuccess) {
+TEST_F(SafeBrowsingPrimaryAccountTokenFetcherTest, SyncSuccess) {
   identity_test_environment_.MakePrimaryAccountAvailable("test@example.com");
   base::Optional<signin::AccessTokenInfo> maybe_account_info;
-  SafeBrowsingTokenFetcher fetcher(
+  SafeBrowsingPrimaryAccountTokenFetcher fetcher(
       identity_test_environment_.identity_manager());
   fetcher.Start(signin::ConsentLevel::kSync,
                 base::BindOnce(

components/safe_browsing/core/realtime/BUILD.gn

@@ -38,7 +38,7 @@ static_library("url_lookup_service") {
     "//components/safe_browsing/core:csd_proto",
     "//components/safe_browsing/core:realtimeapi_proto",
     "//components/safe_browsing/core:verdict_cache_manager",
-    "//components/safe_browsing/core/browser:token_fetcher",
+    "//components/safe_browsing/core/browser/sync",
     "//components/safe_browsing/core/common:safe_browsing_prefs",
     "//components/safe_browsing/core/common:thread_utils",
     "//components/safe_browsing/core/db:v4_protocol_manager_util",

components/safe_browsing/core/realtime/url_lookup_service.cc

@@ -13,7 +13,7 @@
 #include "base/time/time.h"
 #include "components/prefs/pref_service.h"
 #include "components/safe_browsing/buildflags.h"
-#include "components/safe_browsing/core/browser/safe_browsing_token_fetcher.h"
+#include "components/safe_browsing/core/browser/sync/safe_browsing_primary_account_token_fetcher.h"
 #include "components/safe_browsing/core/common/safe_browsing_prefs.h"
 #include "components/safe_browsing/core/common/thread_utils.h"
 #include "components/safe_browsing/core/db/v4_protocol_manager_util.h"
@@ -54,8 +54,8 @@ RealTimeUrlLookupService::RealTimeUrlLookupService(
       pref_service_(pref_service),
       is_off_the_record_(is_off_the_record),
       variations_(variations_service) {
-  token_fetcher_ =
-      std::make_unique<SafeBrowsingTokenFetcher>(identity_manager_);
+  token_fetcher_ = std::make_unique<SafeBrowsingPrimaryAccountTokenFetcher>(
+      identity_manager_);
 }
 
 void RealTimeUrlLookupService::GetAccessToken(