Added detection timeouts and usage of Retry-After HTTP header.

chrome/browser/chromeos/net/network_portal_detector.cc

Index: chrome/browser/chromeos/net/network_portal_detector.cc
diff --git a/chrome/browser/chromeos/net/network_portal_detector.cc b/chrome/browser/chromeos/net/network_portal_detector.cc
index a70418bbfd120d80ea06ce1abe875c3fe0e5e9f9..9e525764a868eda073cdaa01cc337119c5ad94db 100644
--- a/chrome/browser/chromeos/net/network_portal_detector.cc
+++ b/chrome/browser/chromeos/net/network_portal_detector.cc
@@ -7,6 +7,7 @@
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/logging.h"
+#include "base/message_loop.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chromeos/cros/cros_library.h"
 #include "chrome/common/chrome_switches.h"
@@ -20,6 +21,17 @@ namespace chromeos {
 
 namespace {
 
+// Maximum number of portal detections for the same active network
+// after network change.
+const int kMaxRequestAttempts = 3;
+
+// Minimum timeout between consecutive portal checks for the same
+// network.
+const int kMinTimeBetweenAttemptsSec = 3;
+
+// Timeout for a portal check.
+const int kRequestTimeoutSec = 10;
+
 std::string CaptivePortalStateString(
     NetworkPortalDetector::CaptivePortalState state) {
   switch (state) {
@@ -44,7 +56,12 @@ NetworkPortalDetector* g_network_portal_detector = NULL;
 
 NetworkPortalDetector::NetworkPortalDetector(
     const scoped_refptr<net::URLRequestContextGetter>& request_context)
-    : test_url_(CaptivePortalDetector::kDefaultURL) {
+    : test_url_(CaptivePortalDetector::kDefaultURL),
+      weak_ptr_factory_(this),
+      attempt_count_(0),
+      min_time_between_attempts_(
+          base::TimeDelta::FromSeconds(kMinTimeBetweenAttemptsSec)),
+      request_timeout_(base::TimeDelta::FromSeconds(kRequestTimeoutSec)) {
   captive_portal_detector_.reset(
       new CaptivePortalDetector(request_context));
 }
@@ -64,6 +81,9 @@ void NetworkPortalDetector::Init() {
 void NetworkPortalDetector::Shutdown() {
   DCHECK(CalledOnValidThread());
 
+  detection_task_.Cancel();
+  detection_timeout_.Cancel();
+
   captive_portal_detector_->Cancel();
   captive_portal_detector_.reset();
   observers_.Clear();
@@ -74,17 +94,21 @@ void NetworkPortalDetector::Shutdown() {
 
 void NetworkPortalDetector::AddObserver(Observer* observer) {
   DCHECK(CalledOnValidThread());
+
   if (!observers_.HasObserver(observer))
     observers_.AddObserver(observer);
 }
 
 void NetworkPortalDetector::RemoveObserver(Observer* observer) {
   DCHECK(CalledOnValidThread());
+
   observers_.RemoveObserver(observer);
 }
 
 NetworkPortalDetector::CaptivePortalState
 NetworkPortalDetector::GetCaptivePortalState(const Network* network) {
+  DCHECK(CalledOnValidThread());
+
   if (!network)
     return CAPTIVE_PORTAL_STATE_UNKNOWN;
   CaptivePortalStateMap::const_iterator it =
@@ -109,22 +133,27 @@ void NetworkPortalDetector::OnNetworkManagerChanged(NetworkLibrary* cros) {
 
   if (active_network_id_ != new_active_network_id) {
     active_network_id_ = new_active_network_id;
-    if (IsCheckingForPortal()) {
-      // Network is changed, so detection results will be incorrect.
-      state_ = STATE_CHECKING_FOR_PORTAL_NETWORK_CHANGED;
-    } else if (Network::IsConnectedState(connection_state)) {
-      // Start captive portal detection, if possible.
-      DetectCaptivePortal();
+    attempt_count_ = 0;
+    if (IsPortalCheckPending()) {
+      detection_task_.Cancel();
+      detection_timeout_.Cancel();
+    } else if (IsCheckingForPortal()) {
+      captive_portal_detector_->Cancel();
     }
-  } else if (!IsCheckingForPortal() &&
-             Network::IsConnectedState(connection_state)) {
+    state_ = STATE_IDLE;
+  }
+  if (!IsCheckingForPortal() && !IsPortalCheckPending() &&
+      Network::IsConnectedState(connection_state) &&
+      attempt_count_ < kMaxRequestAttempts) {
     DCHECK(active_network);
+
     // Initiate Captive Portal detection only if network's captive
-    // portal state is unknown (e.g. for freshly created networks) or offline.
+    // portal state is unknown (e.g. for freshly created networks) or
+    // offline.
     CaptivePortalState state = GetCaptivePortalState(active_network);
     if (state == CAPTIVE_PORTAL_STATE_UNKNOWN ||
         state == CAPTIVE_PORTAL_STATE_OFFLINE) {
-      DetectCaptivePortal();
+      DetectCaptivePortal(base::TimeDelta());
     }
   }
 }
@@ -150,41 +179,94 @@ bool NetworkPortalDetector::IsEnabled() {
       switches::kDisableChromeCaptivePortalDetector);
 }
 
-void NetworkPortalDetector::DetectCaptivePortal() {
+void NetworkPortalDetector::DetectCaptivePortal(const base::TimeDelta& delay) {
+  DCHECK(!IsPortalCheckPending());
+  DCHECK(!IsCheckingForPortal());
+  DCHECK(attempt_count_ < kMaxRequestAttempts);
+
+  detection_task_.Cancel();
+  detection_timeout_.Cancel();
+  state_ = STATE_PORTAL_CHECK_PENDING;
+
+  next_attempt_delay_ = delay;
+  if (attempt_count_ > 0) {
+    base::TimeTicks now = GetCurrentTimeTicks();
+    base::TimeDelta elapsed_time;
+    if (now > attempt_start_time_)
+      elapsed_time = now - attempt_start_time_;
+    if (elapsed_time < min_time_between_attempts_ &&
+        min_time_between_attempts_ - elapsed_time > next_attempt_delay_) {
+      next_attempt_delay_ = min_time_between_attempts_ - elapsed_time;
+    }
+  }
+  detection_task_.Reset(
+      base::Bind(&NetworkPortalDetector::DetectCaptivePortalTask,
+                 weak_ptr_factory_.GetWeakPtr()));
+  MessageLoop::current()->PostDelayedTask(FROM_HERE,
+                                          detection_task_.callback(),
+                                          next_attempt_delay_);
+}
+
+void NetworkPortalDetector::DetectCaptivePortalTask() {
+  DCHECK(IsPortalCheckPending());
+
   state_ = STATE_CHECKING_FOR_PORTAL;
+
+  ++attempt_count_;
+  attempt_start_time_ = GetCurrentTimeTicks();
+
+  VLOG(1) << "Portal detection started: "
+          << "network=" << active_network_id_ << ", "
+          << "attempt=" << attempt_count_ << " of " << kMaxRequestAttempts;
+
   captive_portal_detector_->DetectCaptivePortal(
       test_url_,
       base::Bind(&NetworkPortalDetector::OnPortalDetectionCompleted,
-                 base::Unretained(this)));
+                 weak_ptr_factory_.GetWeakPtr()));
+  detection_timeout_.Reset(
+      base::Bind(&NetworkPortalDetector::PortalDetectionTimeout,
+                 weak_ptr_factory_.GetWeakPtr()));
+  MessageLoop::current()->PostDelayedTask(FROM_HERE,
+                                          detection_timeout_.callback(),
+                                          request_timeout_);
+}
+
+void NetworkPortalDetector::PortalDetectionTimeout() {
+  DCHECK(CalledOnValidThread());
+  DCHECK(IsCheckingForPortal());
+
+  VLOG(1) << "Portal detection timeout: network=" << active_network_id_;
+
+  captive_portal_detector_->Cancel();
+  CaptivePortalDetector::Results results;
+  results.result = captive_portal::RESULT_NO_RESPONSE;
+  OnPortalDetectionCompleted(results);
 }
 
 void NetworkPortalDetector::OnPortalDetectionCompleted(
     const CaptivePortalDetector::Results& results) {
   DCHECK(CalledOnValidThread());
-
   DCHECK(IsCheckingForPortal());
 
+  VLOG(1) << "Portal detection completed: "
+          << "network=" << active_network_id_ << ", "
+          << "result=" << CaptivePortalDetector::CaptivePortalResultToString(
+              results.result);
+
+  state_ = STATE_IDLE;
+  detection_timeout_.Cancel();
+
   NetworkLibrary* cros = CrosLibrary::Get()->GetNetworkLibrary();
   const Network* active_network = cros->active_network();
   if (!active_network)
     return;
 
-  if (state_ == STATE_CHECKING_FOR_PORTAL_NETWORK_CHANGED) {
-    // Can't use detection results, because network was changed. So
-    // retry portal detection for the active network or, if possible.
-    if (Network::IsConnectedState(active_network->connection_state()) &&
-        GetCaptivePortalState(active_network) ==
-        CAPTIVE_PORTAL_STATE_UNKNOWN) {
-      DetectCaptivePortal();
-    }
-    return;
-  }
-
-  state_ = STATE_IDLE;
-
   switch (results.result) {
     case captive_portal::RESULT_NO_RESPONSE:
-      SetCaptivePortalState(active_network, CAPTIVE_PORTAL_STATE_OFFLINE);
+      if (attempt_count_ >= kMaxRequestAttempts)
+        SetCaptivePortalState(active_network, CAPTIVE_PORTAL_STATE_OFFLINE);
+      else
+        DetectCaptivePortal(results.retry_after_delta);
       break;
     case captive_portal::RESULT_INTERNET_CONNECTED:
       SetCaptivePortalState(active_network, CAPTIVE_PORTAL_STATE_ONLINE);
@@ -197,19 +279,23 @@ void NetworkPortalDetector::OnPortalDetectionCompleted(
   }
 }
 
+bool NetworkPortalDetector::IsPortalCheckPending() const {
+  return state_ == STATE_PORTAL_CHECK_PENDING;
+}
+
 bool NetworkPortalDetector::IsCheckingForPortal() const {
-  return state_ == STATE_CHECKING_FOR_PORTAL ||
-      state_ == STATE_CHECKING_FOR_PORTAL_NETWORK_CHANGED;
+  return state_ == STATE_CHECKING_FOR_PORTAL;
 }
 
 void NetworkPortalDetector::SetCaptivePortalState(const Network* network,
                                                   CaptivePortalState state) {
   DCHECK(network);
+
   CaptivePortalStateMap::const_iterator it =
       captive_portal_state_map_.find(network->unique_id());
   if (it == captive_portal_state_map_.end() ||
       it->second != state) {
-    VLOG(2) << "Updating Chrome Captive Portal state: "
+    VLOG(1) << "Updating Chrome Captive Portal state: "
             << "network=" << network->unique_id() << ", "
             << "state=" << CaptivePortalStateString(state);
     captive_portal_state_map_[network->unique_id()] = state;
@@ -222,4 +308,11 @@ void NetworkPortalDetector::NotifyPortalStateChanged(const Network* network,
   FOR_EACH_OBSERVER(Observer, observers_, OnPortalStateChanged(network, state));
 }
 
+base::TimeTicks NetworkPortalDetector::GetCurrentTimeTicks() const {
+  if (time_ticks_for_testing_.is_null())
+    return base::TimeTicks::Now();
+  else
+    return time_ticks_for_testing_;
+}
+
 }  // namespace chromeos