Add an async ReadTrailers method to QuicChromiumClientStream::Handle

net/quic/chromium/quic_chromium_client_stream_test.cc

Index: net/quic/chromium/quic_chromium_client_stream_test.cc
diff --git a/net/quic/chromium/quic_chromium_client_stream_test.cc b/net/quic/chromium/quic_chromium_client_stream_test.cc
index 0623610f878860b1d4fed8e093a2e23bf693f2fb..76ccfa1d2fc04978f49867bdb001f7473692b92c 100644
--- a/net/quic/chromium/quic_chromium_client_stream_test.cc
+++ b/net/quic/chromium/quic_chromium_client_stream_test.cc
@@ -44,13 +44,6 @@ class MockDelegate : public QuicChromiumClientStream::Delegate {
 
   MOCK_METHOD0(OnSendData, int());
   MOCK_METHOD2(OnSendDataComplete, int(int, bool*));
-  void OnTrailingHeadersAvailable(const SpdyHeaderBlock& headers,
-                                  size_t frame_len) override {
-    trailers_ = headers.Clone();
-    OnTrailingHeadersAvailableMock(headers, frame_len);
-  }
-  MOCK_METHOD2(OnTrailingHeadersAvailableMock,
-               void(const SpdyHeaderBlock& headers, size_t frame_len));
   MOCK_METHOD0(OnClose, void());
   MOCK_METHOD1(OnError, void(int));
   MOCK_METHOD0(HasSendHeadersComplete, bool());
@@ -503,12 +496,12 @@ TEST_P(QuicChromiumClientStreamTest, OnTrailers) {
   trailers[kFinalOffsetHeaderKey] = base::IntToString(strlen(data));
 
   auto t = ProcessTrailers(trailers);
-  base::RunLoop run_loop;
-  EXPECT_CALL(delegate_,
-              OnTrailingHeadersAvailableMock(_, t.uncompressed_header_bytes()))
-      .WillOnce(testing::InvokeWithoutArgs([&run_loop]() { run_loop.Quit(); }));
 
-  run_loop.Run();
+  TestCompletionCallback trailers_callback;
+  EXPECT_EQ(static_cast<int>(t.uncompressed_header_bytes()),
+            handle_->ReadTrailingHeaders(&delegate_.trailers_,
+                                         trailers_callback.callback()));
+  base::RunLoop().RunUntilIdle();

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

Is "base::RunLoop().RunUntilIdle()" needed here?

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

Done.

[Ryan Hamilton, 2017/05/29 16:27:23]

Oh, whoops. Actually, this is needed. It's needed because the
ReadTraileringHeaders posts a task for the OnDataAvailable notification and we
need to wait for it to be delivered. I've added this line back.

Alternatively, we would expect ERR_IO_PENDING from ReadBody below. Do you have a
preference?

 
   // Read the body and verify that it arrives correctly.
   EXPECT_EQ(0,
@@ -550,13 +543,9 @@ TEST_P(QuicChromiumClientStreamTest, MarkTrailersConsumedWhenNotifyDelegate) {
   QuicHeaderList t = ProcessTrailers(trailers);
   EXPECT_FALSE(stream_->IsDoneReading());
 
-  base::RunLoop run_loop2;
-  EXPECT_CALL(delegate_,
-              OnTrailingHeadersAvailableMock(_, t.uncompressed_header_bytes()))
-      .WillOnce(
-          testing::InvokeWithoutArgs([&run_loop2]() { run_loop2.Quit(); }));
-
-  run_loop2.Run();
+  EXPECT_EQ(
+      static_cast<int>(t.uncompressed_header_bytes()),
+      handle_->ReadTrailingHeaders(&delegate_.trailers_, callback.callback()));
 
   // Read the body and verify that it arrives correctly.
   EXPECT_EQ(0, callback.WaitForResult());
@@ -607,13 +596,9 @@ TEST_P(QuicChromiumClientStreamTest, ReadAfterTrailersReceivedButNotDelivered) {
   // Trailers are not delivered
   EXPECT_FALSE(stream_->IsDoneReading());
 
-  base::RunLoop run_loop2;
-  EXPECT_CALL(delegate_,
-              OnTrailingHeadersAvailableMock(_, t.uncompressed_header_bytes()))
-      .WillOnce(
-          testing::InvokeWithoutArgs([&run_loop2]() { run_loop2.Quit(); }));
-
-  run_loop2.Run();
+  EXPECT_EQ(
+      static_cast<int>(t.uncompressed_header_bytes()),
+      handle_->ReadTrailingHeaders(&delegate_.trailers_, callback.callback()));
 
   // Read the body and verify that it arrives correctly.
   // OnDataAvailable() should follow right after and Read() will return 0.