Cleanup: use MockGeolocationArbitrator to test GeolocationProvider

This patch introduces a MockGeolocationArbitrator class for use
with the GeolocationProvider unittests. This avoids having to
instantiate dependencies of the GeolocationArbitrator to test the
GeolocationProvider.

This will make it easier to add a unit test for 
https://codereview.chromium.org/11312210/

TEST=Existing
BUG=

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

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

12 files changed, 277 insertions, 223 deletions

diff --git a/content/browser/geolocation/geolocation_provider.cc b/content/browser/geolocation/geolocation_provider.cc
index 6c07eec..30218af 100644
--- a/content/browser/geolocation/geolocation_provider.cc
+++ b/content/browser/geolocation/geolocation_provider.cc
@@ -11,7 +11,7 @@
 #include "base/logging.h"
 #include "base/memory/singleton.h"
 #include "base/message_loop.h"
-#include "content/browser/geolocation/location_arbitrator.h"
+#include "content/browser/geolocation/location_arbitrator_impl.h"
 #include "content/public/browser/browser_thread.h"
 
 namespace content {
@@ -193,7 +193,7 @@ void GeolocationProvider::CleanUp() {
 }
 
 GeolocationArbitrator* GeolocationProvider::CreateArbitrator() {
-  return new GeolocationArbitrator(this);
+  return new GeolocationArbitratorImpl(this);
 }
 
 }  // namespace content
diff --git a/content/browser/geolocation/geolocation_provider_unittest.cc b/content/browser/geolocation/geolocation_provider_unittest.cc
index f78bbd8..99649ad 100644
--- a/content/browser/geolocation/geolocation_provider_unittest.cc
+++ b/content/browser/geolocation/geolocation_provider_unittest.cc
@@ -8,12 +8,10 @@
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop.h"
 #include "base/string16.h"
-#include "base/synchronization/waitable_event.h"
 #include "base/time.h"
 #include "content/browser/geolocation/geolocation_provider.h"
-#include "content/browser/geolocation/location_arbitrator.h"
 #include "content/browser/geolocation/location_provider.h"
-#include "content/browser/geolocation/mock_location_provider.h"
+#include "content/browser/geolocation/mock_location_arbitrator.h"
 #include "content/public/browser/access_token_store.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/test/test_browser_thread.h"
