Remove content/ dependency in policy_engine.

To make policy_engine build on iOS, replace browser_context with
pref_service and is_off_the_record.


Bug: 1050859,1028755
Change-Id: I343b1b9f83dc1e69cdb5304d0b64f062120e844c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2071159
Commit-Queue: Xinghui Lu <xinghuilu@chromium.org>
Reviewed-by: Varun Khaneja <vakh@chromium.org>
Reviewed-by: Ali Juma <ajuma@chromium.org>
Cr-Commit-Position: refs/heads/master@{#746122}

chrome/browser/chrome_content_browser_client.cc

@@ -4231,7 +4231,7 @@ ChromeContentBrowserClient::CreateURLLoaderThrottles(
         // |safe_browsing_service_| may be unavailable in tests.
         safe_browsing_service_ &&
                 safe_browsing::RealTimePolicyEngine::CanPerformFullURLLookup(
-                    profile)
+                    profile->GetPrefs(), profile->IsOffTheRecord())
             ? safe_browsing::RealTimeUrlLookupServiceFactory::GetForProfile(
                   profile)
             : nullptr;

chrome/browser/safe_browsing/url_lookup_service_factory.cc

@@ -55,7 +55,8 @@ KeyedService* RealTimeUrlLookupServiceFactory::BuildServiceInstanceFor(
   DCHECK(cache_manager);
   return new RealTimeUrlLookupService(
       network::SharedURLLoaderFactory::Create(std::move(url_loader_factory)),
-      cache_manager, IdentityManagerFactory::GetForProfile(profile));
+      cache_manager, IdentityManagerFactory::GetForProfile(profile),
+      profile->GetPrefs(), profile->IsOffTheRecord());
 }
 
 }  // namespace safe_browsing

components/safe_browsing/content/browser/browser_url_loader_throttle.cc

