Add an async ReadTrailers method to QuicChromiumClientStream::Handle

net/quic/chromium/quic_chromium_client_stream.cc

Index: net/quic/chromium/quic_chromium_client_stream.cc
diff --git a/net/quic/chromium/quic_chromium_client_stream.cc b/net/quic/chromium/quic_chromium_client_stream.cc
index a084fe5c87241530bb9d7f47436b4f43381e64a9..b1371d3279b77769975b998b0834c89ebae96e97 100644
--- a/net/quic/chromium/quic_chromium_client_stream.cc
+++ b/net/quic/chromium/quic_chromium_client_stream.cc
@@ -54,10 +54,15 @@ void QuicChromiumClientStream::Handle::OnInitialHeadersAvailable() {
   ResetAndReturn(&read_headers_callback_).Run(rv);
 }
 
-void QuicChromiumClientStream::Handle::OnTrailingHeadersAvailable(
-    const SpdyHeaderBlock& headers,
-    size_t frame_len) {
-  delegate_->OnTrailingHeadersAvailable(headers, frame_len);
+void QuicChromiumClientStream::Handle::OnTrailingHeadersAvailable() {
+  if (!read_headers_callback_)
+    return;  // Wait for ReadInitialHeaders to be called.
+
+  int rv = ERR_QUIC_PROTOCOL_ERROR;
+  if (!stream_->DeliverTrailingHeaders(read_headers_buffer_, &rv))
+    rv = ERR_QUIC_PROTOCOL_ERROR;
+
+  ResetAndReturn(&read_headers_callback_).Run(rv);
 }
 
 void QuicChromiumClientStream::Handle::OnDataAvailable() {
@@ -134,6 +139,21 @@ int QuicChromiumClientStream::Handle::ReadBody(
   return ERR_IO_PENDING;
 }
 
+int QuicChromiumClientStream::Handle::ReadTrailingHeaders(
+    SpdyHeaderBlock* header_block,
+    const CompletionCallback& callback) {
+  if (!stream_)
+    return ERR_CONNECTION_CLOSED;
+
+  int frame_len = 0;
+  if (stream_->DeliverTrailingHeaders(header_block, &frame_len))
+    return frame_len;
+
+  read_headers_buffer_ = header_block;
+  read_headers_callback_ = callback;
+  return ERR_IO_PENDING;
+}
+
 size_t QuicChromiumClientStream::Handle::WriteHeaders(
     SpdyHeaderBlock header_block,
     bool fin,
@@ -312,6 +332,7 @@ QuicChromiumClientStream::QuicChromiumClientStream(
       session_(session),
       can_migrate_(true),
       initial_headers_frame_len_(0),
+      trailing_headers_frame_len_(0),
       weak_factory_(this) {}
 
 QuicChromiumClientStream::~QuicChromiumClientStream() {
@@ -352,8 +373,11 @@ void QuicChromiumClientStream::OnTrailingHeadersComplete(
     size_t frame_len,
     const QuicHeaderList& header_list) {
   QuicSpdyStream::OnTrailingHeadersComplete(fin, frame_len, header_list);
-  NotifyHandleOfTrailingHeadersAvailableLater(received_trailers().Clone(),
-                                              frame_len);
+  trailing_headers_frame_len_ = frame_len;
+  if (handle_) {
+    // The handle will be notified of the headers via a posted task.
+    NotifyHandleOfTrailingHeadersAvailableLater();
+  }
 }
 
 void QuicChromiumClientStream::OnPromiseHeaderList(
@@ -514,21 +538,16 @@ void QuicChromiumClientStream::NotifyHandleOfInitialHeadersAvailable() {
     handle_->OnInitialHeadersAvailable();
 }
 
-void QuicChromiumClientStream::NotifyHandleOfTrailingHeadersAvailableLater(
-    SpdyHeaderBlock headers,
-    size_t frame_len) {
+void QuicChromiumClientStream::NotifyHandleOfTrailingHeadersAvailableLater() {
   DCHECK(handle_);
   base::ThreadTaskRunnerHandle::Get()->PostTask(
       FROM_HERE,
       base::Bind(
           &QuicChromiumClientStream::NotifyHandleOfTrailingHeadersAvailable,
-          weak_factory_.GetWeakPtr(), base::Passed(std::move(headers)),
-          frame_len));
+          weak_factory_.GetWeakPtr()));
 }
 
-void QuicChromiumClientStream::NotifyHandleOfTrailingHeadersAvailable(
-    SpdyHeaderBlock headers,
-    size_t frame_len) {
+void QuicChromiumClientStream::NotifyHandleOfTrailingHeadersAvailable() {
   if (!handle_)
     return;
 
@@ -537,10 +556,7 @@ void QuicChromiumClientStream::NotifyHandleOfTrailingHeadersAvailable(
   MarkTrailersConsumed();

[xunjieli, 2017/05/29 19:39:01]

Sorry I haven't realized earlier. Can we move "MarkTrailersConsumed()" to
DeliverTrailingHeaders() where DeliverTrailingHeaders() returned true? 
|trailers_consumed_| should only be set when received_trailers() have been read
and copied.

[Ryan Hamilton, 2017/05/29 20:29:57]

Ah, good point. Done.

   // Post an async task to notify delegate of the FIN flag.
   NotifyHandleOfDataAvailableLater();
-  net_log_.AddEvent(
-      NetLogEventType::QUIC_CHROMIUM_CLIENT_STREAM_READ_RESPONSE_TRAILERS,
-      base::Bind(&SpdyHeaderBlockNetLogCallback, &headers));
-  handle_->OnTrailingHeadersAvailable(headers, frame_len);
+  handle_->OnTrailingHeadersAvailable();
 }
 
 bool QuicChromiumClientStream::DeliverInitialHeaders(SpdyHeaderBlock* headers,
@@ -558,6 +574,20 @@ bool QuicChromiumClientStream::DeliverInitialHeaders(SpdyHeaderBlock* headers,
   return true;
 }
 
+bool QuicChromiumClientStream::DeliverTrailingHeaders(SpdyHeaderBlock* headers,
+                                                      int* frame_len) {
+  if (received_trailers().empty())
+    return false;
+
+  net_log_.AddEvent(
+      NetLogEventType::QUIC_CHROMIUM_CLIENT_STREAM_READ_RESPONSE_TRAILERS,
+      base::Bind(&SpdyHeaderBlockNetLogCallback, &received_trailers()));
+
+  *headers = received_trailers().Clone();
+  *frame_len = trailing_headers_frame_len_;
+  return true;
+}
+
 void QuicChromiumClientStream::NotifyHandleOfDataAvailableLater() {
   DCHECK(handle_);
   base::ThreadTaskRunnerHandle::Get()->PostTask(