Add timeout check for connectivity_checker.

chromecast/net/connectivity_checker_impl.cc

 // Copyright 2015 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 "chromecast/net/connectivity_checker_impl.h"
 
 #include "base/command_line.h"
 #include "base/logging.h"
 #include "base/message_loop/message_loop.h"
 #include "chromecast/net/net_switches.h"
@@ skipping 10 matching lines @@
 
 namespace {
 
 // How often connectivity checks are performed in seconds.
 const unsigned int kConnectivityPeriodSeconds = 1;
 
 // Number of consecutive connectivity check errors before status is changed
 // to offline.
 const unsigned int kNumErrorsToNotifyOffline = 3;
 
+// Timeout to get response from request.
+const unsigned int kRequestTimeOutInSeconds = 3;
+
 // Default url for connectivity checking.
 const char kDefaultConnectivityCheckUrl[] =
     "https://clients3.google.com/generate_204";
 
 }  // namespace
 
 ConnectivityCheckerImpl::ConnectivityCheckerImpl(
     const scoped_refptr<base::SingleThreadTaskRunner>& task_runner)
     : ConnectivityChecker(),
       task_runner_(task_runner),
       connected_(false),
-      check_errors_(0) {
+      check_errors_(0),
+      request_index_(0) {
   DCHECK(task_runner_.get());
   task_runner->PostTask(FROM_HERE,
                         base::Bind(&ConnectivityCheckerImpl::Initialize, this));
 }
 
 void ConnectivityCheckerImpl::Initialize() {
   base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
   base::CommandLine::StringType check_url_str =
       command_line->GetSwitchValueNative(switches::kConnectivityCheckUrl);
   connectivity_check_url_.reset(new GURL(
@@ skipping 41 matching lines @@
   // Don't check connectivity if network is offline, because Internet could be
   // accessible via netifs ignored.
   if (net::NetworkChangeNotifier::IsOffline())
     return;
 
   // If url_request_ is non-null, there is already a check going on. Don't
   // start another.
   if (url_request_.get())
     return;
 
+  ++request_index_;
   VLOG(1) << "Connectivity check: url=" << *connectivity_check_url_;
   url_request_ = url_request_context_->CreateRequest(
       *connectivity_check_url_, net::MAXIMUM_PRIORITY, this);
   url_request_->set_method("HEAD");
   url_request_->Start();
+
+  // Schedule time out callback.
+  task_runner_->PostDelayedTask(
+      FROM_HERE, base::Bind(&ConnectivityCheckerImpl::OnUrlRequestTimeOut, this,
+                            request_index_),
+      base::TimeDelta::FromSeconds(kRequestTimeOutInSeconds));
 }
 
 void ConnectivityCheckerImpl::OnNetworkChanged(
     net::NetworkChangeNotifier::ConnectionType type) {
   VLOG(2) << "OnNetworkChanged " << type;
   Cancel();
 
   if (type == net::NetworkChangeNotifier::CONNECTION_NONE) {
     SetConnected(false);
     return;
   }
 
   Check();
 }
 
 void ConnectivityCheckerImpl::OnResponseStarted(net::URLRequest* request) {
   int http_response_code =
       (request->status().is_success() &&
        request->response_info().headers.get() != NULL)
           ? request->response_info().headers->response_code()
           : net::HTTP_BAD_REQUEST;
 
   // Clears resources.
   url_request_.reset(NULL);  // URLRequest::Cancel() is called in destructor.

[byungchul, 2015/06/09 20:48:26]

increase request_index_? then, you don't need line 179~182.

And, CancelableCallback is preferred.

[wzhong, 2015/06/09 20:51:45]

Just realized that need to increment for all overrides.

Agreed. Given that, CancelableCallback is preferred.

 
   if (http_response_code < 400) {
     VLOG(1) << "Connectivity check succeeded";
     check_errors_ = 0;
     SetConnected(true);
     return;
   }
   VLOG(1) << "Connectivity check failed: " << http_response_code;
   OnUrlRequestError();
 }
 
 void ConnectivityCheckerImpl::OnSSLCertificateError(
     net::URLRequest* request,
     const net::SSLInfo& ssl_info,
     bool fatal) {
   LOG(ERROR) << "OnSSLCertificateError";

[byungchul, 2015/06/09 20:48:26]

ditto

   OnUrlRequestError();
 }
 
 void ConnectivityCheckerImpl::OnUrlRequestError() {
   ++check_errors_;
   if (check_errors_ > kNumErrorsToNotifyOffline) {
     check_errors_ = kNumErrorsToNotifyOffline;
     SetConnected(false);
   }
   url_request_.reset(NULL);
   // Check again.
   task_runner_->PostDelayedTask(
       FROM_HERE, base::Bind(&ConnectivityCheckerImpl::Check, this),
       base::TimeDelta::FromSeconds(kConnectivityPeriodSeconds));
 }
 
+void ConnectivityCheckerImpl::OnUrlRequestTimeOut(unsigned int request_index) {
+  if (request_index_ != request_index)
+    return;
+  if (!url_request_.get()) {
+    LOG(INFO) << "OnUrlRequestTimeOut, request has completed";
+    return;
+  }
+  VLOG(2) << "UrlRequest timed out";
+  OnUrlRequestError();
+}
+
 void ConnectivityCheckerImpl::OnReadCompleted(net::URLRequest* request,
                                               int bytes_read) {
   NOTREACHED();
 }
 
 void ConnectivityCheckerImpl::Cancel() {
   if (url_request_.get()) {
     VLOG(2) << "Cancel connectivity check in progress";
     url_request_.reset(NULL);  // URLRequest::Cancel() is called in destructor.
   }
 }
 
 }  // namespace chromecast