URLRequestHttpJob::StartTransaction should honour network delegate.

If a network delegate returns net::ERR_BLOCKED_BY_CLIENT inside URLRequestHttpJob::StartTransaction() the job is not cancelled. This does not happen, e.g., when ERR_BLOCKED_BY_CLIENT is returned within URLRequestHttpJob::OnStartCompleted(). This CL attempts to change the behaviour of StartTransaction() to cancel the job on the error signal from the delegate.

BUG=146816
TEST=This is not testable in the browser without custom code changes yet.

Review URL: https://chromiumcodereview.appspot.com/10911151

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

4 files changed, 79 insertions, 14 deletions

diff --git a/chrome_frame/test/net/fake_external_tab.cc b/chrome_frame/test/net/fake_external_tab.cc
index 8608696..943bdcde 100644
--- a/chrome_frame/test/net/fake_external_tab.cc
+++ b/chrome_frame/test/net/fake_external_tab.cc
@@ -208,6 +208,13 @@ void FilterDisabledTests() {
     // the second state is not supported by CF.
     "URLRequestTestHTTP.NetworkDelegateBlockAsynchronously",
 
+    // Tests for cancelling requests in states OnBeforeSendHeaders and
+    // OnHeadersReceived, which do not seem supported by CF.
+    "URLRequestTestHTTP.NetworkDelegateCancelRequestSynchronously2",
+    "URLRequestTestHTTP.NetworkDelegateCancelRequestSynchronously3",
+    "URLRequestTestHTTP.NetworkDelegateCancelRequestAsynchronously2",
+    "URLRequestTestHTTP.NetworkDelegateCancelRequestAsynchronously3",
+
     // Tests that requests can be cancelled while blocking in
     // OnBeforeSendHeaders state. But this state is not supported by CF.
     "URLRequestTestHTTP.NetworkDelegateCancelWhileWaiting2",
diff --git a/net/url_request/url_request_http_job.cc b/net/url_request/url_request_http_job.cc
index 901e3e7..4bffa9f 100644
--- a/net/url_request/url_request_http_job.cc
+++ b/net/url_request/url_request_http_job.cc
@@ -329,11 +329,13 @@ void URLRequestHttpJob::StartTransaction() {
         request_, notify_before_headers_sent_callback_,
         &request_info_.extra_headers);
     // If an extension blocks the request, we rely on the callback to
-    // StartTransactionInternal().
+    // MaybeStartTransactionInternal().
     if (rv == ERR_IO_PENDING) {
       SetBlockedOnDelegate();
       return;
     }
+    MaybeStartTransactionInternal(rv);
+    return;
   }
   StartTransactionInternal();
 }
