Basic Captive Portal detection functionality is extracted into

individual service, so, it can be used without CaptivePortalService.

BUG=115487
TEST=browser_tests:CaptivePortalBrowserTest


Review URL: https://chromiumcodereview.appspot.com/11198066

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

7 files changed, 343 insertions, 204 deletions

diff --git a/chrome/browser/captive_portal/captive_portal_browsertest.cc b/chrome/browser/captive_portal/captive_portal_browsertest.cc
index 6bcec16..f30b972 100644
--- a/chrome/browser/captive_portal/captive_portal_browsertest.cc
+++ b/chrome/browser/captive_portal/captive_portal_browsertest.cc
@@ -1000,7 +1000,7 @@ bool CaptivePortalBrowserTest::CheckPending(Browser* browser) {
   CaptivePortalService* captive_portal_service =
       CaptivePortalServiceFactory::GetForProfile(browser->profile());
 
-  return captive_portal_service->FetchingURL() ||
+  return captive_portal_service->DetectionInProgress() ||
       captive_portal_service->TimerRunning();
 }
 
diff --git a/chrome/browser/captive_portal/captive_portal_detector.cc b/chrome/browser/captive_portal/captive_portal_detector.cc
new file mode 100644
index 0000000..4c59437
--- /dev/null
+++ b/chrome/browser/captive_portal/captive_portal_detector.cc
@@ -0,0 +1,149 @@
+// 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_detector.h"
+
+#include "base/logging.h"
+#include "base/string_number_conversions.h"
+#include "chrome/browser/profiles/profile.h"
+#include "net/base/load_flags.h"
+#include "net/http/http_response_headers.h"
+#include "net/url_request/url_fetcher.h"
+#include "net/url_request/url_request_status.h"
+
+namespace captive_portal {
+
+const char CaptivePortalDetector::kDefaultURL[] =
+    "http://www.gstatic.com/generate_204";
+
+CaptivePortalDetector::CaptivePortalDetector(
+    const scoped_refptr<net::URLRequestContextGetter>& request_context)
+    : request_context_(request_context) {
+}
+
+CaptivePortalDetector::~CaptivePortalDetector() {
+}
+
+void CaptivePortalDetector::DetectCaptivePortal(
+    const GURL& url,
+    const DetectionCallback& detection_callback) {
+  DCHECK(CalledOnValidThread());
+  DCHECK(!FetchingURL());
+  DCHECK(detection_callback_.is_null());
+
+  detection_callback_ = detection_callback;
+
+  // The first 0 means this can use a TestURLFetcherFactory in unit tests.
+  url_fetcher_.reset(net::URLFetcher::Create(0,
+                                             url,
+                                             net::URLFetcher::GET,
+                                             this));
+  url_fetcher_->SetAutomaticallyRetryOn5xx(false);
+  url_fetcher_->SetRequestContext(request_context_);
+
+  // Can't safely use net::LOAD_DISABLE_CERT_REVOCATION_CHECKING here,
+  // since then the connection may be reused without checking the cert.
+  url_fetcher_->SetLoadFlags(
+      net::LOAD_BYPASS_CACHE |
+      net::LOAD_DO_NOT_PROMPT_FOR_LOGIN |
+      net::LOAD_DO_NOT_SAVE_COOKIES |
+      net::LOAD_DO_NOT_SEND_COOKIES |
+      net::LOAD_DO_NOT_SEND_AUTH_DATA);
+  url_fetcher_->Start();
+}
+
+void CaptivePortalDetector::Cancel() {
+  url_fetcher_.reset();
+  detection_callback_.Reset();
+}
+
+void CaptivePortalDetector::OnURLFetchComplete(const net::URLFetcher* source) {
+  DCHECK(CalledOnValidThread());
+  DCHECK(FetchingURL());
+  DCHECK_EQ(url_fetcher_.get(), source);
+  DCHECK(!detection_callback_.is_null());
+
+  Results results;
+  GetCaptivePortalResultFromResponse(url_fetcher_.get(), &results);
+  DetectionCallback callback = detection_callback_;
+  url_fetcher_.reset();
+  detection_callback_.Reset();
+  callback.Run(results);
+}
+
+// Takes a net::URLFetcher that has finished trying to retrieve the test
+// URL, and returns a CaptivePortalService::Result based on its result.
+void CaptivePortalDetector::GetCaptivePortalResultFromResponse(
+    const net::URLFetcher* url_fetcher,
+    Results* results) const {
+  DCHECK(results);
+  DCHECK(!url_fetcher->GetStatus().is_io_pending());
+
+  results->retry_after_delta = base::TimeDelta();
+  results->result = RESULT_NO_RESPONSE;
+
+  // If there's a network error of some sort when fetching a file via HTTP,
+  // there may be a networking problem, rather than a captive portal.
+  // TODO(mmenke):  Consider special handling for redirects that end up at
+  //                errors, especially SSL certificate errors.
+  if (url_fetcher->GetStatus().status() != net::URLRequestStatus::SUCCESS)
+    return;
+
+  // In the case of 503 errors, look for the Retry-After header.
+  int response_code = url_fetcher->GetResponseCode();
+  if (response_code == 503) {
+    net::HttpResponseHeaders* headers = url_fetcher->GetResponseHeaders();
+    std::string retry_after_string;
+
+    // If there's no Retry-After header, nothing else to do.
+    if (!headers->EnumerateHeader(NULL, "Retry-After", &retry_after_string))
+      return;
+
+    // Otherwise, try parsing it as an integer (seconds) or as an HTTP date.
+    int seconds;
+    base::Time full_date;
+    if (base::StringToInt(retry_after_string, &seconds)) {
+      results->retry_after_delta = base::TimeDelta::FromSeconds(seconds);
+    } else if (headers->GetTimeValuedHeader("Retry-After", &full_date)) {
+      base::Time now = GetCurrentTime();
+      if (full_date > now)
+        results->retry_after_delta = full_date - now;
+    }
+    return;
+  }
+
+  // A 511 response (Network Authentication Required) means that the user needs
+  // to login to whatever server issued the response.
+  // See:  http://tools.ietf.org/html/rfc6585
+  if (response_code == 511) {
+    results->result = RESULT_BEHIND_CAPTIVE_PORTAL;
+    return;
+  }
+
+  // Other non-2xx/3xx HTTP responses may indicate server errors.
+  if (response_code >= 400 || response_code < 200)
+    return;
+
+  // A 204 response code indicates there's no captive portal.
+  if (response_code == 204) {
+    results->result = RESULT_INTERNET_CONNECTED;
+    return;
+  }
+
+  // Otherwise, assume it's a captive portal.
+  results->result = RESULT_BEHIND_CAPTIVE_PORTAL;
+}
+
+base::Time CaptivePortalDetector::GetCurrentTime() const {
+  if (time_for_testing_.is_null())
+    return base::Time::Now();
+  else
+    return time_for_testing_;
+}
+
+bool CaptivePortalDetector::FetchingURL() const {
+  return url_fetcher_.get() != NULL;
+}
+
+}  // namespace captive_portal
diff --git a/chrome/browser/captive_portal/captive_portal_detector.h b/chrome/browser/captive_portal/captive_portal_detector.h
new file mode 100644
index 0000000..e1cc1d2
--- /dev/null
+++ b/chrome/browser/captive_portal/captive_portal_detector.h
@@ -0,0 +1,112 @@
+// 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.
+
+#ifndef CHROME_BROWSER_CAPTIVE_PORTAL_CAPTIVE_PORTAL_DETECTOR_H_
+#define CHROME_BROWSER_CAPTIVE_PORTAL_CAPTIVE_PORTAL_DETECTOR_H_
+
+#include "base/basictypes.h"
+#include "base/callback.h"
+#include "base/compiler_specific.h"
+#include "base/memory/ref_counted.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/threading/non_thread_safe.h"
+#include "base/time.h"
+#include "net/url_request/url_fetcher_delegate.h"
+#include "net/url_request/url_request_context_getter.h"
+
+class GURL;
+
+namespace net {
+class URLFetcher;
+}
+
+namespace captive_portal {
+
+// Possible results of an attempt to detect a captive portal.
+enum Result {
+  // There's a confirmed connection to the Internet.
+  RESULT_INTERNET_CONNECTED,
+  // The URL request received a network or HTTP error, or a non-HTTP response.
+  RESULT_NO_RESPONSE,
+  // The URL request apparently encountered a captive portal.  It received a
+  // a valid HTTP response with a 2xx other than 204, 3xx, or 511 status code.
+  RESULT_BEHIND_CAPTIVE_PORTAL,
+  RESULT_COUNT
+};
+
+class CaptivePortalDetector : public net::URLFetcherDelegate,
+                              public base::NonThreadSafe {
+ public:
+  struct Results {
+    Result result;
+    base::TimeDelta retry_after_delta;
+  };
+
+  typedef base::Callback<void(const Results& results)> DetectionCallback;
+
+  // The test URL.  When connected to the Internet, it should return a
+  // blank page with a 204 status code.  When behind a captive portal,
+  // requests for this URL should get an HTTP redirect or a login
+  // page.  When neither is true, no server should respond to requests
+  // for this URL.
+  static const char kDefaultURL[];
+
+  explicit CaptivePortalDetector(
+      const scoped_refptr<net::URLRequestContextGetter>& request_context);
+  virtual ~CaptivePortalDetector();
+
+  // Triggers a check for a captive portal. After completion, runs the
+  // |callback|.
+  void DetectCaptivePortal(const GURL& url, const DetectionCallback& callback);
+
+  // Cancels captive portal check.
+  void Cancel();
+
+ private:
+  friend class CaptivePortalServiceTest;
+
+  // net::URLFetcherDelegate:
+  virtual void OnURLFetchComplete(const net::URLFetcher* source) OVERRIDE;
+
+  // Takes a net::URLFetcher that has finished trying to retrieve the
+  // test URL, and fills a Results struct based on its result.  If the
+  // response is a 503 with a Retry-After header, |retry_after| field
+  // of |results| is populated accordingly.  Otherwise, it's set to
+  // base::TimeDelta().
+  void GetCaptivePortalResultFromResponse(const net::URLFetcher* url_fetcher,
+                                          Results* results) const;
+
+  // Returns the current time. Used only when determining time until a
+  // Retry-After date.
+  base::Time GetCurrentTime() const;
+
+  // Returns true if a captive portal check is currently running.
+  bool FetchingURL() const;
+
+  // Sets current test time. Used by unit tests.
+  void set_time_for_testing(const base::Time& time) {
+    time_for_testing_ = time;
+  }
+
+  // Advances current test time. Used by unit tests.
+  void advance_time_for_testing(const base::TimeDelta& delta) {
+    time_for_testing_ += delta;
+  }
+
+  // URL request context.
+  scoped_refptr<net::URLRequestContextGetter> request_context_;
+
+  DetectionCallback detection_callback_;
+
+  scoped_ptr<net::URLFetcher> url_fetcher_;
+
+  // Test time used by unit tests.
+  base::Time time_for_testing_;
+
+  DISALLOW_COPY_AND_ASSIGN(CaptivePortalDetector);
+};
+
+}  // namespace captive_portal
+
+#endif  // CHROME_BROWSER_CAPTIVE_PORTAL_CAPTIVE_PORTAL_DETECTOR_H_
diff --git a/chrome/browser/captive_portal/captive_portal_service.cc b/chrome/browser/captive_portal/captive_portal_service.cc
index 8d8d59d..d6a013c 100644
--- a/chrome/browser/captive_portal/captive_portal_service.cc
+++ b/chrome/browser/captive_portal/captive_portal_service.cc
@@ -5,21 +5,15 @@
 #include "chrome/browser/captive_portal/captive_portal_service.h"
 
 #include "base/bind.h"
