Fix pipelining crash on canceled user callbacks.

User callbacks are run in separate tasks posted by the pipelining code. If the
user closes their stream before the task runs, we shouldn't try to call the
stale callback.

BUG=101936
TEST=net_unittests


Review URL: http://codereview.chromium.org/8414010

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

net/http/http_pipelined_connection_impl.cc

@@ -223,8 +223,10 @@ int HttpPipelinedConnectionImpl::DoSendComplete(int result) {
         FROM_HERE,
         method_factory_.NewRunnableMethod(
             &HttpPipelinedConnectionImpl::FireUserCallback,
-            send_user_callback_,
+            deferred_request.pipeline_id,
             result));
+    stream_info_map_[deferred_request.pipeline_id].pending_user_callback =
+        send_user_callback_;
     send_user_callback_ = NULL;
   }
   if (result < OK) {
@@ -348,11 +350,12 @@ int HttpPipelinedConnectionImpl::DoReadNextHeaders(int result) {
   if (rv != ERR_IO_PENDING && result == ERR_IO_PENDING) {
     read_next_state_ = READ_STATE_WAITING_FOR_CLOSE;
     read_user_callback_ = stream_info_map_[pipeline_id].read_headers_callback;
+    stream_info_map_[pipeline_id].pending_user_callback = read_user_callback_;
     MessageLoop::current()->PostTask(
         FROM_HERE,
         method_factory_.NewRunnableMethod(
             &HttpPipelinedConnectionImpl::FireUserCallback,
-            read_user_callback_,
+            pipeline_id,
             rv));
   }
   return rv;
@@ -361,12 +364,14 @@ int HttpPipelinedConnectionImpl::DoReadNextHeaders(int result) {
 int HttpPipelinedConnectionImpl::DoReadHeadersComplete(int result) {
   read_next_state_ = READ_STATE_WAITING_FOR_CLOSE;
   if (read_user_callback_) {
+    int pipeline_id = request_order_.front();
     MessageLoop::current()->PostTask(
         FROM_HERE,
         method_factory_.NewRunnableMethod(
             &HttpPipelinedConnectionImpl::FireUserCallback,
-            read_user_callback_,
+            pipeline_id,
             result));
+    stream_info_map_[pipeline_id].pending_user_callback = read_user_callback_;
     read_user_callback_ = NULL;
   }
   return result;
@@ -418,11 +423,13 @@ int HttpPipelinedConnectionImpl::DoEvictPendingReadHeaders(int result) {
       continue;
     }
     if (stream_info_map_[evicted_id].state != STREAM_CLOSED) {
+      stream_info_map_[evicted_id].pending_user_callback =
+          stream_info_map_[evicted_id].read_headers_callback;
       MessageLoop::current()->PostTask(
           FROM_HERE,
           method_factory_.NewRunnableMethod(
               &HttpPipelinedConnectionImpl::FireUserCallback,
-              stream_info_map_[evicted_id].read_headers_callback,
+              evicted_id,
               ERR_PIPELINE_EVICTION));
     }
     stream_info_map_[evicted_id].read_headers_callback = NULL;
@@ -560,11 +567,12 @@ void HttpPipelinedConnectionImpl::GetSSLCertRequestInfo(
       cert_request_info);
 }
 
-void HttpPipelinedConnectionImpl::FireUserCallback(
-    OldCompletionCallback* callback,
-    int result) {
-  CHECK(callback);
-  callback->Run(result);
+void HttpPipelinedConnectionImpl::FireUserCallback(int pipeline_id,
+                                                   int result) {
+  if (ContainsKey(stream_info_map_, pipeline_id)) {
+    CHECK(stream_info_map_[pipeline_id].pending_user_callback);
+    stream_info_map_[pipeline_id].pending_user_callback->Run(result);
+  }
 }
 
 int HttpPipelinedConnectionImpl::depth() const {

net/http/http_pipelined_connection_impl.h

@@ -160,6 +160,7 @@ class NET_EXPORT_PRIVATE HttpPipelinedConnectionImpl
 
     linked_ptr<HttpStreamParser> parser;
     OldCompletionCallback* read_headers_callback;
+    OldCompletionCallback* pending_user_callback;
     StreamState state;
   };
 
@@ -235,7 +236,7 @@ class NET_EXPORT_PRIVATE HttpPipelinedConnectionImpl
   // underlying parser completes SendRequest() or ReadResponseHeaders()
   // synchronously, but we've already returned ERR_IO_PENDING to the user's
   // SendRequest() or ReadResponseHeaders() call into us.
-  void FireUserCallback(OldCompletionCallback* callback, int result);
+  void FireUserCallback(int pipeline_id, int result);
 
   Delegate* delegate_;
   scoped_ptr<ClientSocketHandle> connection_;

net/http/http_pipelined_connection_impl_unittest.cc

@@ -1025,6 +1025,67 @@ TEST_F(HttpPipelinedConnectionImplTest, CloseOtherDuringReadCallback) {
   data_->RunFor(1);
 }
 
+TEST_F(HttpPipelinedConnectionImplTest, CloseBeforeSendCallbackRuns) {
+  MockWrite writes[] = {
+    MockWrite(true, 0, "GET /close.html HTTP/1.1\r\n\r\n"),
+    MockWrite(true, 1, "GET /dummy.html HTTP/1.1\r\n\r\n"),
+  };
+  Initialize(NULL, 0, writes, arraysize(writes));
+
+  scoped_ptr<HttpStream> close_stream(NewTestStream("close.html"));
+  scoped_ptr<HttpStream> dummy_stream(NewTestStream("dummy.html"));
+
+  scoped_ptr<TestOldCompletionCallback> close_callback(
+      new TestOldCompletionCallback);
+  HttpRequestHeaders headers;
+  HttpResponseInfo response;
+  EXPECT_EQ(ERR_IO_PENDING, close_stream->SendRequest(
+      headers, NULL, &response, close_callback.get()));
+
+  data_->RunFor(1);
+  EXPECT_FALSE(close_callback->have_result());
+
+  close_stream->Close(false);
+  close_stream.reset();
+  close_callback.reset();
+
+  MessageLoop::current()->RunAllPending();
+}
+
+TEST_F(HttpPipelinedConnectionImplTest, CloseBeforeReadCallbackRuns) {
+  MockWrite writes[] = {
+    MockWrite(false, 0, "GET /close.html HTTP/1.1\r\n\r\n"),
+    MockWrite(false, 3, "GET /dummy.html HTTP/1.1\r\n\r\n"),
+  };
+  MockRead reads[] = {
+    MockRead(false, 1, "HTTP/1.1 200 OK\r\n"),
+    MockRead(true, 2, "Content-Length: 7\r\n\r\n"),
+  };
+  Initialize(reads, arraysize(reads), writes, arraysize(writes));
+
+  scoped_ptr<HttpStream> close_stream(NewTestStream("close.html"));
+  scoped_ptr<HttpStream> dummy_stream(NewTestStream("dummy.html"));
+
+  HttpRequestHeaders headers;
+  HttpResponseInfo response;
+  EXPECT_EQ(OK,
+            close_stream->SendRequest(headers, NULL, &response, &callback_));
+
+  scoped_ptr<TestOldCompletionCallback> close_callback(
+      new TestOldCompletionCallback);
+  EXPECT_EQ(ERR_IO_PENDING,
+            close_stream->ReadResponseHeaders(close_callback.get()));
+
+  data_->RunFor(1);
+  EXPECT_FALSE(close_callback->have_result());
+
+  close_stream->Close(false);
+  close_stream.reset();
+  close_callback.reset();
+
+  MessageLoop::current()->RunAllPending();
+}
+
 TEST_F(HttpPipelinedConnectionImplTest, OnPipelineHasCapacity) {
   MockWrite writes[] = {
     MockWrite(false, 0, "GET /ok.html HTTP/1.1\r\n\r\n"),