URLRequestHttpJob::StartTransaction should honour network delegate.

net/url_request/url_request_unittest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "build/build_config.h"
 
 #if defined(OS_WIN)
 #include <windows.h>
 #include <shlobj.h>
 #endif
@@ skipping 2045 matching lines @@
     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(1, network_delegate.destroyed_requests());
 }
 
-// Tests that the network delegate can cancel a request synchronously.
+// 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
+  };
+
+  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]);
+
+    TestURLRequestContext context(true);
+    context.set_network_delegate(&network_delegate);
+    context.Init();
+
+    {
+      URLRequest r(url, &d, &context);
+
+      r.Start();
+      MessageLoop::current()->Run();
+
+      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());
+  }
+}
+
+// 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(""));
+}
 
-  TestDelegate d;
-  BlockingNetworkDelegate network_delegate(
-      BlockingNetworkDelegate::SYNCHRONOUS);
-  network_delegate.set_block_on(BlockingNetworkDelegate::ON_BEFORE_URL_REQUEST);
-  network_delegate.set_retval(ERR_EMPTY_RESPONSE);
-
-  TestURLRequestContextWithProxy context(
-      test_server_.host_port_pair().ToString(),
-      &network_delegate);
-
-  {
-    URLRequest r(test_server_.GetURL(""), &d, &context);
-
-    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(1, network_delegate.destroyed_requests());
+// 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
 // URL.
 TEST_F(URLRequestTestHTTP, NetworkDelegateRedirectRequest) {
   ASSERT_TRUE(test_server_.Start());
 
   TestDelegate d;
   BlockingNetworkDelegate network_delegate(
       BlockingNetworkDelegate::AUTO_CALLBACK);
@@ skipping 2689 matching lines @@
 
     EXPECT_FALSE(r.is_pending());
     EXPECT_EQ(1, d->response_started_count());
     EXPECT_FALSE(d->received_data_before_response());
     EXPECT_EQ(d->bytes_received(), static_cast<int>(file_size));
   }
 }
 #endif  // !defined(DISABLE_FTP_SUPPORT)
 
 }  // namespace net