Fix potential crash bug with BidirectionalStreamQuicImpl::SendRequestHeaders

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 5b6ecded6c2751369162c5f19319fa2d9b082176..f03600bdc66929305f93ca8af7320ce64698498b 100644
--- a/net/quic/chromium/bidirectional_stream_quic_impl_unittest.cc
+++ b/net/quic/chromium/bidirectional_stream_quic_impl_unittest.cc
@@ -164,17 +164,13 @@ class TestDelegateBase : public BidirectionalStreamImpl::Delegate {
   void SendData(const scoped_refptr<IOBuffer>& data,
                 int length,
                 bool end_of_stream) {
-    not_expect_callback_ = true;

[Ryan Hamilton, 2017/06/01 00:45:33]

This scares me, because it sure seems like the class and the tests are going to
pains to avoid calling reentrantly into the delegate. We discussed this issue in
chat, and you're confident that such reentrancy is not problematic so I'm
calling NotifyError directly in WriteHeaders(). But I could easily post a task
instead which would be safer but possibly paranoid?

     stream_->SendData(data, length, end_of_stream);
-    not_expect_callback_ = false;
   }
 
   void SendvData(const std::vector<scoped_refptr<IOBuffer>>& data,
                  const std::vector<int>& lengths,
                  bool end_of_stream) {
-    not_expect_callback_ = true;
     stream_->SendvData(data, lengths, end_of_stream);
-    not_expect_callback_ = false;
   }
 
   // Waits until next Delegate callback.
@@ -1221,6 +1217,77 @@ TEST_P(BidirectionalStreamQuicImplTest,
       delegate->GetTotalReceivedBytes());
 }
 
+// Tests that when request headers are delayed and SendData triggers the
+// headers to be sent, if that write fails the stream does not crash.
+TEST_P(BidirectionalStreamQuicImplTest,
+       SendDataWriteErrorCoalesceDataBufferAndHeaderFrame) {
+  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;
+  request.extra_headers.SetHeader("cookie", std::string(2048, 'A'));
+
+  scoped_refptr<IOBuffer> read_buffer(new IOBuffer(kReadBufferSize));
+  std::unique_ptr<DeleteStreamDelegate> delegate(new DeleteStreamDelegate(
+      read_buffer.get(), kReadBufferSize, DeleteStreamDelegate::ON_FAILED));
+  delegate->DoNotSendRequestHeadersAutomatically();
+  delegate->Start(&request, net_log().bound(), session()->CreateHandle());
+  ConfirmHandshake();
+  delegate->WaitUntilNextCallback();  // OnStreamReady
+
+  // Attempt to send the headers and data.
+  const char kBody1[] = "here are some data";
+  scoped_refptr<StringIOBuffer> buf1(new StringIOBuffer(kBody1));
+  delegate->SendData(buf1, buf1->size(), !kFin);
+
+  delegate->WaitUntilNextCallback();  // OnFailed
+  EXPECT_TRUE(delegate->on_failed_called());
+}
+
+// Tests that when request headers are delayed and SendvData triggers the
+// headers to be sent, if that write fails the stream does not crash.
+TEST_P(BidirectionalStreamQuicImplTest,
+       SendvDataWriteErrorCoalesceDataBufferAndHeaderFrame) {
+  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;
+  request.extra_headers.SetHeader("cookie", std::string(2048, 'A'));
+
+  scoped_refptr<IOBuffer> read_buffer(new IOBuffer(kReadBufferSize));
+  std::unique_ptr<DeleteStreamDelegate> delegate(new DeleteStreamDelegate(
+      read_buffer.get(), kReadBufferSize, DeleteStreamDelegate::ON_FAILED));
+  delegate->DoNotSendRequestHeadersAutomatically();
+  delegate->Start(&request, net_log().bound(), session()->CreateHandle());
+  ConfirmHandshake();
+  delegate->WaitUntilNextCallback();  // OnStreamReady
+
+  // Attempt to send the headers and data.
+  const char kBody1[] = "here are some data";
+  const char kBody2[] = "data keep coming";
+  scoped_refptr<StringIOBuffer> buf1(new StringIOBuffer(kBody1));
+  scoped_refptr<StringIOBuffer> buf2(new StringIOBuffer(kBody2));
+  std::vector<int> lengths = {buf1->size(), buf2->size()};
+  delegate->SendvData({buf1, buf2}, lengths, !kFin);
+
+  delegate->WaitUntilNextCallback();  // OnFailed
+  EXPECT_TRUE(delegate->on_failed_called());
+}
+
 TEST_P(BidirectionalStreamQuicImplTest, PostRequest) {
   SetRequest("POST", "/", DEFAULT_PRIORITY);
   size_t spdy_request_headers_frame_length;