Refactored to make MDnsSocketFactory return array of sockets.

net/dns/mdns_client_impl.cc

Index: net/dns/mdns_client_impl.cc
diff --git a/net/dns/mdns_client_impl.cc b/net/dns/mdns_client_impl.cc
index 2a83193d066b6aaf066c914eca62e5471fcc48ea..de04128cc275759bfe70f2ca3964183a4423a638 100644
--- a/net/dns/mdns_client_impl.cc
+++ b/net/dns/mdns_client_impl.cc
@@ -27,12 +27,11 @@ namespace {
 const unsigned MDnsTransactionTimeoutSeconds = 3;
 }
 
-MDnsConnection::SocketHandler::SocketHandler(
-    MDnsConnection* connection, const IPEndPoint& multicast_addr,
-    MDnsConnection::SocketFactory* socket_factory)
-    : socket_(socket_factory->CreateSocket()), connection_(connection),
-      response_(new DnsResponse(dns_protocol::kMaxMulticastSize)),
-      multicast_addr_(multicast_addr) {
+MDnsConnection::SocketHandler::SocketHandler(scoped_ptr<Socket> socket,
+                                             MDnsConnection* connection)
+    : socket_(socket.Pass()),
+      connection_(connection),
+      response_(dns_protocol::kMaxMulticastSize) {
 }
 
 MDnsConnection::SocketHandler::~SocketHandler() {
@@ -45,11 +44,11 @@ int MDnsConnection::SocketHandler::Start() {
 int MDnsConnection::SocketHandler::DoLoop(int rv) {
   do {
     if (rv > 0)
-      connection_->OnDatagramReceived(response_.get(), recv_addr_, rv);
+      connection_->OnDatagramReceived(&response_, recv_addr_, rv);
 
     rv = socket_->RecvFrom(
-        response_->io_buffer(),
-        response_->io_buffer()->size(),
+        response_.io_buffer(),
+        response_.io_buffer()->size(),
         &recv_addr_,
         base::Bind(&MDnsConnection::SocketHandler::OnDatagramReceived,
                    base::Unretained(this)));
@@ -70,31 +69,15 @@ void MDnsConnection::SocketHandler::OnDatagramReceived(int rv) {
 }
 
 int MDnsConnection::SocketHandler::Send(IOBuffer* buffer, unsigned size) {
-  return socket_->SendTo(
-      buffer, size, multicast_addr_,
-      base::Bind(&MDnsConnection::SocketHandler::SendDone,
-                 base::Unretained(this) ));
+  return socket_->SendTo(buffer, size,
+                         base::Bind(&MDnsConnection::SocketHandler::SendDone,
+                                    base::Unretained(this) ));
 }
 
 void MDnsConnection::SocketHandler::SendDone(int rv) {
   // TODO(noamsml): Retry logic.
 }
 
-int MDnsConnection::SocketHandler::Bind() {
-  IPAddressNumber address_any(multicast_addr_.address().size());
-
-  IPEndPoint bind_endpoint(address_any, multicast_addr_.port());
-
-  socket_->AllowAddressReuse();
-  int rv = socket_->Listen(bind_endpoint);
-
-  if (rv < OK) return rv;
-
-  socket_->SetMulticastLoopbackMode(false);
-
-  return socket_->JoinGroup(multicast_addr_.address());
-}
-
 MDnsConnection::MDnsConnection(MDnsConnection::Delegate* delegate) :
       delegate_(delegate) {
 }
@@ -103,24 +86,14 @@ MDnsConnection::~MDnsConnection() {
 }
 
 bool MDnsConnection::Init(MDnsConnection::SocketFactory* socket_factory) {
-  // TODO(vitalybuka): crbug.com/297690 Make socket_factory return list
-  // of initialized sockets.
-  socket_handlers_.push_back(
-      new SocketHandler(this, GetMDnsIPEndPoint(ADDRESS_FAMILY_IPV4),
-                        socket_factory));
-  socket_handlers_.push_back(
-      new SocketHandler(this, GetMDnsIPEndPoint(ADDRESS_FAMILY_IPV6),
-                        socket_factory));
+  ScopedVector<Socket> sockets;
+  socket_factory->CreateSockets(&sockets);
 
-  for (size_t i = 0; i < socket_handlers_.size();) {
-    int rv = socket_handlers_[i]->Bind();
-    if (rv != OK) {
-      socket_handlers_.erase(socket_handlers_.begin() + i);
-      VLOG(1) << "Bind failed, socket=" << i << ", error=" << rv;
-    } else {
-      ++i;
-    }
+  for (size_t i = 0; i < sockets.size(); ++i) {
+    socket_handlers_.push_back(

[szym, 2013/11/27 00:28:09]

Empty sockets (which failed to bind in CreateSockets) should be filtered out
here.

[Vitaly Buka (NO REVIEWS), 2013/11/27 00:45:45]

CreateSockets should filter. That was initial idea, but somehow I missed that
part. i'll put that at line 191

On 2013/11/27 00:28:09, szym wrote:

+        new MDnsConnection::SocketHandler(make_scoped_ptr(sockets[i]), this));
   }
+  sockets.weak_clear();
 
   // All unbound sockets need to be bound before processing untrusted input.
   // This is done for security reasons, so that an attacker can't get an unbound
@@ -167,26 +140,73 @@ void MDnsConnection::OnDatagramReceived(
   delegate_->HandlePacket(response, bytes_read);
 }
 
-class MDnsConnectionSocketFactoryImpl
-    : public MDnsConnection::SocketFactory {
+class SocketImpl : public MDnsConnection::Socket {
  public:
-  MDnsConnectionSocketFactoryImpl();
-  virtual ~MDnsConnectionSocketFactoryImpl();
+  explicit SocketImpl(const IPEndPoint& multicast_addr)
+      : multicast_addr_(multicast_addr),
+        socket_(NULL, NetLog::Source()) {};
+  virtual ~SocketImpl() {};
+
+  int Bind() {
+    IPAddressNumber address_any(multicast_addr_.address().size());
+    IPEndPoint bind_endpoint(address_any, multicast_addr_.port());
+
+    socket_.AllowAddressReuse();
+    int rv = socket_.Listen(bind_endpoint);
+    if (rv < OK)
+      return rv;
+
+    socket_.SetMulticastLoopbackMode(false);
+    return socket_.JoinGroup(multicast_addr_.address());
+  }
 
-  virtual scoped_ptr<DatagramServerSocket> CreateSocket() OVERRIDE;
+  virtual int RecvFrom(IOBuffer* buf,
+                      int buf_len,
+                      IPEndPoint* address,
+                      const CompletionCallback& callback) OVERRIDE {
+    return socket_.RecvFrom(buf, buf_len, address, callback);
+  }
+
+  virtual int SendTo(IOBuffer* buf,
+                     int buf_len,
+                     const CompletionCallback& callback) OVERRIDE {
+   return socket_.SendTo(buf, buf_len, multicast_addr_, callback);
+  }
+
+ private:
+  IPEndPoint multicast_addr_;
+  UDPServerSocket socket_;
+
+  DISALLOW_COPY_AND_ASSIGN(SocketImpl);
 };
 
-MDnsConnectionSocketFactoryImpl::MDnsConnectionSocketFactoryImpl() {
-}
+class MDnsConnectionSocketFactoryImpl
+    : public MDnsConnection::SocketFactory {
+ public:
+  MDnsConnectionSocketFactoryImpl() {};
+  virtual ~MDnsConnectionSocketFactoryImpl() {};
 
-MDnsConnectionSocketFactoryImpl::~MDnsConnectionSocketFactoryImpl() {
-}
+  virtual void CreateSockets(
+      ScopedVector<MDnsConnection::Socket>* sockets) OVERRIDE {
+    sockets->push_back(CreateSocket(ADDRESS_FAMILY_IPV4).release());
+    sockets->push_back(CreateSocket(ADDRESS_FAMILY_IPV6).release());
+  }
 
-scoped_ptr<DatagramServerSocket>
-MDnsConnectionSocketFactoryImpl::CreateSocket() {
-  return scoped_ptr<DatagramServerSocket>(new UDPServerSocket(
-      NULL, NetLog::Source()));
-}
+ private:
+  scoped_ptr<MDnsConnection::Socket> CreateSocket(
+      AddressFamily address_family) {
+    scoped_ptr<SocketImpl> socket(
+        new SocketImpl(GetMDnsIPEndPoint(address_family)));
+    int rv = socket->Bind();
+    if (rv != OK) {
+      VLOG(1) << "Bind failed, family=" << address_family << ", error=" << rv;
+      socket.reset();
+    }
+    return socket.PassAs<MDnsConnection::Socket>();
+  }
+
+  DISALLOW_COPY_AND_ASSIGN(MDnsConnectionSocketFactoryImpl);
+};
 
 // static
 scoped_ptr<MDnsConnection::SocketFactory>