[SPDY] Remove SpdyStream::Delegate::OnSendBody()'s return value

net/spdy/spdy_http_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_http_stream.h"
 
 #include <algorithm>
 #include <list>
 
 #include "base/bind.h"
-#include "base/bind_helpers.h"
 #include "base/logging.h"
 #include "base/message_loop.h"
 #include "base/stringprintf.h"
 #include "net/base/host_port_pair.h"
 #include "net/base/net_log.h"
 #include "net/base/net_util.h"
 #include "net/base/upload_data_stream.h"
 #include "net/http/http_request_headers.h"
 #include "net/http/http_request_info.h"
 #include "net/http/http_response_info.h"
@@ skipping 252 matching lines @@
 }
 
 void SpdyHttpStream::Cancel() {
   callback_.Reset();
   if (stream_) {
     stream_->Cancel();
     DCHECK(!stream_);
   }
 }
 
-void SpdyHttpStream::OnStreamCreated(
-    const CompletionCallback& callback,
-    int rv) {
-  if (rv == OK) {
-    stream_ = stream_request_.ReleaseStream();
-    stream_->SetDelegate(this);
-  }
-  callback.Run(rv);
-}
-
-int SpdyHttpStream::SendData() {
-  CHECK(request_info_ && request_info_->upload_data_stream);
-  CHECK_EQ(0, request_body_buf_->BytesRemaining());
-
-  // Read the data from the request body stream.
-  const int bytes_read = request_info_->upload_data_stream->Read(
-      raw_request_body_buf_, raw_request_body_buf_->size(),
-      base::Bind(
-          base::IgnoreResult(&SpdyHttpStream::OnRequestBodyReadCompleted),
-          weak_factory_.GetWeakPtr()));
-
-  if (bytes_read == ERR_IO_PENDING)
-    return ERR_IO_PENDING;
-  // ERR_IO_PENDING is the only possible error.
-  DCHECK_GE(bytes_read, 0);
-  return OnRequestBodyReadCompleted(bytes_read);
-}
-
 SpdySendStatus SpdyHttpStream::OnSendHeadersComplete() {
   if (!callback_.is_null())
     DoCallback(OK);
   return has_upload_data_ ? MORE_DATA_TO_SEND : NO_MORE_DATA_TO_SEND;
 }
 
