Remove QuicChromiumClientStream::Delegate in favor of async methods.

net/quic/chromium/bidirectional_stream_quic_impl.cc

Index: net/quic/chromium/bidirectional_stream_quic_impl.cc
diff --git a/net/quic/chromium/bidirectional_stream_quic_impl.cc b/net/quic/chromium/bidirectional_stream_quic_impl.cc
index e2301e76669373aee997163238f4870d5ff55bfd..9dce28689ad6c0188e363938243da9c5124704e5 100644
--- a/net/quic/chromium/bidirectional_stream_quic_impl.cc
+++ b/net/quic/chromium/bidirectional_stream_quic_impl.cc
@@ -28,6 +28,7 @@ BidirectionalStreamQuicImpl::BidirectionalStreamQuicImpl(
       delegate_(nullptr),
       response_status_(OK),
       negotiated_protocol_(kProtoUnknown),
+      expect_trailers_(true),
       read_buffer_len_(0),
       headers_bytes_received_(0),
       headers_bytes_sent_(0),
@@ -128,6 +129,7 @@ int BidirectionalStreamQuicImpl::ReadData(IOBuffer* buffer, int buffer_len) {
   if (stream_->IsDoneReading()) {
     // If the write side is closed, OnFinRead() will call
     // BidirectionalStreamQuicImpl::OnClose().
+    expect_trailers_ = false;
     stream_->OnFinRead();
   }
   return rv;
@@ -160,8 +162,7 @@ void BidirectionalStreamQuicImpl::SendData(const scoped_refptr<IOBuffer>& data,
       string_data, end_stream,
       base::Bind(&BidirectionalStreamQuicImpl::OnSendDataComplete,
                  weak_factory_.GetWeakPtr()));
-  DCHECK(rv == OK || rv == ERR_IO_PENDING);
-  if (rv == OK) {
+  if (rv != ERR_IO_PENDING) {
     base::ThreadTaskRunnerHandle::Get()->PostTask(
         FROM_HERE, base::Bind(&BidirectionalStreamQuicImpl::OnSendDataComplete,
                               weak_factory_.GetWeakPtr(), OK));

[xunjieli, 2017/05/31 00:25:22]

Need to pass |rv| instead of hardcoding OK here. 

[Ryan Hamilton, 2017/05/31 02:49:55]

I could do that but OnSendDataComplete has:

  DCHECK(rv == OK || !stream_);

Would you prefer I change == OK to >= 0?

[Ryan Hamilton, 2017/05/31 14:18:26]

Oh. I confused myself. Fixed to pass |rv| and cleaned up OnSendDataComplete()

@@ -194,8 +195,7 @@ void BidirectionalStreamQuicImpl::SendvData(
       base::Bind(&BidirectionalStreamQuicImpl::OnSendDataComplete,
                  weak_factory_.GetWeakPtr()));
 
-  DCHECK(rv == OK || rv == ERR_IO_PENDING);
-  if (rv == OK) {
+  if (rv != ERR_IO_PENDING) {
     base::ThreadTaskRunnerHandle::Get()->PostTask(
         FROM_HERE, base::Bind(&BidirectionalStreamQuicImpl::OnSendDataComplete,
                               weak_factory_.GetWeakPtr(), OK));

[xunjieli, 2017/05/31 00:25:22]

Need to pass |rv| instead of hardcoding OK here. 

[Ryan Hamilton, 2017/05/31 02:49:55]

Ditto. Happy to do whatever you prefer.

[Ryan Hamilton, 2017/05/31 14:18:26]

Done.

@@ -232,38 +232,11 @@ bool BidirectionalStreamQuicImpl::GetLoadTimingInfo(
   return true;
 }
 
-void BidirectionalStreamQuicImpl::OnClose() {
-  DCHECK(stream_);
-
-  if (stream_->connection_error() != QUIC_NO_ERROR ||
-      stream_->stream_error() != QUIC_STREAM_NO_ERROR) {
-    NotifyError(session_->IsCryptoHandshakeConfirmed()
-                    ? ERR_QUIC_PROTOCOL_ERROR
-                    : ERR_QUIC_HANDSHAKE_FAILED);
-    return;
-  }
-
-  if (!stream_->fin_sent() || !stream_->fin_received()) {
-    // The connection must have been closed by the peer with QUIC_NO_ERROR,
-    // which is improper.
-    NotifyError(ERR_UNEXPECTED);
-    return;
-  }
-
-  // The connection was closed normally so there is no need to notify
-  // the delegate.
-  ResetStream();
-}
-
-void BidirectionalStreamQuicImpl::OnError(int error) {
-  NotifyError(error);
-}
-
 void BidirectionalStreamQuicImpl::OnStreamReady(int rv) {
   DCHECK_NE(ERR_IO_PENDING, rv);
   DCHECK(rv == OK || !stream_);
   if (rv == OK) {
-    stream_ = session_->ReleaseStream(this);
+    stream_ = session_->ReleaseStream();
     NotifyStreamReady();
 
     rv = stream_->ReadInitialHeaders(
@@ -319,7 +292,8 @@ void BidirectionalStreamQuicImpl::ReadTrailingHeaders() {
 void BidirectionalStreamQuicImpl::OnReadTrailingHeadersComplete(int rv) {
   DCHECK_NE(ERR_IO_PENDING, rv);
   if (rv < 0) {
-    NotifyError(rv);
+    if (expect_trailers_)
+      NotifyError(rv);
     return;
   }
 
@@ -330,7 +304,6 @@ void BidirectionalStreamQuicImpl::OnReadTrailingHeadersComplete(int rv) {
 }
 
 void BidirectionalStreamQuicImpl::OnReadDataComplete(int rv) {
-  DCHECK_GE(rv, 0);
   read_buffer_ = nullptr;
   read_buffer_len_ = 0;
 
@@ -340,7 +313,12 @@ void BidirectionalStreamQuicImpl::OnReadDataComplete(int rv) {
     stream_->OnFinRead();
   }
 
-  if (delegate_)
+  if (!delegate_)
+    return;
+
+  if (rv < 0)
+    NotifyError(rv);
+  else
     delegate_->OnDataRead(rv);
 }
 
@@ -374,8 +352,6 @@ void BidirectionalStreamQuicImpl::ResetStream() {
   closed_stream_received_bytes_ = stream_->stream_bytes_read();
   closed_stream_sent_bytes_ = stream_->stream_bytes_written();
   closed_is_first_stream_ = stream_->IsFirstStream();
-  stream_->ClearDelegate();
-  stream_ = nullptr;
 }
 
 }  // namespace net