Moved OnBeforeHeadersSent of webRequest API to url_request_http_job.cc

This CL moves the OnBeforeSendHeaders signal from net_http_transaction.cc and net_cache_transaction.cc to url_request_http_job.cc. We do this for several reasons:

- deep in the http stack one URL Request can be split into several http requests
(with different byte ranges).
  - We do not want to expose this implementation detail to extension authors.
  - It is not sufficient to submit only the first OnBeforeSendHeaders event to
the extension: If the extension modifies the headers, this modification should
apply to all http requests.

- from an architectural perspective we do not want to go too deep in order to
allow changing the network stack implementation without being limited by the
published Extension API (needs to be backwards compatible)

BUG=60101
TEST=no


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

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

12 files changed, 88 insertions, 81 deletions

diff --git a/net/base/net_log_event_type_list.h b/net/base/net_log_event_type_list.h
index bf5022a..bca0575 100644
--- a/net/base/net_log_event_type_list.h
+++ b/net/base/net_log_event_type_list.h
@@ -537,9 +537,9 @@ EVENT_TYPE(URL_REQUEST_START_JOB)
 //   }
 EVENT_TYPE(URL_REQUEST_REDIRECTED)
 
-// Measures the time a net::URLRequest is blocked waiting for an extension to
-// respond to the onBefoteRequest extension event.
-EVENT_TYPE(URL_REQUEST_BLOCKED_ON_EXTENSION)
+// Measures the time a net::URLRequest is blocked waiting for a delegate
+// (usually an extension) to respond to the onBeforeRequest extension event.
+EVENT_TYPE(URL_REQUEST_BLOCKED_ON_DELEGATE)
 
 // The specified number of bytes were read from the net::URLRequest.
 // The filtered event is used when the bytes were passed through a filter before
diff --git a/net/base/network_delegate.cc b/net/base/network_delegate.cc
index a236c5d..ecb9a01 100644
--- a/net/base/network_delegate.cc
+++ b/net/base/network_delegate.cc
@@ -17,13 +17,13 @@ int NetworkDelegate::NotifyBeforeURLRequest(URLRequest* request,
   return OnBeforeURLRequest(request, callback, new_url);
 }
 
