Merge 242090 ""Reland" of r241209."

> "Reland" of r241209.
> 
> DialServiceImpl now binds to all ipv4 interfaces instead of just the first ipv4 found. Dedup is done using address family + interface index.
> It now also checks if there are any other open sockets when notifying observers on error and sends a different error code (DIAL_SERVICE_NO_INTERFACES instead of the usual DIAL_SERVICE_SOCKET_ERROR) if there are no other open sockets. The error codes are treated the same upstream currently. Later we should make sure DIAL_SERVICE_SOCKET_ERROR doesn't clear the device list.
> 
> Added unit tests for multiple interfaces and open sockets checking.
> 
> TEST=unit tests, did some manual testing at the lab.
> 
> Review URL: https://codereview.chromium.org/117853004

BUG=300113
TBR=imcheng@chromium.org

Review URL: https://codereview.chromium.org/137723003

git-svn-id: svn://svn.chromium.org/chrome/branches/1750/src@244635 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 77 insertions, 13 deletions

diff --git a/chrome/browser/extensions/api/dial/dial_service.cc b/chrome/browser/extensions/api/dial/dial_service.cc
index 773bdc7..96f5402 100644
--- a/chrome/browser/extensions/api/dial/dial_service.cc
+++ b/chrome/browser/extensions/api/dial/dial_service.cc
@@ -5,6 +5,8 @@
 #include "chrome/browser/extensions/api/dial/dial_service.h"
 
 #include <algorithm>
+#include <set>
+#include <utility>
 
 #include "base/basictypes.h"
 #include "base/callback.h"
