Implemented lazy portal detection.

BUG=169783, 166973, 174061
TEST=unit_tests:NetworkPortalDetectorTest*

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@182249 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/chromeos/net/network_portal_detector.cc

@@ -38,6 +38,11 @@ const int kRequestTimeoutSec = 10;
 // Delay before portal detection caused by changes in proxy settings.
 const int kProxyChangeDelayMs = 500;
 
+// Delay between consecutive portal checks for a network in lazy mode.
+// TODO (ygorshenin@): use exponential backoff or normally distributed
+// random variable instead of this.
+const int kLazyCheckIntervalSec = 30;
+
 std::string CaptivePortalStatusString(
     NetworkPortalDetector::CaptivePortalStatus status) {
   switch (status) {
@@ -73,6 +78,8 @@ NetworkPortalDetector::NetworkPortalDetector(
       test_url_(CaptivePortalDetector::kDefaultURL),
       weak_ptr_factory_(this),
       attempt_count_(0),
+      lazy_detection_enabled_(false),
+      lazy_check_interval_(base::TimeDelta::FromSeconds(kLazyCheckIntervalSec)),
       min_time_between_attempts_(
           base::TimeDelta::FromSeconds(kMinTimeBetweenAttemptsSec)),
       request_timeout_(base::TimeDelta::FromSeconds(kRequestTimeoutSec)) {
@@ -170,18 +177,12 @@ void NetworkPortalDetector::OnNetworkManagerChanged(NetworkLibrary* cros) {
 
   if (network_changed || connection_state_changed) {
     attempt_count_ = 0;
-    if (IsPortalCheckPending()) {
-      detection_task_.Cancel();
-      detection_timeout_.Cancel();
-    } else if (IsCheckingForPortal()) {
-      captive_portal_detector_->Cancel();
-    }
-    state_ = STATE_IDLE;
+    CancelPortalDetection();
   }
 
   if (!IsCheckingForPortal() && !IsPortalCheckPending() &&
       Network::IsConnectedState(active_connection_state_) &&
-      attempt_count_ < kMaxRequestAttempts) {
+      (attempt_count_ < kMaxRequestAttempts || lazy_detection_enabled_)) {
     DCHECK(active_network);
 
     // Initiate Captive Portal detection if network's captive
@@ -202,6 +203,24 @@ void NetworkPortalDetector::OnNetworkChanged(chromeos::NetworkLibrary* cros,
   OnNetworkManagerChanged(cros);
 }
 
+void NetworkPortalDetector::EnableLazyDetection() {
+  if (lazy_detection_enabled_)
+    return;
+  VLOG(1) << "Lazy detection mode enabled";
+  lazy_detection_enabled_ = true;
+  if (!IsPortalCheckPending() && !IsCheckingForPortal())
+    DetectCaptivePortal(base::TimeDelta());
+}
+
+void NetworkPortalDetector::DisableLazyDetection() {
+  if (!lazy_detection_enabled_)
+    return;
+  VLOG(1) << "Lazy detection mode disabled";
+  if (attempt_count_ == kMaxRequestAttempts)
+    CancelPortalDetection();
+  lazy_detection_enabled_ = false;
+}
+
 // static
 NetworkPortalDetector* NetworkPortalDetector::CreateInstance() {
   DCHECK(!g_network_portal_detector);
@@ -226,7 +245,7 @@ bool NetworkPortalDetector::IsEnabled() {
 void NetworkPortalDetector::DetectCaptivePortal(const base::TimeDelta& delay) {
   DCHECK(!IsPortalCheckPending());
   DCHECK(!IsCheckingForPortal());
-  DCHECK(attempt_count_ < kMaxRequestAttempts);
+  DCHECK(attempt_count_ < kMaxRequestAttempts || lazy_detection_enabled_);
 
   detection_task_.Cancel();
   detection_timeout_.Cancel();
@@ -236,11 +255,17 @@ void NetworkPortalDetector::DetectCaptivePortal(const base::TimeDelta& delay) {
   if (attempt_count_ > 0) {
     base::TimeTicks now = GetCurrentTimeTicks();
     base::TimeDelta elapsed_time;
+
+    base::TimeDelta delay_between_attempts = min_time_between_attempts_;
+    if (attempt_count_ == kMaxRequestAttempts) {
+      DCHECK(lazy_detection_enabled_);
+      delay_between_attempts = lazy_check_interval_;
+    }
     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;
+    if (elapsed_time < delay_between_attempts &&
+        delay_between_attempts - elapsed_time > next_attempt_delay_) {
+      next_attempt_delay_ = delay_between_attempts - elapsed_time;
     }
   } else {
     detection_start_time_ = GetCurrentTimeTicks();
@@ -258,11 +283,17 @@ void NetworkPortalDetector::DetectCaptivePortalTask() {
 
   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;
+  if (attempt_count_ < kMaxRequestAttempts) {
+    ++attempt_count_;
+    VLOG(1) << "Portal detection started: "
+            << "network=" << active_network_id_ << ", "
+            << "attempt=" << attempt_count_ << " of " << kMaxRequestAttempts;
+  } else {
+    DCHECK(lazy_detection_enabled_);
+    VLOG(1) << "Lazy portal detection attempt started";
+  }
 
   captive_portal_detector_->DetectCaptivePortal(
       test_url_,
@@ -288,6 +319,16 @@ void NetworkPortalDetector::PortalDetectionTimeout() {
   OnPortalDetectionCompleted(results);
 }
 
+void NetworkPortalDetector::CancelPortalDetection() {
+  if (IsPortalCheckPending()) {
+    detection_task_.Cancel();
+    detection_timeout_.Cancel();
+  } else if (IsCheckingForPortal()) {
+    captive_portal_detector_->Cancel();
+  }
+  state_ = STATE_IDLE;
+}
+
 void NetworkPortalDetector::OnPortalDetectionCompleted(
     const CaptivePortalDetector::Results& results) {
   DCHECK(CalledOnValidThread());
@@ -304,8 +345,10 @@ void NetworkPortalDetector::OnPortalDetectionCompleted(
 
   NetworkLibrary* cros = CrosLibrary::Get()->GetNetworkLibrary();
   const Network* active_network = cros->active_network();
-  if (!active_network)
+  if (!active_network) {
+    TryLazyDetection();
     return;
+  }
 
   CaptivePortalState state;
   state.response_code = results.response_code;
@@ -342,7 +385,18 @@ void NetworkPortalDetector::OnPortalDetectionCompleted(
     default:
       break;
   }
+
+  TryLazyDetection();
 }
+
+void NetworkPortalDetector::TryLazyDetection() {
+  if (!IsPortalCheckPending() && !IsCheckingForPortal() &&
+      lazy_detection_enabled_) {
+    attempt_count_ = kMaxRequestAttempts;
+    DetectCaptivePortal(base::TimeDelta());
+  }
+}
+
 void NetworkPortalDetector::Observe(
     int type,
     const content::NotificationSource& source,

chrome/browser/chromeos/net/network_portal_detector.h

@@ -86,6 +86,13 @@ class NetworkPortalDetector
   virtual void OnNetworkChanged(chromeos::NetworkLibrary* cros,
                                 const chromeos::Network* network) OVERRIDE;
 
+  // Enables lazy detection mode. In this mode portal detection after
+  // first 3 consecutive attemps will be performed once in 30 seconds.
+  void EnableLazyDetection();
+
+  // Dizables lazy detection mode.
+  void DisableLazyDetection();
+
   // Creates an instance of the NetworkPortalDetector.
   static NetworkPortalDetector* CreateInstance();
 
@@ -129,6 +136,10 @@ class NetworkPortalDetector
   void OnPortalDetectionCompleted(
       const captive_portal::CaptivePortalDetector::Results& results);
 
+  // Tries to perform portal detection in "lazy" mode. Does nothing in
+  // the case of already pending/processing detection request.
+  void TryLazyDetection();
+
   // content::NotificationObserver implementation:
   virtual void Observe(int type,
                        const content::NotificationSource& source,
@@ -163,6 +174,12 @@ class NetworkPortalDetector
     min_time_between_attempts_ = delta;
   }
 
+  // Sets default interval between consecutive portal checks for a
+  // network in portal state. Used by unit tests.
+  void set_lazy_check_interval_for_testing(const base::TimeDelta& delta) {
+    lazy_check_interval_ = delta;
+  }
+
   // Sets portal detection timeout. Used by unit tests.
   void set_request_timeout_for_testing(const base::TimeDelta& timeout) {
     request_timeout_ = timeout;
@@ -210,6 +227,13 @@ class NetworkPortalDetector
   // Number of portal detection attemps for an active network.
   int attempt_count_;
 
+  // True if lazy detection is enabled.
+  bool lazy_detection_enabled_;
+
+  // Time between consecutive portal checks for a network in lazy
+  // mode.
+  base::TimeDelta lazy_check_interval_;
+
   // Minimum time between consecutive portal checks for the same
   // active network.
   base::TimeDelta min_time_between_attempts_;

chrome/browser/chromeos/net/network_portal_detector_unittest.cc

@@ -80,6 +80,14 @@ class NetworkPortalDetectorTest
     return network_portal_detector()->state();
   }
 
+  void enable_lazy_detection() {
+    network_portal_detector()->EnableLazyDetection();
+  }
+
+  void disable_lazy_detection() {
+    network_portal_detector()->DisableLazyDetection();
+  }
+
   bool is_state_idle() {
     return (NetworkPortalDetector::STATE_IDLE == state());
   }
@@ -108,6 +116,10 @@ class NetworkPortalDetectorTest
     network_portal_detector()->set_min_time_between_attempts_for_testing(delta);
   }
 
+  void set_lazy_check_interval(const base::TimeDelta& delta) {
+    network_portal_detector()->set_lazy_check_interval_for_testing(delta);
+  }
+
   void set_time_ticks(const base::TimeTicks& time_ticks) {
     network_portal_detector()->set_time_ticks_for_testing(time_ticks);
   }
@@ -458,4 +470,85 @@ TEST_F(NetworkPortalDetectorTest, NoResponseButBehindPortal) {
                    wifi1_network());
 }
 
+TEST_F(NetworkPortalDetectorTest, LazyDetectionForOnlineNetwork) {
+  ASSERT_TRUE(is_state_idle());
+  set_min_time_between_attempts(base::TimeDelta());
+  set_lazy_check_interval(base::TimeDelta());
+
+  SetConnected(wifi1_network());
+  enable_lazy_detection();
+  CompleteURLFetch(net::OK, 204, NULL);
+
+  ASSERT_EQ(3, attempt_count());
+  ASSERT_TRUE(is_state_portal_detection_pending());
+  CheckPortalState(
+      NetworkPortalDetector::CAPTIVE_PORTAL_STATUS_ONLINE, 204,
+      wifi1_network());
+
+  // To run CaptivePortalDetector::DetectCaptivePortal().
+  MessageLoop::current()->RunUntilIdle();
+
+  CompleteURLFetch(net::OK, 204, NULL);
+
+  ASSERT_EQ(3, attempt_count());
+  ASSERT_TRUE(is_state_portal_detection_pending());
+  CheckPortalState(
+      NetworkPortalDetector::CAPTIVE_PORTAL_STATUS_ONLINE, 204,
+      wifi1_network());
+
+  // To run CaptivePortalDetector::DetectCaptivePortal().
+  MessageLoop::current()->RunUntilIdle();
+
+  disable_lazy_detection();
+  ASSERT_TRUE(is_state_idle());
+}
+
+TEST_F(NetworkPortalDetectorTest, LazyDetectionForPortalNetwork) {
+  ASSERT_TRUE(is_state_idle());
+  set_min_time_between_attempts(base::TimeDelta());
+  set_lazy_check_interval(base::TimeDelta());
+
+  SetConnected(wifi1_network());
+  enable_lazy_detection();
+
+  CompleteURLFetch(net::ERR_CONNECTION_CLOSED,
+                   net::URLFetcher::RESPONSE_CODE_INVALID,
+                   NULL);
+  ASSERT_EQ(1, attempt_count());
+  ASSERT_TRUE(is_state_portal_detection_pending());
+  CheckPortalState(NetworkPortalDetector::CAPTIVE_PORTAL_STATUS_UNKNOWN, -1,
+                   wifi1_network());
+
+  // To run CaptivePortalDetector::DetectCaptivePortal().
+  MessageLoop::current()->RunUntilIdle();
+
+  CompleteURLFetch(net::ERR_CONNECTION_CLOSED,
+                   net::URLFetcher::RESPONSE_CODE_INVALID,
+                   NULL);
+  ASSERT_EQ(2, attempt_count());
+  ASSERT_TRUE(is_state_portal_detection_pending());
+  CheckPortalState(NetworkPortalDetector::CAPTIVE_PORTAL_STATUS_UNKNOWN, -1,
+                   wifi1_network());
+
+  // To run CaptivePortalDetector::DetectCaptivePortal().
+  MessageLoop::current()->RunUntilIdle();
+
+  CompleteURLFetch(net::OK, 200, NULL);
+  ASSERT_EQ(3, attempt_count());
+  ASSERT_TRUE(is_state_portal_detection_pending());
+  CheckPortalState(NetworkPortalDetector::CAPTIVE_PORTAL_STATUS_PORTAL, 200,
+                   wifi1_network());
+
+  // To run CaptivePortalDetector::DetectCaptivePortal().
+  MessageLoop::current()->RunUntilIdle();
+  CompleteURLFetch(net::OK, 200, NULL);
+  ASSERT_EQ(3, attempt_count());
+  ASSERT_TRUE(is_state_portal_detection_pending());
+  CheckPortalState(NetworkPortalDetector::CAPTIVE_PORTAL_STATUS_PORTAL, 200,
+                   wifi1_network());
+
+  disable_lazy_detection();
+  ASSERT_TRUE(is_state_idle());
+}
+
 }  // namespace chromeos

chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc

@@ -260,6 +260,18 @@ void RecordNetworkPortalDetectorStats(const std::string& service_path) {
   }
 }
 
+void EnableLazyDetection() {
+  NetworkPortalDetector* detector = NetworkPortalDetector::GetInstance();
+  if (detector)
+    detector->EnableLazyDetection();
+}
+
+void DisableLazyDetection() {
+  NetworkPortalDetector* detector = NetworkPortalDetector::GetInstance();
+  if (detector)
+    detector->DisableLazyDetection();
+}
+
 }  // namespace
 
 // SigninScreenHandler implementation ------------------------------------------
@@ -656,6 +668,8 @@ void SigninScreenHandler::UpdateStateInternal(
       params.SetInteger("lastNetworkType", static_cast<int>(connection_type));
       error_screen_actor_->Show(OobeUI::SCREEN_GAIA_SIGNIN, &params);
     }
+
+    EnableLazyDetection();
   } else {
     if (IsSigninScreenHiddenByError()) {
       LOG(WARNING) << "Hide offline message. state=" << state << ", "
@@ -669,6 +683,8 @@ void SigninScreenHandler::UpdateStateInternal(
         is_gaia_reloaded = true;
       }
     }
+
+    DisableLazyDetection();
   }
 }