Move real-time lookup logic to safe_browsing_url_checker_impl.cc

This CL moves real-time lookup logic from
safe_browsing_url_checker_impl_content.cc to
safe_browsing_url_checker_impl.cc, so it can be used on iOS.
Only WebUI-related logic remains in the content-specific
file.

This CL also makes SafeBrowsingUrlCheckerTests run on iOS.

Bug: 1103218
Change-Id: I9ee4eda4b90d10d75890e213c13e244607e1523c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2288205Reviewed-by: Varun Khaneja <vakh@chromium.org>
Commit-Queue: Ali Juma <ajuma@chromium.org>
Cr-Commit-Position: refs/heads/master@{#786868}

components/safe_browsing/content/browser/safe_browsing_url_checker_impl_content.cc

@@ -5,29 +5,17 @@
 #include "components/safe_browsing/core/browser/safe_browsing_url_checker_impl.h"
 
 #include "base/bind.h"
-#include "base/metrics/histogram_functions.h"
-#include "base/metrics/histogram_macros.h"
-#include "base/metrics/histogram_macros_local.h"
 #include "base/task/post_task.h"
 #include "components/safe_browsing/content/web_ui/safe_browsing_ui.h"
 #include "components/safe_browsing/core/common/safebrowsing_constants.h"
 #include "components/safe_browsing/core/common/thread_utils.h"
-#include "components/safe_browsing/core/realtime/policy_engine.h"
-#include "components/safe_browsing/core/realtime/url_lookup_service_base.h"
 #include "components/safe_browsing/core/web_ui/constants.h"
 
 namespace safe_browsing {
 
-bool SafeBrowsingUrlCheckerImpl::CanPerformFullURLLookup(const GURL& url) {
-  return real_time_lookup_enabled_ &&
-         RealTimePolicyEngine::CanPerformFullURLLookupForResourceType(
-             resource_type_, can_rt_check_subresource_url_) &&
-         RealTimeUrlLookupServiceBase::CanCheckUrl(url);
-}
-
-void SafeBrowsingUrlCheckerImpl::OnRTLookupRequest(
-    std::unique_ptr<RTLookupRequest> request,
-    std::string oauth_token) {
+void SafeBrowsingUrlCheckerImpl::LogRTLookupRequest(
+    const RTLookupRequest& request,
+    const std::string& oauth_token) {
   DCHECK(CurrentlyOnThread(ThreadID::IO));
 
   // The following is to log this RTLookupRequest on any open
@@ -36,27 +24,14 @@ void SafeBrowsingUrlCheckerImpl::OnRTLookupRequest(
       FROM_HERE, CreateTaskTraits(ThreadID::UI),
       base::BindOnce(&WebUIInfoSingleton::AddToRTLookupPings,
                      base::Unretained(WebUIInfoSingleton::GetInstance()),
-                     *request, oauth_token),
+                     request, oauth_token),
       base::BindOnce(&SafeBrowsingUrlCheckerImpl::SetWebUIToken,
                      weak_factory_.GetWeakPtr()));
 }
 
-void SafeBrowsingUrlCheckerImpl::OnRTLookupResponse(
-    bool is_rt_lookup_successful,
-    std::unique_ptr<RTLookupResponse> response) {
+void SafeBrowsingUrlCheckerImpl::LogRTLookupResponse(
+    const RTLookupResponse& response) {
   DCHECK(CurrentlyOnThread(ThreadID::IO));
-  bool is_expected_resource_type =
-      (ResourceType::kMainFrame == resource_type_) ||
-      ((ResourceType::kSubFrame == resource_type_) &&
-       can_rt_check_subresource_url_);
-  DCHECK(is_expected_resource_type);
-
-  const GURL& url = urls_[next_index_].url;
-
-  if (!is_rt_lookup_successful) {
-    PerformHashBasedCheck(url);
-    return;
-  }
 
   if (url_web_ui_token_ != -1) {
     // The following is to log this RTLookupResponse on any open
@@ -65,21 +40,8 @@ void SafeBrowsingUrlCheckerImpl::OnRTLookupResponse(
         FROM_HERE, CreateTaskTraits(ThreadID::UI),
         base::BindOnce(&WebUIInfoSingleton::AddToRTLookupResponses,
                        base::Unretained(WebUIInfoSingleton::GetInstance()),
-                       url_web_ui_token_, *response));
-  }
-
-  SBThreatType sb_threat_type = SB_THREAT_TYPE_SAFE;
-  if (response && (response->threat_info_size() > 0) &&
-      response->threat_info(0).verdict_type() ==
-          RTLookupResponse::ThreatInfo::DANGEROUS) {
-    // TODO(crbug.com/1033692): Only take the first threat info into account
-    // because threat infos are returned in decreasing order of severity.
-    // Consider extend it to support multiple threat types.
-    sb_threat_type =
-        RealTimeUrlLookupServiceBase::GetSBThreatTypeForRTThreatType(
-            response->threat_info(0).threat_type());
+                       url_web_ui_token_, response));
   }
-  OnUrlResult(url, sb_threat_type, ThreatMetadata());
 }
 
 }  // namespace safe_browsing

