SPDY: WriteHeaders should not invoke OnDataSent callback

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 124 matching lines @@
     // We must need to write stream data.
     // Until the headers have been completely sent, we can not be sure
     // that our stream_id is correct.
     DCHECK_GT(io_state_, STATE_SEND_HEADERS_COMPLETE);
     DCHECK_GT(stream_id_, 0u);
     DCHECK(!pending_frames_.empty());
 
     PendingFrame frame = pending_frames_.front();
     pending_frames_.pop_front();
 
+    waiting_completions_.push_back(frame.type);
+
     if (frame.type == TYPE_DATA) {
       // Send queued data frame.
       return frame.data_frame;
     } else {
-      DCHECK(frame.type == TYPE_HEADER);
+      DCHECK(frame.type == TYPE_HEADERS);

[Ryan Hamilton, 2012/08/13 17:10:02]

It doesn't look like the HEADERS frame is added to waiting_completions_.  Is
that expected?

[Takashi Toyoshima, 2012/08/14 09:14:02]

I added frame.type at line 145 for both of TYPE_DATA and TYPE_HEADERS.
Because only type is needed to select which callback should be invoked in
DoOpen().

       // Create actual HEADERS frame just in time because it depends on
       // compression context and should not be reordered after the creation.
       SpdyFrame* header_frame = session_->CreateHeadersFrame(
           stream_id_, *frame.header_block, SpdyControlFlags());
       delete frame.header_block;
       return header_frame;
     }
   }
   NOTREACHED();
 }
@@ skipping 413 matching lines @@
   return DoLoop(OK);
 }
 
 int SpdyStream::WriteHeaders(SpdyHeaderBlock* headers) {
   // Until the first headers by SYN_STREAM have been completely sent, we can
   // not be sure that our stream_id is correct.
   DCHECK_GT(io_state_, STATE_SEND_HEADERS_COMPLETE);
   CHECK_GT(stream_id_, 0u);
 
   PendingFrame frame;
-  frame.type = TYPE_HEADER;
+  frame.type = TYPE_HEADERS;
   frame.header_block = headers;
   pending_frames_.push_back(frame);
 
   SetHasWriteAvailable();
   return ERR_IO_PENDING;
 }
 
 int SpdyStream::WriteStreamData(IOBuffer* data,
                                 int length,
                                 SpdyDataFlags flags) {
@@ skipping 221 matching lines @@
   result = delegate_->OnSendBodyComplete(result, &eof);
   if (!eof)
     io_state_ = STATE_SEND_BODY;
   else
     io_state_ = STATE_WAITING_FOR_RESPONSE;
 
   return result;
 }
 
 int SpdyStream::DoOpen(int result) {
-  if (delegate_)
-    delegate_->OnDataSent(result);
+  if (delegate_) {
+    FrameType type = waiting_completions_.front();
+    waiting_completions_.pop_front();
+    if (type == TYPE_DATA) {
+      delegate_->OnDataSent(result);
+    } else {
+      DCHECK(type == TYPE_HEADERS);
+      delegate_->OnHeadersSent();
+    }
+  }
   io_state_ = STATE_OPEN;
   return result;
 }
 
 void SpdyStream::UpdateHistograms() {
   // We need all timers to be filled in, otherwise metrics can be bogus.
   if (send_time_.is_null() || recv_first_byte_time_.is_null() ||
       recv_last_byte_time_.is_null())
     return;
 
   UMA_HISTOGRAM_TIMES("Net.SpdyStreamTimeToFirstByte",
       recv_first_byte_time_ - send_time_);
   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