[cast_channel] Implement CastSocketService::OpenSocket()

components/cast_channel/cast_socket_service.h

Index: components/cast_channel/cast_socket_service.h
diff --git a/components/cast_channel/cast_socket_service.h b/components/cast_channel/cast_socket_service.h
index e01072620507ec56e43f47321a17223a7111d172..37b46c3fb631393bdf4f4772b81331402e520273 100644
--- a/components/cast_channel/cast_socket_service.h
+++ b/components/cast_channel/cast_socket_service.h
@@ -36,6 +36,44 @@ class CastSocketService : public RefcountedKeyedService {
   // otherwise.
   CastSocket* GetSocket(int channel_id) const;
 
+  CastSocket* GetSocket(const net::IPEndPoint& ip_endpoint) const;
+
+  // Opens cast socket with |ip_endpoint| and invokes |open_cb| when opening
+  // operation finishes. If cast socket with |ip_endpoint| already exists,
+  // invoke |open_cb| directly with existing socket's channel ID.
+  // Parameters:
+  // |ip_endpoint|: IP address of the remote host.

[mark a. foltz, 2017/06/21 17:41:42]

Nit: an endpoint is both an address and a port

[zhaobin, 2017/06/21 21:58:25]

Done.

+  // |channel_auth|: Authentication method used for connecting to a Cast

[mark a. foltz, 2017/06/21 17:41:42]

Remove this comment

[zhaobin, 2017/06/21 21:58:26]

Done.

+  //                 receiver.
+  // |net_log|: Log of socket events.
+  // |connect_timeout|: Connection timeout interval.
+  // |liveness_timeout|: Liveness timeout for connect calls.
+  // |ping_interval|: Ping interval.
+  // |logger|: Log of cast channel events.
+  // |device_capabilities|: Device capabilities.
+  // |open_cb|: Invoked when cast socket opens.

[mark a. foltz, 2017/06/21 17:41:42]

Nit: OnOpenCallback invoked when cast socket is opened.

[zhaobin, 2017/06/21 21:58:26]

Done.

+  int OpenSocket(const net::IPEndPoint& ip_endpoint,
+                 net::NetLog* net_log,
+                 const base::TimeDelta& connect_timeout,
+                 const base::TimeDelta& liveness_timeout,
+                 const base::TimeDelta& ping_interval,
+                 const scoped_refptr<Logger>& logger,
+                 uint64_t device_capabilities,
+                 const CastSocket::OnOpenCallback& open_cb,
+                 CastSocket::Observer* observer);

[mark a. foltz, 2017/06/21 17:41:42]

Please document |observer|.

Is ownership taken?  If so it should be passed by std::unique_ptr.

[zhaobin, 2017/06/21 21:58:25]

No.

[zhaobin, 2017/06/21 21:58:25]

Done.

+
+  // Opens cast socket with |ip_endpoint| and invokes |open_cb| when opening
+  // operation finishes. If cast socket with |ip_endpoint| already exists,
+  // invoke |open_cb| directly with existing socket's channel ID.
+  // |ip_endpoint|: IP endpoint to be connected to.
+  // |net_log|: Net log passed to cast socket.
+  // |open_cb|: first parameter is channel id of newly created socket; second

[mark a. foltz, 2017/06/21 17:41:42]

Can the parameter comments here be copied from above (as I assume they have the
same meanings)?

[zhaobin, 2017/06/21 21:58:25]

Done.

+  // parameter is channel error encountered during channel opening.
+  int OpenSocket(const net::IPEndPoint& ip_endpoint,

[mark a. foltz, 2017/06/21 17:41:42]

Is this ctor invoked anywhere outside of tests?
Maybe it can be removed?

[zhaobin, 2017/06/21 21:58:26]

CastMediaSinkService will call this in
https://codereview.chromium.org/2927833002/.

+                 net::NetLog* net_log,
+                 const CastSocket::OnOpenCallback& open_cb,
+                 CastSocket::Observer* observer);
+
   // Returns an observer corresponding to |id|.
   CastSocket::Observer* GetObserver(const std::string& id);
 
@@ -43,6 +81,9 @@ class CastSocketService : public RefcountedKeyedService {
   void AddObserver(const std::string& id,
                    std::unique_ptr<CastSocket::Observer> observer);
 
+  void SetSocketForTest(

[mark a. foltz, 2017/06/21 17:41:42]

Please document this method

[zhaobin, 2017/06/21 21:58:26]

Done.

+      std::unique_ptr<cast_channel::CastSocket> socket_for_test);

[mark a. foltz, 2017/06/21 17:41:42]

Don't need cast_channel:: here and below

[zhaobin, 2017/06/21 21:58:26]

Done.

+
  private:
   ~CastSocketService() override;
 
@@ -61,6 +102,10 @@ class CastSocketService : public RefcountedKeyedService {
   std::map<std::string, std::unique_ptr<CastSocket::Observer>>
       socket_observer_map_;
 
+  scoped_refptr<cast_channel::Logger> logger_;
+
+  std::unique_ptr<cast_channel::CastSocket> socket_for_test_;
+
   THREAD_CHECKER(thread_checker_);
 
   DISALLOW_COPY_AND_ASSIGN(CastSocketService);