Send requests and receive responses in url_lookup_service_base class.

This CL is for refactoring. There is no functional change.

Move part of SendRequest and OnURLLoaderComplete into the base class.
This can maximize the code that can be reused by the enterprise URL
lookup service.

Add two virtual functions GetTrafficAnnotationTag and
GetRealTimeLookupUrlPrefix that are configurable by the child classes.

Bug: 1085261
Change-Id: Ie895ffceacc15b34653644091763744ca970150d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2247402
Commit-Queue: Xinghui Lu <xinghuilu@chromium.org>
Reviewed-by: Varun Khaneja <vakh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#782085}

chrome/browser/safe_browsing/chrome_enterprise_url_lookup_service.cc

@@ -12,6 +12,7 @@
 #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/verdict_cache_manager.h"
+#include "net/traffic_annotation/network_traffic_annotation.h"
 #include "services/network/public/cpp/shared_url_loader_factory.h"
 #include "url/gurl.h"
 
@@ -22,8 +23,7 @@ ChromeEnterpriseRealTimeUrlLookupService::
         scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
         VerdictCacheManager* cache_manager,
         Profile* profile)
-    : RealTimeUrlLookupServiceBase(cache_manager),
-      url_loader_factory_(url_loader_factory),
+    : RealTimeUrlLookupServiceBase(url_loader_factory, cache_manager),
       profile_(profile) {}
 
 ChromeEnterpriseRealTimeUrlLookupService::
@@ -53,4 +53,16 @@ policy::DMToken ChromeEnterpriseRealTimeUrlLookupService::GetDMToken() const {
   return ::safe_browsing::GetDMToken(profile_);
 }
 
+net::NetworkTrafficAnnotationTag
+ChromeEnterpriseRealTimeUrlLookupService::GetTrafficAnnotationTag() const {
+  // TODO(crbug.com/1085261): Implement this method.
+  return net::NetworkTrafficAnnotationTag::NotReached();
+}
+
+GURL ChromeEnterpriseRealTimeUrlLookupService::GetRealTimeLookupUrl() const {
+  // TODO(crbug.com/1085261): Implement this method.
+  NOTREACHED();
+  return GURL("");
+}
+
 }  // namespace safe_browsing

chrome/browser/safe_browsing/chrome_enterprise_url_lookup_service.h

@@ -11,6 +11,10 @@
 #include "components/safe_browsing/core/realtime/url_lookup_service_base.h"
 #include "url/gurl.h"
 
+namespace net {
+struct NetworkTrafficAnnotationTag;
+}
+
 namespace network {
 class SharedURLLoaderFactory;
 }  // namespace network
@@ -47,10 +51,12 @@ class ChromeEnterpriseRealTimeUrlLookupService
                    RTLookupResponseCallback response_callback) override;
 
  private:
-  policy::DMToken GetDMToken() const;
+  // RealTimeUrlLookupServiceBase:
+  net::NetworkTrafficAnnotationTag GetTrafficAnnotationTag() const override;
 
-  // The URLLoaderFactory we use to issue network requests.
-  scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory_;
+  GURL GetRealTimeLookupUrl() const override;
+
+  policy::DMToken GetDMToken() const;
 
   // Unowned object used for checking profile based settings.
   Profile* profile_;

components/safe_browsing/core/realtime/url_lookup_service.cc

@@ -28,17 +28,11 @@
 #include "net/traffic_annotation/network_traffic_annotation.h"
 #include "services/network/public/cpp/resource_request.h"
 #include "services/network/public/cpp/shared_url_loader_factory.h"
