Use WeakPtrFactory in net::BidirectionalStream when posting task

This CL uses WeakPtrFactory to post task in
net::BidirectionalStream.

BUG=606394

Review-Url: https://codereview.chromium.org/2043863002
Cr-Commit-Position: refs/heads/master@{#398132}

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::NotifyFailed,
+                   weak_factory_.GetWeakPtr(), ERR_DISALLOWED_URL_SCHEME));
     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);
+    NotifyFailed(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);
+  NotifyFailed(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);
+  NotifyFailed(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);
+  NotifyFailed(result);
 }
 
 void BidirectionalStream::OnNeedsProxyAuth(
@@ -326,14 +327,14 @@ void BidirectionalStream::OnNeedsProxyAuth(
     HttpAuthController* auth_controller) {
   DCHECK(stream_request_);
 
-  delegate_->OnFailed(ERR_PROXY_AUTH_REQUESTED);
+  NotifyFailed(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);
+  NotifyFailed(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);
+  NotifyFailed(ERR_HTTPS_PROXY_TUNNEL_RESPONSE);
 }
 
 void BidirectionalStream::OnQuicBroken() {}
 
+void BidirectionalStream::NotifyFailed(int error) {
+  delegate_->OnFailed(error);
+}
+
 }  // namespace net

net/http/bidirectional_stream.h

@@ -12,6 +12,7 @@
 #include "base/compiler_specific.h"
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
+#include "base/memory/weak_ptr.h"
 #include "net/http/bidirectional_stream_impl.h"
 #include "net/http/http_stream_factory.h"
 #include "net/log/net_log.h"
@@ -216,6 +217,9 @@ class NET_EXPORT BidirectionalStream
                                   HttpStream* stream) override;
   void OnQuicBroken() override;
 
+  // Helper method to notify delegate if there is an error.
+  void NotifyFailed(int error);
+
   // BidirectionalStreamRequestInfo used when requesting the stream.
   std::unique_ptr<BidirectionalStreamRequestInfo> request_info_;
   const BoundNetLog net_log_;
@@ -246,6 +250,8 @@ class NET_EXPORT BidirectionalStream
   // List of buffer length.
   std::vector<int> write_buffer_len_list_;
 
+  base::WeakPtrFactory<BidirectionalStream> weak_factory_;
+
   DISALLOW_COPY_AND_ASSIGN(BidirectionalStream);
 };
 

net/http/bidirectional_stream_unittest.cc

@@ -390,6 +390,26 @@ TEST_F(BidirectionalStreamTest, CreateInsecureStream) {
   EXPECT_EQ(ERR_DISALLOWED_URL_SCHEME, delegate.error());
 }
 
+// Creates a BidirectionalStream with an insecure scheme. Destroy the stream
+// without waiting for the OnFailed task to be executed.
+TEST_F(BidirectionalStreamTest,
+       CreateInsecureStreamAndDestroyStreamRightAfter) {
+  std::unique_ptr<BidirectionalStreamRequestInfo> request_info(
+      new BidirectionalStreamRequestInfo);
+  request_info->method = "GET";
+  request_info->url = GURL("http://www.example.org/");
+
+  std::unique_ptr<TestDelegateBase> delegate(new TestDelegateBase(nullptr, 0));
+  HttpNetworkSession::Params params =
+      SpdySessionDependencies::CreateSessionParams(&session_deps_);
+  std::unique_ptr<HttpNetworkSession> session(new HttpNetworkSession(params));
+  delegate->Start(std::move(request_info), session.get());
+  // Reset stream right before the OnFailed task is executed.
+  delegate.reset();
+
+  base::RunLoop().RunUntilIdle();
+}
+
 // Simulates user calling ReadData after END_STREAM has been received in
 // BidirectionalStreamSpdyImpl.
 TEST_F(BidirectionalStreamTest, TestReadDataAfterClose) {