Fix pipelining crash on canceled user callbacks.

net/http/http_pipelined_connection_impl.cc

 // Copyright (c) 2011 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 "net/http/http_pipelined_connection_impl.h"
 
 #include "base/message_loop.h"
 #include "base/stl_util.h"
 #include "net/base/io_buffer.h"
 #include "net/http/http_pipelined_stream.h"
@@ skipping 205 matching lines @@
   }
   if (result < OK) {
     send_next_state_ = SEND_STATE_UNUSABLE;
     usable_ = false;
   }
   if (send_user_callback_) {
     MessageLoop::current()->PostTask(
         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) {
     return result;
   }
   if (deferred_request_queue_.empty()) {
     send_next_state_ = SEND_STATE_NONE;
     return OK;
   }
   send_next_state_ = SEND_STATE_NEXT_REQUEST;
@@ skipping 103 matching lines @@
     CHECK_LE(OK, rv);
     read_next_state_ = READ_STATE_WAITING_FOR_CLOSE;
   }
 
   // |result| == ERR_IO_PENDING means this function was *not* called on the same
   // stack as ReadResponseHeaders(). That means we returned ERR_IO_PENDING to
   // ReadResponseHeaders() earlier and now need to invoke its callback.
   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;
 }
 
 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;
 }
 
 int HttpPipelinedConnectionImpl::DoReadWaitingForClose(int result) {
   read_next_state_ = READ_STATE_WAITING_FOR_CLOSE;
   return result;
 }
 
@@ skipping 31 matching lines @@
 
 int HttpPipelinedConnectionImpl::DoEvictPendingReadHeaders(int result) {
   while (!request_order_.empty()) {
     int evicted_id = request_order_.front();
     request_order_.pop();
     if (!ContainsKey(stream_info_map_, evicted_id) ||
         (stream_info_map_[evicted_id].read_headers_callback == NULL)) {
       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;
   }
   read_next_state_ = READ_STATE_NONE;
   return result;
 }
 
 void HttpPipelinedConnectionImpl::Close(int pipeline_id,
                                         bool not_reusable) {
@@ skipping 117 matching lines @@
 
 void HttpPipelinedConnectionImpl::GetSSLCertRequestInfo(
     int pipeline_id,
     SSLCertRequestInfo* cert_request_info) {
   CHECK(ContainsKey(stream_info_map_, pipeline_id));
   CHECK(stream_info_map_[pipeline_id].parser.get());
   return stream_info_map_[pipeline_id].parser->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);

[willchan no longer on Chromium, 2011/11/01 18:24:43]

In the future, a CHECK like this is unnecessary. We'll crash on the Run() call
anyway. Don't get me wrong, I like CHECKs, but I don't use them in cases like
these where it doesn't improve the diagnostics for the crash, since it has some
non-trivial logging costs in terms of binary size.

+    stream_info_map_[pipeline_id].pending_user_callback->Run(result);
+  }
 }
 
 int HttpPipelinedConnectionImpl::depth() const {
   return stream_info_map_.size();
 }
 
 bool HttpPipelinedConnectionImpl::usable() const {
   return usable_;
 }
 
@@ skipping 25 matching lines @@
 
 HttpPipelinedConnectionImpl::StreamInfo::StreamInfo()
     : read_headers_callback(NULL),
       state(STREAM_CREATED) {
 }
 
 HttpPipelinedConnectionImpl::StreamInfo::~StreamInfo() {
 }
 
 }  // namespace net