Async Safe Browsing check, on mobile

chrome/browser/renderer_host/safe_browsing_resource_throttle.h

Index: chrome/browser/renderer_host/safe_browsing_resource_throttle.h
diff --git a/chrome/browser/renderer_host/safe_browsing_resource_throttle.h b/chrome/browser/renderer_host/safe_browsing_resource_throttle.h
index a21e92a11243b4e44dc46c237f36e1595ed0d681..8e1827ecc12b858bb830dfb7fe190ee487710feb 100644
--- a/chrome/browser/renderer_host/safe_browsing_resource_throttle.h
+++ b/chrome/browser/renderer_host/safe_browsing_resource_throttle.h
@@ -22,28 +22,42 @@ namespace net {
 class URLRequest;
 }
 
-// SafeBrowsingResourceThrottle checks that URLs are "safe" before navigating
-// to them. To be considered "safe", a URL must not appear in the
+// SafeBrowsingResourceThrottle checks that URLs are "safe" before
+// navigating to them. To be considered "safe", a URL must not appear in the
 // malware/phishing blacklists (see SafeBrowsingService for details).
 //
+// On desktop (ifdef SAFE_BROWSING_DB_LOCAL)
+// -----------------------------------------
 // This check is done before requesting the original URL, and additionally
-// before following any subsequent redirect.
+// before following any subsequent redirect.  In the common case the check
+// completes synchronously (no match in the bloom filter), so the request's

[mattm, 2015/07/24 23:24:11]

since we don't actually use a bloom filter these days, might as well s/bloom
filter/prefix set/

[Scott Hess - ex-Googler, 2015/07/24 23:55:18]

Or "in-memory filter", since the implementation isn't relevant here.

[Nathan Parker, 2015/07/28 17:42:12]

Done.

+// flow is un-interrupted.  However if the URL fails this quick check, it
+// has the possibility of being on the blacklist. Now the request is
+// deferred (prevented from starting), and a more expensive safe browsing
+// check is begun (fetches the full hashes).
 //
-// In the common case, the check completes synchronously (no match in the bloom
-// filter), so the request's flow is un-interrupted.
+// On mobile (ifdef SAFE_BROWSING_DB_REMOTE):
+// -----------------------------------------
+// The check is started and runs in parallel with the resource load.  If the
+// check is not complete by the time the headers are loaded, the request is
+// suspended until the URL is classified.  We let the headers load on mobile
+// since the RemoteSafeBrowsingDatabase checks always have some non-zero
+// latency -- there no synchronous pass.  This parallelism helps
+// performance.  Redirects are handled the same way as desktop so they
+// always defer.
 //
-// However if the URL fails this quick check, it has the possibility of being
-// on the blacklist. Now the request is suspended (prevented from starting),
-// and a more expensive safe browsing check is begun (fetches the full hashes).
 //
 // Note that the safe browsing check takes at most kCheckUrlTimeoutMs
 // milliseconds. If it takes longer than this, then the system defaults to
 // treating the URL as safe.
 //
-// Once the safe browsing check has completed, if the URL was decided to be
-// dangerous, a warning page is thrown up and the request remains suspended.
-// If on the other hand the URL was decided to be safe, the request is
-// resumed.
+// If the URL is classified as dangerous, a warning page is thrown up and
+// the request remains suspended.  If the user clicks "proceed" on warning
+// page, we resume the request.
+//
+// Note: The ResourceThrottle interface is called in this order:
+// WillStartRequest once, WillRedirectRequest zero or more times, and then
+// WillProcessReponse once.
 class SafeBrowsingResourceThrottle
     : public content::ResourceThrottle,
       public SafeBrowsingDatabaseManager::Client,
@@ -51,12 +65,15 @@ class SafeBrowsingResourceThrottle
  public:
   SafeBrowsingResourceThrottle(const net::URLRequest* request,
                                content::ResourceType resource_type,
-                               SafeBrowsingService* safe_browsing);
+                               SafeBrowsingService* safe_browsing,
+                               bool defer_at_start);
 
   // content::ResourceThrottle implementation (called on IO thread):
   void WillStartRequest(bool* defer) override;
   void WillRedirectRequest(const net::RedirectInfo& redirect_info,
                            bool* defer) override;
+  void WillProcessResponse(bool* defer) override;
+
   const char* GetNameForLogging() const override;
 
   // SafeBrowsingDabaseManager::Client implementation (called on IO thread):
@@ -67,8 +84,11 @@ class SafeBrowsingResourceThrottle
  private:
   // Describes what phase of the check a throttle is in.
   enum State {
+    // Haven't started checking or checking is complete. Not deferred.
     STATE_NONE,
+    // We have one outstanding URL-check. Could be deferred.
     STATE_CHECKING_URL,
+    // We're displaying a blocking page. Could be deferred.
     STATE_DISPLAYING_BLOCKING_PAGE,
   };
 
@@ -77,6 +97,8 @@ class SafeBrowsingResourceThrottle
     DEFERRED_NONE,
     DEFERRED_START,
     DEFERRED_REDIRECT,
+    DEFERRED_UNCHECKED_REDIRECT,  // unchecked_redirect_url_ is populated.
+    DEFERRED_PROCESSING,
   };
 
   ~SafeBrowsingResourceThrottle() override;
@@ -108,6 +130,10 @@ class SafeBrowsingResourceThrottle
   // request, or following a redirect).
   void ResumeRequest();
 
+  // True if we want to block the starting of requests until they're
+  // deemed safe.  Otherwise we let the resource partially load.
+  const bool defer_at_start_;
+
   State state_;
   DeferState defer_state_;
 
@@ -115,8 +141,8 @@ class SafeBrowsingResourceThrottle
   // when state_ != STATE_CHECKING_URL.
   SBThreatType threat_type_;
 
-  // The time when the outstanding safe browsing check was started.
-  base::TimeTicks url_check_start_time_;
+  // The time when we started deferring the request.
+  base::TimeTicks defer_start_time_;
 
   // Timer to abort the safe browsing check if it takes too long.
   base::OneShotTimer<SafeBrowsingResourceThrottle> timer_;
@@ -124,6 +150,9 @@ class SafeBrowsingResourceThrottle
   // The redirect chain for this resource
   std::vector<GURL> redirect_urls_;
 
+  // If in DEFERRED_UNCHECKED_REDIRECT state, this is the
+  // URL we still need to check before resuming.
+  GURL unchecked_redirect_url_;
   GURL url_being_checked_;
 
   scoped_refptr<SafeBrowsingDatabaseManager> database_manager_;
@@ -134,6 +163,4 @@ class SafeBrowsingResourceThrottle
 
   DISALLOW_COPY_AND_ASSIGN(SafeBrowsingResourceThrottle);
 };
-
-
 #endif  // CHROME_BROWSER_RENDERER_HOST_SAFE_BROWSING_RESOURCE_THROTTLE_H_