-int NetworkDelegate::NotifyBeforeSendHeaders(uint64 request_id,
+int NetworkDelegate::NotifyBeforeSendHeaders(URLRequest* request,
                                              CompletionCallback* callback,
                                              HttpRequestHeaders* headers) {
   DCHECK(CalledOnValidThread());
   DCHECK(headers);
   DCHECK(callback);
-  return OnBeforeSendHeaders(request_id, callback, headers);
+  return OnBeforeSendHeaders(request, callback, headers);
 }
 
 void NetworkDelegate::NotifyRequestSent(
diff --git a/net/base/network_delegate.h b/net/base/network_delegate.h
index 741b7f2..df9fef8 100644
--- a/net/base/network_delegate.h
+++ b/net/base/network_delegate.h
@@ -40,7 +40,7 @@ class NetworkDelegate : public base::NonThreadSafe {
   int NotifyBeforeURLRequest(URLRequest* request,
                              CompletionCallback* callback,
                              GURL* new_url);
-  int NotifyBeforeSendHeaders(uint64 request_id,
+  int NotifyBeforeSendHeaders(URLRequest* request,
                               CompletionCallback* callback,
                               HttpRequestHeaders* headers);
   void NotifyRequestSent(uint64 request_id,
@@ -73,7 +73,7 @@ class NetworkDelegate : public base::NonThreadSafe {
   // read/write |headers| before they get sent out. |callback| and |headers| are
   // valid only until OnHttpTransactionDestroyed is called for this request.
   // Returns a net status code.
-  virtual int OnBeforeSendHeaders(uint64 request_id,
+  virtual int OnBeforeSendHeaders(URLRequest* request,
                                   CompletionCallback* callback,
                                   HttpRequestHeaders* headers) = 0;
 
diff --git a/net/http/http_cache_transaction.cc b/net/http/http_cache_transaction.cc
index 7c5b09d..ac3e373 100644
--- a/net/http/http_cache_transaction.cc
+++ b/net/http/http_cache_transaction.cc
@@ -480,13 +480,6 @@ int HttpCache::Transaction::DoLoop(int result) {
       case STATE_ADD_TO_ENTRY_COMPLETE:
         rv = DoAddToEntryComplete(rv);
         break;
-      case STATE_NOTIFY_BEFORE_SEND_HEADERS:
-        DCHECK_EQ(OK, rv);
-        rv = DoNotifyBeforeSendHeaders();
-        break;
-      case STATE_NOTIFY_BEFORE_SEND_HEADERS_COMPLETE:
-        rv = DoNotifyBeforeSendHeadersComplete(rv);
-        break;
       case STATE_START_PARTIAL_CACHE_VALIDATION:
         DCHECK_EQ(OK, rv);
         rv = DoStartPartialCacheValidation();
@@ -909,56 +902,6 @@ int HttpCache::Transaction::DoAddToEntryComplete(int result) {
   return OK;
 }
 
-int HttpCache::Transaction::DoNotifyBeforeSendHeaders() {
-  // Balanced in DoNotifyBeforeSendHeadersComplete.
-  next_state_ = STATE_NOTIFY_BEFORE_SEND_HEADERS_COMPLETE;
-
-  if (cache_->GetSession() && cache_->GetSession()->network_delegate()) {
-    // Give the delegate a copy of the request headers. We ignore any
-    // modification to these headers, since it doesn't make sense to issue a
-    // request from the cache with modified headers.
-    request_headers_copy_.CopyFrom(request_->extra_headers);
-    return cache_->GetSession()->network_delegate()->NotifyBeforeSendHeaders(
-        request_->request_id, &io_callback_, &request_headers_copy_);
-  }
-
-  return OK;
-}
-
-int HttpCache::Transaction::DoNotifyBeforeSendHeadersComplete(int result) {
-  // We now have access to the cache entry.
-  //
-  //  o if we are a reader for the transaction, then we can start reading the
-  //    cache entry.
-  //
-  //  o if we can read or write, then we should check if the cache entry needs
-  //    to be validated and then issue a network request if needed or just read
-  //    from the cache if the cache entry is already valid.
-  //
-  //  o if we are set to UPDATE, then we are handling an externally
-  //    conditionalized request (if-modified-since / if-none-match). We check
-  //    if the request headers define a validation request.
-  //
-  if (result == net::OK) {
-    switch (mode_) {
-      case READ:
-        result = BeginCacheRead();
-        break;
-      case READ_WRITE:
-        result = BeginPartialCacheValidation();
-        break;
-      case UPDATE:
-        result = BeginExternallyConditionalizedRequest();
-        break;
-      case WRITE:
-      default:
-        NOTREACHED();
-        result = ERR_FAILED;
-    }
-  }
-  return result;
-}
-
 // We may end up here multiple times for a given request.
 int HttpCache::Transaction::DoStartPartialCacheValidation() {
   if (mode_ == NONE)
@@ -1183,8 +1126,35 @@ int HttpCache::Transaction::DoCacheReadResponseComplete(int result) {
   if (response_.headers->GetContentLength() == current_size)
     truncated_ = false;
 
-  next_state_ = STATE_NOTIFY_BEFORE_SEND_HEADERS;
-  return OK;
+  // We now have access to the cache entry.
+  //
+  //  o if we are a reader for the transaction, then we can start reading the
+  //    cache entry.
+  //
+  //  o if we can read or write, then we should check if the cache entry needs
+  //    to be validated and then issue a network request if needed or just read
+  //    from the cache if the cache entry is already valid.
+  //
+  //  o if we are set to UPDATE, then we are handling an externally
+  //    conditionalized request (if-modified-since / if-none-match). We check
+  //    if the request headers define a validation request.
+  //
+  switch (mode_) {
+    case READ:
+      result = BeginCacheRead();
+      break;
+    case READ_WRITE:
+      result = BeginPartialCacheValidation();
+      break;
+    case UPDATE:
+      result = BeginExternallyConditionalizedRequest();
+      break;
+    case WRITE:
+    default:
+      NOTREACHED();
+      result = ERR_FAILED;
+  }
+  return result;
 }
 
 int HttpCache::Transaction::DoCacheWriteResponse() {
diff --git a/net/http/http_cache_transaction.h b/net/http/http_cache_transaction.h
index 0f5b756..21b7443 100644
--- a/net/http/http_cache_transaction.h
+++ b/net/http/http_cache_transaction.h
@@ -142,8 +142,6 @@ class HttpCache::Transaction : public HttpTransaction {
     STATE_DOOM_ENTRY_COMPLETE,
     STATE_ADD_TO_ENTRY,
     STATE_ADD_TO_ENTRY_COMPLETE,
-    STATE_NOTIFY_BEFORE_SEND_HEADERS,
-    STATE_NOTIFY_BEFORE_SEND_HEADERS_COMPLETE,
     STATE_START_PARTIAL_CACHE_VALIDATION,
     STATE_COMPLETE_PARTIAL_CACHE_VALIDATION,
     STATE_UPDATE_CACHED_RESPONSE,
@@ -198,8 +196,6 @@ class HttpCache::Transaction : public HttpTransaction {
   int DoDoomEntryComplete(int result);
   int DoAddToEntry();
   int DoAddToEntryComplete(int result);
-  int DoNotifyBeforeSendHeaders();
-  int DoNotifyBeforeSendHeadersComplete(int result);
   int DoStartPartialCacheValidation();
   int DoCompletePartialCacheValidation(int result);
   int DoUpdateCachedResponse();
diff --git a/net/http/http_network_transaction.cc b/net/http/http_network_transaction.cc
index 403f60f..bbd19b3 100644
--- a/net/http/http_network_transaction.cc
+++ b/net/http/http_network_transaction.cc
@@ -753,11 +753,6 @@ int HttpNetworkTransaction::DoBuildRequest() {
     BuildRequestHeaders(using_proxy);
   }
 
-  if (session_->network_delegate()) {
-    return session_->network_delegate()->NotifyBeforeSendHeaders(
-        request_->request_id, &io_callback_, &request_headers_);
-  }
-
   return OK;
 }
 
diff --git a/net/proxy/network_delegate_error_observer_unittest.cc b/net/proxy/network_delegate_error_observer_unittest.cc
index 0239b53..ca81258 100644
--- a/net/proxy/network_delegate_error_observer_unittest.cc
+++ b/net/proxy/network_delegate_error_observer_unittest.cc
@@ -30,7 +30,7 @@ class TestNetworkDelegate : public net::NetworkDelegate {
                                  GURL* new_url) {
     return net::OK;
   }
-  virtual int OnBeforeSendHeaders(uint64 request_id,
+  virtual int OnBeforeSendHeaders(URLRequest* request,
                                   CompletionCallback* callback,
                                   HttpRequestHeaders* headers) {
     return net::OK;
diff --git a/net/url_request/url_request.cc b/net/url_request/url_request.cc
index 9bcdbbf..f784c7b 100644
--- a/net/url_request/url_request.cc
+++ b/net/url_request/url_request.cc
@@ -377,7 +377,7 @@ void URLRequest::Start() {
     if (context_->network_delegate()->NotifyBeforeURLRequest(
             this, &before_request_callback_, &delegate_redirect_url_) ==
             net::ERR_IO_PENDING) {
-      net_log_.BeginEvent(NetLog::TYPE_URL_REQUEST_BLOCKED_ON_EXTENSION, NULL);
+      net_log_.BeginEvent(NetLog::TYPE_URL_REQUEST_BLOCKED_ON_DELEGATE, NULL);
       return;  // paused
     }
   }
@@ -391,8 +391,12 @@ void URLRequest::BeforeRequestComplete(int error) {
   DCHECK(!job_);
   DCHECK_NE(ERR_IO_PENDING, error);
 
-  net_log_.EndEvent(NetLog::TYPE_URL_REQUEST_BLOCKED_ON_EXTENSION, NULL);
+  net_log_.EndEvent(NetLog::TYPE_URL_REQUEST_BLOCKED_ON_DELEGATE, NULL);
   if (error != OK) {
+    // TODO(battre): Allow passing information of the extension that canceled
+    // the event.
+    net_log_.AddEvent(NetLog::TYPE_CANCELLED,
+        make_scoped_refptr(new NetLogStringParameter("source", "delegate")));
     StartJob(new URLRequestErrorJob(this, error));
   } else if (!delegate_redirect_url_.is_empty()) {
     GURL new_url;
diff --git a/net/url_request/url_request_http_job.cc b/net/url_request/url_request_http_job.cc
index 8091b70..8a7d298 100644
--- a/net/url_request/url_request_http_job.cc
+++ b/net/url_request/url_request_http_job.cc
@@ -23,6 +23,7 @@
 #include "net/base/mime_util.h"
 #include "net/base/net_errors.h"
 #include "net/base/net_util.h"
+#include "net/base/network_delegate.h"
 #include "net/base/sdch_manager.h"
 #include "net/base/ssl_cert_request_info.h"
 #include "net/base/ssl_config_service.h"
@@ -248,6 +249,8 @@ URLRequestHttpJob::URLRequestHttpJob(URLRequest* request)
           this, &URLRequestHttpJob::OnStartCompleted)),
       ALLOW_THIS_IN_INITIALIZER_LIST(read_callback_(
           this, &URLRequestHttpJob::OnReadCompleted)),
+      ALLOW_THIS_IN_INITIALIZER_LIST(notify_before_headers_sent_callback_(
+          this, &URLRequestHttpJob::NotifyBeforeSendHeadersCallback)),
       read_in_progress_(false),
       transaction_(NULL),
       throttling_entry_(URLRequestThrottlerManager::GetInstance()->
@@ -356,6 +359,37 @@ void URLRequestHttpJob::DestroyTransaction() {
 }
 
 void URLRequestHttpJob::StartTransaction() {
+  if (request_->context() && request_->context()->network_delegate()) {
+    int rv = request_->context()->network_delegate()->NotifyBeforeSendHeaders(
+        request_, &notify_before_headers_sent_callback_,
+        &request_info_.extra_headers);
+    // If an extension blocks the request, we rely on the callback to
+    // StartTransactionInternal().
+    if (rv == ERR_IO_PENDING) {
+      request_->net_log().BeginEvent(
+          NetLog::TYPE_URL_REQUEST_BLOCKED_ON_DELEGATE, NULL);
+      return;
+    }
+  }
+  StartTransactionInternal();
+}
+
+void URLRequestHttpJob::NotifyBeforeSendHeadersCallback(int result) {
+  request_->net_log().EndEvent(
+      NetLog::TYPE_URL_REQUEST_BLOCKED_ON_DELEGATE, NULL);
+
+  if (result == OK) {
+    StartTransactionInternal();
+  } else {
+    // TODO(battre): Allow passing information of the extension that canceled
+    // the event.
+    request_->net_log().AddEvent(NetLog::TYPE_CANCELLED,
+        make_scoped_refptr(new NetLogStringParameter("source", "delegate")));
+    NotifyCanceled();
+  }
+}
+
+void URLRequestHttpJob::StartTransactionInternal() {
   // NOTE: This method assumes that request_info_ is already setup properly.
 
   // If we already have a transaction, then we should restart the transaction
diff --git a/net/url_request/url_request_http_job.h b/net/url_request/url_request_http_job.h
index a6ccafd..de29f1a 100644
--- a/net/url_request/url_request_http_job.h
+++ b/net/url_request/url_request_http_job.h
@@ -42,7 +42,7 @@ class URLRequestHttpJob : public URLRequestJob {
   void NotifyDone(const URLRequestStatus& status);
 
   void DestroyTransaction();
-  void StartTransaction();
+
   void AddExtraHeaders();
   void AddCookieHeaderAndStart();
   void SaveCookiesAndNotifyHeadersComplete();
@@ -55,6 +55,7 @@ class URLRequestHttpJob : public URLRequestJob {
 
   void OnStartCompleted(int result);
   void OnReadCompleted(int result);
+  void NotifyBeforeSendHeadersCallback(int result);
 
   bool ShouldTreatAsCertificateError(int result);
 
@@ -105,6 +106,8 @@ class URLRequestHttpJob : public URLRequestJob {
 
   CompletionCallbackImpl<URLRequestHttpJob> start_callback_;
   CompletionCallbackImpl<URLRequestHttpJob> read_callback_;
+  CompletionCallbackImpl<URLRequestHttpJob>
+      notify_before_headers_sent_callback_;
 
   bool read_in_progress_;
 
@@ -150,6 +153,11 @@ class URLRequestHttpJob : public URLRequestJob {
   void RecordCompressionHistograms();
   bool IsCompressibleContent() const;
 
+  // Starts the transaction if extensions using the webrequest API do not
+  // object.
+  void StartTransaction();
+  void StartTransactionInternal();
+
   void RecordPerfHistograms(CompletionCause reason);
   void DoneWithRequest(CompletionCause reason);
 
diff --git a/net/url_request/url_request_test_util.cc b/net/url_request/url_request_test_util.cc
index 67d685e..0bfe9447 100644
--- a/net/url_request/url_request_test_util.cc
+++ b/net/url_request/url_request_test_util.cc
@@ -258,7 +258,7 @@ int TestNetworkDelegate::OnBeforeURLRequest(
 }
 
 int TestNetworkDelegate::OnBeforeSendHeaders(
-    uint64 request_id,
+    net::URLRequest* request,
     net::CompletionCallback* callback,
     net::HttpRequestHeaders* headers) {
   return net::OK;
diff --git a/net/url_request/url_request_test_util.h b/net/url_request/url_request_test_util.h
index 772abc6..f7755b8 100644
--- a/net/url_request/url_request_test_util.h
+++ b/net/url_request/url_request_test_util.h
@@ -185,7 +185,7 @@ class TestNetworkDelegate : public net::NetworkDelegate {
   virtual int OnBeforeURLRequest(net::URLRequest* request,
                                  net::CompletionCallback* callback,
                                  GURL* new_url);
-  virtual int OnBeforeSendHeaders(uint64 request_id,
+  virtual int OnBeforeSendHeaders(net::URLRequest* request,
                                   net::CompletionCallback* callback,
                                   net::HttpRequestHeaders* headers);
   virtual void OnRequestSent(uint64 request_id,