Headless (breaking change): A better GenericURLRequestJob::Delegate API

headless/public/util/generic_url_request_job.h

Index: headless/public/util/generic_url_request_job.h
diff --git a/headless/public/util/generic_url_request_job.h b/headless/public/util/generic_url_request_job.h
index 909befa212766542f30f71f4d3948a5327081ff6..4801bf0ad96480b9359727e86bf5ad87834b62e1 100644
--- a/headless/public/util/generic_url_request_job.h
+++ b/headless/public/util/generic_url_request_job.h
@@ -25,10 +25,91 @@ class HttpResponseHeaders;
 class IOBuffer;
 }  // namespace net
 
+namespace content {
+class ResourceRequestInfo;
+}  // namespace content
+
 namespace headless {
 
 class URLRequestDispatcher;
 
+// Wrapper around net::URLRequest with helpers to access select metadata.
+class Request {
+ public:
+  virtual const net::URLRequest* GetURLRequest() const = 0;
+
+  // The frame from which the request came from.
+  virtual int GetFrameTreeNodeId() const = 0;
+
+  // The devtools agent host id for the page where the request came from.
+  virtual std::string GetDevtoolsAgentHostId() const = 0;

[Sami, 2017/04/13 11:06:31]

nit: DevTools

[alex clarke (OOO till 29th), 2017/04/13 11:16:03]

Done.

+
+  enum ResourceType {

[Sami, 2017/04/13 11:06:31]

bikeshed: enum class?

[alex clarke (OOO till 29th), 2017/04/13 11:16:03]

Done.

+    RESOURCE_TYPE_MAIN_FRAME = 0,
+    RESOURCE_TYPE_SUB_FRAME = 1,
+    RESOURCE_TYPE_STYLESHEET = 2,
+    RESOURCE_TYPE_SCRIPT = 3,
+    RESOURCE_TYPE_IMAGE = 4,
+    RESOURCE_TYPE_FONT_RESOURCE = 5,
+    RESOURCE_TYPE_SUB_RESOURCE = 6,
+    RESOURCE_TYPE_OBJECT = 7,
+    RESOURCE_TYPE_MEDIA = 8,
+    RESOURCE_TYPE_WORKER = 9,
+    RESOURCE_TYPE_SHARED_WORKER = 10,
+    RESOURCE_TYPE_PREFETCH = 11,
+    RESOURCE_TYPE_FAVICON = 12,
+    RESOURCE_TYPE_XHR = 13,
+    RESOURCE_TYPE_PING = 14,
+    RESOURCE_TYPE_SERVICE_WORKER = 15,
+    RESOURCE_TYPE_CSP_REPORT = 16,
+    RESOURCE_TYPE_PLUGIN_RESOURCE = 17,
+    RESOURCE_TYPE_LAST_TYPE
+  };
+
+  virtual ResourceType GetResourceType() const = 0;
+
+ protected:
+  Request() {}
+  virtual ~Request() {}
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(Request);
+};
+
+// Details of a request received by GenericURLRequestJob which must be either
+// Allowed, Blocked, Modified or have it's response Mocked.
+class IncomingRequest : public Request {

[Sami, 2017/04/13 11:06:31]

Let's split this into its own class and have Request as a member to make the
relationship a bit more clear.

[alex clarke (OOO till 29th), 2017/04/13 11:16:03]

Done.

+ public:
+  // Allows the request to proceed as normal.
+  virtual void AllowRequest() = 0;
+
+  // Causes the request to fail with the specified |error|.
+  virtual void BlockRequest(net::Error error) = 0;
+
+  // Allows the request to be completely re-written.
+  virtual void ModifyRequest(
+      const GURL& url,
+      const std::string& method,
+      const std::string& post_data,
+      const net::HttpRequestHeaders& request_headers) = 0;
+
+  struct MockResponseData {
+    int http_response_code;

[Sami, 2017/04/13 11:06:31]

nit: init to zero

[alex clarke (OOO till 29th), 2017/04/13 11:16:03]

Done.

+    std::string response_data;
+  };
+
+  // Instead of fetching the request, |mock_response| is returned instead.
+  virtual void MockResponse(
+      std::unique_ptr<MockResponseData> mock_response) = 0;
+
+ protected:
+  IncomingRequest() {}
+  ~IncomingRequest() override {}
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(IncomingRequest);
+};
+
 // Intended for use in a protocol handler, this ManagedDispatchURLRequestJob has
 // the following features:
 //
@@ -37,47 +118,29 @@ class URLRequestDispatcher;
 //    fetcher is invoked.
 class HEADLESS_EXPORT GenericURLRequestJob
     : public ManagedDispatchURLRequestJob,
-      public URLFetcher::ResultListener {
+      public URLFetcher::ResultListener,
+      public IncomingRequest {
  public:
-  enum class RewriteResult { kAllow, kDeny, kFailure };
-  using RewriteCallback = base::Callback<
-      void(RewriteResult result, const GURL& url, const std::string& method)>;
-
-  struct HttpResponse {
-    GURL final_url;
-    int http_response_code;
-
-    // The HTTP headers and response body. Note the lifetime of |response_data|
-    // is expected to outlive the GenericURLRequestJob.
-    const char* response_data;  // NOT OWNED
-    size_t response_data_size;
-  };
-
   class Delegate {
    public:
-    // Allows the delegate to rewrite the URL for a given request. Return true
-    // to signal that the rewrite is in progress and |callback| will be called
-    // with the result, or false to indicate that no rewriting is necessary.
-    // Called on an arbitrary thread. |callback| can be called on any thread.
-    virtual bool BlockOrRewriteRequest(const GURL& url,
-                                       const std::string& devtools_id,
-                                       const std::string& method,
-                                       const std::string& referrer,
-                                       RewriteCallback callback) = 0;
-
-    // Allows the delegate to synchronously fulfill a request with a reply.
-    // Called on an arbitrary thread.
-    virtual const HttpResponse* MaybeMatchResource(
-        const GURL& url,
-        const std::string& devtools_id,
-        const std::string& method,
-        const net::HttpRequestHeaders& request_headers) = 0;
+    // Notifies the delegate of an IncomingRequest which must either be
+    // allowed, blocked, modifed or it's response mocked. Called on an arbitrary
+    // thread.
+    virtual void OnIncomingRequest(IncomingRequest* incoming_request) = 0;
+
+    // Notifies the delegate of any fetch failure. Called on an arbitrary
+    // thread.
+    virtual void OnResourceLoadFailed(const Request* request,
+                                      net::Error error) = 0;
 
     // Signals that a resource load has finished. Called on an arbitrary thread.
-    virtual void OnResourceLoadComplete(const GURL& final_url,
-                                        const std::string& devtools_id,
-                                        const std::string& mime_type,
-                                        int http_response_code) = 0;
+    virtual void OnResourceLoadComplete(
+        const Request* request,
+        const GURL& final_url,
+        int http_response_code,
+        scoped_refptr<net::HttpResponseHeaders> response_headers,
+        const char* body,
+        size_t body_size) = 0;
 
    protected:
     virtual ~Delegate() {}
@@ -110,29 +173,38 @@ class HEADLESS_EXPORT GenericURLRequestJob
                        const char* body,
                        size_t body_size) override;
 
+  // Request implementation:
+  const net::URLRequest* GetURLRequest() const override;
+  int GetFrameTreeNodeId() const override;
+  std::string GetDevtoolsAgentHostId() const override;
+  ResourceType GetResourceType() const override;
+
+  // IncomingRequest implementation:
+  void AllowRequest() override;
+  void BlockRequest(net::Error error) override;
+  void ModifyRequest(const GURL& url,
+                     const std::string& method,
+                     const std::string& post_data,
+                     const net::HttpRequestHeaders& request_headers) override;
+  void MockResponse(std::unique_ptr<MockResponseData> mock_response) override;
+
  private:
-  static void OnRewriteResult(
-      base::WeakPtr<GenericURLRequestJob> weak_this,
-      const scoped_refptr<base::SingleThreadTaskRunner>& origin_task_runner,
-      RewriteResult result,
-      const GURL& url,
-      const std::string& method);
-  void OnRewriteResultOnOriginThread(RewriteResult result,
-                                     const GURL& url,
-                                     const std::string& method);
   void PrepareCookies(const GURL& rewritten_url,
                       const std::string& method,
-                      const url::Origin& site_for_cookies);
+                      const url::Origin& site_for_cookies,
+                      const base::Closure& done_callback);
   void OnCookiesAvailable(const GURL& rewritten_url,
                           const std::string& method,
+                          const base::Closure& done_callback,
                           const net::CookieList& cookie_list);
 
   std::unique_ptr<URLFetcher> url_fetcher_;
   net::HttpRequestHeaders extra_request_headers_;
   scoped_refptr<net::HttpResponseHeaders> response_headers_;
   scoped_refptr<base::SingleThreadTaskRunner> origin_task_runner_;
-  std::string devtools_request_id_;
+  std::unique_ptr<MockResponseData> mock_response_;
   Delegate* delegate_;          // Not owned.
+  const content::ResourceRequestInfo* request_resource_info_;  // Not owned.
   const char* body_ = nullptr;  // Not owned.
   int http_response_code_ = 0;
   size_t body_size_ = 0;