-#include "base/command_line.h"
+#include "base/bind_helpers.h"
 #include "base/logging.h"
 #include "base/message_loop.h"
 #include "base/metrics/histogram.h"
-#include "base/rand_util.h"
-#include "base/string_number_conversions.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/chrome_notification_types.h"
 #include "chrome/common/pref_names.h"
 #include "content/public/browser/notification_service.h"
-#include "net/base/load_flags.h"
-#include "net/http/http_response_headers.h"
-#include "net/url_request/url_fetcher.h"
-#include "net/url_request/url_request_status.h"
 
 #if defined(OS_MACOSX)
 #include "base/mac/mac_util.h"
@@ -33,14 +27,6 @@ namespace captive_portal {
 
 namespace {
 
-// The test URL.  When connected to the Internet, it should return a blank page
-// with a 204 status code.  When behind a captive portal, requests for this
-// URL should get an HTTP redirect or a login page.  When neither is true,
-// no server should respond to requests for this URL.
-// TODO(mmenke):  Look into getting another domain, for better load management.
-//                Need a cookieless domain, so can't use clients*.google.com.
-const char* const kDefaultTestURL = "http://www.gstatic.com/generate_204";
-
 // Used for histograms.
 const char* const kCaptivePortalResultNames[] = {
   "InternetConnected",
@@ -166,10 +152,11 @@ CaptivePortalService::RecheckPolicy::RecheckPolicy()
 CaptivePortalService::CaptivePortalService(Profile* profile)
     : profile_(profile),
       state_(STATE_IDLE),
+      captive_portal_detector_(profile->GetRequestContext()),
       enabled_(false),
       last_detection_result_(RESULT_INTERNET_CONNECTED),
       num_checks_with_same_result_(0),
-      test_url_(kDefaultTestURL) {
+      test_url_(CaptivePortalDetector::kDefaultURL) {
   // The order matters here:
   // |resolve_errors_with_web_service_| must be initialized and |backoff_entry_|
   // created before the call to UpdateEnabledState.
@@ -206,7 +193,6 @@ void CaptivePortalService::DetectCaptivePortal() {
 void CaptivePortalService::DetectCaptivePortalInternal() {
   DCHECK(CalledOnValidThread());
   DCHECK(state_ == STATE_TIMER_RUNNING || state_ == STATE_IDLE);
-  DCHECK(!FetchingURL());
   DCHECK(!TimerRunning());
 
   state_ = STATE_CHECKING_FOR_PORTAL;
@@ -220,42 +206,26 @@ void CaptivePortalService::DetectCaptivePortalInternal() {
     return;
   }
 
-  // The first 0 means this can use a TestURLFetcherFactory in unit tests.
-  url_fetcher_.reset(net::URLFetcher::Create(0,
-                                             test_url_,
-                                             net::URLFetcher::GET,
-                                             this));
-  url_fetcher_->SetAutomaticallyRetryOn5xx(false);
-  url_fetcher_->SetRequestContext(profile_->GetRequestContext());
-  // Can't safely use net::LOAD_DISABLE_CERT_REVOCATION_CHECKING here,
-  // since then the connection may be reused without checking the cert.
-  url_fetcher_->SetLoadFlags(
-      net::LOAD_BYPASS_CACHE |
-      net::LOAD_DO_NOT_PROMPT_FOR_LOGIN |
-      net::LOAD_DO_NOT_SAVE_COOKIES |
-      net::LOAD_DO_NOT_SEND_COOKIES |
-      net::LOAD_DO_NOT_SEND_AUTH_DATA);
-  url_fetcher_->Start();
+  captive_portal_detector_.DetectCaptivePortal(
+      test_url_, base::Bind(
+          &CaptivePortalService::OnPortalDetectionCompleted,
+          base::Unretained(this)));
 }
 
-void CaptivePortalService::OnURLFetchComplete(const net::URLFetcher* source) {
+void CaptivePortalService::OnPortalDetectionCompleted(
+    const CaptivePortalDetector::Results& results) {
   DCHECK(CalledOnValidThread());
   DCHECK_EQ(STATE_CHECKING_FOR_PORTAL, state_);
-  DCHECK(FetchingURL());
   DCHECK(!TimerRunning());
-  DCHECK_EQ(url_fetcher_.get(), source);
   DCHECK(enabled_);
 
+  Result result = results.result;
+  const base::TimeDelta& retry_after_delta = results.retry_after_delta;
   base::TimeTicks now = GetCurrentTimeTicks();
 
-  base::TimeDelta retry_after_delta;
-  Result new_result = GetCaptivePortalResultFromResponse(source,
-                                                         &retry_after_delta);
-  url_fetcher_.reset();
-
   // Record histograms.
   UMA_HISTOGRAM_ENUMERATION("CaptivePortal.DetectResult",
-                            new_result,
+                            result,
                             RESULT_COUNT);
 
   // If this isn't the first captive portal result, record stats.
@@ -263,7 +233,7 @@ void CaptivePortalService::OnURLFetchComplete(const net::URLFetcher* source) {
     UMA_HISTOGRAM_LONG_TIMES("CaptivePortal.TimeBetweenChecks",
                              now - last_check_time_);
 
-    if (last_detection_result_ != new_result) {
+    if (last_detection_result_ != result) {
       // If the last result was different from the result of the latest test,
       // record histograms about the previous period over which the result was
       // the same.
@@ -273,13 +243,13 @@ void CaptivePortalService::OnURLFetchComplete(const net::URLFetcher* source) {
     }
   }
 
-  if (last_check_time_.is_null() || new_result != last_detection_result_) {
+  if (last_check_time_.is_null() || result != last_detection_result_) {
     first_check_time_with_same_result_ = now;
     num_checks_with_same_result_ = 1;
 
     // Reset the backoff entry both to update the default time and clear
     // previous failures.
-    ResetBackoffEntry(new_result);
+    ResetBackoffEntry(result);
 
     backoff_entry_->SetCustomReleaseTime(now + retry_after_delta);
     // The BackoffEntry is not informed of this request, so there's no delay
@@ -298,7 +268,7 @@ void CaptivePortalService::OnURLFetchComplete(const net::URLFetcher* source) {
 
   last_check_time_ = now;
 
-  OnResult(new_result);
+  OnResult(result);
 }
 
 void CaptivePortalService::Observe(
@@ -372,10 +342,10 @@ void CaptivePortalService::UpdateEnabledState() {
   ResetBackoffEntry(last_detection_result_);
 
   if (state_ == STATE_CHECKING_FOR_PORTAL || state_ == STATE_TIMER_RUNNING) {
-    // If a captive portal check was running or pending, stop the check or the
-    // timer.
-    url_fetcher_.reset();
+    // If a captive portal check was running or pending, cancel check
+    // and the timer.
     check_captive_portal_timer_.Stop();
+    captive_portal_detector_.Cancel();
     state_ = STATE_IDLE;
 
     // Since a captive portal request was queued or running, something may be
@@ -384,74 +354,15 @@ void CaptivePortalService::UpdateEnabledState() {
   }
 }
 
-// Takes a net::URLFetcher that has finished trying to retrieve the test
-// URL, and returns a CaptivePortalService::Result based on its result.
-Result CaptivePortalService::GetCaptivePortalResultFromResponse(
-    const net::URLFetcher* url_fetcher,
-    base::TimeDelta* retry_after) const {
-  DCHECK(retry_after);
-  DCHECK(!url_fetcher->GetStatus().is_io_pending());
-
-  *retry_after = base::TimeDelta();
-
-  // If there's a network error of some sort when fetching a file via HTTP,
-  // there may be a networking problem, rather than a captive portal.
-  // TODO(mmenke):  Consider special handling for redirects that end up at
-  //                errors, especially SSL certificate errors.
-  if (url_fetcher->GetStatus().status() != net::URLRequestStatus::SUCCESS)
-    return RESULT_NO_RESPONSE;
-
-  // In the case of 503 errors, look for the Retry-After header.
-  int response_code = url_fetcher->GetResponseCode();
-  if (response_code == 503) {
-    net::HttpResponseHeaders* headers = url_fetcher->GetResponseHeaders();
-    std::string retry_after_string;
-
-    // If there's no Retry-After header, nothing else to do.
-    if (!headers->EnumerateHeader(NULL, "Retry-After", &retry_after_string))
-      return RESULT_NO_RESPONSE;
-
-    // Otherwise, try parsing it as an integer (seconds) or as an HTTP date.
-    int seconds;
-    base::Time full_date;
-    if (base::StringToInt(retry_after_string, &seconds)) {
-      *retry_after = base::TimeDelta::FromSeconds(seconds);
-    } else if (headers->GetTimeValuedHeader("Retry-After", &full_date)) {
-      base::Time now = GetCurrentTime();
-      if (full_date > now)
-        *retry_after = full_date - now;
-    }
-    return RESULT_NO_RESPONSE;
-  }
-
-  // A 511 response (Network Authentication Required) means that the user needs
-  // to login to whatever server issued the response.
-  // See:  http://tools.ietf.org/html/rfc6585
-  if (response_code == 511)
-    return RESULT_BEHIND_CAPTIVE_PORTAL;
-
-  // Other non-2xx/3xx HTTP responses may indicate server errors.
-  if (response_code >= 400 || response_code < 200)
-    return RESULT_NO_RESPONSE;
-
-  // A 204 response code indicates there's no captive portal.
-  if (response_code == 204)
-    return RESULT_INTERNET_CONNECTED;
-
-  // Otherwise, assume it's a captive portal.
-  return RESULT_BEHIND_CAPTIVE_PORTAL;
-}
-
-base::Time CaptivePortalService::GetCurrentTime() const {
-  return base::Time::Now();
-}
-
 base::TimeTicks CaptivePortalService::GetCurrentTimeTicks() const {
-  return base::TimeTicks::Now();
+  if (time_ticks_for_testing_.is_null())
+    return base::TimeTicks::Now();
+  else
+    return time_ticks_for_testing_;
 }
 
-bool CaptivePortalService::FetchingURL() const {
-  return url_fetcher_.get() != NULL;
+bool CaptivePortalService::DetectionInProgress() const {
+  return state_ == STATE_CHECKING_FOR_PORTAL;
 }
 
 bool CaptivePortalService::TimerRunning() const {
diff --git a/chrome/browser/captive_portal/captive_portal_service.h b/chrome/browser/captive_portal/captive_portal_service.h
index 6ad53b2..82b3d71 100644
--- a/chrome/browser/captive_portal/captive_portal_service.h
+++ b/chrome/browser/captive_portal/captive_portal_service.h
@@ -11,32 +11,16 @@
 #include "base/time.h"
 #include "base/timer.h"
 #include "chrome/browser/api/prefs/pref_member.h"
+#include "chrome/browser/captive_portal/captive_portal_detector.h"
 #include "chrome/browser/profiles/profile_keyed_service.h"
 #include "content/public/browser/notification_observer.h"
 #include "googleurl/src/gurl.h"
 #include "net/base/backoff_entry.h"
-#include "net/url_request/url_fetcher_delegate.h"
 
 class Profile;
 
-namespace net {
-class URLFetcher;
-}
-
 namespace captive_portal {
 
-// Possible results of an attempt to detect a captive portal.
-enum Result {
-  // There's a confirmed connection to the Internet.
-  RESULT_INTERNET_CONNECTED,
-  // The URL request received a network or HTTP error, or a non-HTTP response.
-  RESULT_NO_RESPONSE,
-  // The URL request apparently encountered a captive portal.  It received a
-  // a valid HTTP response with a 2xx, 3xx, or 511 status code, other than 204.
-  RESULT_BEHIND_CAPTIVE_PORTAL,
-  RESULT_COUNT
-};
-
 // Service that checks for captive portals when queried, and sends a
 // NOTIFICATION_CAPTIVE_PORTAL_CHECK_RESULT with the Profile as the source and
 // a CaptivePortalService::Results as the details.
@@ -45,7 +29,6 @@ enum Result {
 // be accessed on the UI thread.
 // Design doc: https://docs.google.com/document/d/1k-gP2sswzYNvryu9NcgN7q5XrsMlUdlUdoW9WRaEmfM/edit
 class CaptivePortalService : public ProfileKeyedService,
-                             public net::URLFetcherDelegate,
                              public content::NotificationObserver,
                              public base::NonThreadSafe {
  public:
@@ -129,8 +112,9 @@ class CaptivePortalService : public ProfileKeyedService,
   // is disabled, just acts like there's an Internet connection.
   void DetectCaptivePortalInternal();
 
-  // net::URLFetcherDelegate:
-  virtual void OnURLFetchComplete(const net::URLFetcher* source) OVERRIDE;
+  // Called by CaptivePortalDetector when detection completes.
+  void OnPortalDetectionCompleted(
+      const CaptivePortalDetector::Results& results);
 
   // content::NotificationObserver:
   virtual void Observe(
@@ -157,22 +141,10 @@ class CaptivePortalService : public ProfileKeyedService,
   //               Android, since it lacks the Browser class.
   void UpdateEnabledState();
 
-  // Takes a net::URLFetcher that has finished trying to retrieve the test
-  // URL, and returns a captive_portal::Result based on its result.
-  // If the response is a 503 with a Retry-After header, |retry_after| is
-  // populated accordingly.  Otherwise, it's set to base::TimeDelta().
-  Result GetCaptivePortalResultFromResponse(const net::URLFetcher* url_fetcher,
-                                            base::TimeDelta* retry_after) const;
+  // Returns the current TimeTicks.
+  base::TimeTicks GetCurrentTimeTicks() const;
 
-  // Returns the current time.  Used only when determining time until a
-  // Retry-After date.  Overridden by unit tests.
-  virtual base::Time GetCurrentTime() const;
-
-  // Returns the current TimeTicks.  Overridden by unit tests.
-  virtual base::TimeTicks GetCurrentTimeTicks() const;
-
-  // Returns true if a captive portal check is currently running.
-  bool FetchingURL() const;
+  bool DetectionInProgress() const;
 
   // Returns true if the timer to try and detect a captive portal is running.
   bool TimerRunning() const;
@@ -183,11 +155,24 @@ class CaptivePortalService : public ProfileKeyedService,
 
   void set_test_url(const GURL& test_url) { test_url_ = test_url; }
 
+  // Sets current test time ticks. Used by unit tests.
+  void set_time_ticks_for_testing(const base::TimeTicks& time_ticks) {
+    time_ticks_for_testing_ = time_ticks;
+  }
+
+  // Advances current test time ticks. Used by unit tests.
+  void advance_time_ticks_for_testing(const base::TimeDelta& delta) {
+    time_ticks_for_testing_ += delta;
+  }
+
   // The profile that owns this CaptivePortalService.
   Profile* profile_;
 
   State state_;
 
+  // Detector for checking active network for a portal state.
+  CaptivePortalDetector captive_portal_detector_;
+
   // True if the service is enabled.  When not enabled, all checks will return
   // RESULT_INTERNET_CONNECTED.
   bool enabled_;
@@ -213,8 +198,6 @@ class CaptivePortalService : public ProfileKeyedService,
   // is considered a "failure", to trigger throttling.
   scoped_ptr<net::BackoffEntry> backoff_entry_;
 
-  scoped_ptr<net::URLFetcher> url_fetcher_;
-
   // URL that returns a 204 response code when connected to the Internet.
   GURL test_url_;
 
@@ -227,6 +210,9 @@ class CaptivePortalService : public ProfileKeyedService,
 
   static TestingState testing_state_;
 
+  // Test time ticks used by unit tests.
+  base::TimeTicks time_ticks_for_testing_;
+
   DISALLOW_COPY_AND_ASSIGN(CaptivePortalService);
 };
 
diff --git a/chrome/browser/captive_portal/captive_portal_service_unittest.cc b/chrome/browser/captive_portal/captive_portal_service_unittest.cc
index f6714ea..9a88335 100644
--- a/chrome/browser/captive_portal/captive_portal_service_unittest.cc
+++ b/chrome/browser/captive_portal/captive_portal_service_unittest.cc
@@ -39,47 +39,7 @@ scoped_refptr<net::HttpResponseHeaders> CreateResponseHeaders(
   return new net::HttpResponseHeaders(raw_headers);
 }
 
-// Allows setting the time, for testing timers.
-class TestCaptivePortalService : public CaptivePortalService {
- public:
-  // The initial time does not matter, except it must not be NULL.
-  explicit TestCaptivePortalService(Profile* profile)
-      : CaptivePortalService(profile),
-        time_ticks_(base::TimeTicks::Now()),
-        time_(base::Time::Now()) {
-  }
-
-  virtual ~TestCaptivePortalService() {}
-
-  void AdvanceTime(base::TimeDelta delta) {
-    time_ticks_ += delta;
-    time_ += delta;
-  }
-
-  // CaptivePortalService:
-  virtual base::TimeTicks GetCurrentTimeTicks() const OVERRIDE {
-    return time_ticks_;
-  }
-
-  virtual base::Time GetCurrentTime() const OVERRIDE {
-    return time_;
-  }
-
-  void set_time(base::Time time) {
-    time_= time;
-  }
-
- private:
-  base::TimeTicks time_ticks_;
-
-  // Not necessarily consistent with |time_ticks_|.  Used solely with
-  // Retry-After dates.
-  base::Time time_;
-
-  DISALLOW_COPY_AND_ASSIGN(TestCaptivePortalService);
-};
-
-// An observer watches the CaptivePortalService.  It tracks the last
+// An observer watches the CaptivePortalDetector.  It tracks the last
 // received result and the total number of received results.
 class CaptivePortalObserver : public content::NotificationObserver {
  public:
@@ -149,8 +109,7 @@ class CaptivePortalServiceTest : public testing::Test {
     CaptivePortalService::set_state_for_testing(testing_state);
 
     profile_.reset(new TestingProfile());
-    service_.reset(new TestCaptivePortalService(profile_.get()));
-    scoped_ptr<TestCaptivePortalService> service_;
+    service_.reset(CreateTestCaptivePortalService(profile_.get()));
 
     // Use no delays for most tests.
     set_initial_backoff_no_portal(base::TimeDelta());
@@ -182,7 +141,7 @@ class CaptivePortalServiceTest : public testing::Test {
 
   // Calls the corresponding CaptivePortalService function.
   void OnURLFetchComplete(net::URLFetcher* fetcher) {
-    service()->OnURLFetchComplete(fetcher);
+    service()->captive_portal_detector_.OnURLFetchComplete(fetcher);
   }
 
   // Triggers a captive portal check, then simulates the URL request
@@ -207,7 +166,7 @@ class CaptivePortalServiceTest : public testing::Test {
     ASSERT_EQ(CaptivePortalService::STATE_IDLE, service()->state());
     ASSERT_EQ(expected_delay, GetTimeUntilNextRequest());
 
-    service()->AdvanceTime(expected_delay);
+    AdvanceTime(expected_delay);
     ASSERT_EQ(base::TimeDelta(), GetTimeUntilNextRequest());
 
     CaptivePortalObserver observer(profile(), service());
@@ -256,7 +215,7 @@ class CaptivePortalServiceTest : public testing::Test {
     ASSERT_EQ(CaptivePortalService::STATE_IDLE, service()->state());
     ASSERT_EQ(expected_delay, GetTimeUntilNextRequest());
 
-    service()->AdvanceTime(expected_delay);
+    AdvanceTime(expected_delay);
     ASSERT_EQ(base::TimeDelta(), GetTimeUntilNextRequest());
 
     CaptivePortalObserver observer(profile(), service());
@@ -287,8 +246,27 @@ class CaptivePortalServiceTest : public testing::Test {
     RunTest(expected_result, net_error, status_code, 1600, NULL);
   }
 
+  CaptivePortalService* CreateTestCaptivePortalService(Profile* profile) {
+    scoped_ptr<CaptivePortalService> service(new CaptivePortalService(profile));
+    service->set_time_ticks_for_testing(base::TimeTicks::Now());
+    service->captive_portal_detector_.set_time_for_testing(base::Time::Now());
+    return service.release();
+  }
+
+  // Changes test time for the service and service's captive portal
+  // detector.
+  void AdvanceTime(const base::TimeDelta& delta) {
+    service()->advance_time_ticks_for_testing(delta);
+    service()->captive_portal_detector_.advance_time_for_testing(delta);
+  }
+
+  // Sets test time for service's captive portal detector.
+  void SetTime(const base::Time& time) {
+    service()->captive_portal_detector_.set_time_for_testing(time);
+  }
+
   bool FetchingURL() {
-    return service()->FetchingURL();
+    return service()->captive_portal_detector_.FetchingURL();
   }
 
   bool TimerRunning() {
@@ -331,7 +309,7 @@ class CaptivePortalServiceTest : public testing::Test {
 
   TestingProfile* profile() { return profile_.get(); }
 
-  TestCaptivePortalService* service() { return service_.get(); }
+  CaptivePortalService* service() { return service_.get(); }
 
  private:
   // Stores the initial CaptivePortalService::TestingState so it can be restored
@@ -342,7 +320,7 @@ class CaptivePortalServiceTest : public testing::Test {
 
   // Note that the construction order of these matters.
   scoped_ptr<TestingProfile> profile_;
-  scoped_ptr<TestCaptivePortalService> service_;
+  scoped_ptr<CaptivePortalService> service_;
 };
 
 // Test that the CaptivePortalService returns the expected result codes in
@@ -370,8 +348,9 @@ TEST_F(CaptivePortalServiceTest, CaptivePortalResultCodes) {
 TEST_F(CaptivePortalServiceTest, CaptivePortalTwoProfiles) {
   Initialize(CaptivePortalService::SKIP_OS_CHECK_FOR_TESTING);
   TestingProfile profile2;
-  TestCaptivePortalService service2(&profile2);
-  CaptivePortalObserver observer2(&profile2, &service2);
+  scoped_ptr<CaptivePortalService> service2(
+      CreateTestCaptivePortalService(&profile2));
+  CaptivePortalObserver observer2(&profile2, service2.get());
 
   RunTest(RESULT_INTERNET_CONNECTED, net::OK, 204, 0, NULL);
   EXPECT_EQ(0, observer2.num_results_received());
@@ -598,7 +577,7 @@ TEST_F(CaptivePortalServiceTest, CaptivePortalRetryAfterDate) {
   base::Time start_time;
   ASSERT_TRUE(
       base::Time::FromString("Tue, 17 Apr 2012 18:02:00 GMT", &start_time));
-  service()->set_time(start_time);
+  SetTime(start_time);
 
   RunTest(RESULT_NO_RESPONSE,
           net::OK,
diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi
index f2d9129..a090c42 100644
--- a/chrome/chrome_browser.gypi
+++ b/chrome/chrome_browser.gypi
@@ -419,6 +419,8 @@
         'browser/feedback/feedback_data.h',
         'browser/feedback/feedback_util.cc',
         'browser/feedback/feedback_util.h',
+        'browser/captive_portal/captive_portal_detector.cc',
+        'browser/captive_portal/captive_portal_detector.h',
         'browser/captive_portal/captive_portal_login_detector.cc',
         'browser/captive_portal/captive_portal_login_detector.h',
         'browser/captive_portal/captive_portal_service.cc',