URLRequestHttpJob::StartTransaction should honour network delegate.

net/url_request/url_request_unittest.cc

Index: net/url_request/url_request_unittest.cc
diff --git a/net/url_request/url_request_unittest.cc b/net/url_request/url_request_unittest.cc
index 42e8e6ef7296db3241f2ebb2f7bfd6614d8d618f..5ff08977ef701004562f69e36165a7acf3cd0ba1 100644
--- a/net/url_request/url_request_unittest.cc
+++ b/net/url_request/url_request_unittest.cc
@@ -2063,32 +2063,57 @@ 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 various tages and cancels it with ERR_BLOCKED_BY_CLIENT.
+void NetworkDelegateCancelRequest(BlockingNetworkDelegate::BlockMode block_mode,
+                                  const GURL& url) {
+  static const BlockingNetworkDelegate::Stage blocking_stages[] = {
+    BlockingNetworkDelegate::ON_BEFORE_URL_REQUEST,

[erikwright (departed), 2012/09/26 15:41:42]

Could you just make the blocking stage a parameter to this function and then
define six separate test-cases instead of two?

Or, alternatively, refactor this to be a parameterized test.

http://code.google.com/p/googletest/wiki/AdvancedGuide#How_to_Write_Value-Par...

[vabr (Chromium), 2012/09/26 16:04:08]

Done.
I disabled all of them in Chrome Frame, except for the OnBeforeRequest ones.
I did not go for the alternative, as the first option seems to be a pattern
already (see other URLRequestTestHTTP tests with numbers at the end of name). I
guess the refactoring would be significant enough to have a separate CL and
crbug dedicated to it.

But thanks for the pointer to the docs, they are quite interesting.

+    BlockingNetworkDelegate::ON_BEFORE_SEND_HEADERS,
+    BlockingNetworkDelegate::ON_HEADERS_RECEIVED
+  };
 
-  TestDelegate d;
-  BlockingNetworkDelegate network_delegate(
-      BlockingNetworkDelegate::SYNCHRONOUS);
-  network_delegate.set_block_on(BlockingNetworkDelegate::ON_BEFORE_URL_REQUEST);
-  network_delegate.set_retval(ERR_EMPTY_RESPONSE);
+  for (size_t i = 0; i < arraysize(blocking_stages); ++i) {
+    TestDelegate d;
+    BlockingNetworkDelegate network_delegate(block_mode);
+    network_delegate.set_retval(ERR_BLOCKED_BY_CLIENT);
+    network_delegate.set_block_on(blocking_stages[i]);
 
-  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();
+      r.Start();
+      MessageLoop::current()->Run();
 
-    EXPECT_EQ(URLRequestStatus::FAILED, r.status().status());
-    EXPECT_EQ(ERR_EMPTY_RESPONSE, r.status().error());
-    EXPECT_EQ(1, network_delegate.created_requests());
-    EXPECT_EQ(0, network_delegate.destroyed_requests());
+      EXPECT_EQ(URLRequestStatus::FAILED, r.status().status());
+      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());
   }
-  EXPECT_EQ(1, network_delegate.destroyed_requests());
+}
+
+// Tests that the network delegate can cancel a request synchronously in
+// various stages of the request.
+TEST_F(URLRequestTestHTTP, NetworkDelegateCancelRequestSynchronously) {
+  ASSERT_TRUE(test_server_.Start());
+  NetworkDelegateCancelRequest(BlockingNetworkDelegate::SYNCHRONOUS,
+                               test_server_.GetURL(""));
+}
+
+// Tests that the network delegate can cancel a request asynchronously in
+// various stages of the request.
+TEST_F(URLRequestTestHTTP, NetworkDelegateCancelRequestAsynchronously) {
+  ASSERT_TRUE(test_server_.Start());
+  NetworkDelegateCancelRequest(BlockingNetworkDelegate::AUTO_CALLBACK,
+                               test_server_.GetURL(""));
 }
 
 // Tests that the network delegate can block and redirect a request to a new