Fix crash when socket connect callback is destroyed before being called

This is very similar to http://crbug.com/868220

TBR=dcheng@chromium.org

Bug: 875856
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: Ifa6e29c60d593f16d154cae551ccef6c12540c4c
Reviewed-on: https://chromium-review.googlesource.com/1184391
Commit-Queue: Clark DuVall <cduvall@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Helen Li <xunjieli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585258}

services/network/public/mojom/network_context.mojom

@@ -484,6 +484,9 @@ interface NetworkContext {
   // to receive data over the connection. On failure, |result| is a network
   // error code.
   //
+  // If the client closes |socket| before this completes, the callback will be
+  // invoked with net::ERR_ABORTED.
+  //
   // Any sockets that are created but are yet to be destroyed will be destroyed
   // when NetworkContext goes away.
   CreateTCPConnectedSocket(

services/network/tcp_connected_socket.cc

@@ -46,7 +46,16 @@ TCPConnectedSocket::TCPConnectedSocket(
       std::move(send_pipe_handle), traffic_annotation);
 }
 
-TCPConnectedSocket::~TCPConnectedSocket() {}
+TCPConnectedSocket::~TCPConnectedSocket() {
+  if (connect_callback_) {
+    // If |this| is destroyed when connect hasn't completed, tell the consumer
+    // that request has been aborted.
+    std::move(connect_callback_)
+        .Run(net::ERR_ABORTED, base::nullopt, base::nullopt,
+             mojo::ScopedDataPipeConsumerHandle(),
+             mojo::ScopedDataPipeProducerHandle());
+  }
+}
 
 void TCPConnectedSocket::Connect(
     const base::Optional<net::IPEndPoint>& local_addr,

services/network/tcp_socket_unittest.cc

@@ -333,6 +333,7 @@ class TCPSocketTest : public testing::Test {
   }
 
   TestSocketObserver* observer() { return &test_observer_; }
+  SocketFactory* factory() { return factory_.get(); }
 
  private:
   base::test::ScopedTaskEnvironment scoped_task_environment_;
@@ -1055,6 +1056,36 @@ TEST_F(TCPSocketWithMockSocketTest, SetNoDelayAndKeepAlive) {
   }
 }
 
+TEST_F(TCPSocketWithMockSocketTest, SocketDestroyedBeforeConnectCompletes) {
+  std::vector<net::MockRead> reads;
+  std::vector<net::MockWrite> writes;
+  net::StaticSocketDataProvider data_provider(reads, writes);
+  data_provider.set_connect_data(
+      net::MockConnect(net::ASYNC, net::ERR_IO_PENDING));
+  mock_client_socket_factory_.AddSocketDataProvider(&data_provider);
+
+  mojom::TCPConnectedSocketPtr client_socket;
+  net::IPEndPoint server_addr(net::IPAddress::IPv4Localhost(), 1234);
+  net::AddressList remote_addr_list(server_addr);
+  int net_error = net::OK;
+  base::RunLoop run_loop;
+  factory()->CreateTCPConnectedSocket(
+      base::nullopt, remote_addr_list, TRAFFIC_ANNOTATION_FOR_TESTS,
+      mojo::MakeRequest(&client_socket), nullptr,
+      base::BindLambdaForTesting(
+          [&](int result,
+              const base::Optional<net::IPEndPoint>& actual_local_addr,
+              const base::Optional<net::IPEndPoint>& peer_addr,
+              mojo::ScopedDataPipeConsumerHandle receive_pipe_handle,
+              mojo::ScopedDataPipeProducerHandle send_pipe_handle) {
+            net_error = result;
+            run_loop.Quit();
+          }));
+  client_socket.reset();
+  run_loop.Run();
+  EXPECT_EQ(net::ERR_ABORTED, net_error);
+}
+
 // Tests the case where net::ServerSocket::Listen() succeeds but
 // net::ServerSocket::GetLocalAddress() fails. This should still be considered
 // as a failure.