@@ -159,11 +159,9 @@ BrowserURLLoaderThrottle::BrowserURLLoaderThrottle(
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
   // Decide whether to do real time URL lookups or not.
-  content::WebContents* web_contents = web_contents_getter.Run();
   bool real_time_lookup_enabled =
-      web_contents ? RealTimePolicyEngine::CanPerformFullURLLookup(
-                         web_contents->GetBrowserContext())
-                   : false;
+      url_lookup_service ? url_lookup_service->CanPerformFullURLLookup()
+                         : false;
 
   io_checker_ = std::make_unique<CheckerOnIO>(
       std::move(delegate_getter), frame_tree_node_id, web_contents_getter,

components/safe_browsing/core/browser/BUILD.gn

@@ -19,6 +19,7 @@ jumbo_source_set("browser") {
     "//components/safe_browsing/core/common:thread_utils",
     "//components/safe_browsing/core/db:database_manager",
     "//components/safe_browsing/core/db:util",
+    "//components/safe_browsing/core/realtime:policy_engine",
     "//components/safe_browsing/core/realtime:url_lookup_service",
     "//components/safe_browsing/core/web_ui:constants",
     "//components/security_interstitials/core:unsafe_resource",
@@ -29,10 +30,7 @@ jumbo_source_set("browser") {
     sources += [ "//components/safe_browsing/ios/browser/safe_browsing_url_checker_impl_ios.cc" ]
   } else {
     sources += [ "//components/safe_browsing/content/browser/safe_browsing_url_checker_impl_content.cc" ]
-    deps += [
-      "//components/safe_browsing/content/web_ui",
-      "//components/safe_browsing/core/realtime:policy_engine",
-    ]
+    deps += [ "//components/safe_browsing/content/web_ui" ]
   }
 }
 

components/safe_browsing/core/realtime/BUILD.gn

@@ -9,7 +9,9 @@ static_library("url_lookup_service") {
   ]
 
   deps = [
+    ":policy_engine",
     "//base:base",
+    "//components/prefs",
     "//components/safe_browsing/core:realtimeapi_proto",
     "//components/safe_browsing/core:verdict_cache_manager",
     "//components/safe_browsing/core/common:thread_utils",
@@ -20,52 +22,46 @@ static_library("url_lookup_service") {
   ]
 }
 
-# TODO(crbug.com/1028755): Remove this condition once there are no longer any
-# dependencies on content/.
-if (!is_ios) {
-  static_library("policy_engine") {
-    sources = [
-      "policy_engine.cc",
-      "policy_engine.h",
-    ]
+static_library("policy_engine") {
+  sources = [
+    "policy_engine.cc",
+    "policy_engine.h",
+  ]
 
-    deps = [
-      "//base:base",
-      "//components/prefs",
-      "//components/safe_browsing/core:features",
-      "//components/safe_browsing/core:realtimeapi_proto",
-      "//components/safe_browsing/core/common",
-      "//components/safe_browsing/core/common:safe_browsing_prefs",
-      "//components/unified_consent",
-      "//components/user_prefs",
-      "//content/public/browser",
-    ]
-  }
+  deps = [
+    "//base:base",
+    "//components/prefs",
+    "//components/safe_browsing/core:features",
+    "//components/safe_browsing/core:realtimeapi_proto",
+    "//components/safe_browsing/core/common",
+    "//components/safe_browsing/core/common:safe_browsing_prefs",
+    "//components/unified_consent",
+    "//components/user_prefs",
+  ]
+}
 
-  source_set("unit_tests") {
-    testonly = true
-    sources = [
-      "policy_engine_unittest.cc",
-      "url_lookup_service_unittest.cc",
-    ]
-    deps = [
-      ":policy_engine",
-      ":url_lookup_service",
-      "//base/test:test_support",
-      "//components/content_settings/core/browser",
-      "//components/safe_browsing/core:features",
-      "//components/safe_browsing/core:verdict_cache_manager",
-      "//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",
-      "//content/test:test_support",
-      "//services/network:test_support",
-      "//services/network/public/cpp:cpp",
-      "//testing/gtest",
-    ]
-  }
+source_set("unit_tests") {
+  testonly = true
+  sources = [
+    "policy_engine_unittest.cc",
+    "url_lookup_service_unittest.cc",
+  ]
+  deps = [
+    ":policy_engine",
+    ":url_lookup_service",
+    "//base/test:test_support",
+    "//components/content_settings/core/browser",
+    "//components/safe_browsing/core:features",
+    "//components/safe_browsing/core:verdict_cache_manager",
+    "//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",
+    "//services/network:test_support",
+    "//services/network/public/cpp:cpp",
+    "//testing/gtest",
+  ]
 }

components/safe_browsing/core/realtime/policy_engine.cc

@@ -13,7 +13,6 @@
 #include "components/safe_browsing/core/features.h"
 #include "components/unified_consent/pref_names.h"
 #include "components/user_prefs/user_prefs.h"
-#include "content/public/browser/browser_context.h"
 
 #if defined(OS_ANDROID)
 #include "base/metrics/field_trial_params.h"
@@ -44,35 +43,34 @@ bool RealTimePolicyEngine::IsUrlLookupEnabled() {
 }
 
 // static
-bool RealTimePolicyEngine::IsUserOptedIn(
-    content::BrowserContext* browser_context) {
-  PrefService* pref_service = user_prefs::UserPrefs::Get(browser_context);
+bool RealTimePolicyEngine::IsUserOptedIn(PrefService* pref_service) {
   return pref_service->GetBoolean(
       unified_consent::prefs::kUrlKeyedAnonymizedDataCollectionEnabled);
 }
 
 // static
-bool RealTimePolicyEngine::IsEnabledByPolicy(
-    content::BrowserContext* browser_context) {
+// TODO(crbug.com/1050859): Remove this method.
+bool RealTimePolicyEngine::IsEnabledByPolicy() {
   return false;
 }
 
 // static
-bool RealTimePolicyEngine::CanPerformFullURLLookup(
-    content::BrowserContext* browser_context) {
-  if (browser_context->IsOffTheRecord())
+bool RealTimePolicyEngine::CanPerformFullURLLookup(PrefService* pref_service,
+                                                   bool is_off_the_record) {
+  if (is_off_the_record)
     return false;
 
-  if (IsEnabledByPolicy(browser_context))
+  if (IsEnabledByPolicy())
     return true;
 
-  return IsUrlLookupEnabled() && IsUserOptedIn(browser_context);
+  return IsUrlLookupEnabled() && IsUserOptedIn(pref_service);
 }
 
 // static
 bool RealTimePolicyEngine::CanPerformFullURLLookupWithToken(
-    content::BrowserContext* browser_context) {
-  if (!CanPerformFullURLLookup(browser_context)) {
+    PrefService* pref_service,
+    bool is_off_the_record) {
+  if (!CanPerformFullURLLookup(pref_service, is_off_the_record)) {
     return false;
   }
 

components/safe_browsing/core/realtime/policy_engine.h

@@ -7,9 +7,7 @@
 
 #include "build/build_config.h"
 
-namespace content {
-class BrowserContext;
-}
+class PrefService;
 
 namespace safe_browsing {
 
@@ -39,13 +37,14 @@ class RealTimePolicyEngine {
 
   // Return true if the feature to enable full URL lookups is enabled and the
   // allowlist fetch is enabled for the profile represented by
-  // |browser_context|.
-  static bool CanPerformFullURLLookup(content::BrowserContext* browser_context);
+  // |pref_service|.
+  static bool CanPerformFullURLLookup(PrefService* pref_service,
+                                      bool is_off_the_record);
 
   // Return true if the OAuth token should be associated with the URL lookup
   // pings.
-  static bool CanPerformFullURLLookupWithToken(
-      content::BrowserContext* browser_context);
+  static bool CanPerformFullURLLookupWithToken(PrefService* pref_service,
+                                               bool is_off_the_record);
 
   friend class SafeBrowsingService;
   friend class SafeBrowsingUIHandler;
@@ -55,10 +54,10 @@ class RealTimePolicyEngine {
   static bool IsUrlLookupEnabled();
 
   // Is user opted-in to the feature?
-  static bool IsUserOptedIn(content::BrowserContext* browser_context);
+  static bool IsUserOptedIn(PrefService* pref_service);
 
   // Is the feature enabled due to enterprise policy?
-  static bool IsEnabledByPolicy(content::BrowserContext* browser_context);
+  static bool IsEnabledByPolicy();
 
   friend class RealTimePolicyEngineTest;
 };  // class RealTimePolicyEngine

components/safe_browsing/core/realtime/policy_engine_unittest.cc

@@ -14,7 +14,6 @@
 #include "components/unified_consent/pref_names.h"
 #include "components/unified_consent/unified_consent_service.h"
 #include "components/user_prefs/user_prefs.h"
-#include "content/public/test/test_browser_context.h"
 #include "testing/platform_test.h"
 
 #if defined(OS_ANDROID)
@@ -29,22 +28,21 @@ class RealTimePolicyEngineTest : public PlatformTest {
   RealTimePolicyEngineTest() : task_environment_(CreateTestTaskEnvironment()) {}
 
   void SetUp() override {
-    user_prefs::UserPrefs::Set(&test_context_, &pref_service_);
     RegisterProfilePrefs(pref_service_.registry());
     unified_consent::UnifiedConsentService::RegisterPrefs(
         pref_service_.registry());
   }
 
   bool IsUserOptedIn() {
-    return RealTimePolicyEngine::IsUserOptedIn(&test_context_);
+    return RealTimePolicyEngine::IsUserOptedIn(&pref_service_);
   }
 
-  bool CanPerformFullURLLookup() {
-    return RealTimePolicyEngine::CanPerformFullURLLookup(&test_context_);
+  bool CanPerformFullURLLookup(bool is_off_the_record) {
+    return RealTimePolicyEngine::CanPerformFullURLLookup(&pref_service_,
+                                                         is_off_the_record);
   }
 
   std::unique_ptr<base::test::TaskEnvironment> task_environment_;
-  content::TestBrowserContext test_context_;
   sync_preferences::TestingPrefServiceSyncable pref_service_;
 };
 
@@ -64,7 +62,7 @@ TEST_F(RealTimePolicyEngineTest, TestCanPerformFullURLLookup_LargeMemorySize) {
   pref_service_.SetUserPref(
       unified_consent::prefs::kUrlKeyedAnonymizedDataCollectionEnabled,
       std::make_unique<base::Value>(true));
-  EXPECT_TRUE(CanPerformFullURLLookup());
+  EXPECT_TRUE(CanPerformFullURLLookup(/* is_off_the_record */ false));
 }
 
 TEST_F(RealTimePolicyEngineTest, TestCanPerformFullURLLookup_SmallMemorySize) {
@@ -80,7 +78,7 @@ TEST_F(RealTimePolicyEngineTest, TestCanPerformFullURLLookup_SmallMemorySize) {
   pref_service_.SetUserPref(
       unified_consent::prefs::kUrlKeyedAnonymizedDataCollectionEnabled,
       std::make_unique<base::Value>(true));
-  EXPECT_FALSE(CanPerformFullURLLookup());
+  EXPECT_FALSE(CanPerformFullURLLookup(/* is_off_the_record */ false));
 }
 
 TEST_F(RealTimePolicyEngineTest,
@@ -89,7 +87,7 @@ TEST_F(RealTimePolicyEngineTest,
   feature_list.InitWithFeaturesAndParameters(
       /* enabled_features */ {},
       /* disabled_features */ {kRealTimeUrlLookupEnabled});
-  EXPECT_FALSE(CanPerformFullURLLookup());
+  EXPECT_FALSE(CanPerformFullURLLookup(/* is_off_the_record */ false));
 }
 #endif  // defined(OS_ANDROID)
 
@@ -99,14 +97,14 @@ TEST_F(RealTimePolicyEngineTest, TestCanPerformFullURLLookup_EnabledByPolicy) {
                                std::make_unique<base::Value>(true));
   // Verifies that setting the pref still doesn't enable the feature.
   // See crbug.com/1030815 for details.
-  EXPECT_FALSE(CanPerformFullURLLookup());
+  EXPECT_FALSE(CanPerformFullURLLookup(/* is_off_the_record */ false));
 }
 
 TEST_F(RealTimePolicyEngineTest,
        TestCanPerformFullURLLookup_DisabledUrlLookup) {
   base::test::ScopedFeatureList feature_list;
   feature_list.InitAndDisableFeature(kRealTimeUrlLookupEnabled);
-  EXPECT_FALSE(CanPerformFullURLLookup());
+  EXPECT_FALSE(CanPerformFullURLLookup(/* is_off_the_record */ false));
 }
 
 TEST_F(RealTimePolicyEngineTest,
@@ -114,8 +112,7 @@ TEST_F(RealTimePolicyEngineTest,
   base::test::ScopedFeatureList feature_list;
   pref_service_.SetManagedPref(prefs::kSafeBrowsingRealTimeLookupEnabled,
                                std::make_unique<base::Value>(true));
-  test_context_.set_is_off_the_record(true);
-  EXPECT_FALSE(CanPerformFullURLLookup());
+  EXPECT_FALSE(CanPerformFullURLLookup(/* is_off_the_record */ true));
 }
 
 TEST_F(RealTimePolicyEngineTest,

components/safe_browsing/core/realtime/url_lookup_service.cc

@@ -10,8 +10,10 @@
 #include "base/strings/string_piece.h"
 #include "base/task/post_task.h"
 #include "base/time/time.h"
+#include "components/prefs/pref_service.h"
 #include "components/safe_browsing/core/common/thread_utils.h"
 #include "components/safe_browsing/core/db/v4_protocol_manager_util.h"
+#include "components/safe_browsing/core/realtime/policy_engine.h"
 #include "components/safe_browsing/core/verdict_cache_manager.h"
 #include "components/signin/public/identity_manager/identity_manager.h"
 #include "net/base/ip_address.h"
@@ -52,12 +54,17 @@ GURL SanitizeURL(const GURL& url) {
 RealTimeUrlLookupService::RealTimeUrlLookupService(
     scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
     VerdictCacheManager* cache_manager,
-    signin::IdentityManager* identity_manager)
+    signin::IdentityManager* identity_manager,
+    PrefService* pref_service,
+    bool is_off_the_record)
     : url_loader_factory_(url_loader_factory),
       cache_manager_(cache_manager),
-      identity_manager_(identity_manager) {
+      identity_manager_(identity_manager),
+      pref_service_(pref_service),
+      is_off_the_record_(is_off_the_record) {
   DCHECK(cache_manager_);
   DCHECK(identity_manager_);
+  DCHECK(pref_service_);
 }
 
 void RealTimeUrlLookupService::StartLookup(
@@ -324,6 +331,11 @@ void RealTimeUrlLookupService::ResetFailures() {
   backoff_timer_.Stop();
 }
 
+bool RealTimeUrlLookupService::CanPerformFullURLLookup() const {
+  return RealTimePolicyEngine::CanPerformFullURLLookup(pref_service_,
+                                                       is_off_the_record_);
+}
+
 // static
 SBThreatType RealTimeUrlLookupService::GetSBThreatTypeForRTThreatType(
     RTLookupResponse::ThreatInfo::ThreatType rt_threat_type) {

components/safe_browsing/core/realtime/url_lookup_service.h

@@ -27,6 +27,8 @@ namespace signin {
 class IdentityManager;
 }
 
+class PrefService;
+
 namespace safe_browsing {
 
 using RTLookupRequestCallback =
@@ -45,12 +47,19 @@ class RealTimeUrlLookupService : public KeyedService {
   RealTimeUrlLookupService(
       scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
       VerdictCacheManager* cache_manager,
-      signin::IdentityManager* identity_manager);
+      signin::IdentityManager* identity_manager,
+      PrefService* pref_service,
+      bool is_off_the_record);
   ~RealTimeUrlLookupService() override;
 
   // Returns true if |url|'s scheme can be checked.
   bool CanCheckUrl(const GURL& url) const;
 
+  // Returns true if real time URL lookup is enabled. The check is based on
+  // pref settings of the associated profile, whether the profile is an off the
+  // record profile and the finch flag.
+  bool CanPerformFullURLLookup() const;
+
   // Returns true if the real time lookups are currently in backoff mode due to
   // too many prior errors. If this happens, the checking falls back to
   // local hash-based method.
@@ -150,6 +159,13 @@ class RealTimeUrlLookupService : public KeyedService {
   // token is enabled.
   signin::IdentityManager* identity_manager_;
 
+  // Unowned object used for getting preference settings.
+  PrefService* pref_service_;
+
+  // A boolean indicates whether the profile associated with this
+  // |url_lookup_service| is an off the record profile.
+  bool is_off_the_record_;
+
   friend class RealTimeUrlLookupServiceTest;
 
   base::WeakPtrFactory<RealTimeUrlLookupService> weak_factory_{this};

components/safe_browsing/core/realtime/url_lookup_service_unittest.cc

@@ -39,7 +39,8 @@ class RealTimeUrlLookupServiceTest : public PlatformTest {
     signin::IdentityTestEnvironment identity_test_env;
     rt_service_ = std::make_unique<RealTimeUrlLookupService>(
         test_shared_loader_factory_, cache_manager_.get(),
-        identity_test_env.identity_manager());
+        identity_test_env.identity_manager(), &test_pref_service_,
+        /* is_off_the_record */ false);
   }
 
   void TearDown() override {

components/safe_browsing/ios/BUILD.gn

@@ -13,5 +13,6 @@ source_set("unit_tests") {
     "//components/safe_browsing/core/common",
     "//components/safe_browsing/core/common:unit_tests",
     "//components/safe_browsing/core/db:unit_tests_local_db",
+    "//components/safe_browsing/core/realtime:unit_tests",
   ]
 }