Remove QuicChromiumClientStream::Delegate in favor of async methods.

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 2432a5c1c785c0369aeb26b7391a38279b75591a..5f7a7b1c22c11ce48e264be0858c1114af3a6d53 100644
--- a/net/quic/chromium/quic_http_stream_test.cc
+++ b/net/quic/chromium/quic_http_stream_test.cc
@@ -1403,7 +1403,7 @@ TEST_P(QuicHttpStreamTest, DestroyedEarly) {
   SetResponse("404 OK", "hello world!");
   // In the course of processing this packet, the QuicHttpStream close itself.
   size_t response_size = 0;
-  ProcessPacket(ConstructResponseHeadersPacket(2, kFin, &response_size));
+  ProcessPacket(ConstructResponseHeadersPacket(2, !kFin, &response_size));

[xunjieli, 2017/05/31 00:25:22]

Why is kFin changed to !kFin?

[Ryan Hamilton, 2017/05/31 02:49:56]

Ah! Good question :) So with the new code, if kFin is set, then when the HEADERS
are received, the stream becomes closed the "normal" way (fin sent and
received). As a result, the write on line 1383 does not happen (because the
stream is not reset). By using !kFin, the stream does not close normally and
instead it's the callback (of QuicHttpStreamTest::CloseStream) which actually
closes the stream.

I believe the old code passes because of buggy close handling in QuicHttpStream
which results in the stream being reset even though it is already closed. I can
make this change in a stand-alone CL if you'd prefer?

[xunjieli, 2017/06/01 15:30:32]

Acknowledged.

 
   base::RunLoop().RunUntilIdle();
 
@@ -1469,38 +1469,6 @@ TEST_P(QuicHttpStreamTest, Priority) {
             stream_->GetTotalReceivedBytes());
 }
 
-// Regression test for http://crbug.com/294870
-TEST_P(QuicHttpStreamTest, CheckPriorityWithNoDelegate) {

[xunjieli, 2017/05/31 00:25:22]

Why is this test deleted?

[Ryan Hamilton, 2017/05/31 02:49:56]

Ah, because this test is all about what happens when a stream has no delegate.
But there is no such thing as a delegate any longer, so this test has become
invalid.

[xunjieli, 2017/06/01 15:30:32]

Acknowledged.

-  SetRequest("GET", "/", MEDIUM);
-  QuicStreamOffset header_stream_offset = 0;
-  AddWrite(ConstructInitialSettingsPacket(&header_stream_offset));
-  AddWrite(ConstructClientRstStreamPacket(2));
-
-  Initialize();
-
-  request_.method = "GET";
-  request_.url = GURL("https://www.example.org/");
-
-  EXPECT_EQ(OK, stream_->InitializeStream(&request_, MEDIUM, net_log_.bound(),
-                                          callback_.callback()));
-
-  // Check that priority is highest.
-  QuicChromiumClientStream::Handle* reliable_stream =
-      QuicHttpStreamPeer::GetQuicChromiumClientStream(stream_.get());
-  DCHECK(reliable_stream);
-  QuicChromiumClientStream::Delegate* delegate = reliable_stream->GetDelegate();
-  DCHECK(delegate);
-  DCHECK_EQ(kV3HighestPriority, reliable_stream->priority());
-
-  // Set Delegate to nullptr and make sure Priority returns highest
-  // priority.
-  reliable_stream->ClearDelegate();
-  DCHECK_EQ(kV3HighestPriority, reliable_stream->priority());
-
-  EXPECT_EQ(0, stream_->GetTotalSentBytes());
-  EXPECT_EQ(0, stream_->GetTotalReceivedBytes());
-}
-
 TEST_P(QuicHttpStreamTest, SessionClosedDuringDoLoop) {
   SetRequest("POST", "/", DEFAULT_PRIORITY);
   size_t spdy_request_headers_frame_length;