Captive portal service: Don't use a retry delay after getting a new result.

chrome/browser/captive_portal/captive_portal_service_unittest.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/captive_portal/captive_portal_service.h"
 
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/message_loop.h"
 #include "base/test/test_timeouts.h"
@@ skipping 145 matching lines @@
     set_initial_backoff_no_portal(base::TimeDelta());
     set_initial_backoff_portal(base::TimeDelta());
 
     // Disable jitter, so can check exact values.
     set_jitter_factor(0.0);
 
     // These values make checking exponential backoff easier.
     set_multiply_factor(2.0);
     set_maximum_backoff(base::TimeDelta::FromSeconds(1600));
 
-    // This means backoff starts after the first "failure", which is the second
-    // captive portal test in a row that ends up with the same result.
-    set_num_errors_to_ignore(0);
+    // This means backoff starts after the second "failure", which is the third
+    // captive portal test in a row that ends up with the same result.  Since
+    // the first request uses no delay, this means the delays will be in
+    // the pattern 0, 0, 100, 200, 400, etc.  There are two zeros because the
+    // first check never has a delay, and the first check to have a new result
+    // is followed by no delay.
+    set_num_errors_to_ignore(1);
 
     EnableCaptivePortalDetectionPreference(true);
   }
 
   // Sets the captive portal checking preference.
   void EnableCaptivePortalDetectionPreference(bool enabled) {
     profile()->GetPrefs()->SetBoolean(prefs::kAlternateErrorPagesEnabled,
                                       enabled);
   }
 
