Fix for spdy cancelling ServerBoundCertServiceRequest

net/spdy/spdy_stream.cc

 // Copyright (c) 2012 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/spdy/spdy_stream.h"
 
 #include "base/bind.h"
 #include "base/logging.h"
 #include "base/message_loop.h"
 #include "base/stringprintf.h"
@@ skipping 143 matching lines @@
       SpdyFrame* header_frame = session_->CreateHeadersFrame(
           stream_id_, *frame.header_block, SpdyControlFlags());
       delete frame.header_block;
       return header_frame;
     }
   }
   NOTREACHED();
 }
 
 SpdyStream::~SpdyStream() {
+  if (domain_bound_cert_request_handle_ != NULL) {
+    ServerBoundCertService* sbc_service = session_->GetServerBoundCertService();
+    sbc_service->CancelRequest(domain_bound_cert_request_handle_);
+    domain_bound_cert_request_handle_ = NULL;

[Ryan Hamilton, 2012/12/28 16:30:53]

This seems like a reasonable place to put this logic.  However, I think it would
be a good idea to use RAII.  Instead of making the RequestHandle a void*, it
could be an object with a destructor that would cancel the request (using
basically this code).  Then when someone make a request to the cert service,
they would store the handle in a scoped_ptr and would be guaranteed that it was
cleaned up on destruction.  Does this sounds feasible?  (I recently did
something like this in QUIC, and was happy with how it turned out.) 
SingleRequestHostResolver is a good example of wrapping up a request into a RAII
object.

[mattm, 2013/01/09 04:27:51]

Done.

[Ryan Hamilton, 2013/01/09 04:48:03]

Nice!  A bit more code to implement the RequestHandle, but it sure makes the
calling code trivial.

+  }
   UpdateHistograms();
   while (!pending_frames_.empty()) {
     PendingFrame frame = pending_frames_.back();
     pending_frames_.pop_back();
     if (frame.type == TYPE_DATA)
       delete frame.data_frame;
     else
       delete frame.header_block;
   }
 }
@@ skipping 373 matching lines @@
   Delegate* delegate = delegate_;
   delegate_ = NULL;
   if (delegate)
     delegate->OnClose(status);
 }
 
 void SpdyStream::Cancel() {
   if (cancelled())
     return;
 
+  if (domain_bound_cert_request_handle_ != NULL) {
+    ServerBoundCertService* sbc_service = session_->GetServerBoundCertService();
+    sbc_service->CancelRequest(domain_bound_cert_request_handle_);
+    domain_bound_cert_request_handle_ = NULL;
+  }
+
   cancelled_ = true;
   if (session_->IsStreamActive(stream_id_))
     session_->ResetStream(stream_id_, CANCEL, "");
   else if (stream_id_ == 0)
     session_->CloseCreatedStream(this, CANCEL);
 }
 
 void SpdyStream::Close() {
   if (stream_id_ != 0)
     session_->CloseStream(stream_id_, net::OK);
@@ skipping 73 matching lines @@
 
 GURL SpdyStream::GetUrl() const {
   DCHECK(HasUrl());
 
   const SpdyHeaderBlock& headers = (pushed_) ? *response_ : *request_;
   return GetUrlFromHeaderBlock(headers, GetProtocolVersion(), pushed_);
 }
 
 void SpdyStream::OnGetDomainBoundCertComplete(int result) {
   DCHECK_EQ(STATE_GET_DOMAIN_BOUND_CERT_COMPLETE, io_state_);
+  domain_bound_cert_request_handle_ = NULL;
   DoLoop(result);
 }
 
 int SpdyStream::DoLoop(int result) {
   do {
     State state = io_state_;
     io_state_ = STATE_NONE;
     switch (state) {
       // State machine 1: Send headers and body.
       case STATE_GET_DOMAIN_BOUND_CERT:
@@ skipping 198 matching lines @@
   UMA_HISTOGRAM_TIMES("Net.SpdyStreamDownloadTime",
       recv_last_byte_time_ - recv_first_byte_time_);
   UMA_HISTOGRAM_TIMES("Net.SpdyStreamTime",
       recv_last_byte_time_ - send_time_);
 
   UMA_HISTOGRAM_COUNTS("Net.SpdySendBytes", send_bytes_);
   UMA_HISTOGRAM_COUNTS("Net.SpdyRecvBytes", recv_bytes_);
 }
 
 }  // namespace net