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

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

 // Copyright (c) 2012 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.
 
 #include "chrome/browser/extensions/api/dial/dial_service.h"
 
 #include <algorithm>
 
 #include "base/basictypes.h"
 #include "base/callback.h"
@@ skipping 24 matching lines @@
 using net::IPEndPoint;
 using net::NetworkInterface;
 using net::NetworkInterfaceList;
 using net::StringIOBuffer;
 using net::UDPSocket;
 
 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;
 
 // The timeout (MX) set for responses.
 const int kDialResponseTimeoutSecs = 2;
 
 // The multicast IP address for discovery.
 const char kDialRequestAddress[] = "239.255.255.250";
 
@@ skipping 38 matching lines @@
       version.Version().c_str(),
       version.OSType().c_str()));
   // 1500 is a good MTU value for most Ethernet LANs.
   DCHECK(request.size() <= 1500);
   return request;
 }
 
 }  // 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) +
+                  TimeDelta::FromSeconds(kDialResponseTimeoutSecs)),
+    request_interval_(TimeDelta::FromMilliseconds(kDialRequestIntervalMillis))
+ {
   IPAddressNumber address;
   bool result = net::ParseIPLiteralToNumber(kDialRequestAddress, &address);
   DCHECK(result);
   send_address_ = IPEndPoint(address, kDialRequestPort);
   send_buffer_ = new StringIOBuffer(BuildRequest());
   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() {
   DCHECK(thread_checker_.CalledOnValidThread());
 }
 
 void DialServiceImpl::AddObserver(Observer* observer) {
   DCHECK(thread_checker_.CalledOnValidThread());
   observer_list_.AddObserver(observer);
 }
 
 void DialServiceImpl::RemoveObserver(Observer* observer) {
   DCHECK(thread_checker_.CalledOnValidThread());
   observer_list_.RemoveObserver(observer);
 }
 
 bool DialServiceImpl::HasObserver(Observer* observer) {
   DCHECK(thread_checker_.CalledOnValidThread());
   return observer_list_.HasObserver(observer);
 }
 
 bool DialServiceImpl::Discover() {
   DCHECK(thread_checker_.CalledOnValidThread());
   if (discovery_active_)
     return false;
   discovery_active_ = true;
 
   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;
+
+  // TODO(mfoltz): Add a net::NetworkChangeNotifier() to listen for connection

[justinlin, 2013/02/07 09:03:24]

This TODO no longer needed.

[mark a. foltz, 2013/02/07 22:36:55]

Done.

+  // 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));
 }
 
-bool DialServiceImpl::BindAndWriteSocket(
-      const NetworkInterface& bind_interface) {
+bool DialServiceImpl::BindSocketAndSendRequest(
+      const IPAddressNumber& bind_ip_address) {
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(!socket_.get());
 
   net::RandIntCallback rand_cb = base::Bind(&base::RandInt);
   socket_.reset(new UDPSocket(net::DatagramSocket::RANDOM_BIND,
                               rand_cb,
                               net_log_,
                               net_log_source_));
   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()) {

[justinlin, 2013/02/07 09:03:24]

no braces

[mark a. foltz, 2013/02/07 22:36:55]

Done.

+    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(),
       send_buffer_->size(), send_address_,
       base::Bind(&DialServiceImpl::OnSocketWrite, this));
   bool result_ok = CheckResult("SendTo", result);
   if (result_ok && result > 0) {
     // 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;
 }
 
 void DialServiceImpl::OnSocketWrite(int result) {
   DCHECK(thread_checker_.CalledOnValidThread());
   is_writing_ = false;
   if (!CheckResult("OnSocketWrite", result))
     return;
-
   if (result != send_buffer_->size()) {
     DLOG(ERROR) << "Sent " << result << " chars, expected "
                << send_buffer_->size() << " chars";
   }
   // If discovery is inactive, no reason to notify observers.
   if (!discovery_active_) {
     DVLOG(1) << "Request sent after discovery finished.  Ignoring.";
     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_) {
+    if (request_interval_ <= TimeDelta::FromSeconds(0)) {

[justinlin, 2013/02/07 09:03:24]

Is this check needed? Seems like scheduling a timer at 0 seconds would pretty
much do the same thing.

[mark a. foltz, 2013/02/07 22:36:55]

Scheduling a zero-delay timer didn't result in a transfer of control flow to the
posted task, so the unit test would exit before any of the posted tasks fired. 
It might be clearer to disable the timers by setting a flag like
DisableTimersForTest()?

[justinlin, 2013/02/07 22:42:05]

Ah, if it's for testing, then I think it's possible to test without needing
this. You just need a MessageLoop, and use "RunLoop run_loop;
run_loop.RunUntilIdle();".
If you need to wait for an event, you can also use "run_loop.QuitClosure()" and
run_loop.Run()". I have an example here:
https://code.google.com/p/chromium/codesearch#chrome/src/content/browser/rend...

But, I think, depending on the test, run_loop.RunUntilIdle should be enough.

[mark a. foltz, 2013/02/08 00:04:50]

Thanks for the tip.  I wasn't able to use RunLoop since the socket libraries
expect a MessageLoop with type = TYPE_IO, but RunUntilIdle worked there too. 

+      SendOneRequest();
+    } else if (num_requests_sent_ == 1) {
+      request_timer_.Start(FROM_HERE,
+                           request_interval_,
+                           this,
+                           &DialServiceImpl::SendOneRequest);
+    } else {
+      DCHECK(request_timer_.IsRunning());

[justinlin, 2013/02/07 09:03:24]

I'm not sure this DCHECK is valid. Since the DialService can be shutdown
arbitrarily due to network events, it can probably happen that we scheduled a
SocketRead, but we end up being shutdown when this is run.

[mark a. foltz, 2013/02/07 22:36:55]

Good point.  Removed.

+    }
+  }
 }
 
 bool DialServiceImpl::ReadSocket() {
   DCHECK(thread_checker_.CalledOnValidThread());
   if (!socket_.get()) {
     DLOG(WARNING) << "Socket not connected.";
     return false;
   }
 
   if (is_reading_) {
@@ skipping 111 matching lines @@
     DVLOG(1) << "Malformed or missing " << kSsdpConfigIdHeader << ": "
              << config_id;
   }
 
   return true;
 }
 
 void DialServiceImpl::SendNetworkList(const NetworkInterfaceList& networks) {
   DCHECK(thread_checker_.CalledOnValidThread());
   if (!networks.size()) {
-    DVLOG(1) << "No network interfaces found!";
+    DVLOG(1) << "No valid network interfaces found!";
+    FinishDiscovery();
     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.
 
   // TODO(mfoltz): Support IPV6 multicast.  http://crbug.com/165286
   for (NetworkInterfaceList::const_iterator iter = networks.begin();
        iter != networks.end(); iter++) {
     DVLOG(1) << "Found " << iter->name << ", "
              << net::IPAddressToString(iter->address);
     if (iter->address.size() == net::kIPv4AddressSize) {
       interface = &*iter;
       break;
     }
   }
 
   if (interface == NULL) {
     DVLOG(1) << "Could not find a valid interface to bind.";
+    FinishDiscovery();
   } else {
-    BindAndWriteSocket(*interface);
+    BindSocketAndSendRequest(interface->address);
   }
 }
 
 void DialServiceImpl::DoGetNetworkList() {
   NetworkInterfaceList list;
   bool success = net::GetNetworkList(&list);
   if (!success) {
     DVLOG(1) << "Could not retrieve network list!";
-    return;
+    list.clear();

[justinlin, 2013/02/07 09:03:24]

This isn't needed as the vector is already initialized to empty, also please
sync to head and rebase as some of this has changed, I think.

[mark a. foltz, 2013/02/07 22:36:55]

Done.  Will rebase.

   }
   BrowserThread::PostTask(BrowserThread::IO, FROM_HERE, base::Bind(
       &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));
+}
+
 void DialServiceImpl::CloseSocket() {
   DCHECK(thread_checker_.CalledOnValidThread());
   is_reading_ = false;
   is_writing_ = false;
   if (!socket_.get())
     return;
   socket_.reset();
 }
 
 bool DialServiceImpl::CheckResult(const char* operation, int result) {
   DCHECK(thread_checker_.CalledOnValidThread());
   DVLOG(1) << "Operation " << operation << " result " << result;
   if (result < net::OK && result != net::ERR_IO_PENDING) {
     CloseSocket();
     std::string error_str(net::ErrorToString(result));
     DVLOG(0) << "dial socket error: " << error_str;
     FOR_EACH_OBSERVER(Observer, observer_list_, OnError(this, error_str));
     return false;
   }
   return true;
 }
 
 }  // namespace extensions