@@ -468,18 +470,40 @@ void DialServiceImpl::StartDiscovery() {
 
 void DialServiceImpl::SendNetworkList(const NetworkInterfaceList& networks) {
   DCHECK(thread_checker_.CalledOnValidThread());
+  typedef std::pair<uint32, net::AddressFamily> InterfaceIndexAddressFamily;
+  std::set<InterfaceIndexAddressFamily> interface_index_addr_family_seen;
   std::vector<IPAddressNumber> ip_addresses;
 
-  // Returns the first IPv4 address found.  If there is a need for discovery
-  // across multiple networks, we could manage multiple sockets.
+  // Binds a socket to each IPv4 network interface found. Note that
+  // there may be duplicates in |networks|, so address family + interface index
+  // is used to identify unique interfaces.
   // TODO(mfoltz): Support IPV6 multicast.  http://crbug.com/165286
   for (NetworkInterfaceList::const_iterator iter = networks.begin();
        iter != networks.end(); ++iter) {
+    net::AddressFamily addr_family = net::GetAddressFamily(iter->address);
     DVLOG(1) << "Found " << iter->name << ", "
-             << net::IPAddressToString(iter->address);
-    if (iter->address.size() == net::kIPv4AddressSize) {
-      ip_addresses.push_back(iter->address);
-      break;
+             << net::IPAddressToString(iter->address)
+             << ", address family: " << addr_family;
+    if (addr_family == net::ADDRESS_FAMILY_IPV4) {
+      InterfaceIndexAddressFamily interface_index_addr_family =
+          std::make_pair(iter->interface_index, addr_family);
+      bool inserted = interface_index_addr_family_seen
+          .insert(interface_index_addr_family)
+          .second;
+      // We have not seen this interface before, so add its IP address to the
+      // discovery list.
+      if (inserted) {
+        VLOG(2) << "Encountered "
+                << "interface index: " << iter->interface_index << ", "
+                << "address family: " << addr_family << " for the first time, "
+                << "adding IP address " << net::IPAddressToString(iter->address)
+                << " to list.";
+        ip_addresses.push_back(iter->address);
+      } else {
+        VLOG(2) << "Already encountered "
+                << "interface index: " << iter->interface_index << ", "
+                << "address family: " << addr_family << " before, not adding.";
+      }
     }
   }
 
@@ -488,7 +512,7 @@ void DialServiceImpl::SendNetworkList(const NetworkInterfaceList& networks) {
 
 void DialServiceImpl::DiscoverOnAddresses(
     const std::vector<IPAddressNumber>& ip_addresses) {
-  if (ip_addresses.size() == 0) {
+  if (ip_addresses.empty()) {
     DVLOG(1) << "Could not find a valid interface to bind. Finishing discovery";
     FinishDiscovery();
     return;
@@ -505,9 +529,8 @@ void DialServiceImpl::DiscoverOnAddresses(
 
   for (std::vector<IPAddressNumber>::const_iterator iter = ip_addresses.begin();
        iter != ip_addresses.end();
-       ++iter) {
+       ++iter)
     BindAndAddSocket(*iter);
-  }
 
   SendOneRequest();
 }
@@ -560,6 +583,8 @@ void DialServiceImpl::NotifyOnDiscoveryRequest() {
   // If we need to send additional requests, schedule a timer to do so.
   if (num_requests_sent_ < max_requests_ && num_requests_sent_ == 1) {
     VLOG(2) << "Scheduling timer to send additional requests";
+    // TODO(imcheng): Move this to SendOneRequest() once the implications are
+    // understood.
     request_timer_.Start(FROM_HERE,
                          request_interval_,
                          this,
@@ -580,8 +605,12 @@ void DialServiceImpl::NotifyOnDeviceDiscovered(
 
 void DialServiceImpl::NotifyOnError() {
   DCHECK(thread_checker_.CalledOnValidThread());
+  // TODO(imcheng): Modify upstream so that the device list is not cleared
+  // when it could still potentially discover devices on other sockets.
   FOR_EACH_OBSERVER(Observer, observer_list_,
-                    OnError(this, DIAL_SERVICE_SOCKET_ERROR));
+                    OnError(this,
+                            HasOpenSockets() ? DIAL_SERVICE_SOCKET_ERROR
+                                             : DIAL_SERVICE_NO_INTERFACES));
 }
 
 void DialServiceImpl::FinishDiscovery() {
diff --git a/chrome/browser/extensions/api/dial/dial_service.h b/chrome/browser/extensions/api/dial/dial_service.h
index 3ce7b2d..82efb3e 100644
--- a/chrome/browser/extensions/api/dial/dial_service.h
+++ b/chrome/browser/extensions/api/dial/dial_service.h
@@ -126,8 +126,8 @@ class DialServiceImpl : public DialService,
     // Creates a socket using |net_log| and |net_log_source| and binds it to
     // |bind_ip_address|.
     bool CreateAndBindSocket(const net::IPAddressNumber& bind_ip_address,
-                    net::NetLog* net_log,
-                    net::NetLog::Source net_log_source);
+                             net::NetLog* net_log,
+                             net::NetLog::Source net_log_source);
 
     // Sends a single discovery request |send_buffer| to |send_address|
     // over the socket.
@@ -195,6 +195,7 @@ class DialServiceImpl : public DialService,
     // Marks whether there is an active read callback.
     bool is_reading_;
 
+    FRIEND_TEST_ALL_PREFIXES(DialServiceTest, TestNotifyOnError);
     FRIEND_TEST_ALL_PREFIXES(DialServiceTest, TestOnDeviceDiscovered);
     FRIEND_TEST_ALL_PREFIXES(DialServiceTest, TestOnDiscoveryRequest);
     FRIEND_TEST_ALL_PREFIXES(DialServiceTest, TestResponseParsing);
@@ -204,11 +205,14 @@ class DialServiceImpl : public DialService,
   // Starts the control flow for one discovery cycle.
   void StartDiscovery();
 
-  // Send the network list to IO thread.
+  // For each network interface in |list|, finds all unqiue IPv4 network
+  // interfaces and call |DiscoverOnAddresses()| with their IP addresses.
   void SendNetworkList(const net::NetworkInterfaceList& list);
 
   // Calls |BindAndAddSocket()| for each address in |ip_addresses|, calls
   // |SendOneRequest()|, and start the timer to finish discovery if needed.
+  // The (Address family, interface index) of each address in |ip_addresses|
+  // must be unique. If |ip_address| is empty, calls |FinishDiscovery()|.
   void DiscoverOnAddresses(
       const std::vector<net::IPAddressNumber>& ip_addresses);
 
@@ -285,6 +289,8 @@ class DialServiceImpl : public DialService,
 
   friend class DialServiceTest;
   FRIEND_TEST_ALL_PREFIXES(DialServiceTest, TestSendMultipleRequests);
+  FRIEND_TEST_ALL_PREFIXES(DialServiceTest, TestMultipleNetworkInterfaces);
+  FRIEND_TEST_ALL_PREFIXES(DialServiceTest, TestNotifyOnError);
   FRIEND_TEST_ALL_PREFIXES(DialServiceTest, TestOnDeviceDiscovered);
   FRIEND_TEST_ALL_PREFIXES(DialServiceTest, TestOnDiscoveryFinished);
   FRIEND_TEST_ALL_PREFIXES(DialServiceTest, TestOnDiscoveryRequest);
diff --git a/chrome/browser/extensions/api/dial/dial_service_unittest.cc b/chrome/browser/extensions/api/dial/dial_service_unittest.cc
index 2e1753b..02d5e09 100644
--- a/chrome/browser/extensions/api/dial/dial_service_unittest.cc
+++ b/chrome/browser/extensions/api/dial/dial_service_unittest.cc
@@ -67,11 +67,40 @@ TEST_F(DialServiceTest, TestSendMultipleRequests) {
   EXPECT_CALL(mock_observer_, OnDiscoveryRequest(A<DialService*>())).Times(4);
   EXPECT_CALL(mock_observer_, OnDiscoveryFinished(A<DialService*>())).Times(1);
   dial_service_.BindAndAddSocket(mock_ip_);
+  EXPECT_EQ(1u, dial_service_.dial_sockets_.size());
   dial_service_.SendOneRequest();
   loop.RunUntilIdle();
   dial_service_.FinishDiscovery();
 }
 
+TEST_F(DialServiceTest, TestMultipleNetworkInterfaces) {
+  base::MessageLoop loop(base::MessageLoop::TYPE_IO);
+  // Setting the finish delay to zero disables the timer that invokes
+  // FinishDiscovery().
+  dial_service_.finish_delay_ = TimeDelta::FromSeconds(0);
+  dial_service_.request_interval_ = TimeDelta::FromSeconds(0);
+  dial_service_.max_requests_ = 4;
+  dial_service_.discovery_active_ = true;
+  net::NetworkInterfaceList interface_list;
+  interface_list.push_back(net::NetworkInterface("network1", 0, mock_ip_, 0));
+  interface_list.push_back(net::NetworkInterface("network2", 1, mock_ip_, 0));
+  interface_list.push_back(net::NetworkInterface("network3", 2, mock_ip_, 0));
+
+  // "network4" is equivalent to "network2" because both the address family
+  // and interface index are the same.
+  interface_list.push_back(net::NetworkInterface("network4", 1, mock_ip_, 0));
+
+  // 3 sockets * 4 requests per socket = 12 requests
+  EXPECT_CALL(mock_observer_, OnDiscoveryRequest(A<DialService*>())).Times(12);
+  EXPECT_CALL(mock_observer_, OnDiscoveryFinished(A<DialService*>())).Times(1);
+
+  dial_service_.SendNetworkList(interface_list);
+  EXPECT_EQ(3u, dial_service_.dial_sockets_.size());
+
+  loop.RunUntilIdle();
+  dial_service_.FinishDiscovery();
+}
+
 TEST_F(DialServiceTest, TestOnDiscoveryRequest) {
   dial_service_.discovery_active_ = true;
   dial_service_.num_requests_sent_ = 1;