- Invoke FinishDiscovery() if no network interfaces are detected.

chrome/browser/extensions/api/dial/dial_service.cc

Index: chrome/browser/extensions/api/dial/dial_service.cc
diff --git a/chrome/browser/extensions/api/dial/dial_service.cc b/chrome/browser/extensions/api/dial/dial_service.cc
index 743fcf8a5c92faa22544916b7de8c7da22ab2147..83d1ad42c00e7834f6666d38d1c7077315cb3137 100644
--- a/chrome/browser/extensions/api/dial/dial_service.cc
+++ b/chrome/browser/extensions/api/dial/dial_service.cc
@@ -42,8 +42,8 @@ namespace extensions {
 
 namespace {
 
-// The total number of requests to make.
-const int kDialNumRequests = 1;
+// The total number of requests to make per discovery cycle.
+const int kDialMaxRequests = 4;
 
 // The interval to wait between successive requests.
 const int kDialRequestIntervalMillis = 1000;
@@ -102,8 +102,16 @@ std::string BuildRequest() {
 }  // namespace
 
 DialServiceImpl::DialServiceImpl(net::NetLog* net_log)
-  : is_writing_(false), is_reading_(false), discovery_active_(false),
-    num_requests_sent_(0) {
+  : is_writing_(false),
+    is_reading_(false),
+    discovery_active_(false),
+    num_requests_sent_(0),
+    max_requests_(kDialMaxRequests),
+    finish_delay_(TimeDelta::FromMilliseconds((kDialMaxRequests - 1) *
+                                                kDialRequestIntervalMillis) +

[justinlin, 2013/02/08 08:56:20]

Align this with the "(" above.

[mark a. foltz, 2013/02/08 22:07:58]

Done.

+                  TimeDelta::FromSeconds(kDialResponseTimeoutSecs)),
+    request_interval_(TimeDelta::FromMilliseconds(kDialRequestIntervalMillis))
+ {
   IPAddressNumber address;
   bool result = net::ParseIPLiteralToNumber(kDialRequestAddress, &address);
   DCHECK(result);
@@ -112,9 +120,6 @@ DialServiceImpl::DialServiceImpl(net::NetLog* net_log)
   net_log_ = net_log;
   net_log_source_.type = net::NetLog::SOURCE_UDP_SOCKET;
   net_log_source_.id = net_log_->NextID();
-  finish_delay_ = TimeDelta::FromMilliseconds((kDialNumRequests - 1) *
-                                              kDialRequestIntervalMillis) +
-    TimeDelta::FromSeconds(kDialResponseTimeoutSecs);
 }
 
 DialServiceImpl::~DialServiceImpl() {
@@ -144,24 +149,22 @@ bool DialServiceImpl::Discover() {
 
   DVLOG(1) << "Discovery started.";
 
-  // TODO(mfoltz): Send multiple requests.
-  StartRequest();
+  StartDiscovery();
   return true;
 }
 
-void DialServiceImpl::FinishDiscovery() {
+void DialServiceImpl::StartDiscovery() {
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(discovery_active_);
-  DVLOG(1) << "Discovery finished.";
-  CloseSocket();
-  finish_timer_.Stop();
-  discovery_active_ = false;
-  num_requests_sent_ = 0;
-  FOR_EACH_OBSERVER(Observer, observer_list_, OnDiscoveryFinished(this));
+  if (socket_.get())
+    return;
+
+  BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE, base::Bind(
+      &DialServiceImpl::DoGetNetworkList, this));
 }
 
-bool DialServiceImpl::BindAndWriteSocket(
-      const NetworkInterface& bind_interface) {
+bool DialServiceImpl::BindSocketAndSendRequest(
+      const IPAddressNumber& bind_ip_address) {
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(!socket_.get());
 
@@ -173,29 +176,42 @@ bool DialServiceImpl::BindAndWriteSocket(
   socket_->AllowBroadcast();
 
   // Schedule a timer to finish the discovery process (and close the socket).
-  finish_timer_.Start(FROM_HERE,
-                      finish_delay_,
-                      this,
-                      &DialServiceImpl::FinishDiscovery);
+  if (finish_delay_ > TimeDelta::FromSeconds(0)) {
+    finish_timer_.Start(FROM_HERE,
+                        finish_delay_,
+                        this,
+                        &DialServiceImpl::FinishDiscovery);
+  }
 
   // 0 means bind a random port
-  IPEndPoint address(bind_interface.address, 0);
+  IPEndPoint address(bind_ip_address, 0);
 
   if (!CheckResult("Bind", socket_->Bind(address)))
     return false;
 
+  recv_buffer_ = new IOBufferWithSize(kDialRecvBufferSize);
+  if (!ReadSocket())
+    return false;
+  SendOneRequest();
+  return true;
+}
+
+void DialServiceImpl::SendOneRequest() {
+  if (num_requests_sent_ == max_requests_) {
+    request_timer_.Stop();
+    return;
+  }
+  num_requests_sent_++;
   if (!socket_.get()) {
     DLOG(WARNING) << "Socket not connected.";
-    return false;
+    return;
   }
-
-  recv_buffer_ = new IOBufferWithSize(kDialRecvBufferSize);
-  ReadSocket();
-
   if (is_writing_) {
     DVLOG(1) << "Already writing.";
-    return false;
+    return;
   }
+  DVLOG(1) << "Sending request " << num_requests_sent_ << "/"
+           << max_requests_;
   is_writing_ = true;
   int result = socket_->SendTo(
       send_buffer_.get(),
@@ -206,21 +222,7 @@ bool DialServiceImpl::BindAndWriteSocket(
     // Synchronous write.
     OnSocketWrite(result);
   }
-  return result_ok;
-}
-
-void DialServiceImpl::StartRequest() {
-  DCHECK(thread_checker_.CalledOnValidThread());
-  DCHECK(discovery_active_);
-  if (socket_.get())
-    return;
-
-  // TODO(mfoltz): Add a net::NetworkChangeNotifier() to listen for connection
-  // type/IP address changes, and notify via observer.  Also sanity check the
-  // connection type, i.e. !IsOffline && !IsCellular
-  // http://crbug.com/165290
-  BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE, base::Bind(
-      &DialServiceImpl::DoGetNetworkList, this));
+  return;

[justinlin, 2013/02/08 08:56:20]

Don't need this return

[mark a. foltz, 2013/02/08 22:07:58]

Done.

 }
 
 void DialServiceImpl::OnSocketWrite(int result) {
@@ -228,7 +230,6 @@ void DialServiceImpl::OnSocketWrite(int result) {
   is_writing_ = false;
   if (!CheckResult("OnSocketWrite", result))
     return;
-
   if (result != send_buffer_->size()) {
     DLOG(ERROR) << "Sent " << result << " chars, expected "
                << send_buffer_->size() << " chars";
@@ -239,7 +240,13 @@ void DialServiceImpl::OnSocketWrite(int result) {
     return;
   }
   FOR_EACH_OBSERVER(Observer, observer_list_, OnDiscoveryRequest(this));
-  num_requests_sent_++;
+  // If we need to send additional requests, schedule a timer to do so.
+  if (num_requests_sent_ < max_requests_ && num_requests_sent_ == 1) {
+    request_timer_.Start(FROM_HERE,
+                         request_interval_,
+                         this,
+                         &DialServiceImpl::SendOneRequest);
+  }
 }
 
 bool DialServiceImpl::ReadSocket() {
@@ -370,11 +377,6 @@ bool DialServiceImpl::ParseResponse(const std::string& response,
 
 void DialServiceImpl::SendNetworkList(const NetworkInterfaceList& networks) {
   DCHECK(thread_checker_.CalledOnValidThread());
-  if (!networks.size()) {
-    DVLOG(1) << "No network interfaces found!";
-    return;
-  }
-
   const NetworkInterface* interface = NULL;
   // Returns the first IPv4 address found.  If there is a need for discovery
   // across multiple networks, we could manage multiple sockets.
@@ -392,8 +394,9 @@ void DialServiceImpl::SendNetworkList(const NetworkInterfaceList& networks) {
 
   if (interface == NULL) {
     DVLOG(1) << "Could not find a valid interface to bind.";
+    FinishDiscovery();
   } else {
-    BindAndWriteSocket(*interface);
+    BindSocketAndSendRequest(interface->address);
   }
 }
 
@@ -407,6 +410,18 @@ void DialServiceImpl::DoGetNetworkList() {
       &DialServiceImpl::SendNetworkList, this, list));
 }
 
+void DialServiceImpl::FinishDiscovery() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK(discovery_active_);
+  DVLOG(1) << "Discovery finished.";
+  CloseSocket();
+  finish_timer_.Stop();
+  request_timer_.Stop();
+  discovery_active_ = false;
+  num_requests_sent_ = 0;
+  FOR_EACH_OBSERVER(Observer, observer_list_, OnDiscoveryFinished(this));

[justinlin, 2013/02/08 08:56:20]

Don't need to do this in the CL, but I kinda feel that it's DialRegistry that
should have the observer list, and not DialService. DialRegistry should be the
sole listener to DialService, and clients should attach to DialService, but we
can do that later if you agree.

[mark a. foltz, 2013/02/08 22:07:58]

I agree with you that DialService is exclusively for use by DialRegistry at the
moment but it doesn't seem to make a big difference in complexity whether there
is an observer list or a single observer for DialService.

+}
+
 void DialServiceImpl::CloseSocket() {
   DCHECK(thread_checker_.CalledOnValidThread());
   is_reading_ = false;