@@ skipping 88 matching lines @@
     EXPECT_FALSE(TimerRunning());
     EXPECT_EQ(1, observer.num_results_received());
     EXPECT_EQ(RESULT_INTERNET_CONNECTED, observer.captive_portal_result());
   }
 
   // Tests exponential backoff.  Prior to calling, the relevant recheck settings
   // must be set to have a minimum time of 100 seconds, with 2 checks before
   // starting exponential backoff.
   void RunBackoffTest(Result expected_result, int net_error, int status_code) {
     RunTest(expected_result, net_error, status_code, 0, NULL);
+    RunTest(expected_result, net_error, status_code, 0, NULL);
     RunTest(expected_result, net_error, status_code, 100, NULL);
     RunTest(expected_result, net_error, status_code, 200, NULL);
     RunTest(expected_result, net_error, status_code, 400, NULL);
     RunTest(expected_result, net_error, status_code, 800, NULL);
     RunTest(expected_result, net_error, status_code, 1600, NULL);
     RunTest(expected_result, net_error, status_code, 1600, NULL);
   }
 
   bool FetchingURL() {
     return service()->FetchingURL();
@@ skipping 108 matching lines @@
   Initialize(true);
 
   // This value should have no effect on this test, until the end.
   set_initial_backoff_portal(base::TimeDelta::FromSeconds(1));
 
   set_initial_backoff_no_portal(base::TimeDelta::FromSeconds(100));
   RunBackoffTest(RESULT_INTERNET_CONNECTED, net::OK, 204);
 
   // Make sure that getting a new result resets the timer.
   RunTest(RESULT_BEHIND_CAPTIVE_PORTAL, net::OK, 200, 1600, NULL);
+  RunTest(RESULT_BEHIND_CAPTIVE_PORTAL, net::OK, 200, 0, NULL);
   RunTest(RESULT_BEHIND_CAPTIVE_PORTAL, net::OK, 200, 1, NULL);
+  RunTest(RESULT_BEHIND_CAPTIVE_PORTAL, net::OK, 200, 2, NULL);
 }
 
 // Checks exponential backoff when there's an HTTP error.
 TEST_F(CaptivePortalServiceTest, CaptivePortalRecheckError) {
   Initialize(true);
 
   // This value should have no effect on this test.
   set_initial_backoff_portal(base::TimeDelta::FromDays(1));
 
   set_initial_backoff_no_portal(base::TimeDelta::FromSeconds(100));
   RunBackoffTest(RESULT_NO_RESPONSE, net::OK, 500);
 
   // Make sure that getting a new result resets the timer.
   RunTest(RESULT_INTERNET_CONNECTED, net::OK, 204, 1600, NULL);
+  RunTest(RESULT_INTERNET_CONNECTED, net::OK, 204, 0, NULL);
   RunTest(RESULT_INTERNET_CONNECTED, net::OK, 204, 100, NULL);
 }
 
 // Checks exponential backoff when there's a captive portal.
 TEST_F(CaptivePortalServiceTest, CaptivePortalRecheckBehindPortal) {
   Initialize(true);
 
   // This value should have no effect on this test, until the end.
   set_initial_backoff_no_portal(base::TimeDelta::FromSeconds(250));
 
   set_initial_backoff_portal(base::TimeDelta::FromSeconds(100));
   RunBackoffTest(RESULT_BEHIND_CAPTIVE_PORTAL, net::OK, 200);
 
   // Make sure that getting a new result resets the timer.
   RunTest(RESULT_INTERNET_CONNECTED, net::OK, 204, 1600, NULL);
+  RunTest(RESULT_INTERNET_CONNECTED, net::OK, 204, 0, NULL);
   RunTest(RESULT_INTERNET_CONNECTED, net::OK, 204, 250, NULL);
 }
 
 // Check that everything works as expected when captive portal checking is
 // disabled, including throttling.  Then enables it again and runs another test.
 TEST_F(CaptivePortalServiceTest, CaptivePortalPrefDisabled) {
   Initialize(true);
 
   // This value should have no effect on this test.
   set_initial_backoff_no_portal(base::TimeDelta::FromDays(1));
@@ skipping 105 matching lines @@
   Initialize(false);
   RunDisabledTest(0);
 }
 
 // Checks that jitter gives us values in the correct range.
 TEST_F(CaptivePortalServiceTest, CaptivePortalJitter) {
   Initialize(true);
   set_jitter_factor(0.3);
   set_initial_backoff_no_portal(base::TimeDelta::FromSeconds(100));
   RunTest(RESULT_INTERNET_CONNECTED, net::OK, 204, 0, NULL);
+  RunTest(RESULT_INTERNET_CONNECTED, net::OK, 204, 0, NULL);
 
   for (int i = 0; i < 50; ++i) {
     int interval_sec = GetTimeUntilNextRequest().InSeconds();
     // Allow for roundoff, though shouldn't be necessary.
     EXPECT_LE(69, interval_sec);
     EXPECT_LE(interval_sec, 101);
   }
 }
 
 // Check a Retry-After header that contains a delay in seconds.
 TEST_F(CaptivePortalServiceTest, CaptivePortalRetryAfterSeconds) {
   Initialize(true);
   set_initial_backoff_no_portal(base::TimeDelta::FromSeconds(100));
+  const char* retry_after = "HTTP/1.1 503 OK\nRetry-After: 101\n\n";
 
-  RunTest(RESULT_NO_RESPONSE,
-          net::OK,
-          503,
-          0,
-          "HTTP/1.1 503 OK\nRetry-After: 101\n\n");
+  // Check that Retry-After headers work both on the first request to return a
+  // result and on subsequent requests.
+  RunTest(RESULT_NO_RESPONSE, net::OK, 503, 0, retry_after);
+  RunTest(RESULT_NO_RESPONSE, net::OK, 503, 101, retry_after);
+  RunTest(RESULT_INTERNET_CONNECTED, net::OK, 204, 101, NULL);
 
-  // Run another captive portal check to make sure the time until the next check
-  // is as expected.
-  RunTest(RESULT_INTERNET_CONNECTED, net::OK, 204, 101, NULL);
-  EXPECT_EQ(base::TimeDelta::FromSeconds(100), GetTimeUntilNextRequest());
+  // Make sure that there's no effect on the next captive portal check after
+  // login.
+  EXPECT_EQ(base::TimeDelta::FromSeconds(0), GetTimeUntilNextRequest());
 }
 
 // Check that the RecheckPolicy is still respected on 503 responses with
 // Retry-After headers.
 TEST_F(CaptivePortalServiceTest, CaptivePortalRetryAfterSecondsTooShort) {
   Initialize(true);
   set_initial_backoff_no_portal(base::TimeDelta::FromSeconds(100));
+  const char* retry_after = "HTTP/1.1 503 OK\nRetry-After: 99\n\n";
 
-  RunTest(RESULT_NO_RESPONSE,
-          net::OK,
-          503,
-          0,
-          "HTTP/1.1 503 OK\nRetry-After: 99\n\n");
+  RunTest(RESULT_NO_RESPONSE, net::OK, 503, 0, retry_after);
+  // Normally would be no delay on the first check with a new result.
+  RunTest(RESULT_NO_RESPONSE, net::OK, 503, 99, retry_after);
   EXPECT_EQ(base::TimeDelta::FromSeconds(100), GetTimeUntilNextRequest());
 }
 
 // Check a Retry-After header that contains a date.
 TEST_F(CaptivePortalServiceTest, CaptivePortalRetryAfterDate) {
   Initialize(true);
   set_initial_backoff_no_portal(base::TimeDelta::FromSeconds(50));
 
   // base has a function to get a time in the right format from a string, but
   // not the other way around.
   base::Time start_time;
   ASSERT_TRUE(
       base::Time::FromString("Tue, 17 Apr 2012 18:02:00 GMT", &start_time));
   service()->set_time(start_time);
 
   RunTest(RESULT_NO_RESPONSE,
           net::OK,
           503,
           0,
           "HTTP/1.1 503 OK\nRetry-After: Tue, 17 Apr 2012 18:02:51 GMT\n\n");
   EXPECT_EQ(base::TimeDelta::FromSeconds(51), GetTimeUntilNextRequest());
 }
 
 // Check invalid Retry-After headers are ignored.
 TEST_F(CaptivePortalServiceTest, CaptivePortalRetryAfterInvalid) {
   Initialize(true);
   set_initial_backoff_no_portal(base::TimeDelta::FromSeconds(100));
+  const char* retry_after = "HTTP/1.1 503 OK\nRetry-After: Christmas\n\n";
 
-  RunTest(RESULT_NO_RESPONSE,
-          net::OK,
-          503,
-          0,
-          "HTTP/1.1 503 OK\nRetry-After: Christmas\n\n");
+  RunTest(RESULT_NO_RESPONSE, net::OK, 503, 0, retry_after);
+  RunTest(RESULT_NO_RESPONSE, net::OK, 503, 0, retry_after);
   EXPECT_EQ(base::TimeDelta::FromSeconds(100), GetTimeUntilNextRequest());
 }
 
 }  // namespace captive_portal