Fix cancellation of a pair of URLRequestJob subclasses

content/browser/webui/url_data_manager_backend_unittest.cc

 // Copyright 2014 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 "base/memory/scoped_ptr.h"
 #include "base/run_loop.h"
 #include "content/browser/webui/url_data_manager_backend.h"
 #include "content/public/test/mock_resource_context.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "net/http/http_response_headers.h"
 #include "net/http/http_response_info.h"
 #include "net/url_request/url_request_context.h"
 #include "net/url_request/url_request_job.h"
+#include "net/url_request/url_request_job_factory_impl.h"
+#include "net/url_request/url_request_test_util.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace content {
 
-namespace {
+class CancelAfterFirstReadDelegate : public net::TestDelegate {
+ public:
+   CancelAfterFirstReadDelegate() {}
+   ~CancelAfterFirstReadDelegate() override {}
 
-// Create a request for a chrome://resource URL passing the supplied |origin|
-// header and checking that the response header Access-Control-Allow-Origin has
-// value |expected_access_control_allow_origin_value|.
-void RunAccessControlAllowOriginTest(
-    const std::string& origin,
-    const std::string& expected_access_control_allow_origin_value) {
-  TestBrowserThreadBundle thread_bundle;
-  MockResourceContext resource_context;
-  scoped_ptr<net::URLRequestJobFactory::ProtocolHandler> protocol_handler(
-      URLDataManagerBackend::CreateProtocolHandler(&resource_context, false,
-                                                   nullptr, nullptr));
-  net::URLRequestContext url_request_context;
-  scoped_ptr<net::URLRequest> request = url_request_context.CreateRequest(
-      GURL("chrome://resources/polymer/v1_0/polymer/polymer-extracted.js"),
-      net::HIGHEST, nullptr);
-  request->SetExtraRequestHeaderByName("Origin", origin, true);
-  scoped_refptr<net::URLRequestJob> job =
-      protocol_handler->MaybeCreateJob(request.get(), nullptr);
-  ASSERT_TRUE(job);
-  job->Start();
+   // Cancels the request, after starting the first read.
+   void OnResponseStarted(net::URLRequest* request) override {
+     // net::TestDelegate will start the first read.
+     TestDelegate::OnResponseStarted(request);
+     request->Cancel();
+   }

[davidben, 2015/08/05 18:56:39]

Do you want to override OnReadCompleted like the other one, to assert we read -1
bytes?

[mmenke, 2015/08/05 20:00:24]

Done.  I've gone ahead and make the classes identical (Except for the runloop,
which the other class doesn't really need, but given the extra thread hop that
test uses, to make sure no action is pending, I'm happier keeping it there)

+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(CancelAfterFirstReadDelegate);
+};
+
+class UrlDataManagerBackendTest : public testing::Test {
+ public:
+  UrlDataManagerBackendTest() {
+    // URLRequestJobFactory takes ownership of the passed in ProtocolHandler.
+    url_request_job_factory_.SetProtocolHandler(
+        "chrome",
+        URLDataManagerBackend::CreateProtocolHandler(
+            &resource_context_, false, nullptr, nullptr));
+    url_request_context_.set_job_factory(&url_request_job_factory_);
+  }
+
+  scoped_ptr<net::URLRequest> CreateRequest(net::URLRequest::Delegate* delegate,
+                                            const char* origin) {
+    scoped_ptr<net::URLRequest> request = url_request_context_.CreateRequest(
+        GURL("chrome://resources/polymer/v1_0/polymer/polymer-extracted.js"),
+        net::HIGHEST, delegate);
+    request->SetExtraRequestHeaderByName("Origin", origin, true);
+    return request.Pass();
+  }
+
+ protected:
+  TestBrowserThreadBundle thread_bundle_;
+  MockResourceContext resource_context_;
+  net::URLRequestJobFactoryImpl url_request_job_factory_;
+  net::URLRequestContext url_request_context_;
+  net::TestDelegate delegate_;
+};
+
+TEST_F(UrlDataManagerBackendTest, AccessControlAllowOriginChromeUrl) {
+  scoped_ptr<net::URLRequest> request(
+      CreateRequest(&delegate_, "chrome://webui"));
+  request->Start();
   base::RunLoop().RunUntilIdle();
-  net::HttpResponseInfo response;
-  job->GetResponseInfo(&response);
-  EXPECT_TRUE(response.headers->HasHeaderValue(
-      "Access-Control-Allow-Origin",
-      expected_access_control_allow_origin_value));
+  EXPECT_TRUE(request->response_headers()->HasHeaderValue(
+      "Access-Control-Allow-Origin", "chrome://webui"));
 }
 
-}  // namespace
-
-TEST(UrlDataManagerBackendTest, AccessControlAllowOriginChromeUrl) {
-  RunAccessControlAllowOriginTest("chrome://webui", "chrome://webui");
+TEST_F(UrlDataManagerBackendTest, AccessControlAllowOriginNonChromeUrl) {
+  scoped_ptr<net::URLRequest> request(
+      CreateRequest(&delegate_, "http://www.example.com"));
+  request->Start();
+  base::RunLoop().RunUntilIdle();
+  EXPECT_TRUE(request->response_headers()->HasHeaderValue(
+      "Access-Control-Allow-Origin", "null"));
 }
 
-TEST(UrlDataManagerBackendTest, AccessControlAllowOriginNonChromeUrl) {
-  RunAccessControlAllowOriginTest("http://www.example.com", "null");
+// Check that the URLRequestJob isn't passed headers after cancellation.

[davidben, 2015/08/05 18:56:39]

URLRequestJob -> URLRequest?

[mmenke, 2015/08/05 20:00:24]

Done.

+TEST_F(UrlDataManagerBackendTest, CancelBeforeResponseStarts) {
+  scoped_ptr<net::URLRequest> request(
+      CreateRequest(&delegate_, "chrome://webui"));
+  request->Start();
+  request->Cancel();
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(net::URLRequestStatus::CANCELED, request->status().status());
+  EXPECT_EQ(1, delegate_.response_started_count());
+}
+
+// Check that the URLRequestJob isn't passed data after cancellation.

[davidben, 2015/08/05 18:56:39]

Ditto.

[mmenke, 2015/08/05 20:00:24]

Done.

+TEST_F(UrlDataManagerBackendTest, CancelAfterFirstReadStarted) {
+  CancelAfterFirstReadDelegate cancel_delegate;
+  scoped_ptr<net::URLRequest> request(
+      CreateRequest(&cancel_delegate, "chrome://webui"));
+  request->Start();
+  base::RunLoop().RunUntilIdle();
+
+  EXPECT_EQ(net::URLRequestStatus::CANCELED, request->status().status());
+  EXPECT_EQ(1, cancel_delegate.response_started_count());
+  EXPECT_EQ("", cancel_delegate.data_received());
 }
 
 }  // namespace content