@@ -344,6 +346,10 @@ void URLRequestHttpJob::NotifyBeforeSendHeadersCallback(int result) {
   // Check that there are no callbacks to already canceled requests.
   DCHECK_NE(URLRequestStatus::CANCELED, GetStatus().status());
 
+  MaybeStartTransactionInternal(result);
+}
+
+void URLRequestHttpJob::MaybeStartTransactionInternal(int result) {
   if (result == OK) {
     StartTransactionInternal();
   } else {
@@ -351,6 +357,7 @@ void URLRequestHttpJob::NotifyBeforeSendHeadersCallback(int result) {
     request_->net_log().AddEvent(NetLog::TYPE_CANCELLED,
                                  NetLog::StringCallback("source", &source));
     NotifyCanceled();
+    NotifyStartError(URLRequestStatus(URLRequestStatus::FAILED, result));
   }
 }
 
diff --git a/net/url_request/url_request_http_job.h b/net/url_request/url_request_http_job.h
index 22e092ec..3457166 100644
--- a/net/url_request/url_request_http_job.h
+++ b/net/url_request/url_request_http_job.h
@@ -159,6 +159,9 @@ class URLRequestHttpJob : public URLRequestJob {
   // Starts the transaction if extensions using the webrequest API do not
   // object.
   void StartTransaction();
+  // If |result| is net::OK, calls StartTransactionInternal. Otherwise notifies
+  // cancellation.
+  void MaybeStartTransactionInternal(int result);
   void StartTransactionInternal();
 
   void RecordPerfHistograms(CompletionCause reason);
diff --git a/net/url_request/url_request_unittest.cc b/net/url_request/url_request_unittest.cc
index 42e8e6e..2dbe023 100644
--- a/net/url_request/url_request_unittest.cc
+++ b/net/url_request/url_request_unittest.cc
@@ -2063,34 +2063,82 @@ TEST_F(URLRequestTestHTTP, NetworkDelegateCancelRequest) {
   EXPECT_EQ(1, network_delegate.destroyed_requests());
 }
 
-// Tests that the network delegate can cancel a request synchronously.
-TEST_F(URLRequestTestHTTP, NetworkDelegateCancelRequestSynchronously) {
-  ASSERT_TRUE(test_server_.Start());
-
+// Helper function for NetworkDelegateCancelRequestAsynchronously and
+// NetworkDelegateCancelRequestSynchronously. Sets up a blocking network
+// delegate operating in |block_mode| and a request for |url|. It blocks the
+// request in |stage| and cancels it with ERR_BLOCKED_BY_CLIENT.
+void NetworkDelegateCancelRequest(BlockingNetworkDelegate::BlockMode block_mode,
+                                  BlockingNetworkDelegate::Stage stage,
+                                  const GURL& url) {
   TestDelegate d;
-  BlockingNetworkDelegate network_delegate(
-      BlockingNetworkDelegate::SYNCHRONOUS);
-  network_delegate.set_block_on(BlockingNetworkDelegate::ON_BEFORE_URL_REQUEST);
-  network_delegate.set_retval(ERR_EMPTY_RESPONSE);
+  BlockingNetworkDelegate network_delegate(block_mode);
+  network_delegate.set_retval(ERR_BLOCKED_BY_CLIENT);
+  network_delegate.set_block_on(stage);
 
-  TestURLRequestContextWithProxy context(
-      test_server_.host_port_pair().ToString(),
-      &network_delegate);
+  TestURLRequestContext context(true);
+  context.set_network_delegate(&network_delegate);
+  context.Init();
 
   {
-    URLRequest r(test_server_.GetURL(""), &d, &context);
+    URLRequest r(url, &d, &context);
 
     r.Start();
     MessageLoop::current()->Run();
 
     EXPECT_EQ(URLRequestStatus::FAILED, r.status().status());
-    EXPECT_EQ(ERR_EMPTY_RESPONSE, r.status().error());
+    EXPECT_EQ(ERR_BLOCKED_BY_CLIENT, r.status().error());
     EXPECT_EQ(1, network_delegate.created_requests());
     EXPECT_EQ(0, network_delegate.destroyed_requests());
   }
   EXPECT_EQ(1, network_delegate.destroyed_requests());
 }
 
+// The following 3 tests check that the network delegate can cancel a request
+// synchronously in various stages of the request.
+TEST_F(URLRequestTestHTTP, NetworkDelegateCancelRequestSynchronously1) {
+  ASSERT_TRUE(test_server_.Start());
+  NetworkDelegateCancelRequest(BlockingNetworkDelegate::SYNCHRONOUS,
+                               BlockingNetworkDelegate::ON_BEFORE_URL_REQUEST,
+                               test_server_.GetURL(""));
+}
+
+TEST_F(URLRequestTestHTTP, NetworkDelegateCancelRequestSynchronously2) {
+  ASSERT_TRUE(test_server_.Start());
+  NetworkDelegateCancelRequest(BlockingNetworkDelegate::SYNCHRONOUS,
+                               BlockingNetworkDelegate::ON_BEFORE_SEND_HEADERS,
+                               test_server_.GetURL(""));
+}
+
+TEST_F(URLRequestTestHTTP, NetworkDelegateCancelRequestSynchronously3) {
+  ASSERT_TRUE(test_server_.Start());
+  NetworkDelegateCancelRequest(BlockingNetworkDelegate::SYNCHRONOUS,
+                               BlockingNetworkDelegate::ON_HEADERS_RECEIVED,
+                               test_server_.GetURL(""));
+}
+
+// The following 3 tests check that the network delegate can cancel a request
+// asynchronously in various stages of the request.
+TEST_F(URLRequestTestHTTP, NetworkDelegateCancelRequestAsynchronously1) {
+  ASSERT_TRUE(test_server_.Start());
+  NetworkDelegateCancelRequest(BlockingNetworkDelegate::AUTO_CALLBACK,
+                               BlockingNetworkDelegate::ON_BEFORE_URL_REQUEST,
+                               test_server_.GetURL(""));
+}
+
+TEST_F(URLRequestTestHTTP, NetworkDelegateCancelRequestAsynchronously2) {
+  ASSERT_TRUE(test_server_.Start());
+  NetworkDelegateCancelRequest(BlockingNetworkDelegate::AUTO_CALLBACK,
+                               BlockingNetworkDelegate::ON_BEFORE_SEND_HEADERS,
+                               test_server_.GetURL(""));
+}
+
+TEST_F(URLRequestTestHTTP, NetworkDelegateCancelRequestAsynchronously3) {
+  ASSERT_TRUE(test_server_.Start());
+  NetworkDelegateCancelRequest(BlockingNetworkDelegate::AUTO_CALLBACK,
+                               BlockingNetworkDelegate::ON_HEADERS_RECEIVED,
+                               test_server_.GetURL(""));
+}
+
 // Tests that the network delegate can block and redirect a request to a new
 // URL.
 TEST_F(URLRequestTestHTTP, NetworkDelegateRedirectRequest) {