[cast_channel] Implement CastSocketService::OpenSocket()

components/cast_channel/cast_socket_service.h

 // Copyright 2017 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 COMPONENTS_CAST_CHANNEL_CAST_CHANNEL_SERVICE_H_
 #define COMPONENTS_CAST_CHANNEL_CAST_CHANNEL_SERVICE_H_
 
 #include <map>
 #include <memory>
 
@@ skipping 18 matching lines @@
   int AddSocket(std::unique_ptr<CastSocket> socket);
 
   // Removes the CastSocket corresponding to |channel_id| from the
   // CastSocketRegistry. Returns nullptr if no such CastSocket exists.
   std::unique_ptr<CastSocket> RemoveSocket(int channel_id);
 
   // Returns the socket corresponding to |channel_id| if one exists, or nullptr
   // 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);
 
   // Adds |observer| to |socket_observer_map_| keyed by |id|.
   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;
 
   // RefcountedKeyedService implementation.
   void ShutdownOnUIThread() override;
 
   // Used to generate CastSocket id.
   static int last_channel_id_;
 
   // The collection of CastSocket keyed by channel_id.
   std::map<int, std::unique_ptr<CastSocket>> sockets_;
 
   // Map of CastSocket::Observer keyed by observer id. For extension side
   // observers, id is extension_id; For browser side observers, id is a hard
   // coded string.
   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);
 };
 
 }  // namespace cast_channel
 
 #endif  // COMPONENTS_CAST_CHANNEL_CAST_CHANNEL_SERVICE_H_