-int SpdyHttpStream::OnSendBody() {
+void SpdyHttpStream::OnSendBody() {
   CHECK(request_info_ && request_info_->upload_data_stream);
-  const bool eof = request_info_->upload_data_stream->IsEOF();
   if (request_body_buf_->BytesRemaining() > 0) {
-    stream_->QueueStreamData(
-        request_body_buf_,
-        request_body_buf_->BytesRemaining(),
-        eof ? DATA_FLAG_FIN : DATA_FLAG_NONE);
-    return ERR_IO_PENDING;
+    SendRequestBodyData();
+  } else {
+    // We shouldn't be called if there's no more data to read.
+    CHECK(!request_info_->upload_data_stream->IsEOF());
+    ReadAndSendRequestBodyData();
   }
-
-  // The entire body data has been sent.
-  if (eof)
-    return OK;
-
-  return SendData();
 }
 
 SpdySendStatus SpdyHttpStream::OnSendBodyComplete(size_t bytes_sent) {
   // |status| is the number of bytes written to the SPDY stream.
   CHECK(request_info_ && request_info_->upload_data_stream);
   DCHECK_GE(static_cast<int>(bytes_sent), 0);
   DCHECK_LE(static_cast<int>(bytes_sent), request_body_buf_->BytesRemaining());
 
   request_body_buf_->DidConsume(static_cast<int>(bytes_sent));
 
@@ skipping 110 matching lines @@
   stream_.reset();
   bool invoked_callback = false;
   if (status == net::OK) {
     // We need to complete any pending buffered read now.
     invoked_callback = DoBufferedReadCallback();
   }
   if (!invoked_callback && !callback_.is_null())
     DoCallback(status);
 }
 
+void SpdyHttpStream::OnStreamCreated(
+    const CompletionCallback& callback,
+    int rv) {
+  if (rv == OK) {
+    stream_ = stream_request_.ReleaseStream();
+    stream_->SetDelegate(this);
+  }
+  callback.Run(rv);
+}
+
+void SpdyHttpStream::ReadAndSendRequestBodyData() {
+  CHECK(request_info_ && request_info_->upload_data_stream);
+  CHECK_EQ(0, request_body_buf_->BytesRemaining());
+
+  // Read the data from the request body stream.
+  const int rv = request_info_->upload_data_stream->Read(
+      raw_request_body_buf_, raw_request_body_buf_->size(),
+      base::Bind(
+          &SpdyHttpStream::OnRequestBodyReadCompleted,
+          weak_factory_.GetWeakPtr()));
+
+  if (rv != ERR_IO_PENDING) {
+    // ERR_IO_PENDING is the only possible error.
+    CHECK_GE(rv, 0);
+    OnRequestBodyReadCompleted(rv);
+  }
+}
+
+void SpdyHttpStream::OnRequestBodyReadCompleted(int status) {
+  CHECK_GE(status, 0);
+  request_body_buf_ = new DrainableIOBuffer(raw_request_body_buf_, status);
+  SendRequestBodyData();
+}
+
+void SpdyHttpStream::SendRequestBodyData() {
+  const bool eof = request_info_->upload_data_stream->IsEOF();
+  if (eof) {
+    CHECK_GE(request_body_buf_->BytesRemaining(), 0);

[Ryan Hamilton, 2013/05/22 03:17:30]

Alternatively:
CHECK_GE(request_body_buf_->BytesRemaining(), eof ? 0 : 1);

Why CHECK not DCHECK?  Is this to get crash reports to help debug?

[akalin, 2013/05/22 08:40:05]

Hmm, I prefer the split-out CHECK, simply because one knows from examining a
crash dump what the value of eof is (from the line number).
Yeah, pretty much. If this happens in the wild, I'd like to know about it.

+  } else {
+    CHECK_GT(request_body_buf_->BytesRemaining(), 0);
+  }
+  stream_->QueueStreamData(request_body_buf_,
+                           request_body_buf_->BytesRemaining(),
+                           eof ? DATA_FLAG_FIN : DATA_FLAG_NONE);
+}
+
 void SpdyHttpStream::ScheduleBufferedReadCallback() {
   // If there is already a scheduled DoBufferedReadCallback, don't issue
   // another one.  Mark that we have received more data and return.
   if (buffered_read_callback_pending_) {
     more_read_data_pending_ = true;
     return;
   }
 
   more_read_data_pending_ = false;
   buffered_read_callback_pending_ = true;
@@ skipping 53 matching lines @@
 void SpdyHttpStream::DoCallback(int rv) {
   CHECK_NE(rv, ERR_IO_PENDING);
   CHECK(!callback_.is_null());
 
   // Since Run may result in being called back, clear user_callback_ in advance.
   CompletionCallback c = callback_;
   callback_.Reset();
   c.Run(rv);
 }
 
-int SpdyHttpStream::OnRequestBodyReadCompleted(int status) {
-  DCHECK_GE(status, 0);
-
-  request_body_buf_ = new DrainableIOBuffer(raw_request_body_buf_, status);
-
-  const bool eof = request_info_->upload_data_stream->IsEOF();
-  stream_->QueueStreamData(request_body_buf_,
-                           request_body_buf_->BytesRemaining(),
-                           eof ? DATA_FLAG_FIN : DATA_FLAG_NONE);
-  return ERR_IO_PENDING;
-}
-
 void SpdyHttpStream::GetSSLInfo(SSLInfo* ssl_info) {
   DCHECK(stream_);
   bool using_npn;
   NextProto protocol_negotiated = kProtoUnknown;
   stream_->GetSSLInfo(ssl_info, &using_npn, &protocol_negotiated);
 }
 
 void SpdyHttpStream::GetSSLCertRequestInfo(
     SSLCertRequestInfo* cert_request_info) {
   DCHECK(stream_);
   stream_->GetSSLCertRequestInfo(cert_request_info);
 }
 
 bool SpdyHttpStream::IsSpdyHttpStream() const {
   return true;
 }
 
 void SpdyHttpStream::Drain(HttpNetworkSession* session) {
   Close(false);
   delete this;
 }
 
 }  // namespace net