Add an async ReadTrailers method to QuicChromiumClientStream::Handle

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 214 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::OnTrailingHeadersAvailable(
-    const SpdyHeaderBlock& headers,
-    size_t frame_len) {
-  headers_bytes_received_ += frame_len;
-  if (delegate_)
-    delegate_->OnTrailersReceived(headers);
-  // |this| can be destroyed after this point.
-}
-
 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;
   }
@@ skipping 49 matching lines @@
   if (rv < 0) {
     NotifyError(rv);
     return;
   }
 
   headers_bytes_received_ += rv;
   negotiated_protocol_ = kProtoQUIC;
   connect_timing_ = session_->GetConnectTiming();
   if (delegate_)
     delegate_->OnHeadersReceived(initial_headers_);
+  base::ThreadTaskRunnerHandle::Get()->PostTask(
+      FROM_HERE, base::Bind(&BidirectionalStreamQuicImpl::ReadTrailingHeaders,
+                            weak_factory_.GetWeakPtr()));

[xunjieli, 2017/05/26 15:53:05]

Should we post this task before calling into the |delegate_| ?
If |delegate_| destroys |this| during the callback, then we will have a
segmentation fault when dereferencing |weak_factory_|, right?

[Ryan Hamilton, 2017/05/26 22:35:50]

Indeed! I noticed this in a followup, but forgot to upload the new patch set. :(

+}
+
+void BidirectionalStreamQuicImpl::ReadTrailingHeaders() {
+  int rv = stream_->ReadTrailingHeaders(
+      &trailing_headers_,
+      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);
+    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 (delegate_)
     delegate_->OnDataRead(rv);
 }
 
@@ skipping 25 matching lines @@
   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