-#include "services/network/public/cpp/simple_url_loader.h"
 
 namespace safe_browsing {
 
 namespace {
 
-const char kRealTimeLookupUrlPrefix[] =
-    "https://safebrowsing.google.com/safebrowsing/clientreport/realtime";
-
-const size_t kURLLookupTimeoutDurationInSeconds = 10;  // 10 seconds.
-
 constexpr char kAuthHeaderBearer[] = "Bearer ";
 
 }  // namespace
@@ -54,8 +48,7 @@ RealTimeUrlLookupService::RealTimeUrlLookupService(
     bool is_under_advanced_protection,
     bool is_off_the_record,
     variations::VariationsService* variations_service)
-    : RealTimeUrlLookupServiceBase(cache_manager),
-      url_loader_factory_(url_loader_factory),
+    : RealTimeUrlLookupServiceBase(url_loader_factory, cache_manager),
       identity_manager_(identity_manager),
       sync_service_(sync_service),
       pref_service_(pref_service),
@@ -126,43 +119,8 @@ void RealTimeUrlLookupService::SendRequest(
                             ChromeUserPopulation::UserPopulation_MAX + 1);
   std::string req_data;
   request->SerializeToString(&req_data);
-  net::NetworkTrafficAnnotationTag traffic_annotation =
-      net::DefineNetworkTrafficAnnotation("safe_browsing_realtime_url_lookup",
-                                          R"(
-        semantics {
-          sender: "Safe Browsing"
-          description:
-            "When Safe Browsing can't detect that a URL is safe based on its "
-            "local database, it sends the top-level URL to Google to verify it "
-            "before showing a warning to the user."
-          trigger:
-            "When a main frame URL fails to match the local hash-prefix "
-            "database of known safe URLs and a valid result from a prior "
-            "lookup is not already cached, this will be sent."
-          data: "The main frame URL that did not match the local safelist."
-          destination: GOOGLE_OWNED_SERVICE
-        }
-        policy {
-          cookies_allowed: YES
-          cookies_store: "Safe Browsing cookie store"
-          setting:
-            "Users can disable Safe Browsing real time URL checks by "
-            "unchecking 'Protect you and your device from dangerous sites' in "
-            "Chromium settings under Privacy, or by unchecking 'Make searches "
-            "and browsing better (Sends URLs of pages you visit to Google)' in "
-            "Chromium settings under Privacy."
-          chrome_policy {
-            UrlKeyedAnonymizedDataCollectionEnabled {
-              policy_options {mode: MANDATORY}
-              UrlKeyedAnonymizedDataCollectionEnabled: false
-            }
-          }
-        })");
 
-  auto resource_request = std::make_unique<network::ResourceRequest>();
-  resource_request->url = GURL(kRealTimeLookupUrlPrefix);
-  resource_request->load_flags = net::LOAD_DISABLE_CACHE;
-  resource_request->method = "POST";
+  auto resource_request = GetResourceRequest();
   if (access_token_info.has_value()) {
     resource_request->headers.SetHeader(
         net::HttpRequestHeaders::kAuthorization,
@@ -171,20 +129,8 @@ void RealTimeUrlLookupService::SendRequest(
   base::UmaHistogramBoolean("SafeBrowsing.RT.HasTokenInRequest",
                             access_token_info.has_value());
 
-  std::unique_ptr<network::SimpleURLLoader> owned_loader =
-      network::SimpleURLLoader::Create(std::move(resource_request),
-                                       traffic_annotation);
-  network::SimpleURLLoader* loader = owned_loader.get();
-  owned_loader->AttachStringForUpload(req_data, "application/octet-stream");
-  owned_loader->SetTimeoutDuration(
-      base::TimeDelta::FromSeconds(kURLLookupTimeoutDurationInSeconds));
-  owned_loader->DownloadToStringOfUnboundedSizeUntilCrashAndDie(
-      url_loader_factory_.get(),
-      base::BindOnce(&RealTimeUrlLookupService::OnURLLoaderComplete,
-                     weak_factory_.GetWeakPtr(), url, loader,
-                     base::TimeTicks::Now()));
-
-  pending_requests_[owned_loader.release()] = std::move(response_callback);
+  SendRequestInternal(std::move(resource_request), req_data, url,
+                      std::move(response_callback));
 
   base::PostTask(FROM_HERE, CreateTaskTraits(ThreadID::IO),
                  base::BindOnce(std::move(request_callback), std::move(request),
@@ -193,60 +139,8 @@ void RealTimeUrlLookupService::SendRequest(
                                     : ""));
 }
 
-void RealTimeUrlLookupService::Shutdown() {
-  for (auto& pending : pending_requests_) {
-    // Treat all pending requests as safe.
-    auto response = std::make_unique<RTLookupResponse>();
-    std::move(pending.second)
-        .Run(/* is_rt_lookup_successful */ true, std::move(response));
-    delete pending.first;
-  }
-  pending_requests_.clear();
-}
-
 RealTimeUrlLookupService::~RealTimeUrlLookupService() {}
 
-void RealTimeUrlLookupService::OnURLLoaderComplete(
-    const GURL& url,
-    network::SimpleURLLoader* url_loader,
-    base::TimeTicks request_start_time,
-    std::unique_ptr<std::string> response_body) {
-  DCHECK(CurrentlyOnThread(ThreadID::UI));
-
-  auto it = pending_requests_.find(url_loader);
-  DCHECK(it != pending_requests_.end()) << "Request not found";
-
-  UMA_HISTOGRAM_TIMES("SafeBrowsing.RT.Network.Time",
-                      base::TimeTicks::Now() - request_start_time);
-
-  int net_error = url_loader->NetError();
-  int response_code = 0;
-  if (url_loader->ResponseInfo() && url_loader->ResponseInfo()->headers)
-    response_code = url_loader->ResponseInfo()->headers->response_code();
-  V4ProtocolManagerUtil::RecordHttpResponseOrErrorCode(
-      "SafeBrowsing.RT.Network.Result", net_error, response_code);
-
-  auto response = std::make_unique<RTLookupResponse>();
-  bool is_rt_lookup_successful = (net_error == net::OK) &&
-                                 (response_code == net::HTTP_OK) &&
-                                 response->ParseFromString(*response_body);
-  base::UmaHistogramBoolean("SafeBrowsing.RT.IsLookupSuccessful",
-                            is_rt_lookup_successful);
-  is_rt_lookup_successful ? HandleLookupSuccess() : HandleLookupError();
-
-  MayBeCacheRealTimeUrlVerdict(url, *response);
-
-  UMA_HISTOGRAM_COUNTS_100("SafeBrowsing.RT.ThreatInfoSize",
-                           response->threat_info_size());
-
-  base::PostTask(FROM_HERE, CreateTaskTraits(ThreadID::IO),
-                 base::BindOnce(std::move(it->second), is_rt_lookup_successful,
-                                std::move(response)));
-
-  delete it->first;
-  pending_requests_.erase(it);
-}
-
 std::unique_ptr<RTLookupRequest> RealTimeUrlLookupService::FillRequestProto(
     const GURL& url) {
   auto request = std::make_unique<RTLookupRequest>();
@@ -301,4 +195,45 @@ bool RealTimeUrlLookupService::CanCheckSafeBrowsingDb() const {
   return true;
 }
 
+net::NetworkTrafficAnnotationTag
+RealTimeUrlLookupService::GetTrafficAnnotationTag() const {
+  return net::DefineNetworkTrafficAnnotation(
+      "safe_browsing_realtime_url_lookup",
+      R"(
+        semantics {
+          sender: "Safe Browsing"
+          description:
+            "When Safe Browsing can't detect that a URL is safe based on its "
+            "local database, it sends the top-level URL to Google to verify it "
+            "before showing a warning to the user."
+          trigger:
+            "When a main frame URL fails to match the local hash-prefix "
+            "database of known safe URLs and a valid result from a prior "
+            "lookup is not already cached, this will be sent."
+          data: "The main frame URL that did not match the local safelist."
+          destination: GOOGLE_OWNED_SERVICE
+        }
+        policy {
+          cookies_allowed: YES
+          cookies_store: "Safe Browsing cookie store"
+          setting:
+            "Users can disable Safe Browsing real time URL checks by "
+            "unchecking 'Protect you and your device from dangerous sites' in "
+            "Chromium settings under Privacy, or by unchecking 'Make searches "
+            "and browsing better (Sends URLs of pages you visit to Google)' in "
+            "Chromium settings under Privacy."
+          chrome_policy {
+            UrlKeyedAnonymizedDataCollectionEnabled {
+              policy_options {mode: MANDATORY}
+              UrlKeyedAnonymizedDataCollectionEnabled: false
+            }
+          }
+        })");
+}
+
+GURL RealTimeUrlLookupService::GetRealTimeLookupUrl() const {
+  return GURL(
+      "https://safebrowsing.google.com/safebrowsing/clientreport/realtime");
+}
+
 }  // namespace safe_browsing

components/safe_browsing/core/realtime/url_lookup_service.h

@@ -21,8 +21,11 @@
 #include "components/signin/public/identity_manager/access_token_info.h"
 #include "url/gurl.h"
 
+namespace net {
+struct NetworkTrafficAnnotationTag;
+}
+
 namespace network {
-class SimpleURLLoader;
 class SharedURLLoaderFactory;
 }  // namespace network
 
@@ -75,13 +78,11 @@ class RealTimeUrlLookupService : public RealTimeUrlLookupServiceBase {
                    RTLookupRequestCallback request_callback,
                    RTLookupResponseCallback response_callback) override;
 
-  // KeyedService:
-  // Called before the actual deletion of the object.
-  void Shutdown() override;
-
  private:
-  using PendingRTLookupRequests =
-      base::flat_map<network::SimpleURLLoader*, RTLookupResponseCallback>;
+  // RealTimeUrlLookupServiceBase:
+  net::NetworkTrafficAnnotationTag GetTrafficAnnotationTag() const override;
+
+  GURL GetRealTimeLookupUrl() const override;
 
   // Called when the access token is obtained from |token_fetcher_|.
   void OnGetAccessToken(
@@ -101,26 +102,11 @@ class RealTimeUrlLookupService : public RealTimeUrlLookupServiceBase {
 
   bool IsHistorySyncEnabled();
 
-  // Called when the response from the real-time lookup remote endpoint is
-  // received. |url_loader| is the unowned loader that was used to send the
-  // request. |request_start_time| is the time when the request was sent.
-  // |response_body| is the response received. |url| is used for calling
-  // |MayBeCacheRealTimeUrlVerdict|.
-  void OnURLLoaderComplete(const GURL& url,
-                           network::SimpleURLLoader* url_loader,
-                           base::TimeTicks request_start_time,
-                           std::unique_ptr<std::string> response_body);
-
   std::unique_ptr<RTLookupRequest> FillRequestProto(const GURL& url);
 
   // Returns true if real time URL lookup with GAIA token is enabled.
   bool CanPerformFullURLLookupWithToken() const;
 
-  PendingRTLookupRequests pending_requests_;
-
-  // The URLLoaderFactory we use to issue network requests.
-  scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory_;
-
   // Unowned object used for getting access token when real time url check with
   // token is enabled.
   signin::IdentityManager* identity_manager_;

components/safe_browsing/core/realtime/url_lookup_service_base.cc

@@ -16,6 +16,10 @@
 #include "net/base/ip_address.h"
 #include "net/base/load_flags.h"
 #include "net/base/url_util.h"
+#include "net/http/http_status_code.h"
+#include "net/traffic_annotation/network_traffic_annotation.h"
+#include "services/network/public/cpp/shared_url_loader_factory.h"
+#include "services/network/public/cpp/simple_url_loader.h"
 
 namespace safe_browsing {
 
@@ -26,11 +30,14 @@ const size_t kMaxFailuresToEnforceBackoff = 3;
 const size_t kMinBackOffResetDurationInSeconds = 5 * 60;   //  5 minutes.
 const size_t kMaxBackOffResetDurationInSeconds = 30 * 60;  // 30 minutes.
 
+const size_t kURLLookupTimeoutDurationInSeconds = 10;  // 10 seconds.
+
 }  // namespace
 
 RealTimeUrlLookupServiceBase::RealTimeUrlLookupServiceBase(
+    scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
     VerdictCacheManager* cache_manager)
-    : cache_manager_(cache_manager) {}
+    : url_loader_factory_(url_loader_factory), cache_manager_(cache_manager) {}
 
 RealTimeUrlLookupServiceBase::~RealTimeUrlLookupServiceBase() = default;
 
@@ -193,4 +200,85 @@ void RealTimeUrlLookupServiceBase::MayBeCacheRealTimeUrlVerdict(
   }
 }
 
+std::unique_ptr<network::ResourceRequest>
+RealTimeUrlLookupServiceBase::GetResourceRequest() {
+  auto resource_request = std::make_unique<network::ResourceRequest>();
+  resource_request->url = GetRealTimeLookupUrl();
+  resource_request->load_flags = net::LOAD_DISABLE_CACHE;
+  resource_request->method = "POST";
+  return resource_request;
+}
+
+void RealTimeUrlLookupServiceBase::SendRequestInternal(
+    std::unique_ptr<network::ResourceRequest> resource_request,
+    const std::string& req_data,
+    const GURL& url,
+    RTLookupResponseCallback response_callback) {
+  std::unique_ptr<network::SimpleURLLoader> owned_loader =
+      network::SimpleURLLoader::Create(std::move(resource_request),
+                                       GetTrafficAnnotationTag());
+  network::SimpleURLLoader* loader = owned_loader.get();
+  owned_loader->AttachStringForUpload(req_data, "application/octet-stream");
+  owned_loader->SetTimeoutDuration(
+      base::TimeDelta::FromSeconds(kURLLookupTimeoutDurationInSeconds));
+  owned_loader->DownloadToStringOfUnboundedSizeUntilCrashAndDie(
+      url_loader_factory_.get(),
+      base::BindOnce(&RealTimeUrlLookupServiceBase::OnURLLoaderComplete,
+                     GetWeakPtr(), url, loader, base::TimeTicks::Now()));
+
+  pending_requests_[owned_loader.release()] = std::move(response_callback);
+}
+
+void RealTimeUrlLookupServiceBase::OnURLLoaderComplete(
+    const GURL& url,
+    network::SimpleURLLoader* url_loader,
+    base::TimeTicks request_start_time,
+    std::unique_ptr<std::string> response_body) {
+  DCHECK(CurrentlyOnThread(ThreadID::UI));
+
+  auto it = pending_requests_.find(url_loader);
+  DCHECK(it != pending_requests_.end()) << "Request not found";
+
+  UMA_HISTOGRAM_TIMES("SafeBrowsing.RT.Network.Time",
+                      base::TimeTicks::Now() - request_start_time);
+
+  int net_error = url_loader->NetError();
+  int response_code = 0;
+  if (url_loader->ResponseInfo() && url_loader->ResponseInfo()->headers)
+    response_code = url_loader->ResponseInfo()->headers->response_code();
+  V4ProtocolManagerUtil::RecordHttpResponseOrErrorCode(
+      "SafeBrowsing.RT.Network.Result", net_error, response_code);
+
+  auto response = std::make_unique<RTLookupResponse>();
+  bool is_rt_lookup_successful = (net_error == net::OK) &&
+                                 (response_code == net::HTTP_OK) &&
+                                 response->ParseFromString(*response_body);
+  base::UmaHistogramBoolean("SafeBrowsing.RT.IsLookupSuccessful",
+                            is_rt_lookup_successful);
+  is_rt_lookup_successful ? HandleLookupSuccess() : HandleLookupError();
+
+  MayBeCacheRealTimeUrlVerdict(url, *response);
+
+  UMA_HISTOGRAM_COUNTS_100("SafeBrowsing.RT.ThreatInfoSize",
+                           response->threat_info_size());
+
+  base::PostTask(FROM_HERE, CreateTaskTraits(ThreadID::IO),
+                 base::BindOnce(std::move(it->second), is_rt_lookup_successful,
+                                std::move(response)));
+
+  delete it->first;
+  pending_requests_.erase(it);
+}
+
+void RealTimeUrlLookupServiceBase::Shutdown() {
+  for (auto& pending : pending_requests_) {
+    // Treat all pending requests as safe.
+    auto response = std::make_unique<RTLookupResponse>();
+    std::move(pending.second)
+        .Run(/* is_rt_lookup_successful */ true, std::move(response));
+    delete pending.first;
+  }
+  pending_requests_.clear();
+}
+
 }  // namespace safe_browsing

components/safe_browsing/core/realtime/url_lookup_service_base.h

@@ -19,6 +19,16 @@
 #include "components/safe_browsing/core/proto/realtimeapi.pb.h"
 #include "url/gurl.h"
 
+namespace net {
+struct NetworkTrafficAnnotationTag;
+}
+
+namespace network {
+struct ResourceRequest;
+class SimpleURLLoader;
+class SharedURLLoaderFactory;
+}  // namespace network
+
 namespace safe_browsing {
 
 using RTLookupRequestCallback =
@@ -33,7 +43,9 @@ class VerdictCacheManager;
 // lookup feature.
 class RealTimeUrlLookupServiceBase : public KeyedService {
  public:
-  explicit RealTimeUrlLookupServiceBase(VerdictCacheManager* cache_manager);
+  explicit RealTimeUrlLookupServiceBase(
+      scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
+      VerdictCacheManager* cache_manager);
   ~RealTimeUrlLookupServiceBase() override;
 
   // Returns true if |url|'s scheme can be checked.
@@ -74,12 +86,23 @@ class RealTimeUrlLookupServiceBase : public KeyedService {
                            RTLookupRequestCallback request_callback,
                            RTLookupResponseCallback response_callback) = 0;
 
+  // KeyedService:
+  // Called before the actual deletion of the object.
+  void Shutdown() override;
+
  protected:
   // Fragments, usernames and passwords are removed, because fragments are only
   // used for local navigations and usernames/passwords are too privacy
   // sensitive.
   static GURL SanitizeURL(const GURL& url);
 
+  // Returns the traffic annotation tag that is attached in the simple URL
+  // loader.
+  virtual net::NetworkTrafficAnnotationTag GetTrafficAnnotationTag() const = 0;
+
+  // Returns the endpoint that the URL lookup will be sent to.
+  virtual GURL GetRealTimeLookupUrl() const = 0;
+
   // Returns the duration of the next backoff. Starts at
   // |kMinBackOffResetDurationInSeconds| and increases exponentially until it
   // reaches |kMaxBackOffResetDurationInSeconds|.
@@ -106,7 +129,29 @@ class RealTimeUrlLookupServiceBase : public KeyedService {
   // |cache_manager|.
   void MayBeCacheRealTimeUrlVerdict(const GURL& url, RTLookupResponse response);
 
+  // Get a resource request with URL, load_flags and method set.
+  std::unique_ptr<network::ResourceRequest> GetResourceRequest();
+
+  void SendRequestInternal(
+      std::unique_ptr<network::ResourceRequest> resource_request,
+      const std::string& req_data,
+      const GURL& url,
+      RTLookupResponseCallback response_callback);
+
  private:
+  using PendingRTLookupRequests =
+      base::flat_map<network::SimpleURLLoader*, RTLookupResponseCallback>;
+
+  // Called when the response from the real-time lookup remote endpoint is
+  // received. |url_loader| is the unowned loader that was used to send the
+  // request. |request_start_time| is the time when the request was sent.
+  // |response_body| is the response received. |url| is used for calling
+  // |MayBeCacheRealTimeUrlVerdict|.
+  void OnURLLoaderComplete(const GURL& url,
+                           network::SimpleURLLoader* url_loader,
+                           base::TimeTicks request_start_time,
+                           std::unique_ptr<std::string> response_body);
+
   // Count of consecutive failures to complete URL lookup requests. When it
   // reaches |kMaxFailuresToEnforceBackoff|, we enter the backoff mode. It gets
   // reset when we complete a lookup successfully or when the backoff reset
@@ -126,9 +171,15 @@ class RealTimeUrlLookupServiceBase : public KeyedService {
   // If this timer is running, backoff is in effect.
   base::OneShotTimer backoff_timer_;
 
+  // The URLLoaderFactory we use to issue network requests.
+  scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory_;
+
   // Unowned object used for getting and storing real time url check cache.
   VerdictCacheManager* cache_manager_;
 
+  // All requests that are sent but haven't received a response yet.
+  PendingRTLookupRequests pending_requests_;
+
   base::WeakPtrFactory<RealTimeUrlLookupServiceBase> weak_factory_{this};
 
   DISALLOW_COPY_AND_ASSIGN(RealTimeUrlLookupServiceBase);