Avoid crashing when a BidirectionalStreamQuicImpl is deleted during OnStreamReady.

net/quic/chromium/bidirectional_stream_quic_impl_unittest.cc

Index: net/quic/chromium/bidirectional_stream_quic_impl_unittest.cc
diff --git a/net/quic/chromium/bidirectional_stream_quic_impl_unittest.cc b/net/quic/chromium/bidirectional_stream_quic_impl_unittest.cc
index d4c0d028ca871c69d925203d9496bc12dc56bb66..32a7cfbd9a61d24798cf816bbe2dea3a29c6e52f 100644
--- a/net/quic/chromium/bidirectional_stream_quic_impl_unittest.cc
+++ b/net/quic/chromium/bidirectional_stream_quic_impl_unittest.cc
@@ -285,6 +285,7 @@ class DeleteStreamDelegate : public TestDelegateBase {
  public:
   // Specifies in which callback the stream can be deleted.
   enum Phase {
+    ON_STREAM_READY,
     ON_HEADERS_RECEIVED,
     ON_DATA_READ,
     ON_TRAILERS_RECEIVED,
@@ -295,13 +296,18 @@ class DeleteStreamDelegate : public TestDelegateBase {
       : TestDelegateBase(buf, buf_len), phase_(phase) {}
   ~DeleteStreamDelegate() override {}
 
+  void OnStreamReady(bool request_headers_sent) override {
+    TestDelegateBase::OnStreamReady(request_headers_sent);
+    if (phase_ == ON_STREAM_READY)
+      DeleteStream();
+  }
+
   void OnHeadersReceived(const SpdyHeaderBlock& response_headers) override {
     // Make a copy of |response_headers| before the stream is deleted, since
     // the headers are owned by the stream.
     SpdyHeaderBlock headers_copy = response_headers.Clone();
-    if (phase_ == ON_HEADERS_RECEIVED) {
+    if (phase_ == ON_HEADERS_RECEIVED)
       DeleteStream();
-    }
     TestDelegateBase::OnHeadersReceived(headers_copy);
   }
 
@@ -398,6 +404,11 @@ class BidirectionalStreamQuicImplTest
     writes_.push_back(PacketToWrite(SYNCHRONOUS, packet.release()));
   }
 
+  // Adds a write error to the list of expected writes.
+  void AddWriteError(IoMode mode, int rv) {
+    writes_.push_back(PacketToWrite(mode, rv));
+  }
+
   void ProcessPacket(std::unique_ptr<QuicReceivedPacket> packet) {
     connection_->ProcessUdpPacket(
         QuicSocketAddress(QuicSocketAddressImpl(self_addr_)),
@@ -594,20 +605,29 @@ class BidirectionalStreamQuicImplTest
 
   std::unique_ptr<QuicReceivedPacket> ConstructClientRstStreamPacket(
       QuicPacketNumber packet_number) {
-    return ConstructRstStreamCancelledPacket(packet_number, 0, &client_maker_);
+    return ConstructRstStreamCancelledPacket(packet_number, !kIncludeVersion, 0,
+                                             &client_maker_);
   }
 
   std::unique_ptr<QuicReceivedPacket> ConstructServerRstStreamPacket(
       QuicPacketNumber packet_number) {
-    return ConstructRstStreamCancelledPacket(packet_number, 0, &server_maker_);
+    return ConstructRstStreamCancelledPacket(packet_number, !kIncludeVersion, 0,
+                                             &server_maker_);
+  }
+
+  std::unique_ptr<QuicReceivedPacket> ConstructClientEarlyRstStreamPacket(
+      QuicPacketNumber packet_number) {
+    return ConstructRstStreamCancelledPacket(packet_number, kIncludeVersion, 0,
+                                             &client_maker_);
   }
 
   std::unique_ptr<QuicReceivedPacket> ConstructRstStreamCancelledPacket(
       QuicPacketNumber packet_number,
+      bool include_version,
       size_t bytes_written,
       QuicTestPacketMaker* maker) {
     std::unique_ptr<QuicReceivedPacket> packet(
-        maker->MakeRstPacket(packet_number, !kIncludeVersion, stream_id_,
+        maker->MakeRstPacket(packet_number, include_version, stream_id_,
                              QUIC_STREAM_CANCELLED, bytes_written));
     DVLOG(2) << "packet(" << packet_number << "): " << std::endl
              << QuicTextUtils::HexDump(packet->AsStringPiece());
@@ -1609,6 +1629,60 @@ TEST_P(BidirectionalStreamQuicImplTest, SessionClosedBeforeReadData) {
             delegate->GetTotalReceivedBytes());
 }
 
+TEST_P(BidirectionalStreamQuicImplTest, SessionCloseDuringOnStreamReady) {
+  SetRequest("POST", "/", DEFAULT_PRIORITY);
+  QuicStreamOffset header_stream_offset = 0;
+  AddWrite(ConstructInitialSettingsPacket(1, &header_stream_offset));
+  AddWriteError(SYNCHRONOUS, ERR_CONNECTION_REFUSED);
+
+  Initialize();
+
+  BidirectionalStreamRequestInfo request;
+  request.method = "POST";
+  request.url = GURL("http://www.google.com/");
+  request.end_stream_on_headers = false;
+  request.priority = DEFAULT_PRIORITY;
+
+  scoped_refptr<IOBuffer> read_buffer(new IOBuffer(kReadBufferSize));
+  std::unique_ptr<DeleteStreamDelegate> delegate(new DeleteStreamDelegate(
+      read_buffer.get(), kReadBufferSize, DeleteStreamDelegate::ON_FAILED));
+  delegate->Start(&request, net_log().bound(), session()->CreateHandle());
+  ConfirmHandshake();
+  delegate->WaitUntilNextCallback();  // OnStreamReady
+
+  EXPECT_EQ(0, delegate->on_data_read_count());
+  EXPECT_EQ(0, delegate->on_data_sent_count());
+}

[xunjieli, 2017/05/31 20:17:42]

nit: could use a blank line after }.

[Ryan Hamilton, 2017/05/31 20:30:36]

Done.

+TEST_P(BidirectionalStreamQuicImplTest, DeleteStreamDuringOnStreamReady) {
+  SetRequest("POST", "/", DEFAULT_PRIORITY);
+  size_t spdy_request_headers_frame_length;
+  QuicStreamOffset header_stream_offset = 0;
+  AddWrite(ConstructInitialSettingsPacket(1, &header_stream_offset));
+  AddWrite(ConstructRequestHeadersPacketInner(
+      2, GetNthClientInitiatedStreamId(0), !kFin, DEFAULT_PRIORITY,
+      &spdy_request_headers_frame_length, &header_stream_offset));
+  AddWrite(ConstructClientEarlyRstStreamPacket(3));
+
+  Initialize();
+
+  BidirectionalStreamRequestInfo request;
+  request.method = "POST";
+  request.url = GURL("http://www.google.com/");
+  request.end_stream_on_headers = false;
+  request.priority = DEFAULT_PRIORITY;
+
+  scoped_refptr<IOBuffer> read_buffer(new IOBuffer(kReadBufferSize));
+  std::unique_ptr<DeleteStreamDelegate> delegate(
+      new DeleteStreamDelegate(read_buffer.get(), kReadBufferSize,
+                               DeleteStreamDelegate::ON_STREAM_READY));
+  delegate->Start(&request, net_log().bound(), session()->CreateHandle());
+  ConfirmHandshake();
+  delegate->WaitUntilNextCallback();  // OnStreamReady
+
+  EXPECT_EQ(0, delegate->on_data_read_count());
+  EXPECT_EQ(0, delegate->on_data_sent_count());
+}
+
 TEST_P(BidirectionalStreamQuicImplTest, DeleteStreamAfterReadData) {
   SetRequest("POST", "/", DEFAULT_PRIORITY);
   size_t spdy_request_headers_frame_length;