Fix potential crash bug with BidirectionalStreamQuicImpl::SendRequestHeaders

net/quic/chromium/bidirectional_stream_quic_impl.cc

Index: net/quic/chromium/bidirectional_stream_quic_impl.cc
diff --git a/net/quic/chromium/bidirectional_stream_quic_impl.cc b/net/quic/chromium/bidirectional_stream_quic_impl.cc
index c3864fce8561b7ad838f3baef728c3a58ca391ac..77c7c30065820303cba00f9194e182a6fb8cf925 100644
--- a/net/quic/chromium/bidirectional_stream_quic_impl.cc
+++ b/net/quic/chromium/bidirectional_stream_quic_impl.cc
@@ -80,6 +80,10 @@ void BidirectionalStreamQuicImpl::Start(
 }
 
 void BidirectionalStreamQuicImpl::SendRequestHeaders() {
+  WriteHeaders();
+}
+
+bool BidirectionalStreamQuicImpl::WriteHeaders() {
   DCHECK(!has_sent_headers_);
   if (!stream_) {
     LOG(ERROR)
@@ -87,7 +91,7 @@ void BidirectionalStreamQuicImpl::SendRequestHeaders() {
     base::ThreadTaskRunnerHandle::Get()->PostTask(
         FROM_HERE, base::Bind(&BidirectionalStreamQuicImpl::NotifyError,
                               weak_factory_.GetWeakPtr(), ERR_UNEXPECTED));
-    return;
+    return false;
   }
 
   SpdyHeaderBlock headers;
@@ -103,9 +107,11 @@ void BidirectionalStreamQuicImpl::SendRequestHeaders() {
   size_t headers_bytes_sent = stream_->WriteHeaders(
       std::move(headers), request_info_->end_stream_on_headers, nullptr);

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

If this fails synchronously, then the stream will end up calling OnError on the
bidi stream which will call NotifyError synchronously. That makes me nervous,
because NotifyError() is called via PostTask up at the start of this method. But
maybe it's fine to call into the delegate reentrantly?

   if (!guard.get())
-    return;
+    return false;
+
   headers_bytes_sent_ += headers_bytes_sent;
   has_sent_headers_ = true;
+  return true;
 }
 
 int BidirectionalStreamQuicImpl::ReadData(IOBuffer* buffer, int buffer_len) {
@@ -156,7 +162,9 @@ void BidirectionalStreamQuicImpl::SendData(const scoped_refptr<IOBuffer>& data,
     // single data buffer.
     bundler =
         session_->CreatePacketBundler(QuicConnection::SEND_ACK_IF_PENDING);
-    SendRequestHeaders();
+    // Sending the request might result in |this| being deleted.
+    if (!WriteHeaders())
+      return;
   }
 
   QuicStringPiece string_data(data->data(), length);
@@ -190,7 +198,9 @@ void BidirectionalStreamQuicImpl::SendvData(
       session_->CreatePacketBundler(QuicConnection::SEND_ACK_IF_PENDING));
   if (!has_sent_headers_) {
     DCHECK(!send_request_headers_automatically_);
-    SendRequestHeaders();
+    // Sending the request might result in |this| being deleted.
+    if (!WriteHeaders())
+      return;
   }
 
   int rv = stream_->WritevStreamData(
@@ -373,13 +383,10 @@ void BidirectionalStreamQuicImpl::NotifyError(int error) {
 }
 
 void BidirectionalStreamQuicImpl::NotifyStreamReady() {
-  if (send_request_headers_automatically_) {
-    // Sending the request might result in |this| being deleted.
-    auto guard = weak_factory_.GetWeakPtr();
-    SendRequestHeaders();
-    if (!guard.get())
-      return;
-  }
+  // Sending the request might result in |this| being deleted.
+  if (send_request_headers_automatically_ && !WriteHeaders())
+    return;
+
   if (delegate_)
     delegate_->OnStreamReady(has_sent_headers_);
 }