components/safe_browsing/core/browser/BUILD.gn

@@ -50,23 +50,23 @@ if (!is_ios) {
       "//net:extras",
     ]
   }
+}
 
-  source_set("safe_browsing_url_checker_unittest") {
-    testonly = true
-    sources = [ "safe_browsing_url_checker_impl_unittest.cc" ]
-    deps = [
-      ":browser",
-      "//base/test:test_support",
-      "//components/safe_browsing/core:csd_proto",
-      "//components/safe_browsing/core/common:test_support",
-      "//components/safe_browsing/core/common:thread_utils",
-      "//components/safe_browsing/core/db:test_database_manager",
-      "//components/safe_browsing/core/db:util",
-      "//components/safe_browsing/core/realtime:url_lookup_service",
-      "//content/public/browser:browser",
-      "//testing/gtest",
-    ]
-  }
+source_set("safe_browsing_url_checker_unittest") {
+  testonly = true
+  sources = [ "safe_browsing_url_checker_impl_unittest.cc" ]
+  deps = [
+    ":browser",
+    "//base/test:test_support",
+    "//components/safe_browsing/core:csd_proto",
+    "//components/safe_browsing/core/common:test_support",
+    "//components/safe_browsing/core/common:thread_utils",
+    "//components/safe_browsing/core/db:test_database_manager",
+    "//components/safe_browsing/core/db:util",
+    "//components/safe_browsing/core/realtime:url_lookup_service",
+    "//services/network/public/cpp",
+    "//testing/gtest",
+  ]
 }
 
 source_set("referrer_chain_provider") {

components/safe_browsing/core/browser/safe_browsing_url_checker_impl.cc

@@ -566,4 +566,52 @@ void SafeBrowsingUrlCheckerImpl::PerformHashBasedCheck(const GURL& url) {
   }
 }
 
+bool SafeBrowsingUrlCheckerImpl::CanPerformFullURLLookup(const GURL& url) {
+  return real_time_lookup_enabled_ &&
+         RealTimePolicyEngine::CanPerformFullURLLookupForResourceType(
+             resource_type_, can_rt_check_subresource_url_) &&
+         RealTimeUrlLookupServiceBase::CanCheckUrl(url);
+}
+
+void SafeBrowsingUrlCheckerImpl::OnRTLookupRequest(
+    std::unique_ptr<RTLookupRequest> request,
+    std::string oauth_token) {
+  DCHECK(CurrentlyOnThread(ThreadID::IO));
+
+  LogRTLookupRequest(*request, oauth_token);
+}
+
+void SafeBrowsingUrlCheckerImpl::OnRTLookupResponse(
+    bool is_rt_lookup_successful,
+    std::unique_ptr<RTLookupResponse> response) {
+  DCHECK(CurrentlyOnThread(ThreadID::IO));
+  bool is_expected_resource_type =
+      (ResourceType::kMainFrame == resource_type_) ||
+      ((ResourceType::kSubFrame == resource_type_) &&
+       can_rt_check_subresource_url_);
+  DCHECK(is_expected_resource_type);
+
+  const GURL& url = urls_[next_index_].url;
+
+  if (!is_rt_lookup_successful) {
+    PerformHashBasedCheck(url);
+    return;
+  }
+
+  LogRTLookupResponse(*response);
+
+  SBThreatType sb_threat_type = SB_THREAT_TYPE_SAFE;
+  if (response && (response->threat_info_size() > 0) &&
+      response->threat_info(0).verdict_type() ==
+          RTLookupResponse::ThreatInfo::DANGEROUS) {
+    // TODO(crbug.com/1033692): Only take the first threat info into account
+    // because threat infos are returned in decreasing order of severity.
+    // Consider extend it to support multiple threat types.
+    sb_threat_type =
+        RealTimeUrlLookupServiceBase::GetSBThreatTypeForRTThreatType(
+            response->threat_info(0).threat_type());
+  }
+  OnUrlResult(url, sb_threat_type, ThreatMetadata());
+}
+
 }  // namespace safe_browsing

