Remove DCHECK in HttpStreamParser::ReadResponseHeaders.

The DCHECK dereferenced the request's HttpResponseInfo.  It turns out
that when an HttpNetworkTransaction is cancelled and an
HttpResponseDrainer is used, the HttpResponseInfo is deleted with the
HttpStreamParser's knowledge.

This CL also adds a pair of tests for the situation.

BUG=368418

Review URL: https://codereview.chromium.org/284913003

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

net/http/http_network_transaction_unittest.cc

@@ -10159,6 +10159,56 @@ TEST_P(HttpNetworkTransactionTest, SimpleCancel) {
   base::MessageLoop::current()->RunUntilIdle();
 }
 
+// Test that if a transaction is cancelled after receiving the headers, the
+// stream is drained properly and added back to the socket pool.  The main
+// purpose of this test is to make sure that an HttpStreamParser can be read
+// from after the HttpNetworkTransaction and the objects it owns have been
+// deleted.
+// See http://crbug.com/368418
+TEST_P(HttpNetworkTransactionTest, CancelAfterHeaders) {
+  MockRead data_reads[] = {
+    MockRead(ASYNC, "HTTP/1.1 200 OK\r\n"),
+    MockRead(ASYNC, "Content-Length: 2\r\n"),
+    MockRead(ASYNC, "Connection: Keep-Alive\r\n\r\n"),
+    MockRead(ASYNC, "1"),
+    // 2 async reads are necessary to trigger a ReadResponseBody call after the
+    // HttpNetworkTransaction has been deleted.
+    MockRead(ASYNC, "2"),
+    MockRead(SYNCHRONOUS, ERR_IO_PENDING),  // Should never read this.
+  };
+  StaticSocketDataProvider data(data_reads, arraysize(data_reads), NULL, 0);
+  session_deps_.socket_factory->AddSocketDataProvider(&data);
+
+  scoped_refptr<HttpNetworkSession> session(CreateSession(&session_deps_));
+
+  {
+    HttpRequestInfo request;
+    request.method = "GET";
+    request.url = GURL("http://www.google.com/");
+    request.load_flags = 0;
+
+    HttpNetworkTransaction trans(DEFAULT_PRIORITY, session);
+    TestCompletionCallback callback;
+
+    int rv = trans.Start(&request, callback.callback(), BoundNetLog());
+    EXPECT_EQ(ERR_IO_PENDING, rv);
+    callback.WaitForResult();
+
+    const HttpResponseInfo* response = trans.GetResponseInfo();
+    ASSERT_TRUE(response != NULL);
+    EXPECT_TRUE(response->headers.get() != NULL);
+    EXPECT_EQ("HTTP/1.1 200 OK", response->headers->GetStatusLine());
+
+    // The transaction and HttpRequestInfo are deleted.
+  }
+
+  // Let the HttpResponseBodyDrainer drain the socket.
+  base::MessageLoop::current()->RunUntilIdle();
+
+  // Socket should now be idle, waiting to be reused.
+  EXPECT_EQ(1, GetIdleSocketCountInTransportSocketPool(session));
+}
+
 // Test a basic GET request through a proxy.
 TEST_P(HttpNetworkTransactionTest, ProxyGet) {
   session_deps_.proxy_service.reset(

net/http/http_stream_parser.cc

@@ -345,9 +345,6 @@ int HttpStreamParser::ReadResponseBody(IOBuffer* buf, int buf_len,
   if (io_state_ == STATE_DONE)
     return OK;
 
-  // Must have response headers with a non-1xx error code.
-  DCHECK_NE(1, response_->headers->response_code() / 100);
-
   user_read_buf_ = buf;
   user_read_buf_len_ = buf_len;
   io_state_ = STATE_READ_BODY;

net/http/http_stream_parser_unittest.cc

@@ -804,6 +804,63 @@ TEST(HttpStreamParser, ReceivedBytesIncludesContinueHeader) {
   EXPECT_EQ(response_size, get_runner.parser()->received_bytes());
 }
 
+// Test that an HttpStreamParser can be read from after it's received headers
+// and data structures owned by its owner have been deleted.  This happens
+// when a ResponseBodyDrainer is used.
+TEST(HttpStreamParser, ReadAfterUnownedObjectsDestroyed) {
+  MockWrite writes[] = {
+    MockWrite(SYNCHRONOUS, 0,
+              "GET /foo.html HTTP/1.1\r\n\r\n"),
+    MockWrite(SYNCHRONOUS, 1, "1"),
+  };
+
+  const int kBodySize = 1;
+  MockRead reads[] = {
+    MockRead(SYNCHRONOUS, 5, "HTTP/1.1 200 OK\r\n"),
+    MockRead(SYNCHRONOUS, 6, "Content-Length: 1\r\n\r\n"),
+    MockRead(SYNCHRONOUS, 6, "Connection: Keep-Alive\r\n\r\n"),
+    MockRead(SYNCHRONOUS, 7, "1"),
+    MockRead(SYNCHRONOUS, 0, 8),  // EOF
+  };
+
+  StaticSocketDataProvider data(reads, arraysize(reads), writes,
+                                arraysize(writes));
+  data.set_connect_data(MockConnect(SYNCHRONOUS, OK));
+
+  scoped_ptr<MockTCPClientSocket> transport(
+      new MockTCPClientSocket(AddressList(), NULL, &data));
+
+  TestCompletionCallback callback;
+  ASSERT_EQ(OK, transport->Connect(callback.callback()));
+
+  scoped_ptr<ClientSocketHandle> socket_handle(new ClientSocketHandle);
+  socket_handle->SetSocket(transport.PassAs<StreamSocket>());
+
+  scoped_ptr<HttpRequestInfo> request_info(new HttpRequestInfo());
+  request_info->method = "GET";
+  request_info->url = GURL("http://somewhere/foo.html");
+
+  scoped_refptr<GrowableIOBuffer> read_buffer(new GrowableIOBuffer);
+  HttpStreamParser parser(socket_handle.get(), request_info.get(),
+                          read_buffer.get(), BoundNetLog());
+
+  scoped_ptr<HttpRequestHeaders> request_headers(new HttpRequestHeaders());
+  scoped_ptr<HttpResponseInfo> response_info(new HttpResponseInfo());
+  ASSERT_EQ(OK, parser.SendRequest("GET /foo.html HTTP/1.1\r\n",
+            *request_headers, response_info.get(), callback.callback()));
+  ASSERT_EQ(OK, parser.ReadResponseHeaders(callback.callback()));
+
+  // If the object that owns the HttpStreamParser is deleted, it takes the
+  // objects passed to the HttpStreamParser with it.
+  request_info.reset();
+  request_headers.reset();
+  response_info.reset();
+
+  scoped_refptr<IOBuffer> body_buffer(new IOBuffer(kBodySize));
+  ASSERT_EQ(kBodySize, parser.ReadResponseBody(
+      body_buffer.get(), kBodySize, callback.callback()));
+}
+
 }  // namespace
 
 }  // namespace net