Remove QuicChromiumClientStream::Delegate in favor of async methods.

net/quic/chromium/bidirectional_stream_quic_impl.cc

 // Copyright 2016 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/quic/chromium/bidirectional_stream_quic_impl.h"
 
 #include <utility>
 
 #include "base/bind.h"
 #include "base/location.h"
@@ skipping 10 matching lines @@
 namespace net {
 
 BidirectionalStreamQuicImpl::BidirectionalStreamQuicImpl(
     std::unique_ptr<QuicChromiumClientSession::Handle> session)
     : session_(std::move(session)),
       stream_(nullptr),
       request_info_(nullptr),
       delegate_(nullptr),
       response_status_(OK),
       negotiated_protocol_(kProtoUnknown),
+      expect_trailers_(true),
       read_buffer_len_(0),
       headers_bytes_received_(0),
       headers_bytes_sent_(0),
       closed_stream_received_bytes_(0),
       closed_stream_sent_bytes_(0),
       closed_is_first_stream_(false),
       has_sent_headers_(false),
       send_request_headers_automatically_(true),
       weak_factory_(this) {}
 
@@ skipping 80 matching lines @@
     read_buffer_len_ = buffer_len;
     return ERR_IO_PENDING;
   }
 
   if (rv < 0)
     return rv;
 
   if (stream_->IsDoneReading()) {
     // If the write side is closed, OnFinRead() will call
     // BidirectionalStreamQuicImpl::OnClose().
+    expect_trailers_ = false;
     stream_->OnFinRead();
   }
   return rv;
 }
 
 void BidirectionalStreamQuicImpl::SendData(const scoped_refptr<IOBuffer>& data,
                                            int length,
                                            bool end_stream) {
   DCHECK(length > 0 || (length == 0 && end_stream));
   if (!stream_) {
@@ skipping 12 matching lines @@
     bundler =
         session_->CreatePacketBundler(QuicConnection::SEND_ACK_IF_PENDING);
     SendRequestHeaders();
   }
 
   QuicStringPiece string_data(data->data(), length);
   int rv = stream_->WriteStreamData(
       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()

   }
 }
 
 void BidirectionalStreamQuicImpl::SendvData(
     const std::vector<scoped_refptr<IOBuffer>>& buffers,
     const std::vector<int>& lengths,
     bool end_stream) {
   DCHECK_EQ(buffers.size(), lengths.size());
 
   if (!stream_) {
     LOG(ERROR) << "Trying to send data after stream has been destroyed.";
     base::ThreadTaskRunnerHandle::Get()->PostTask(
         FROM_HERE, base::Bind(&BidirectionalStreamQuicImpl::NotifyError,
                               weak_factory_.GetWeakPtr(), ERR_UNEXPECTED));
     return;
   }
 
   std::unique_ptr<QuicConnection::ScopedPacketBundler> bundler(
       session_->CreatePacketBundler(QuicConnection::SEND_ACK_IF_PENDING));
   if (!has_sent_headers_) {
     DCHECK(!send_request_headers_automatically_);
     SendRequestHeaders();
   }
 
   int rv = stream_->WritevStreamData(
       buffers, lengths, 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]

Ditto. Happy to do whatever you prefer.

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

Done.

   }
 }
 
 NextProto BidirectionalStreamQuicImpl::GetProtocol() const {
   return negotiated_protocol_;
 }
 
 int64_t BidirectionalStreamQuicImpl::GetTotalReceivedBytes() const {
   if (stream_)
     return headers_bytes_received_ + stream_->stream_bytes_read();
@@ skipping 13 matching lines @@
     is_first_stream = stream_->IsFirstStream();
   if (is_first_stream) {
     load_timing_info->socket_reused = false;
     load_timing_info->connect_timing = connect_timing_;
   } else {
     load_timing_info->socket_reused = true;
   }
   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(
         &initial_headers_,
         base::Bind(&BidirectionalStreamQuicImpl::OnReadInitialHeadersComplete,
                    weak_factory_.GetWeakPtr()));
     if (rv == ERR_IO_PENDING)
       return;
 
     OnReadInitialHeadersComplete(rv);
@@ skipping 35 matching lines @@
       base::Bind(&BidirectionalStreamQuicImpl::OnReadTrailingHeadersComplete,
                  weak_factory_.GetWeakPtr()));
 
   if (rv != ERR_IO_PENDING)
     OnReadTrailingHeadersComplete(rv);
 }
 
 void BidirectionalStreamQuicImpl::OnReadTrailingHeadersComplete(int rv) {
   DCHECK_NE(ERR_IO_PENDING, rv);
   if (rv < 0) {
-    NotifyError(rv);
+    if (expect_trailers_)
+      NotifyError(rv);
     return;
   }
 
   headers_bytes_received_ += rv;
 
   if (delegate_)
     delegate_->OnTrailersReceived(trailing_headers_);
 }
 
 void BidirectionalStreamQuicImpl::OnReadDataComplete(int rv) {
-  DCHECK_GE(rv, 0);
   read_buffer_ = nullptr;
   read_buffer_len_ = 0;
 
   if (stream_->IsDoneReading()) {
     // If the write side is closed, OnFinRead() will call
     // BidirectionalStreamQuicImpl::OnClose().
     stream_->OnFinRead();
   }
 
-  if (delegate_)
+  if (!delegate_)
+    return;
+
+  if (rv < 0)
+    NotifyError(rv);
+  else
     delegate_->OnDataRead(rv);
 }
 
 void BidirectionalStreamQuicImpl::NotifyError(int error) {
   DCHECK_NE(OK, error);
   DCHECK_NE(ERR_IO_PENDING, error);
 
   ResetStream();
   if (delegate_) {
     response_status_ = error;
@@ skipping 13 matching lines @@
   if (delegate_)
     delegate_->OnStreamReady(has_sent_headers_);
 }
 
 void BidirectionalStreamQuicImpl::ResetStream() {
   if (!stream_)
     return;
   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