Add an async ReadInitialHeaders method to QuicChromiumClientStream::Handle

net/quic/chromium/quic_http_stream_test.cc

Index: net/quic/chromium/quic_http_stream_test.cc
diff --git a/net/quic/chromium/quic_http_stream_test.cc b/net/quic/chromium/quic_http_stream_test.cc
index ff43b73adc0bb46dc6fd9662594eec77388c44a4..587a4acc1971c083da4a106fbb9a3e5cec2c81cf 100644
--- a/net/quic/chromium/quic_http_stream_test.cc
+++ b/net/quic/chromium/quic_http_stream_test.cc
@@ -107,11 +107,6 @@ class AutoClosingStream : public QuicHttpStream {
       HttpServerProperties* http_server_properties)
       : QuicHttpStream(std::move(session), http_server_properties) {}
 
-  void OnInitialHeadersAvailable(const SpdyHeaderBlock& headers,
-                                 size_t frame_len) override {
-    Close(false);
-  }
-
   void OnTrailingHeadersAvailable(const SpdyHeaderBlock& headers,
                                   size_t frame_len) override {
     Close(false);
@@ -185,6 +180,9 @@ class QuicHttpStreamPeer {
 };
 
 class QuicHttpStreamTest : public ::testing::TestWithParam<QuicVersion> {
+ public:
+  void CloseStream(QuicHttpStream* stream, int /*rv*/) { stream->Close(false); }
+
  protected:
   static const bool kFin = true;
   static const bool kIncludeVersion = true;
@@ -1159,10 +1157,8 @@ TEST_P(QuicHttpStreamTest, SendPostRequest) {
   ProcessPacket(ConstructResponseHeadersPacket(
       2, !kFin, &spdy_response_headers_frame_length));
 
-  // The headers have arrived, but they are delivered asynchronously.
-  EXPECT_THAT(stream_->ReadResponseHeaders(callback_.callback()),
-              IsError(ERR_IO_PENDING));
-  EXPECT_THAT(callback_.WaitForResult(), IsOk());
+  // The headers have already arrived.
+  EXPECT_THAT(stream_->ReadResponseHeaders(callback_.callback()), IsOk());
   ASSERT_TRUE(response_.headers.get());
   EXPECT_EQ(200, response_.headers->response_code());
   EXPECT_TRUE(response_.headers->HasHeaderValue("Content-Type", "text/plain"));
@@ -1232,10 +1228,8 @@ TEST_P(QuicHttpStreamTest, SendChunkedPostRequest) {
   ProcessPacket(ConstructResponseHeadersPacket(
       2, !kFin, &spdy_response_headers_frame_length));
 
-  // The headers have arrived, but they are delivered asynchronously
-  EXPECT_THAT(stream_->ReadResponseHeaders(callback_.callback()),
-              IsError(ERR_IO_PENDING));
-  EXPECT_THAT(callback_.WaitForResult(), IsOk());
+  // The headers have already arrived.
+  EXPECT_THAT(stream_->ReadResponseHeaders(callback_.callback()), IsOk());
   ASSERT_TRUE(response_.headers.get());
   EXPECT_EQ(200, response_.headers->response_code());
   EXPECT_TRUE(response_.headers->HasHeaderValue("Content-Type", "text/plain"));
@@ -1305,10 +1299,8 @@ TEST_P(QuicHttpStreamTest, SendChunkedPostRequestWithFinalEmptyDataPacket) {
   ProcessPacket(ConstructResponseHeadersPacket(
       2, !kFin, &spdy_response_headers_frame_length));
 
-  // The headers have arrived, but they are delivered asynchronously
-  EXPECT_THAT(stream_->ReadResponseHeaders(callback_.callback()),
-              IsError(ERR_IO_PENDING));
-  EXPECT_THAT(callback_.WaitForResult(), IsOk());
+  // The headers have already arrived.
+  EXPECT_THAT(stream_->ReadResponseHeaders(callback_.callback()), IsOk());
   ASSERT_TRUE(response_.headers.get());
   EXPECT_EQ(200, response_.headers->response_code());
   EXPECT_TRUE(response_.headers->HasHeaderValue("Content-Type", "text/plain"));
@@ -1373,10 +1365,8 @@ TEST_P(QuicHttpStreamTest, SendChunkedPostRequestWithOneEmptyDataPacket) {
   ProcessPacket(ConstructResponseHeadersPacket(
       2, !kFin, &spdy_response_headers_frame_length));
 
-  // The headers have arrived, but they are delivered asynchronously
-  EXPECT_THAT(stream_->ReadResponseHeaders(callback_.callback()),
-              IsError(ERR_IO_PENDING));
-  EXPECT_THAT(callback_.WaitForResult(), IsOk());
+  // The headers have already arrived.
+  EXPECT_THAT(stream_->ReadResponseHeaders(callback_.callback()), IsOk());
   ASSERT_TRUE(response_.headers.get());
   EXPECT_EQ(200, response_.headers->response_code());
   EXPECT_TRUE(response_.headers->HasHeaderValue("Content-Type", "text/plain"));
@@ -1427,7 +1417,9 @@ TEST_P(QuicHttpStreamTest, DestroyedEarly) {
 
   // Ack the request.
   ProcessPacket(ConstructServerAckPacket(1, 0, 0, 0));
-  EXPECT_THAT(stream_->ReadResponseHeaders(callback_.callback()),
+  EXPECT_THAT(stream_->ReadResponseHeaders(
+                  base::Bind(&QuicHttpStreamTest::CloseStream,
+                             base::Unretained(this), stream_.get())),
               IsError(ERR_IO_PENDING));
 
   // Send the response with a body.
@@ -1444,7 +1436,7 @@ TEST_P(QuicHttpStreamTest, DestroyedEarly) {
   EXPECT_EQ(static_cast<int64_t>(spdy_request_headers_frame_length),
             stream_->GetTotalSentBytes());
   // Zero since the stream is closed before processing the headers.
-  EXPECT_EQ(0, stream_->GetTotalReceivedBytes());
+  EXPECT_EQ(31, stream_->GetTotalReceivedBytes());
 }
 
 TEST_P(QuicHttpStreamTest, Priority) {
@@ -1480,7 +1472,9 @@ TEST_P(QuicHttpStreamTest, Priority) {
 
   // Ack the request.
   ProcessPacket(ConstructServerAckPacket(1, 0, 0, 0));
-  EXPECT_THAT(stream_->ReadResponseHeaders(callback_.callback()),
+  EXPECT_THAT(stream_->ReadResponseHeaders(
+                  base::Bind(&QuicHttpStreamTest::CloseStream,

[xunjieli, 2017/05/10 18:58:08]

This test doesn't need the CloseStream/"Close early" logic. We could use
|callback_.callback()| and wait for callback_ to complete.

[Ryan Hamilton, 2017/05/10 19:33:46]

Actually the test does rely on the stream closing currently because of the 3rd
write. But that doesn't seem important to the test, so I'll nuke it. Done.

+                             base::Unretained(this), stream_.get())),
               IsError(ERR_IO_PENDING));
 
   // Send the response with a body.
@@ -1497,7 +1491,7 @@ TEST_P(QuicHttpStreamTest, Priority) {
   EXPECT_EQ(static_cast<int64_t>(spdy_request_headers_frame_length),
             stream_->GetTotalSentBytes());
   // Zero since the stream is closed before processing the headers.

[xunjieli, 2017/05/10 18:58:08]

The comment is no longer accurate. Could you update this line?

[Ryan Hamilton, 2017/05/10 19:33:46]

Done.

-  EXPECT_EQ(0, stream_->GetTotalReceivedBytes());
+  EXPECT_EQ(31, stream_->GetTotalReceivedBytes());

[xunjieli, 2017/05/10 18:58:08]

Can we get "31" from ConstructResponseHeadersPacket() ?

[Ryan Hamilton, 2017/05/10 19:33:46]

Done.

 }
 
 // Regression test for http://crbug.com/294870