Use WeakPtrFactory in net::BidirectionalStream when posting task

net/http/bidirectional_stream.cc

Index: net/http/bidirectional_stream.cc
diff --git a/net/http/bidirectional_stream.cc b/net/http/bidirectional_stream.cc
index 9bf1c4886321494a00d05b97739a43b53de6f2c5..cc8a2827f69a8ff10f82ebb5ae93a9c294280c12 100644
--- a/net/http/bidirectional_stream.cc
+++ b/net/http/bidirectional_stream.cc
@@ -85,7 +85,8 @@ BidirectionalStream::BidirectionalStream(
       send_request_headers_automatically_(send_request_headers_automatically),
       request_headers_sent_(false),
       delegate_(delegate),
-      timer_(std::move(timer)) {
+      timer_(std::move(timer)),
+      weak_factory_(this) {
   DCHECK(delegate_);
   DCHECK(request_info_);
 
@@ -104,8 +105,8 @@ BidirectionalStream::BidirectionalStream(
   if (!request_info_->url.SchemeIs(url::kHttpsScheme)) {
     base::ThreadTaskRunnerHandle::Get()->PostTask(
         FROM_HERE,
-        base::Bind(&BidirectionalStream::Delegate::OnFailed,
-                   base::Unretained(delegate_), ERR_DISALLOWED_URL_SCHEME));
+        base::Bind(&BidirectionalStream::NotifyError,
+                   weak_factory_.GetWeakPtr(), ERR_DISALLOWED_URL_SCHEME));

[mmenke, 2016/06/06 19:34:00]

You may want a test for this, since this could cause actual crashes in
production code - create a bidirectional stream for a non-HTTPS URL, then delete
the BidirectionalStream without waiting, and the spin the message loop.  Could
also do one where you wait for the response, if you don't have one already.

[xunjieli, 2016/06/06 19:52:56]

Done. I have a unit where I wait for the response. I added a new one where we
don't wait, and this crashes without the patch. Could you take another look?
Thanks!

     return;
   }
 
@@ -220,7 +221,7 @@ void BidirectionalStream::OnHeadersReceived(
   HttpResponseInfo response_info;
   if (!SpdyHeadersToHttpResponse(response_headers, HTTP2, &response_info)) {
     DLOG(WARNING) << "Invalid headers";
-    delegate_->OnFailed(ERR_FAILED);
+    NotifyError(ERR_FAILED);
     return;
   }
   if (net_log_.IsCapturing()) {
@@ -270,7 +271,7 @@ void BidirectionalStream::OnTrailersReceived(const SpdyHeaderBlock& trailers) {
 }
 
 void BidirectionalStream::OnFailed(int status) {
-  delegate_->OnFailed(status);
+  NotifyError(status);
 }
 
 void BidirectionalStream::OnStreamReady(const SSLConfig& used_ssl_config,
@@ -306,7 +307,7 @@ void BidirectionalStream::OnStreamFailed(int result,
   DCHECK_NE(result, ERR_IO_PENDING);
   DCHECK(stream_request_);
 
-  delegate_->OnFailed(result);
+  NotifyError(result);
 }
 
 void BidirectionalStream::OnCertificateError(int result,
@@ -316,7 +317,7 @@ void BidirectionalStream::OnCertificateError(int result,
   DCHECK_NE(result, ERR_IO_PENDING);
   DCHECK(stream_request_);
 
-  delegate_->OnFailed(result);
+  NotifyError(result);
 }
 
 void BidirectionalStream::OnNeedsProxyAuth(
@@ -326,14 +327,14 @@ void BidirectionalStream::OnNeedsProxyAuth(
     HttpAuthController* auth_controller) {
   DCHECK(stream_request_);
 
-  delegate_->OnFailed(ERR_PROXY_AUTH_REQUESTED);
+  NotifyError(ERR_PROXY_AUTH_REQUESTED);
 }
 
 void BidirectionalStream::OnNeedsClientAuth(const SSLConfig& used_ssl_config,
                                             SSLCertRequestInfo* cert_info) {
   DCHECK(stream_request_);
 
-  delegate_->OnFailed(ERR_SSL_CLIENT_AUTH_CERT_NEEDED);
+  NotifyError(ERR_SSL_CLIENT_AUTH_CERT_NEEDED);
 }
 
 void BidirectionalStream::OnHttpsProxyTunnelResponse(
@@ -343,9 +344,13 @@ void BidirectionalStream::OnHttpsProxyTunnelResponse(
     HttpStream* stream) {
   DCHECK(stream_request_);
 
-  delegate_->OnFailed(ERR_HTTPS_PROXY_TUNNEL_RESPONSE);
+  NotifyError(ERR_HTTPS_PROXY_TUNNEL_RESPONSE);
 }
 
 void BidirectionalStream::OnQuicBroken() {}
 
+void BidirectionalStream::NotifyError(int error) {

[mmenke, 2016/06/06 19:34:00]

nit:  May want to call this NotifyFailed instead, to match the name of the
delegate's method.

[xunjieli, 2016/06/06 19:52:56]

Done.

+  delegate_->OnFailed(error);
+}
+
 }  // namespace net