Multicast socket API

net/udp/udp_socket_win.h

Index: net/udp/udp_socket_win.h
diff --git a/net/udp/udp_socket_win.h b/net/udp/udp_socket_win.h
index ce17050583efd9e307418940cff034242ad4e473..aff347b6863c9d50e8618ebf539dfb4bd55f6120 100644
--- a/net/udp/udp_socket_win.h
+++ b/net/udp/udp_socket_win.h
@@ -7,6 +7,7 @@
 
 #include <winsock2.h>
 
+#include "base/hash_tables.h"

[wtc, 2013/04/16 19:59:42]

I believe "base/hash_tables.h" should be removed because
I don't see any use of base::hash_* in this file.

[Bei Zhang, 2013/04/17 17:53:03]

Done.

 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/threading/non_thread_safe.h"
@@ -115,6 +116,29 @@ class NET_EXPORT UDPSocketWin : NON_EXPORTED_BASE(public base::NonThreadSafe) {
   // called before Bind().
   void AllowBroadcast();
 
+  // Join the multicast group.
+  // |group_address| is the group address to join, could be either
+  // IPv4 or IPv6 address.

[wtc, 2013/04/16 19:59:42]

Nit: add "an" before "IPv4". Make the same change to line 126.

Please document the 'int' return value of all the new functions.

[Bei Zhang, 2013/04/17 17:53:03]

Done.

+  int JoinGroup(const net::IPAddressNumber& group_address) const;

[wtc, 2013/04/16 19:59:42]

I suggest naming these two methods JoinMulticastGroup and
LeaveMulticastGroup.  Would that be too wordy?

Please remove "net::" throughout this file because it is
inside the net namespace.

+
+  // Leave the multicast group.
+  // |group_address| is the group address to leave, could be either
+  // IPv4 or IPv6 address. If the socket hasn't joined the group,
+  // it will be ignored.
+  // It's optional to leave the multicast group before destroying
+  // the socket. It will be done by the OS.
+  int LeaveGroup(const net::IPAddressNumber& group_address) const;
+
+  // Set the time-to-live option for udp packets sent to the multicast

[wtc, 2013/04/16 19:59:42]

Nit: udp => UDP

Also on line 137.

[Bei Zhang, 2013/04/17 17:53:03]

Done.

+  // group address. Initially this value is 1. Cannot be negative or

[wtc, 2013/04/16 19:59:42]

Initially => By default ?

Or we can say "The default value of this option is 1."

Also on line 139.

[Bei Zhang, 2013/04/17 17:53:03]

Done.

+  // more than 255.

[wtc, 2013/04/16 19:59:42]

Can this value be 0?

[Bei Zhang, 2013/04/17 17:53:03]

Yes. It means loopback only.

On 2013/04/16 19:59:42, wtc wrote:

+  int SetMulticastTimeToLive(int time_to_live);
+
+  // Set the loopback flag for udp socket. If this flag is true, the host
+  // will receive package sent to the joined group from itself.

[wtc, 2013/04/16 19:59:42]

package => packets ?

[Bei Zhang, 2013/04/17 17:53:03]

Done.

+  // Initially this value is true.
+  int SetMulticastLoopbackMode(bool loopback);
+
  private:
   enum SocketOptions {
     SOCKET_OPTION_REUSE_ADDRESS = 1 << 0,
@@ -160,11 +184,17 @@ class NET_EXPORT UDPSocketWin : NON_EXPORTED_BASE(public base::NonThreadSafe) {
   bool ReceiveAddressToIPEndpoint(IPEndPoint* address) const;
 
   SOCKET socket_;
+  int sock_addr_family_;

[wtc, 2013/04/16 19:59:42]

Nit: you can omit "sock_" in this member's name because
that is implied.

[Bei Zhang, 2013/04/17 17:53:03]

Done.

 
   // Bitwise-or'd combination of SocketOptions. Specifies the set of
   // options that should be applied to |socket_| before Bind().
   int socket_options_;
 
+  // Multicast socket options cached for SetSocketOption.
+  // Cannot be used after Bind().

[wtc, 2013/04/16 19:59:42]

"Cannot be used after Bind()" seems to contradicts the
implementation in the .cc file.

[Bei Zhang, 2013/04/17 17:53:03]

Done.

+  int multicast_time_to_live_;
+  bool multicast_loopback_mode_;

[wtc, 2013/04/16 19:59:42]

I think multicast_loopback_mode_ should be replaced by a
bit flag in socket_options_.  See lines 143-146 and 189-191.

[Bei Zhang, 2013/04/17 17:53:03]

Done.

+
   // How to do source port binding, used only when UDPSocket is part of
   // UDPClientSocket, since UDPServerSocket provides Bind.
   DatagramSocket::BindType bind_type_;