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

Add comment explaining semantics of OnSendBody().
Audit SpdyStream::Delegate implementations to either
comply with those semantics or CHECK-fail.

Strengthen some related checks in SpdyHttpStream.

Fix typo in spdy_stream_spdy3_unittest.cc.

BUG=242288
R=rch@chromium.org

Review URL: https://codereview.chromium.org/15555003

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

12 files changed, 101 insertions, 97 deletions

diff --git a/net/spdy/spdy_http_stream.cc b/net/spdy/spdy_http_stream.cc
index 96995b8..3129412 100644
--- a/net/spdy/spdy_http_stream.cc
+++ b/net/spdy/spdy_http_stream.cc
@@ -8,7 +8,6 @@
 #include <list>
 
 #include "base/bind.h"
-#include "base/bind_helpers.h"
 #include "base/logging.h"
 #include "base/message_loop.h"
 #include "base/stringprintf.h"
@@ -281,56 +280,21 @@ void SpdyHttpStream::Cancel() {
   }
 }
 
-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) {
@@ -461,6 +425,52 @@ void SpdyHttpStream::OnClose(int status) {
     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);
+  } 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.
@@ -534,18 +544,6 @@ void SpdyHttpStream::DoCallback(int rv) {
   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;
diff --git a/net/spdy/spdy_http_stream.h b/net/spdy/spdy_http_stream.h
index bfb5076..5f405d3 100644
--- a/net/spdy/spdy_http_stream.h
+++ b/net/spdy/spdy_http_stream.h
@@ -85,7 +85,7 @@ class NET_EXPORT_PRIVATE SpdyHttpStream : public SpdyStream::Delegate,
 
   // SpdyStream::Delegate implementation.
   virtual SpdySendStatus OnSendHeadersComplete() OVERRIDE;
-  virtual int OnSendBody() OVERRIDE;
+  virtual void OnSendBody() OVERRIDE;
   virtual SpdySendStatus OnSendBodyComplete(size_t bytes_sent) OVERRIDE;
   virtual int OnResponseReceived(const SpdyHeaderBlock& response,
                                  base::Time response_time,
@@ -98,15 +98,21 @@ class NET_EXPORT_PRIVATE SpdyHttpStream : public SpdyStream::Delegate,
  private:
   void OnStreamCreated(const CompletionCallback& callback, int rv);
 
-  // Reads the data (whether chunked or not) from the request body stream and
-  // sends the data by calling WriteStreamData on the underlying SpdyStream.
-  int SendData();
+  // Reads the data (whether chunked or not) from the request body
+  // stream and sends it. The read and subsequent sending may happen
+  // asynchronously.
+  void ReadAndSendRequestBodyData();
+
+  // Called when data has just been read from the request body stream;
+  // does the actual sending of data.
+  void OnRequestBodyReadCompleted(int status);
+
+  // Queues some request body data to be sent.
+  void SendRequestBodyData();
 
   // Call the user callback.
   void DoCallback(int rv);
 
-  int OnRequestBodyReadCompleted(int status);
-
   void ScheduleBufferedReadCallback();
 
   // Returns true if the callback is invoked.
diff --git a/net/spdy/spdy_proxy_client_socket.cc b/net/spdy/spdy_proxy_client_socket.cc
index 2fcb899..5bc617c 100644
--- a/net/spdy/spdy_proxy_client_socket.cc
+++ b/net/spdy/spdy_proxy_client_socket.cc
@@ -468,18 +468,17 @@ SpdySendStatus SpdyProxyClientSocket::OnSendHeadersComplete() {
   return NO_MORE_DATA_TO_SEND;
 }
 
-int SpdyProxyClientSocket::OnSendBody() {
+void SpdyProxyClientSocket::OnSendBody() {
   // Because we use |spdy_stream_| via STATE_OPEN (ala WebSockets)
-  // OnSendBody() should never be called.
-  NOTREACHED();
-  return ERR_UNEXPECTED;
+  // OnSendBody() must never be called.
+  CHECK(false);
 }
 
 SpdySendStatus SpdyProxyClientSocket::OnSendBodyComplete(
     size_t /*bytes_sent*/) {
   // Because we use |spdy_stream_| via STATE_OPEN (ala WebSockets)
-  // OnSendBodyComplete() should never be called.
-  NOTREACHED();
+  // OnSendBodyComplete() must never be called.
+  CHECK(false);
   return NO_MORE_DATA_TO_SEND;
 }
 
diff --git a/net/spdy/spdy_proxy_client_socket.h b/net/spdy/spdy_proxy_client_socket.h
index f5b1baa..ee187d7 100644
--- a/net/spdy/spdy_proxy_client_socket.h
+++ b/net/spdy/spdy_proxy_client_socket.h
@@ -92,7 +92,7 @@ class NET_EXPORT_PRIVATE SpdyProxyClientSocket : public ProxyClientSocket,
 
   // SpdyStream::Delegate implementation.
   virtual SpdySendStatus OnSendHeadersComplete() OVERRIDE;
-  virtual int OnSendBody() OVERRIDE;
+  virtual void OnSendBody() OVERRIDE;
   virtual SpdySendStatus OnSendBodyComplete(size_t bytes_sent) OVERRIDE;
   virtual int OnResponseReceived(const SpdyHeaderBlock& response,
                                  base::Time response_time,
diff --git a/net/spdy/spdy_session_spdy3_unittest.cc b/net/spdy/spdy_session_spdy3_unittest.cc
index fa54b16..21fe59a 100644
--- a/net/spdy/spdy_session_spdy3_unittest.cc
+++ b/net/spdy/spdy_session_spdy3_unittest.cc
@@ -3200,13 +3200,12 @@ class StreamClosingDelegate : public test::StreamDelegateWithBody {
     stream_to_close_ = stream_to_close;
   }
 
-  virtual int OnSendBody() OVERRIDE {
-    int rv = test::StreamDelegateWithBody::OnSendBody();
+  virtual void OnSendBody() OVERRIDE {
+    test::StreamDelegateWithBody::OnSendBody();
     if (stream_to_close_) {
       stream_to_close_->Close();
       EXPECT_EQ(NULL, stream_to_close_.get());
     }
-    return rv;
   }
 
  private:
diff --git a/net/spdy/spdy_stream.cc b/net/spdy/spdy_stream.cc
index 3dc00e9..936fdf9 100644
--- a/net/spdy/spdy_stream.cc
+++ b/net/spdy/spdy_stream.cc
@@ -910,9 +910,8 @@ int SpdyStream::DoSendBody() {
   // data portion, of course.
   io_state_ = STATE_SEND_BODY_COMPLETE;
   CHECK(delegate_);
-  int status = delegate_->OnSendBody();
-  CHECK(status == OK || status == ERR_IO_PENDING);
-  return status;
+  delegate_->OnSendBody();
+  return ERR_IO_PENDING;
 }
 
 int SpdyStream::DoSendBodyComplete(int result) {
diff --git a/net/spdy/spdy_stream.h b/net/spdy/spdy_stream.h
index 8ead4e6..b9ecfe9 100644
--- a/net/spdy/spdy_stream.h
+++ b/net/spdy/spdy_stream.h
@@ -60,10 +60,14 @@ class NET_EXPORT_PRIVATE SpdyStream {
     // Returns true if no more data to be sent after SYN frame.
     virtual SpdySendStatus OnSendHeadersComplete() = 0;
 
-    // Called when stream is ready to send data.
-    // Returns network error code. OK when it successfully sent data.
-    // ERR_IO_PENDING when performing operation asynchronously.
-    virtual int OnSendBody() = 0;
+    // Called when the stream is ready to send body data.  The
+    // delegate must call QueueStreamData() on the stream, either
+    // immediately or asynchronously (e.g., if the data to be send has
+    // to be read asynchronously).
+    //
+    // Called only when OnSendHeadersComplete() or
+    // OnSendBodyComplete() returns MORE_DATA_TO_SEND.
+    virtual void OnSendBody() = 0;
 
     // Called when body data has been sent. |bytes_sent| is the number
     // of bytes that has been sent (may be zero). Must return whether
diff --git a/net/spdy/spdy_stream_spdy3_unittest.cc b/net/spdy/spdy_stream_spdy3_unittest.cc
index f009043..b08dcd1 100644
--- a/net/spdy/spdy_stream_spdy3_unittest.cc
+++ b/net/spdy/spdy_stream_spdy3_unittest.cc
@@ -544,8 +544,8 @@ TEST_F(SpdyStreamSpdy3Test, ResumeAfterSendWindowSizeAdjustRequestResponse) {
 }
 
 // Given an unstall function, runs a test to make sure that a
-// bidrectional (i.e., non-HTTP-like) stream resumes after a stall and
-// unstall.
+// bidirectional (i.e., non-HTTP-like) stream resumes after a stall
+// and unstall.
 void SpdyStreamSpdy3Test::RunResumeAfterUnstallBidirectionalTest(
     const UnstallFunction& unstall_function) {
   GURL url(kStreamUrl);
diff --git a/net/spdy/spdy_stream_test_util.cc b/net/spdy/spdy_stream_test_util.cc
index b434f3e..c9a06fd 100644
--- a/net/spdy/spdy_stream_test_util.cc
+++ b/net/spdy/spdy_stream_test_util.cc
@@ -24,8 +24,8 @@ SpdySendStatus ClosingDelegate::OnSendHeadersComplete() {
   return NO_MORE_DATA_TO_SEND;
 }
 
-int ClosingDelegate::OnSendBody() {
-  return OK;
+void ClosingDelegate::OnSendBody() {
+  ADD_FAILURE() << "OnSendBody should not be called";
 }
 
 SpdySendStatus ClosingDelegate::OnSendBodyComplete(size_t /*bytes_sent*/) {
@@ -131,9 +131,10 @@ StreamDelegateDoNothing::StreamDelegateDoNothing(
 StreamDelegateDoNothing::~StreamDelegateDoNothing() {
 }
 
-int StreamDelegateDoNothing::OnSendBody() {
-  return OK;
+void StreamDelegateDoNothing::OnSendBody() {
+  ADD_FAILURE() << "OnSendBody should not be called";
 }
+
 SpdySendStatus StreamDelegateDoNothing::OnSendBodyComplete(
     size_t /*bytes_sent*/) {
   return NO_MORE_DATA_TO_SEND;
@@ -150,10 +151,10 @@ StreamDelegateSendImmediate::StreamDelegateSendImmediate(
 StreamDelegateSendImmediate::~StreamDelegateSendImmediate() {
 }
 
-int StreamDelegateSendImmediate::OnSendBody() {
+void StreamDelegateSendImmediate::OnSendBody() {
   ADD_FAILURE() << "OnSendBody should not be called";
-  return ERR_UNEXPECTED;
 }
+
 SpdySendStatus StreamDelegateSendImmediate::OnSendBodyComplete(
     size_t /*bytes_sent*/) {
   ADD_FAILURE() << "OnSendBodyComplete should not be called";
@@ -192,10 +193,9 @@ SpdySendStatus StreamDelegateWithBody::OnSendHeadersComplete() {
   return MORE_DATA_TO_SEND;
 }
 
-int StreamDelegateWithBody::OnSendBody() {
+void StreamDelegateWithBody::OnSendBody() {
   stream()->QueueStreamData(buf_.get(), buf_->BytesRemaining(),
                             DATA_FLAG_NONE);
-  return ERR_IO_PENDING;
 }
 
 SpdySendStatus StreamDelegateWithBody::OnSendBodyComplete(size_t bytes_sent) {
diff --git a/net/spdy/spdy_stream_test_util.h b/net/spdy/spdy_stream_test_util.h
index 2dacb63..334f1eb 100644
--- a/net/spdy/spdy_stream_test_util.h
+++ b/net/spdy/spdy_stream_test_util.h
@@ -27,7 +27,7 @@ class ClosingDelegate : public SpdyStream::Delegate {
 
   // SpdyStream::Delegate implementation.
   virtual SpdySendStatus OnSendHeadersComplete() OVERRIDE;
-  virtual int OnSendBody() OVERRIDE;
+  virtual void OnSendBody() OVERRIDE;
   virtual SpdySendStatus OnSendBodyComplete(size_t bytes_sent) OVERRIDE;
   virtual int OnResponseReceived(const SpdyHeaderBlock& response,
                                  base::Time response_time,
@@ -52,7 +52,7 @@ class StreamDelegateBase : public SpdyStream::Delegate {
   virtual ~StreamDelegateBase();
 
   virtual SpdySendStatus OnSendHeadersComplete() OVERRIDE;
-  virtual int OnSendBody() = 0;
+  virtual void OnSendBody() = 0;
   virtual SpdySendStatus OnSendBodyComplete(size_t bytes_sent) = 0;
   virtual int OnResponseReceived(const SpdyHeaderBlock& response,
                                  base::Time response_time,
@@ -103,7 +103,7 @@ class StreamDelegateDoNothing : public StreamDelegateBase {
   StreamDelegateDoNothing(const base::WeakPtr<SpdyStream>& stream);
   virtual ~StreamDelegateDoNothing();
 
-  virtual int OnSendBody() OVERRIDE;
+  virtual void OnSendBody() OVERRIDE;
   virtual SpdySendStatus OnSendBodyComplete(size_t bytes_sent) OVERRIDE;
 };
 
@@ -116,7 +116,7 @@ class StreamDelegateSendImmediate : public StreamDelegateBase {
                               base::StringPiece data);
   virtual ~StreamDelegateSendImmediate();
 
-  virtual int OnSendBody() OVERRIDE;
+  virtual void OnSendBody() OVERRIDE;
   virtual SpdySendStatus OnSendBodyComplete(size_t bytes_sent) OVERRIDE;
   virtual int OnResponseReceived(const SpdyHeaderBlock& response,
                                  base::Time response_time,
@@ -135,7 +135,7 @@ class StreamDelegateWithBody : public StreamDelegateBase {
   virtual ~StreamDelegateWithBody();
 
   virtual SpdySendStatus OnSendHeadersComplete() OVERRIDE;
-  virtual int OnSendBody() OVERRIDE;
+  virtual void OnSendBody() OVERRIDE;
   virtual SpdySendStatus OnSendBodyComplete(size_t bytes_sent) OVERRIDE;
 
   int body_data_sent() const { return body_data_sent_; }
diff --git a/net/spdy/spdy_websocket_stream.cc b/net/spdy/spdy_websocket_stream.cc
index d503fa2..8dc2f4a 100644
--- a/net/spdy/spdy_websocket_stream.cc
+++ b/net/spdy/spdy_websocket_stream.cc
@@ -86,13 +86,12 @@ SpdySendStatus SpdyWebSocketStream::OnSendHeadersComplete() {
   return NO_MORE_DATA_TO_SEND;
 }
 
-int SpdyWebSocketStream::OnSendBody() {
-  NOTREACHED();
-  return ERR_UNEXPECTED;
+void SpdyWebSocketStream::OnSendBody() {
+  CHECK(false);
 }
 
 SpdySendStatus SpdyWebSocketStream::OnSendBodyComplete(size_t bytes_sent) {
-  NOTREACHED();
+  CHECK(false);
   return NO_MORE_DATA_TO_SEND;
 }
 
diff --git a/net/spdy/spdy_websocket_stream.h b/net/spdy/spdy_websocket_stream.h
index a589ae0..77fe1ed 100644
--- a/net/spdy/spdy_websocket_stream.h
+++ b/net/spdy/spdy_websocket_stream.h
@@ -75,7 +75,7 @@ class NET_EXPORT_PRIVATE SpdyWebSocketStream
 
   // SpdyStream::Delegate
   virtual SpdySendStatus OnSendHeadersComplete() OVERRIDE;
-  virtual int OnSendBody() OVERRIDE;
+  virtual void OnSendBody() OVERRIDE;
   virtual SpdySendStatus OnSendBodyComplete(size_t bytes_sent) OVERRIDE;
   virtual int OnResponseReceived(const SpdyHeaderBlock& response,
                                  base::Time response_time,