[Cast Channel] Fix "leaky" CastChannelOpenFunction.

extensions/browser/api/cast_channel/cast_socket.h

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef EXTENSIONS_BROWSER_API_CAST_CHANNEL_CAST_SOCKET_H_
 #define EXTENSIONS_BROWSER_API_CAST_CHANNEL_CAST_SOCKET_H_
 
 #include <stdint.h>
 
 #include <queue>
@@ skipping 52 matching lines @@
   ~CastSocket() override {}
 
   // Used by BrowserContextKeyedAPIFactory.
   static const char* service_name() { return "CastSocketImplManager"; }
 
   // Connects the channel to the peer. If successful, the channel will be in
   // READY_STATE_OPEN.  DO NOT delete the CastSocket object in |callback|.
   // Instead use Close().
   // |callback| will be invoked with any ChannelError that occurred, or
   // CHANNEL_ERROR_NONE if successful.
+  // If the CastSocket is destroyed while the connection is pending, |callback|
+  // will be invoked with CHANNEL_ERROR_UNKNOWN. In this case, invoking

[mark a. foltz, 2017/02/21 18:30:01]

CHANNEL_ERROR_SOCKET_ERROR is used elsewhere for socket-level issues IIRC.

[imcheng, 2017/02/21 22:55:48]

socket_error is used for errors with read/write socket operations. I chose to
use unknown here since it is an edge case and it is unlikely the extension will
receive it anyway (since it is being unloaded).

+  // |callback| must not result in an re-entrancy behavior.

[mark a. foltz, 2017/02/21 18:30:01]

s/an/any/

[imcheng, 2017/02/21 22:55:48]

Done.

   // |delegate| receives message receipt and error events.
   // Ownership of |delegate| is transferred to this CastSocket.
   virtual void Connect(std::unique_ptr<CastTransport::Delegate> delegate,
                        base::Callback<void(ChannelError)> callback) = 0;
 
   // Closes the channel if not already closed. On completion, the channel will
   // be in READY_STATE_CLOSED.
   //
   // It is fine to delete this object in |callback|.
   virtual void Close(const net::CompletionCallback& callback) = 0;
@@ skipping 233 matching lines @@
   std::unique_ptr<net::SSLClientSocket> socket_;
 
   // Certificate of the peer. This field may be empty if the peer
   // certificate is not yet fetched.
   scoped_refptr<net::X509Certificate> peer_cert_;
 
   // Reply received from the receiver to a challenge request.
   std::unique_ptr<CastMessage> challenge_reply_;
 
   // Callback invoked when the socket is connected or fails to connect.
+  // TODO(imcheng): Change to OnceCallback? OnceCallback is move-only which

[mark a. foltz, 2017/02/21 18:30:01]

I think the benefit is pretty slight here, not sure it's worth a TODO. 
Personally, I'll probably use the new callbacks for new code but not try to
retrofit existing code.

[imcheng, 2017/02/21 22:55:47]

Ok, removed TODO.

+  // is an issue for gmock.
   base::Callback<void(ChannelError)> connect_callback_;
 
   // Callback invoked by |connect_timeout_timer_| to cancel the connection.
   base::CancelableClosure connect_timeout_callback_;
 
   // Duration to wait before timing out.
   base::TimeDelta connect_timeout_;
 
   // Timer invoked when the connection has timed out.
   std::unique_ptr<base::Timer> connect_timeout_timer_;
@@ skipping 42 matching lines @@
   // information.
   AuthTransportDelegate* auth_delegate_;
 
   DISALLOW_COPY_AND_ASSIGN(CastSocketImpl);
 };
 }  // namespace cast_channel
 }  // namespace api
 }  // namespace extensions
 
 #endif  // EXTENSIONS_BROWSER_API_CAST_CHANNEL_CAST_SOCKET_H_