Increased delay before next portal detection attempt in the case of !ONLINE -> ONLINE transition.

BUG=377725
TEST=manual

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

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

chrome/browser/chromeos/net/network_portal_detector_impl.cc

@@ -37,6 +37,22 @@ const int kProxyChangeDelaySec = 1;
 // offline state in a row before notification is sent to observers.
 const int kMaxOfflineResultsBeforeReport = 3;
 
+// Delay before portal detection attempt after !ONLINE -> !ONLINE
+// transition.
+const int kShortInitialDelayBetweenAttemptsMs = 600;
+
+// Maximum timeout before portal detection attempts after !ONLINE ->
+// !ONLINE transition.
+const int kShortMaximumDelayBetweenAttemptsMs = 2 * 60 * 1000;
+
+// Delay before portal detection attempt after !ONLINE -> ONLINE
+// transition.
+const int kLongInitialDelayBetweenAttemptsMs = 30 * 1000;
+
+// Maximum timeout before portal detection attempts after !ONLINE ->
+// ONLINE transition.
+const int kLongMaximumDelayBetweenAttemptsMs = 5 * 60 * 1000;
+
 const NetworkState* DefaultNetwork() {
   return NetworkHandler::Get()->network_state_handler()->DefaultNetwork();
 }
@@ -207,13 +223,12 @@ NetworkPortalDetectorImpl::NetworkPortalDetectorImpl(
       test_url_(CaptivePortalDetector::kDefaultURL),
       enabled_(false),
       strategy_(PortalDetectorStrategy::CreateById(
-          PortalDetectorStrategy::STRATEGY_ID_LOGIN_SCREEN)),
+          PortalDetectorStrategy::STRATEGY_ID_LOGIN_SCREEN, this)),
       last_detection_result_(CAPTIVE_PORTAL_STATUS_UNKNOWN),
       same_detection_result_count_(0),
       no_response_result_count_(0),
       weak_factory_(this) {
   captive_portal_detector_.reset(new CaptivePortalDetector(request_context));
-  strategy_->set_delegate(this);
 
   registrar_.Add(this,
                  chrome::NOTIFICATION_LOGIN_PROXY_CHANGED,
@@ -307,8 +322,7 @@ void NetworkPortalDetectorImpl::SetStrategy(
     PortalDetectorStrategy::StrategyId id) {
   if (id == strategy_->Id())
     return;
-  strategy_.reset(PortalDetectorStrategy::CreateById(id).release());
-  strategy_->set_delegate(this);
+  strategy_ = PortalDetectorStrategy::CreateById(id, this).Pass();
   StopDetection();
   StartDetectionIfIdle();
 }
@@ -502,7 +516,15 @@ void NetworkPortalDetectorImpl::OnAttemptCompleted(
   if (last_detection_result_ != state.status) {
     last_detection_result_ = state.status;
     same_detection_result_count_ = 1;
-    strategy_->Reset();
+    net::BackoffEntry::Policy policy = strategy_->policy();
+    if (state.status == CAPTIVE_PORTAL_STATUS_ONLINE) {
+      policy.initial_delay_ms = kLongInitialDelayBetweenAttemptsMs;
+      policy.maximum_backoff_ms = kLongMaximumDelayBetweenAttemptsMs;
+    } else {
+      policy.initial_delay_ms = kShortInitialDelayBetweenAttemptsMs;
+      policy.maximum_backoff_ms = kShortMaximumDelayBetweenAttemptsMs;
+    }
+    strategy_->SetPolicyAndReset(policy);
   } else {
     ++same_detection_result_count_;
   }

chromeos/network/portal_detector/network_portal_detector_strategy.cc

@@ -17,14 +17,13 @@ const NetworkState* DefaultNetwork() {
   return NetworkHandler::Get()->network_state_handler()->DefaultNetwork();
 }
 
-// TODO (ygorshenin@): reuse net::BackoffEntry for strategies.
-
 class LoginScreenStrategy : public PortalDetectorStrategy {
  public:
   static const int kBaseAttemptTimeoutSec = 5;
   static const int kMaxAttemptTimeoutSec = 30;
 
-  LoginScreenStrategy() {}
+  explicit LoginScreenStrategy(PortalDetectorStrategy::Delegate* delegate)
+      : PortalDetectorStrategy(delegate) {}
   virtual ~LoginScreenStrategy() {}
 
  protected:
@@ -51,7 +50,8 @@ class ErrorScreenStrategy : public PortalDetectorStrategy {
  public:
   static const int kAttemptTimeoutSec = 15;
 
-  ErrorScreenStrategy() {}
+  explicit ErrorScreenStrategy(PortalDetectorStrategy::Delegate* delegate)
+      : PortalDetectorStrategy(delegate) {}
   virtual ~ErrorScreenStrategy() {}
 
  protected:
@@ -71,7 +71,8 @@ class SessionStrategy : public PortalDetectorStrategy {
   static const int kFastAttemptTimeoutSec = 3;
   static const int kSlowAttemptTimeoutSec = 5;
 
-  SessionStrategy() {}
+  explicit SessionStrategy(PortalDetectorStrategy::Delegate* delegate)
+      : PortalDetectorStrategy(delegate) {}
   virtual ~SessionStrategy() {}
 
  protected:
@@ -91,6 +92,26 @@ class SessionStrategy : public PortalDetectorStrategy {
 
 }  // namespace
 
+// PortalDetectorStrategy::BackoffEntryImpl ------------------------------------
+
+class PortalDetectorStrategy::BackoffEntryImpl : public net::BackoffEntry {
+ public:
+  BackoffEntryImpl(const net::BackoffEntry::Policy* const policy,
+                   PortalDetectorStrategy::Delegate* delegate)
+      : net::BackoffEntry(policy), delegate_(delegate) {}
+  virtual ~BackoffEntryImpl() {}
+
+  // net::BackoffEntry overrides:
+  virtual base::TimeTicks ImplGetTimeNow() const OVERRIDE {
+    return delegate_->GetCurrentTimeTicks();
+  }
+
+ private:
+  PortalDetectorStrategy::Delegate* delegate_;
+
+  DISALLOW_COPY_AND_ASSIGN(BackoffEntryImpl);
+};
+
 // PortalDetectorStrategy -----------------------------------------------------
 
 // static
@@ -107,12 +128,13 @@ base::TimeDelta PortalDetectorStrategy::next_attempt_timeout_for_testing_;
 bool PortalDetectorStrategy::next_attempt_timeout_for_testing_initialized_ =
     false;
 
-PortalDetectorStrategy::PortalDetectorStrategy()
-    : net::BackoffEntry(&policy_), delegate_(NULL) {
-  // First three attempts with the same result are performed with
-  // 600ms delay between them. Delay before every consecutive attempt
-  // is multplied by 2.0. Also, 30% fuzz factor is used for each
-  // delay.
+PortalDetectorStrategy::PortalDetectorStrategy(Delegate* delegate)
+    : delegate_(delegate) {
+  // First |policy_.num_errors_to_ignore| attempts with the same
+  // result are performed with |policy_.initial_delay_ms| between
+  // them. Delay before every consecutive attempt is multplied by
+  // |policy_.multiply_factor_|. Also, |policy_.jitter_factor| is used
+  // for each delay.
   policy_.num_errors_to_ignore = 3;
   policy_.initial_delay_ms = 600;
   policy_.multiply_factor = 2.0;
@@ -120,6 +142,7 @@ PortalDetectorStrategy::PortalDetectorStrategy()
   policy_.maximum_backoff_ms = 2 * 60 * 1000;
   policy_.entry_lifetime_ms = -1;
   policy_.always_use_initial_delay = true;
+  backoff_entry_.reset(new BackoffEntryImpl(&policy_, delegate_));
 }
 
 PortalDetectorStrategy::~PortalDetectorStrategy() {
@@ -127,14 +150,17 @@ PortalDetectorStrategy::~PortalDetectorStrategy() {
 
 // statc
 scoped_ptr<PortalDetectorStrategy> PortalDetectorStrategy::CreateById(
-    StrategyId id) {
+    StrategyId id,
+    Delegate* delegate) {
   switch (id) {
     case STRATEGY_ID_LOGIN_SCREEN:
-      return scoped_ptr<PortalDetectorStrategy>(new LoginScreenStrategy());
+      return scoped_ptr<PortalDetectorStrategy>(
+          new LoginScreenStrategy(delegate));
     case STRATEGY_ID_ERROR_SCREEN:
-      return scoped_ptr<PortalDetectorStrategy>(new ErrorScreenStrategy());
+      return scoped_ptr<PortalDetectorStrategy>(
+          new ErrorScreenStrategy(delegate));
     case STRATEGY_ID_SESSION:
-      return scoped_ptr<PortalDetectorStrategy>(new SessionStrategy());
+      return scoped_ptr<PortalDetectorStrategy>(new SessionStrategy(delegate));
     default:
       NOTREACHED();
       return scoped_ptr<PortalDetectorStrategy>(
@@ -145,7 +171,7 @@ scoped_ptr<PortalDetectorStrategy> PortalDetectorStrategy::CreateById(
 base::TimeDelta PortalDetectorStrategy::GetDelayTillNextAttempt() {
   if (delay_till_next_attempt_for_testing_initialized_)
     return delay_till_next_attempt_for_testing_;
-  return net::BackoffEntry::GetTimeUntilRelease();
+  return backoff_entry_->GetTimeUntilRelease();
 }
 
 base::TimeDelta PortalDetectorStrategy::GetNextAttemptTimeout() {
@@ -155,15 +181,17 @@ base::TimeDelta PortalDetectorStrategy::GetNextAttemptTimeout() {
 }
 
 void PortalDetectorStrategy::Reset() {
-  net::BackoffEntry::Reset();
+  backoff_entry_->Reset();
 }
 
-void PortalDetectorStrategy::OnDetectionCompleted() {
-  net::BackoffEntry::InformOfRequest(false);
+void PortalDetectorStrategy::SetPolicyAndReset(
+    const net::BackoffEntry::Policy& policy) {
+  policy_ = policy;
+  backoff_entry_.reset(new BackoffEntryImpl(&policy_, delegate_));
 }
 
-base::TimeTicks PortalDetectorStrategy::ImplGetTimeNow() const {
-  return delegate_->GetCurrentTimeTicks();
+void PortalDetectorStrategy::OnDetectionCompleted() {
+  backoff_entry_->InformOfRequest(false);
 }
 
 base::TimeDelta PortalDetectorStrategy::GetNextAttemptTimeoutImpl() {

chromeos/network/portal_detector/network_portal_detector_strategy.h

@@ -15,7 +15,7 @@
 
 namespace chromeos {
 
-class CHROMEOS_EXPORT PortalDetectorStrategy : protected net::BackoffEntry {
+class CHROMEOS_EXPORT PortalDetectorStrategy {
  public:
   enum StrategyId {
     STRATEGY_ID_LOGIN_SCREEN,
@@ -40,9 +40,8 @@ class CHROMEOS_EXPORT PortalDetectorStrategy : protected net::BackoffEntry {
 
   virtual ~PortalDetectorStrategy();
 
-  static scoped_ptr<PortalDetectorStrategy> CreateById(StrategyId id);
-
-  void set_delegate(Delegate* delegate) { delegate_ = delegate; }
+  static scoped_ptr<PortalDetectorStrategy> CreateById(StrategyId id,
+                                                       Delegate* delegate);
 
   // Returns delay before next detection attempt. This delay is needed
   // to separate detection attempts in time.
@@ -56,21 +55,26 @@ class CHROMEOS_EXPORT PortalDetectorStrategy : protected net::BackoffEntry {
   // Resets strategy to the initial state.
   void Reset();
 
+  const net::BackoffEntry::Policy& policy() const { return policy_; }
+
+  // Resets strategy to the initial stater and sets custom policy.
+  void SetPolicyAndReset(const net::BackoffEntry::Policy& policy);
+
   // Should be called when portal detection is completed and timeout before next
   // attempt should be adjusted.
   void OnDetectionCompleted();
 
  protected:
-  PortalDetectorStrategy();
+  class BackoffEntryImpl;
 
-  // net::BackoffEntry overrides:
-  virtual base::TimeTicks ImplGetTimeNow() const OVERRIDE;
+  explicit PortalDetectorStrategy(Delegate* delegate);
 
   // Interface for subclasses:
   virtual base::TimeDelta GetNextAttemptTimeoutImpl();
 
   Delegate* delegate_;
   net::BackoffEntry::Policy policy_;
+  scoped_ptr<BackoffEntryImpl> backoff_entry_;
 
  private:
   friend class NetworkPortalDetectorImplTest;