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

3 files changed, 107 insertions, 3 deletions

diff --git a/net/http/http_network_transaction_unittest.cc b/net/http/http_network_transaction_unittest.cc
index cf388d3..b47afdf 100644
--- a/net/http/http_network_transaction_unittest.cc
+++ b/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(
diff --git a/net/http/http_stream_parser.cc b/net/http/http_stream_parser.cc
index 820c1ba..30ba279 100644
--- a/net/http/http_stream_parser.cc
+++ b/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;
diff --git a/net/http/http_stream_parser_unittest.cc b/net/http/http_stream_parser_unittest.cc
index dcaf1f3..3391039 100644
--- a/net/http/http_stream_parser_unittest.cc
+++ b/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