@@ -30,89 +28,26 @@ namespace content {
 
 class LocationProviderForTestArbitrator : public GeolocationProvider {
  public:
-  explicit LocationProviderForTestArbitrator(base::WaitableEvent* event)
-      : event_(event) {
-  }
-
+  LocationProviderForTestArbitrator() : mock_arbitrator_(NULL) {}
   virtual ~LocationProviderForTestArbitrator() {}
 
- protected:
-  // GeolocationProvider implementation:
-  virtual GeolocationArbitrator* CreateArbitrator() OVERRIDE;
-
- private:
-  base::WaitableEvent* event_;
-};
-
-class StartStopMockLocationProvider : public MockLocationProvider {
- public:
-  StartStopMockLocationProvider(base::WaitableEvent* event) :
-      MockLocationProvider(&instance_),
-      event_(event) {
+  // Only valid for use on the geolocation thread.
+  MockGeolocationArbitrator* mock_arbitrator() const {
+    return mock_arbitrator_;
   }
 
-  virtual ~StartStopMockLocationProvider() {
-    event_->Signal();
-  }
-
- private:
-  base::WaitableEvent* event_;
-};
-
-// The AccessTokenStore will be accessed from the geolocation helper thread. The
-// existing FakeAccessTokenStore class cannot be used here because it is based
-// on gmock and gmock is not thread-safe on Windows.
-// See: http://code.google.com/p/googlemock/issues/detail?id=156
-class TestingAccessTokenStore : public AccessTokenStore {
- public:
-  TestingAccessTokenStore(base::WaitableEvent* event) : event_(event) {}
-
-  virtual void LoadAccessTokens(const LoadAccessTokensCallbackType& callback)
-        OVERRIDE {
-    callback.Run(AccessTokenSet(), NULL);
-    event_->Signal();
-  }
-
-  virtual void SaveAccessToken(const GURL& server_url,
-                               const string16& access_token) OVERRIDE {}
-
  protected:
-  virtual ~TestingAccessTokenStore() {}
-
- private:
-  base::WaitableEvent* event_;
-};
-
-class TestGeolocationArbitrator : public GeolocationArbitrator {
- public:
-  TestGeolocationArbitrator(GeolocationObserver* observer,
-                            base::WaitableEvent* event)
-      : GeolocationArbitrator(observer),
-        event_(event) {
-  }
-
-  virtual AccessTokenStore* NewAccessTokenStore() OVERRIDE {
-    return new TestingAccessTokenStore(event_);
-  }
-
-  virtual LocationProviderBase* NewNetworkLocationProvider(
-      AccessTokenStore* access_token_store,
-      net::URLRequestContextGetter* context,
-      const GURL& url,
-      const string16& access_token) OVERRIDE {
-    return new StartStopMockLocationProvider(event_);
-  }
-
-  virtual LocationProviderBase* NewSystemLocationProvider() OVERRIDE  {
-    return NULL;
-  }
+  // GeolocationProvider implementation:
+  virtual GeolocationArbitrator* CreateArbitrator() OVERRIDE;
 
  private:
-  base::WaitableEvent* event_;
+  MockGeolocationArbitrator* mock_arbitrator_;
 };
 
 GeolocationArbitrator* LocationProviderForTestArbitrator::CreateArbitrator() {
-  return new TestGeolocationArbitrator(this, event_);
+  DCHECK(mock_arbitrator_ == NULL);
+  mock_arbitrator_ = new MockGeolocationArbitrator;
+  return mock_arbitrator_;
 }
 
 class NullGeolocationObserver : public GeolocationObserver {
@@ -175,83 +110,94 @@ class GeolocationProviderTest : public testing::Test {
   GeolocationProviderTest()
       : message_loop_(),
         io_thread_(BrowserThread::IO, &message_loop_),
-        event_(false, false),
-        provider_(new LocationProviderForTestArbitrator(&event_)) {
+        provider_(new LocationProviderForTestArbitrator) {
   }
 
-  ~GeolocationProviderTest() {
-  }
+  ~GeolocationProviderTest() {}
 
-  void WaitAndReset() {
-    event_.Wait();
-    event_.Reset();
-  }
+  LocationProviderForTestArbitrator* provider() { return provider_.get(); }
+
+  // Called on test thread.
+  bool ProvidersStarted();
+
+ private:
+  // Called on provider thread.
+  void GetProvidersStarted(bool* started);
 
   MessageLoop message_loop_;
   TestBrowserThread io_thread_;
-
-  base::WaitableEvent event_;
   scoped_ptr<LocationProviderForTestArbitrator> provider_;
 };
 
+
+bool GeolocationProviderTest::ProvidersStarted() {
+  DCHECK(provider_->IsRunning());
+  DCHECK(MessageLoop::current() == &message_loop_);
+  bool started;
+  provider_->message_loop_proxy()->PostTaskAndReply(
+      FROM_HERE,
+      base::Bind(&GeolocationProviderTest::GetProvidersStarted,
+                 base::Unretained(this),
+                 &started),
+      MessageLoop::QuitClosure());
+  message_loop_.Run();
+  return started;
+}
+
+void GeolocationProviderTest::GetProvidersStarted(bool* started) {
+  DCHECK(MessageLoop::current() == provider_->message_loop());
+  *started = provider_->mock_arbitrator()->providers_started();
+}
+
+
 // Regression test for http://crbug.com/59377
 TEST_F(GeolocationProviderTest, OnPermissionGrantedWithoutObservers) {
-  EXPECT_FALSE(provider_->HasPermissionBeenGranted());
-  provider_->OnPermissionGranted();
-  EXPECT_TRUE(provider_->HasPermissionBeenGranted());
+  EXPECT_FALSE(provider()->HasPermissionBeenGranted());
+  provider()->OnPermissionGranted();
+  EXPECT_TRUE(provider()->HasPermissionBeenGranted());
 }
 
 TEST_F(GeolocationProviderTest, StartStop) {
-  EXPECT_FALSE(provider_->IsRunning());
+  EXPECT_FALSE(provider()->IsRunning());
   NullGeolocationObserver null_observer;
   GeolocationObserverOptions options;
-  provider_->AddObserver(&null_observer, options);
-  EXPECT_TRUE(provider_->IsRunning());
-  // Wait for token load request from the arbitrator to come through.
-  WaitAndReset();
-
-  EXPECT_EQ(MockLocationProvider::instance_->state_,
-            MockLocationProvider::LOW_ACCURACY);
-  provider_->RemoveObserver(&null_observer);
-  // Wait for the providers to be stopped now that all clients are gone.
-  WaitAndReset();
-  EXPECT_TRUE(provider_->IsRunning());
+  provider()->AddObserver(&null_observer, options);
+  EXPECT_TRUE(provider()->IsRunning());
+  EXPECT_TRUE(ProvidersStarted());
+
+  provider()->RemoveObserver(&null_observer);
+  EXPECT_FALSE(ProvidersStarted());
+  EXPECT_TRUE(provider()->IsRunning());
 }
 
 TEST_F(GeolocationProviderTest, OverrideLocationForTesting) {
   Geoposition position;
   position.error_code = Geoposition::ERROR_CODE_POSITION_UNAVAILABLE;
-  provider_->OverrideLocationForTesting(position);
+  provider()->OverrideLocationForTesting(position);
   // Adding an observer when the location is overridden should synchronously
   // update the observer with our overridden position.
   MockGeolocationObserver mock_observer;
   EXPECT_CALL(mock_observer, OnLocationUpdate(GeopositionEq(position)));
-  provider_->AddObserver(&mock_observer, GeolocationObserverOptions());
-  // Wait for token load request from the arbitrator to come through.
-  WaitAndReset();
-
-  provider_->RemoveObserver(&mock_observer);
+  provider()->AddObserver(&mock_observer, GeolocationObserverOptions());
+  provider()->RemoveObserver(&mock_observer);
   // Wait for the providers to be stopped now that all clients are gone.
-  WaitAndReset();
+  EXPECT_FALSE(ProvidersStarted());
 }
 
 TEST_F(GeolocationProviderTest, Callback) {
   MockGeolocationCallbackWrapper callback_wrapper;
-  provider_->RequestCallback(
+  provider()->RequestCallback(
       base::Bind(&MockGeolocationCallbackWrapper::Callback,
                  base::Unretained(&callback_wrapper)));
-  // Wait for token load request from the arbitrator to come through.
-  WaitAndReset();
-
   Geoposition position;
   position.latitude = 12;
   position.longitude = 34;
   position.accuracy = 56;
   position.timestamp = base::Time::Now();
   EXPECT_CALL(callback_wrapper, Callback(GeopositionEq(position)));
-  provider_->OverrideLocationForTesting(position);
+  provider()->OverrideLocationForTesting(position);
   // Wait for the providers to be stopped now that all clients are gone.
-  WaitAndReset();
+  EXPECT_FALSE(ProvidersStarted());
 }
 
 }  // namespace content
diff --git a/content/browser/geolocation/location_arbitrator.h b/content/browser/geolocation/location_arbitrator.h
index 7f21d6d..443a6a3d 100644
--- a/content/browser/geolocation/location_arbitrator.h
+++ b/content/browser/geolocation/location_arbitrator.h
@@ -5,101 +5,35 @@
 #ifndef CONTENT_BROWSER_GEOLOCATION_LOCATION_ARBITRATOR_H_
 #define CONTENT_BROWSER_GEOLOCATION_LOCATION_ARBITRATOR_H_
 
-#include "base/memory/scoped_vector.h"
-#include "base/string16.h"
-#include "base/time.h"
-#include "content/browser/geolocation/location_provider.h"
-#include "content/browser/geolocation/geolocation_observer.h"
 #include "content/common/content_export.h"
-#include "content/public/browser/access_token_store.h"
-#include "content/public/common/geoposition.h"
-#include "net/url_request/url_request_context_getter.h"
-
-namespace net {
-class URLRequestContextGetter;
-}
 
 namespace content {
-class AccessTokenStore;
-class LocationProviderBase;
+
+struct GeolocationObserverOptions;
 
 // This class is responsible for handling updates from multiple underlying
 // providers and resolving them to a single 'best' location fix at any given
 // moment.
-class CONTENT_EXPORT GeolocationArbitrator
-    : public LocationProviderBase::ListenerInterface {
- public:
-  // Number of milliseconds newer a location provider has to be that it's worth
-  // switching to this location provider on the basis of it being fresher
-  // (regardles of relative accuracy). Public for tests.
-  static const int64 kFixStaleTimeoutMilliseconds;
-
-  explicit GeolocationArbitrator(GeolocationObserver* observer);
-  virtual ~GeolocationArbitrator();
-
-  static GURL DefaultNetworkProviderURL();
+class CONTENT_EXPORT GeolocationArbitrator {
+public:
+  virtual ~GeolocationArbitrator() {};
 
   // See more details in geolocation_provider.
-  void StartProviders(const GeolocationObserverOptions& options);
-  void StopProviders();
+  virtual void StartProviders(const GeolocationObserverOptions& options) = 0;
+  virtual void StopProviders() = 0;
 
   // Called everytime permission is granted to a page for using geolocation.
   // This may either be through explicit user action (e.g. responding to the
   // infobar prompt) or inferred from a persisted site permission.
   // The arbitrator will inform all providers of this, which may in turn use
   // this information to modify their internal policy.
-  void OnPermissionGranted();
+  virtual void OnPermissionGranted() = 0;
 
   // Returns true if this arbitrator has received at least one call to
   // OnPermissionGranted().
-  bool HasPermissionBeenGranted() const;
-
-  // ListenerInterface
-  virtual void LocationUpdateAvailable(LocationProviderBase* provider) OVERRIDE;
-
- protected:
-
-  AccessTokenStore* GetAccessTokenStore();
-
-  // These functions are useful for injection of dependencies in derived
-  // testing classes.
-  virtual AccessTokenStore* NewAccessTokenStore();
-  virtual LocationProviderBase* NewNetworkLocationProvider(
-      AccessTokenStore* access_token_store,
-      net::URLRequestContextGetter* context,
-      const GURL& url,
-      const string16& access_token);
-  virtual LocationProviderBase* NewSystemLocationProvider();
-  virtual base::Time GetTimeNow() const;
-
- private:
-  // Takes ownership of |provider| on entry; it will either be added to
-  // |providers_| or deleted on error (e.g. it fails to start).
-  void RegisterProvider(LocationProviderBase* provider);
-  void OnAccessTokenStoresLoaded(
-      AccessTokenStore::AccessTokenSet access_token_store,
-      net::URLRequestContextGetter* context_getter);
-  void DoStartProviders();
-  // Returns true if |new_position| is an improvement over |old_position|.
-  // Set |from_same_provider| to true if both the positions came from the same
-  // provider.
-  bool IsNewPositionBetter(const Geoposition& old_position,
-                           const Geoposition& new_position,
-                           bool from_same_provider) const;
-
-  scoped_refptr<AccessTokenStore> access_token_store_;
-  GeolocationObserver* observer_;
-  ScopedVector<LocationProviderBase> providers_;
-  GeolocationObserverOptions current_provider_options_;
-  // The provider which supplied the current |position_|
-  const LocationProviderBase* position_provider_;
-  bool is_permission_granted_;
-  // The current best estimate of our position.
-  Geoposition position_;
-
-  DISALLOW_COPY_AND_ASSIGN(GeolocationArbitrator);
+  virtual bool HasPermissionBeenGranted() const = 0;
 };
 
 }  // namespace content
 
-#endif  // CONTENT_BROWSER_GEOLOCATION_LOCATION_ARBITRATOR_H_
+#endif  // CONTENT_BROWSER_GEOLOCATION_LOCATION_ARBITRATOR_IMPL_H_
diff --git a/content/browser/geolocation/location_arbitrator.cc b/content/browser/geolocation/location_arbitrator_impl.cc
index 229e5ad..97cae8e 100644
--- a/content/browser/geolocation/location_arbitrator.cc
+++ b/content/browser/geolocation/location_arbitrator_impl.cc
@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "content/browser/geolocation/location_arbitrator.h"
+#include "content/browser/geolocation/location_arbitrator_impl.h"
 
 #include <map>
 
@@ -22,24 +22,24 @@ const char* kDefaultNetworkProviderUrl =
 
 // To avoid oscillations, set this to twice the expected update interval of a
 // a GPS-type location provider (in case it misses a beat) plus a little.
-const int64 GeolocationArbitrator::kFixStaleTimeoutMilliseconds =
+const int64 GeolocationArbitratorImpl::kFixStaleTimeoutMilliseconds =
     11 * base::Time::kMillisecondsPerSecond;
 
-GeolocationArbitrator::GeolocationArbitrator(
+GeolocationArbitratorImpl::GeolocationArbitratorImpl(
     GeolocationObserver* observer)
     : observer_(observer),
       position_provider_(NULL),
       is_permission_granted_(false) {
 }
 
-GeolocationArbitrator::~GeolocationArbitrator() {
+GeolocationArbitratorImpl::~GeolocationArbitratorImpl() {
 }
 
-GURL GeolocationArbitrator::DefaultNetworkProviderURL() {
+GURL GeolocationArbitratorImpl::DefaultNetworkProviderURL() {
   return GURL(kDefaultNetworkProviderUrl);
 }
 
-void GeolocationArbitrator::OnPermissionGranted() {
+void GeolocationArbitratorImpl::OnPermissionGranted() {
   is_permission_granted_ = true;
   for (ScopedVector<LocationProviderBase>::iterator i = providers_.begin();
       i != providers_.end(); ++i) {
@@ -47,32 +47,32 @@ void GeolocationArbitrator::OnPermissionGranted() {
   }
 }
 
-void GeolocationArbitrator::StartProviders(
+void GeolocationArbitratorImpl::StartProviders(
     const GeolocationObserverOptions& options) {
   // Stash options as OnAccessTokenStoresLoaded has not yet been called.
   current_provider_options_ = options;
   if (providers_.empty()) {
     DCHECK(DefaultNetworkProviderURL().is_valid());
     GetAccessTokenStore()->LoadAccessTokens(
-        base::Bind(&GeolocationArbitrator::OnAccessTokenStoresLoaded,
+        base::Bind(&GeolocationArbitratorImpl::OnAccessTokenStoresLoaded,
                    base::Unretained(this)));
   } else {
     DoStartProviders();
   }
 }
 
-void GeolocationArbitrator::DoStartProviders() {
+void GeolocationArbitratorImpl::DoStartProviders() {
   for (ScopedVector<LocationProviderBase>::iterator i = providers_.begin();
        i != providers_.end(); ++i) {
     (*i)->StartProvider(current_provider_options_.use_high_accuracy);
   }
 }
 
-void GeolocationArbitrator::StopProviders() {
+void GeolocationArbitratorImpl::StopProviders() {
   providers_.clear();
 }
 
-void GeolocationArbitrator::OnAccessTokenStoresLoaded(
+void GeolocationArbitratorImpl::OnAccessTokenStoresLoaded(
     AccessTokenStore::AccessTokenSet access_token_set,
     net::URLRequestContextGetter* context_getter) {
   if (!providers_.empty()) {
@@ -95,7 +95,7 @@ void GeolocationArbitrator::OnAccessTokenStoresLoaded(
   DoStartProviders();
 }
 
-void GeolocationArbitrator::RegisterProvider(
+void GeolocationArbitratorImpl::RegisterProvider(
     LocationProviderBase* provider) {
   if (!provider)
     return;
@@ -105,7 +105,7 @@ void GeolocationArbitrator::RegisterProvider(
   providers_.push_back(provider);
 }
 
-void GeolocationArbitrator::LocationUpdateAvailable(
+void GeolocationArbitratorImpl::LocationUpdateAvailable(
     LocationProviderBase* provider) {
   DCHECK(provider);
   Geoposition new_position;
@@ -120,17 +120,17 @@ void GeolocationArbitrator::LocationUpdateAvailable(
   observer_->OnLocationUpdate(position_);
 }
 
-AccessTokenStore* GeolocationArbitrator::NewAccessTokenStore() {
+AccessTokenStore* GeolocationArbitratorImpl::NewAccessTokenStore() {
   return GetContentClient()->browser()->CreateAccessTokenStore();
 }
 
-AccessTokenStore* GeolocationArbitrator::GetAccessTokenStore() {
+AccessTokenStore* GeolocationArbitratorImpl::GetAccessTokenStore() {
   if (!access_token_store_.get())
     access_token_store_ = NewAccessTokenStore();
   return access_token_store_.get();
 }
 
-LocationProviderBase* GeolocationArbitrator::NewNetworkLocationProvider(
+LocationProviderBase* GeolocationArbitratorImpl::NewNetworkLocationProvider(
     AccessTokenStore* access_token_store,
     net::URLRequestContextGetter* context,
     const GURL& url,
@@ -144,15 +144,15 @@ LocationProviderBase* GeolocationArbitrator::NewNetworkLocationProvider(
 #endif
 }
 
-LocationProviderBase* GeolocationArbitrator::NewSystemLocationProvider() {
+LocationProviderBase* GeolocationArbitratorImpl::NewSystemLocationProvider() {
   return content::NewSystemLocationProvider();
 }
 
-base::Time GeolocationArbitrator::GetTimeNow() const {
+base::Time GeolocationArbitratorImpl::GetTimeNow() const {
   return base::Time::Now();
 }
 
-bool GeolocationArbitrator::IsNewPositionBetter(
+bool GeolocationArbitratorImpl::IsNewPositionBetter(
     const Geoposition& old_position, const Geoposition& new_position,
     bool from_same_provider) const {
   // Updates location_info if it's better than what we currently have,
@@ -178,7 +178,7 @@ bool GeolocationArbitrator::IsNewPositionBetter(
   return false;
 }
 
-bool GeolocationArbitrator::HasPermissionBeenGranted() const {
+bool GeolocationArbitratorImpl::HasPermissionBeenGranted() const {
   return is_permission_granted_;
 }
 
diff --git a/content/browser/geolocation/location_arbitrator_impl.h b/content/browser/geolocation/location_arbitrator_impl.h
new file mode 100644
index 0000000..e7b4d50
--- /dev/null
+++ b/content/browser/geolocation/location_arbitrator_impl.h
@@ -0,0 +1,99 @@
+// 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 CONTENT_BROWSER_GEOLOCATION_LOCATION_ARBITRATOR_IMPL_H_
+#define CONTENT_BROWSER_GEOLOCATION_LOCATION_ARBITRATOR_IMPL_H_
+
+#include "base/memory/scoped_vector.h"
+#include "base/string16.h"
+#include "base/time.h"
+#include "content/browser/geolocation/geolocation_observer.h"
+#include "content/browser/geolocation/location_arbitrator.h"
+#include "content/browser/geolocation/location_provider.h"
+#include "content/common/content_export.h"
+#include "content/public/browser/access_token_store.h"
+#include "content/public/common/geoposition.h"
+#include "net/url_request/url_request_context_getter.h"
+
+namespace net {
+class URLRequestContextGetter;
+}
+
+namespace content {
+class AccessTokenStore;
+class LocationProviderBase;
+
+// This class is responsible for handling updates from multiple underlying
+// providers and resolving them to a single 'best' location fix at any given
+// moment.
+class CONTENT_EXPORT GeolocationArbitratorImpl
+    : public GeolocationArbitrator,
+      public LocationProviderBase::ListenerInterface {
+ public:
+  // Number of milliseconds newer a location provider has to be that it's worth
+  // switching to this location provider on the basis of it being fresher
+  // (regardles of relative accuracy). Public for tests.
+  static const int64 kFixStaleTimeoutMilliseconds;
+
+  explicit GeolocationArbitratorImpl(GeolocationObserver* observer);
+  virtual ~GeolocationArbitratorImpl();
+
+  static GURL DefaultNetworkProviderURL();
+
+  // GeolocationArbitrator
+  virtual void StartProviders(const GeolocationObserverOptions& options)
+      OVERRIDE;
+  virtual void StopProviders() OVERRIDE;
+  virtual void OnPermissionGranted() OVERRIDE;
+  virtual bool HasPermissionBeenGranted() const OVERRIDE;
+
+  // ListenerInterface
+  virtual void LocationUpdateAvailable(LocationProviderBase* provider) OVERRIDE;
+
+ protected:
+
+  AccessTokenStore* GetAccessTokenStore();
+
+  // These functions are useful for injection of dependencies in derived
+  // testing classes.
+  virtual AccessTokenStore* NewAccessTokenStore();
+  virtual LocationProviderBase* NewNetworkLocationProvider(
+      AccessTokenStore* access_token_store,
+      net::URLRequestContextGetter* context,
+      const GURL& url,
+      const string16& access_token);
+  virtual LocationProviderBase* NewSystemLocationProvider();
+  virtual base::Time GetTimeNow() const;
+
+ private:
+  // Takes ownership of |provider| on entry; it will either be added to
+  // |providers_| or deleted on error (e.g. it fails to start).
+  void RegisterProvider(LocationProviderBase* provider);
+  void OnAccessTokenStoresLoaded(
+      AccessTokenStore::AccessTokenSet access_token_store,
+      net::URLRequestContextGetter* context_getter);
+  void DoStartProviders();
+  // Returns true if |new_position| is an improvement over |old_position|.
+  // Set |from_same_provider| to true if both the positions came from the same
+  // provider.
+  bool IsNewPositionBetter(const Geoposition& old_position,
+                           const Geoposition& new_position,
+                           bool from_same_provider) const;
+
+  scoped_refptr<AccessTokenStore> access_token_store_;
+  GeolocationObserver* observer_;
+  ScopedVector<LocationProviderBase> providers_;
+  GeolocationObserverOptions current_provider_options_;
+  // The provider which supplied the current |position_|
+  const LocationProviderBase* position_provider_;
+  bool is_permission_granted_;
+  // The current best estimate of our position.
+  Geoposition position_;
+
+  DISALLOW_COPY_AND_ASSIGN(GeolocationArbitratorImpl);
+};
+
+}  // namespace content
+
+#endif  // CONTENT_BROWSER_GEOLOCATION_LOCATION_ARBITRATOR_IMPL_H_
diff --git a/content/browser/geolocation/location_arbitrator_unittest.cc b/content/browser/geolocation/location_arbitrator_impl_unittest.cc
index ae4a84e..b385f86 100644
--- a/content/browser/geolocation/location_arbitrator_unittest.cc
+++ b/content/browser/geolocation/location_arbitrator_impl_unittest.cc
@@ -5,7 +5,7 @@
 #include "base/memory/scoped_ptr.h"
 #include "content/browser/geolocation/fake_access_token_store.h"
 #include "content/browser/geolocation/geolocation_observer.h"
-#include "content/browser/geolocation/location_arbitrator.h"
+#include "content/browser/geolocation/location_arbitrator_impl.h"
 #include "content/browser/geolocation/location_provider.h"
 #include "content/browser/geolocation/mock_location_provider.h"
 #include "content/public/common/geoposition.h"
@@ -61,12 +61,12 @@ void SetReferencePosition(MockLocationProvider* provider) {
 
 namespace {
 
-class TestingGeolocationArbitrator : public GeolocationArbitrator {
+class TestingGeolocationArbitrator : public GeolocationArbitratorImpl {
  public:
   TestingGeolocationArbitrator(
       GeolocationObserver* observer,
       AccessTokenStore* access_token_store)
-      : GeolocationArbitrator(observer),
+      : GeolocationArbitratorImpl(observer),
         cell_(NULL),
         gps_(NULL),
         access_token_store_(access_token_store) {
@@ -130,7 +130,7 @@ class GeolocationLocationArbitratorTest : public testing::Test {
   base::TimeDelta SwitchOnFreshnessCliff() {
     // Add 1, to ensure it meets any greater-than test.
     return base::TimeDelta::FromMilliseconds(
-        GeolocationArbitrator::kFixStaleTimeoutMilliseconds + 1);
+        GeolocationArbitratorImpl::kFixStaleTimeoutMilliseconds + 1);
   }
 
   MockLocationProvider* cell() {
diff --git a/content/browser/geolocation/mock_location_arbitrator.cc b/content/browser/geolocation/mock_location_arbitrator.cc
new file mode 100644
index 0000000..a01fe56
--- /dev/null
+++ b/content/browser/geolocation/mock_location_arbitrator.cc
@@ -0,0 +1,34 @@
+// 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 "content/browser/geolocation/mock_location_arbitrator.h"
+
+#include "base/message_loop.h"
+#include "content/public/common/geoposition.h"
+
+namespace content {
+
+MockGeolocationArbitrator::MockGeolocationArbitrator()
+    : permission_granted_(false),
+      providers_started_(false) {
+}
+
+void MockGeolocationArbitrator::StartProviders(
+    const GeolocationObserverOptions& options) {
+  providers_started_ = true;;
+}
+
+void MockGeolocationArbitrator::StopProviders() {
+  providers_started_ = false;
+}
+
+void MockGeolocationArbitrator::OnPermissionGranted() {
+  permission_granted_ = true;
+}
+
+bool MockGeolocationArbitrator::HasPermissionBeenGranted() const {
+  return permission_granted_;
+}
+
+}  // namespace content
diff --git a/content/browser/geolocation/mock_location_arbitrator.h b/content/browser/geolocation/mock_location_arbitrator.h
new file mode 100644
index 0000000..dde6878
--- /dev/null
+++ b/content/browser/geolocation/mock_location_arbitrator.h
@@ -0,0 +1,38 @@
+// 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 CONTENT_BROWSER_GEOLOCATION_MOCK_LOCATION_ARBITRATOR_H_
+#define CONTENT_BROWSER_GEOLOCATION_MOCK_LOCATION_ARBITRATOR_H_
+
+#include "base/basictypes.h"
+#include "base/compiler_specific.h"
+#include "content/browser/geolocation/location_arbitrator.h"
+
+namespace content {
+
+struct Geoposition;
+
+class MockGeolocationArbitrator : public GeolocationArbitrator {
+ public:
+  MockGeolocationArbitrator();
+
+  bool providers_started() const { return providers_started_; }
+
+  // GeolocationArbitrator:
+  virtual void StartProviders(const GeolocationObserverOptions& options)
+      OVERRIDE;
+  virtual void StopProviders() OVERRIDE;
+  virtual void OnPermissionGranted() OVERRIDE;
+  virtual bool HasPermissionBeenGranted() const OVERRIDE;
+
+ private:
+  bool permission_granted_;
+  bool providers_started_;
+
+  DISALLOW_COPY_AND_ASSIGN(MockGeolocationArbitrator);
+};
+
+}  // namespace content
+
+#endif  //  CONTENT_BROWSER_GEOLOCATION_MOCK_LOCATION_ARBITRATOR_H_
diff --git a/content/browser/geolocation/network_location_provider_unittest.cc b/content/browser/geolocation/network_location_provider_unittest.cc
index b70a334..f46f2ee 100644
--- a/content/browser/geolocation/network_location_provider_unittest.cc
+++ b/content/browser/geolocation/network_location_provider_unittest.cc
@@ -11,7 +11,7 @@
 #include "base/utf_string_conversions.h"
 #include "base/values.h"
 #include "content/browser/geolocation/fake_access_token_store.h"
-#include "content/browser/geolocation/location_arbitrator.h"
+#include "content/browser/geolocation/location_arbitrator_impl.h"
 #include "content/browser/geolocation/network_location_provider.h"
 #include "net/url_request/test_url_fetcher_factory.h"
 #include "net/url_request/url_request_status.h"
@@ -264,7 +264,7 @@ class GeolocationNetworkProviderTest : public testing::Test {
     // Check to see that the api key is being appended for the default
     // network provider url.
     bool is_default_url = UrlWithoutQuery(request_url) ==
-        UrlWithoutQuery(GeolocationArbitrator::DefaultNetworkProviderURL());
+        UrlWithoutQuery(GeolocationArbitratorImpl::DefaultNetworkProviderURL());
     EXPECT_EQ(is_default_url, !request_url.query().empty());
 
     const std::string& upload_data = request.upload_data();
@@ -347,7 +347,7 @@ TEST_F(GeolocationNetworkProviderTest, StartProvider) {
 }
 
 TEST_F(GeolocationNetworkProviderTest, StartProviderDefaultUrl) {
-  test_server_url_ = GeolocationArbitrator::DefaultNetworkProviderURL();
+  test_server_url_ = GeolocationArbitratorImpl::DefaultNetworkProviderURL();
   scoped_ptr<LocationProviderBase> provider(CreateProvider(true));
   EXPECT_TRUE(provider->StartProvider(false));
   net::TestURLFetcher* fetcher = get_url_fetcher_and_advance_id();
diff --git a/content/browser/geolocation/network_location_request.cc b/content/browser/geolocation/network_location_request.cc
index ab53aa5..b4aac01 100644
--- a/content/browser/geolocation/network_location_request.cc
+++ b/content/browser/geolocation/network_location_request.cc
@@ -13,7 +13,7 @@
 #include "base/string_number_conversions.h"
 #include "base/utf_string_conversions.h"
 #include "base/values.h"
-#include "content/browser/geolocation/location_arbitrator.h"
+#include "content/browser/geolocation/location_arbitrator_impl.h"
 #include "content/public/common/geoposition.h"
 #include "google_apis/google_api_keys.h"
 #include "net/base/escape.h"
@@ -156,7 +156,7 @@ struct AccessPointLess {
 };
 
 GURL FormRequestURL(const GURL& url) {
-  if (url == GeolocationArbitrator::DefaultNetworkProviderURL()) {
+  if (url == GeolocationArbitratorImpl::DefaultNetworkProviderURL()) {
     std::string api_key = google_apis::GetAPIKey();
     if (!api_key.empty()) {
       std::string query(url.query());
diff --git a/content/content_browser.gypi b/content/content_browser.gypi
index f35d9f1..925fddb 100644
--- a/content/content_browser.gypi
+++ b/content/content_browser.gypi
@@ -422,8 +422,9 @@
     'browser/geolocation/gps_location_provider_linux.h',
     'browser/geolocation/location_api_adapter_android.cc',
     'browser/geolocation/location_api_adapter_android.h',
-    'browser/geolocation/location_arbitrator.cc',
     'browser/geolocation/location_arbitrator.h',
+    'browser/geolocation/location_arbitrator_impl.cc',
+    'browser/geolocation/location_arbitrator_impl.h',
     'browser/geolocation/location_provider_android.cc',
     'browser/geolocation/location_provider_android.h',
     'browser/geolocation/location_provider.cc',
diff --git a/content/content_tests.gypi b/content/content_tests.gypi
index 9ee16c6..cba279c 100644
--- a/content/content_tests.gypi
+++ b/content/content_tests.gypi
@@ -85,6 +85,8 @@
         'browser/download/mock_download_file.h',
         'browser/geolocation/fake_access_token_store.cc',
         'browser/geolocation/fake_access_token_store.h',
+        'browser/geolocation/mock_location_arbitrator.cc',
+        'browser/geolocation/mock_location_arbitrator.h',
         'browser/geolocation/mock_location_provider.cc',
         'browser/geolocation/mock_location_provider.h',
         'browser/renderer_host/media/mock_media_observer.cc',
@@ -272,7 +274,7 @@
         'browser/geolocation/device_data_provider_unittest.cc',
         'browser/geolocation/geolocation_provider_unittest.cc',
         'browser/geolocation/gps_location_provider_unittest_linux.cc',
-        'browser/geolocation/location_arbitrator_unittest.cc',
+        'browser/geolocation/location_arbitrator_impl_unittest.cc',
         'browser/geolocation/network_location_provider_unittest.cc',
         'browser/geolocation/wifi_data_provider_common_unittest.cc',
         'browser/geolocation/wifi_data_provider_linux_unittest.cc',