components/safe_browsing/core/browser/safe_browsing_url_checker_impl.h

@@ -192,6 +192,13 @@ class SafeBrowsingUrlCheckerImpl : public mojom::SafeBrowsingUrlChecker,
   void OnRTLookupResponse(bool is_rt_lookup_successful,
                           std::unique_ptr<RTLookupResponse> response);
 
+  // Logs |request| on any open chrome://safe-browsing pages.
+  void LogRTLookupRequest(const RTLookupRequest& request,
+                          const std::string& oauth_token);
+
+  // Logs |response| on any open chrome://safe-browsing pages.
+  void LogRTLookupResponse(const RTLookupResponse& response);
+
   void SetWebUIToken(int token);
 
   security_interstitials::UnsafeResource MakeUnsafeResource(

components/safe_browsing/core/browser/safe_browsing_url_checker_impl_unittest.cc

@@ -19,7 +19,6 @@
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "testing/platform_test.h"
-#include "third_party/blink/public/mojom/loader/resource_load_info.mojom-shared.h"
 
 using ::testing::_;
 
@@ -228,7 +227,7 @@ class SafeBrowsingUrlCheckerTest : public PlatformTest {
         mock_web_contents_getter;
     return std::make_unique<SafeBrowsingUrlCheckerImpl>(
         net::HttpRequestHeaders(), /*load_flags=*/0,
-        blink::mojom::ResourceType::kMainFrame,
+        static_cast<blink::mojom::ResourceType>(ResourceType::kMainFrame),
         /*has_user_gesture=*/false, url_checker_delegate_,
         mock_web_contents_getter.Get(), real_time_lookup_enabled,
         /*can_rt_check_subresource_url=*/false, can_check_safe_browsing_db,

components/safe_browsing/ios/BUILD.gn

@@ -10,6 +10,7 @@ source_set("unit_tests") {
     "//components/safe_browsing/core:ping_manager_unittest",
     "//components/safe_browsing/core:verdict_cache_manager_unittest",
     "//components/safe_browsing/core/browser",
+    "//components/safe_browsing/core/browser:safe_browsing_url_checker_unittest",
     "//components/safe_browsing/core/common",
     "//components/safe_browsing/core/common:unit_tests",
     "//components/safe_browsing/core/db:unit_tests_local_db",

components/safe_browsing/ios/browser/safe_browsing_url_checker_impl_ios.cc

@@ -6,21 +6,19 @@
 
 namespace safe_browsing {
 
-bool SafeBrowsingUrlCheckerImpl::CanPerformFullURLLookup(const GURL& url) {
-  // TODO(crbug.com/1028755): Support real-time checks on iOS.
-  return false;
+void SafeBrowsingUrlCheckerImpl::LogRTLookupRequest(
+    const RTLookupRequest& request,
+    const std::string& oauth_token) {
+  // TODO(crbug.com/1103222): Log this request on open chrome://safe-browsing
+  // pages once chrome://safe-browsing works on iOS, or log this request to
+  // stderr.
 }
 
-void SafeBrowsingUrlCheckerImpl::OnRTLookupRequest(
-    std::unique_ptr<RTLookupRequest> request,
-    std::string oauth_token) {
-  NOTREACHED();
-}
-
-void SafeBrowsingUrlCheckerImpl::OnRTLookupResponse(
-    bool is_rt_lookup_successful,
-    std::unique_ptr<RTLookupResponse> response) {
-  NOTREACHED();
+void SafeBrowsingUrlCheckerImpl::LogRTLookupResponse(
+    const RTLookupResponse& response) {
+  // TODO(crbug.com/1103222): Log this response on open chrome://safe-browsing
+  // pages once chrome://safe-browsing works on iOS, or log this response to
+  // stderr.
 }
 
 }  // namespace safe_browsing