Add an async method to QuicChromiumClientSession::Handle for

net/quic/chromium/quic_network_transaction_unittest.cc

 // Copyright (c) 2012 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 <algorithm>
 #include <memory>
 #include <ostream>
 #include <string>
 #include <utility>
 #include <vector>
@@ skipping 4282 matching lines @@
   net_log_.GetEntries(&entries);
   EXPECT_LT(0u, entries.size());
 
   // Check that we logged a QUIC_HTTP_STREAM_ADOPTED_PUSH_STREAM
   int pos = ExpectLogContainsSomewhere(
       entries, 0, NetLogEventType::QUIC_HTTP_STREAM_ADOPTED_PUSH_STREAM,
       NetLogEventPhase::NONE);
   EXPECT_LT(0, pos);
 }
 
+// Regression test for http://crbug.com/719461 in which a promised stream
+// is closed before the pushed headers arrive, but after the connection
+// is closed and before the callbacks are executed.
+TEST_P(QuicNetworkTransactionTest, CancelServerPushAfterConnectionClose) {
+  session_params_.origins_to_force_quic_on.insert(
+      HostPortPair::FromString("mail.example.org:443"));
+
+  MockQuicData mock_quic_data;
+  QuicStreamOffset header_stream_offset = 0;
+  // Initial SETTINGS frame.
+  mock_quic_data.AddWrite(
+      ConstructInitialSettingsPacket(1, &header_stream_offset));
+  // First request: GET https://mail.example.org/
+  mock_quic_data.AddWrite(ConstructClientRequestHeadersPacket(
+      2, GetNthClientInitiatedStreamId(0), true, true,
+      GetRequestHeaders("GET", "https", "/"), &header_stream_offset));
+  QuicStreamOffset server_header_offset = 0;
+  // Server promise for: https://mail.example.org/pushed.jpg
+  mock_quic_data.AddRead(ConstructServerPushPromisePacket(
+      1, GetNthClientInitiatedStreamId(0), GetNthServerInitiatedStreamId(0),
+      false, GetRequestHeaders("GET", "https", "/pushed.jpg"),
+      &server_header_offset, &server_maker_));
+  // Response headers for first request.
+  mock_quic_data.AddRead(ConstructServerResponseHeadersPacket(
+      2, GetNthClientInitiatedStreamId(0), false, false,
+      GetResponseHeaders("200 OK"), &server_header_offset));
+  // Client ACKs the response headers.
+  mock_quic_data.AddWrite(ConstructClientAckPacket(3, 2, 1, 1));
+  // Response body for first request.
+  mock_quic_data.AddRead(ConstructServerDataPacket(
+      3, GetNthClientInitiatedStreamId(0), false, true, 0, "hello!"));
+  // Write error for the third request.
+  mock_quic_data.AddWrite(SYNCHRONOUS, ERR_FAILED);
+  mock_quic_data.AddRead(ASYNC, ERR_IO_PENDING);  // No more data to read
+  mock_quic_data.AddRead(ASYNC, 0);               // EOF
+  mock_quic_data.AddSocketDataToFactory(&socket_factory_);
+
+  CreateSession();
+
+  // Send a request which triggers a push promise from the server.
+  SendRequestAndExpectQuicResponse("hello!");
+
+  {
+    // Start a push transaction that will be cancelled after the connection
+    // is closed, but before the callback is executed.
+    request_.url = GURL("https://mail.example.org/pushed.jpg");
+    HttpNetworkTransaction trans(DEFAULT_PRIORITY, session_.get());

[xunjieli, 2017/06/22 14:43:30]

Minor nit: could you name this trans2, callback2? 

[Ryan Hamilton, 2017/06/22 20:40:38]

Done.

+    TestCompletionCallback callback;
+    int rv = trans.Start(&request_, callback.callback(), net_log_.bound());
+    EXPECT_THAT(rv, IsError(ERR_IO_PENDING));
+    base::RunLoop().RunUntilIdle();
+
+    // Cause the connection to close on a write error.
+    HttpNetworkTransaction trans2(DEFAULT_PRIORITY, session_.get());

[xunjieli, 2017/06/22 14:43:30]

Minor nit: could you name this trans3, callback3 and request3? It took me a
while to realize this is the third request and there is another one in
SendRequestAndExpectQuicResponse().

[Ryan Hamilton, 2017/06/22 20:40:38]

Done.

+    TestCompletionCallback callback2;
+    HttpRequestInfo request2;
+    request2.method = "GET";
+    request2.url = GURL("https://mail.example.org/");
+    request2.load_flags = 0;
+    EXPECT_THAT(trans2.Start(&request2, callback2.callback(), net_log_.bound()),
+                IsError(ERR_IO_PENDING));
+
+    base::RunLoop().RunUntilIdle();

[xunjieli, 2017/06/22 14:43:30]

|callback2| has completed at this point, right? To make it more explicit, should
we wait for callback2 to complete instead of using RunUntilIdle()?

[Ryan Hamilton, 2017/06/22 20:40:38]

Sure. Good point. In the process of doing this, I ended up switching to using a
unique_ptr for the 2nd request in order to ensure that it is deleted before we
WaitForResult on callback3. I think it's clearer now. PTAL.

+
+    // When |trans| goes out of scope, the underlying stream will be closed
+  }
+}
+
 TEST_P(QuicNetworkTransactionTest, QuicForceHolBlocking) {
   session_params_.quic_force_hol_blocking = true;
   session_params_.origins_to_force_quic_on.insert(
       HostPortPair::FromString("mail.example.org:443"));
 
   MockQuicData mock_quic_data;
 
   QuicStreamOffset offset = 0;
   mock_quic_data.AddWrite(ConstructInitialSettingsPacket(1, &offset));
   mock_quic_data.AddWrite(ConstructClientRequestHeadersPacket(
@@ skipping 714 matching lines @@
 
   request_.url = GURL("https://mail.example.org/pushed.jpg");
   ChunkedUploadDataStream upload_data(0);
   upload_data.AppendData("1", 1, true);
   request_.upload_data_stream = &upload_data;
   SendRequestAndExpectQuicResponse("and hello!");
 }
 
 }  // namespace test
 }  // namespace net