Implement blocking for webRequest.onBeforeRequest extension event.

I did some measurements with a Release build of chrome, both manually and via the page cycler tests. It seems that a simple empty blocking event listener can add anywhere from a 1 to 30ms delay to request times, largely depending on how many requests are in the queue (when many requests come at once, the last ones to be processed by the extension are delayed the longest).

From page cycler data (on my local machine), the average increase in page load time seems to be around 6ms. This is independent of total page load time (which makes sense).

BUG=60101
TEST=covered by apitests

Review URL: http://codereview.chromium.org/6574049

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@77339 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 48 insertions, 8 deletions

diff --git a/net/url_request/url_request.cc b/net/url_request/url_request.cc
index 5af99ea..229611ca 100644
--- a/net/url_request/url_request.cc
+++ b/net/url_request/url_request.cc
@@ -19,6 +19,7 @@
 #include "net/http/http_response_headers.h"
 #include "net/http/http_util.h"
 #include "net/url_request/url_request_context.h"
+#include "net/url_request/url_request_error_job.h"
 #include "net/url_request/url_request_job.h"
 #include "net/url_request/url_request_job_manager.h"
 #include "net/url_request/url_request_netlog_params.h"
@@ -131,6 +132,9 @@ URLRequest::URLRequest(const GURL& url, Delegate* delegate)
 }
 
 URLRequest::~URLRequest() {
+  if (before_request_callback_)
+    before_request_callback_->Cancel();
+
   Cancel();
 
   if (job_)
@@ -354,6 +358,19 @@ GURL URLRequest::GetSanitizedReferrer() const {
 }
 
 void URLRequest::Start() {
+  response_info_.request_time = Time::Now();
+
+  if (context_ && context_->network_delegate()) {
+    before_request_callback_ = new CancelableCompletionCallback<URLRequest>(
+      this, &URLRequest::BeforeRequestComplete);
+    before_request_callback_->AddRef();  // balanced in BeforeRequestComplete
+    if (context_->network_delegate()->NotifyBeforeURLRequest(
+        this, before_request_callback_)) {
+      net_log_.BeginEvent(NetLog::TYPE_URL_REQUEST_BLOCKED_ON_EXTENSION, NULL);
+      return;  // paused
+    }
+  }
+
   StartJob(URLRequestJobManager::GetInstance()->CreateJob(this));
 }
 
@@ -363,11 +380,6 @@ void URLRequest::StartJob(URLRequestJob* job) {
   DCHECK(!is_pending_);
   DCHECK(!job_);
 
-  // TODO(mpcomplete): pass in request ID?
-  // TODO(mpcomplete): allow delegate to potentially delay/cancel request.
-  if (context_ && context_->network_delegate())
-    context_->network_delegate()->NotifyBeforeURLRequest(this);
-
   net_log_.BeginEvent(
       net::NetLog::TYPE_URL_REQUEST_START_JOB,
       make_scoped_refptr(new URLRequestStartEventParameters(
@@ -381,7 +393,6 @@ void URLRequest::StartJob(URLRequestJob* job) {
 
   is_pending_ = true;
 
-  response_info_.request_time = Time::Now();
   response_info_.was_cached = false;
 
   // Don't allow errors to be sent from within Start().
@@ -391,6 +402,18 @@ void URLRequest::StartJob(URLRequestJob* job) {
   job_->Start();
 }
 
+void URLRequest::BeforeRequestComplete(int error) {
+  DCHECK(!job_);
+
+  net_log_.EndEvent(NetLog::TYPE_URL_REQUEST_BLOCKED_ON_EXTENSION, NULL);
+  before_request_callback_->Release();  // balanced in Start
+  if (error != net::OK) {
+    StartJob(new URLRequestErrorJob(this, error));
+  } else {
+    StartJob(URLRequestJobManager::GetInstance()->CreateJob(this));
+  }
+}
+
 void URLRequest::Restart() {
   // Should only be called if the original job didn't make any progress.
   DCHECK(job_ && !job_->has_response_started());
@@ -539,6 +562,7 @@ void URLRequest::PrepareToRestart() {
   OrphanJob();
 
   response_info_ = net::HttpResponseInfo();
+  response_info_.request_time = Time::Now();
   status_ = URLRequestStatus();
   is_pending_ = false;
 }
diff --git a/net/url_request/url_request.h b/net/url_request/url_request.h
index 849a03b..72ab4d1 100644
--- a/net/url_request/url_request.h
+++ b/net/url_request/url_request.h
@@ -17,6 +17,7 @@
 #include "base/string16.h"
 #include "base/threading/non_thread_safe.h"
 #include "googleurl/src/gurl.h"
+#include "net/base/completion_callback.h"
 #include "net/base/load_states.h"
 #include "net/base/net_log.h"
 #include "net/base/request_priority.h"
@@ -607,6 +608,13 @@ class URLRequest : public base::NonThreadSafe {
   // passed values.
   void DoCancel(int os_error, const net::SSLInfo& ssl_info);
 
+  // Resumes or blocks a request paused by the NetworkDelegate::OnBeforeRequest
+  // handler. If |blocked| is true, the request is blocked and an error page is
+  // returned indicating so. This should only be called after Start is called
+  // and OnBeforeRequest returns true (signalling that the request should be
+  // paused).
+  void BeforeRequestComplete(int error);
+
   // Contextual information used for this request (can be NULL). This contains
   // most of the dependencies which are shared between requests (disk cache,
   // cookie store, socket pool, etc.)
@@ -664,6 +672,11 @@ class URLRequest : public base::NonThreadSafe {
 
   base::debug::LeakTracker<URLRequest> leak_tracker_;
 
+  // Callback passed to the network delegate to notify us when a blocked request
+  // is ready to be resumed or canceled.
+  scoped_refptr< CancelableCompletionCallback<URLRequest> >
+      before_request_callback_;
+
   DISALLOW_COPY_AND_ASSIGN(URLRequest);
 };
 
diff --git a/net/url_request/url_request_test_util.cc b/net/url_request/url_request_test_util.cc
index 5e76729..8fd7cab 100644
--- a/net/url_request/url_request_test_util.cc
+++ b/net/url_request/url_request_test_util.cc
@@ -296,7 +296,9 @@ TestNetworkDelegate::TestNetworkDelegate()
 
 TestNetworkDelegate::~TestNetworkDelegate() {}
 
-void TestNetworkDelegate::OnBeforeURLRequest(net::URLRequest* request) {
+bool TestNetworkDelegate::OnBeforeURLRequest(
+    net::URLRequest* request, net::CompletionCallback* callback) {
+  return false;
 }
 
 void TestNetworkDelegate::OnSendHttpRequest(
diff --git a/net/url_request/url_request_test_util.h b/net/url_request/url_request_test_util.h
index 47ecff4..d166f9e 100644
--- a/net/url_request/url_request_test_util.h
+++ b/net/url_request/url_request_test_util.h
@@ -201,7 +201,8 @@ class TestNetworkDelegate : public net::NetworkDelegate {
 
  private:
   // net::NetworkDelegate:
-  virtual void OnBeforeURLRequest(net::URLRequest* request);
+  virtual bool OnBeforeURLRequest(net::URLRequest* request,
+                                  net::CompletionCallback* callback);
   virtual void OnSendHttpRequest(net::HttpRequestHeaders* headers);
   virtual void OnResponseStarted(net::URLRequest* request);
   virtual void OnReadCompleted(net::URLRequest* request, int bytes_read);