Refactoring geolocation code to work on its own thread.

Split the arbitrator into a class that handles the threading and a class that handles location provider arbitration. Changed unit tests and MockLocationProvider to work with this new model.

BUG=None
TEST= --gtest_filter=*Geolo*

Review URL: http://codereview.chromium.org/3548008

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

13 files changed, 483 insertions, 414 deletions

diff --git a/chrome/browser/geolocation/geolocation_browsertest.cc b/chrome/browser/geolocation/geolocation_browsertest.cc
index 6617f90..5a26866 100644
--- a/chrome/browser/geolocation/geolocation_browsertest.cc
+++ b/chrome/browser/geolocation/geolocation_browsertest.cc
@@ -171,11 +171,10 @@ class GeolocationNotificationObserver : public NotificationObserver {
   std::string javascript_response_;
 };
 
-void NotifyGeopositionOnIOThread(const Geoposition& geoposition) {
-  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO));
+void NotifyGeoposition(const Geoposition& geoposition) {
   DCHECK(MockLocationProvider::instance_);
   MockLocationProvider::instance_->position_ = geoposition;
-  MockLocationProvider::instance_->UpdateListeners();
+  MockLocationProvider::instance_->HandlePositionChanged();
   LOG(WARNING) << "MockLocationProvider listeners updated";
 }
 
@@ -456,8 +455,7 @@ IN_PROC_BROWSER_TEST_F(GeolocationBrowserTest, IFramesWithFreshPosition) {
   // MockLocationProvider must have been created.
   ASSERT_TRUE(MockLocationProvider::instance_);
   Geoposition fresh_position = GeopositionFromLatLong(3.17, 4.23);
-  ChromeThread::PostTask(ChromeThread::IO, FROM_HERE, NewRunnableFunction(
-      &NotifyGeopositionOnIOThread, fresh_position));
+  NotifyGeoposition(fresh_position);
   WaitForJSPrompt();
   CheckGeoposition(fresh_position);
 
@@ -486,8 +484,7 @@ IN_PROC_BROWSER_TEST_F(GeolocationBrowserTest, IFramesWithCachedPosition) {
   // MockLocationProvider must have been created.
   ASSERT_TRUE(MockLocationProvider::instance_);
   Geoposition cached_position = GeopositionFromLatLong(5.67, 8.09);
-  ChromeThread::PostTask(ChromeThread::IO, FROM_HERE, NewRunnableFunction(
-      &NotifyGeopositionOnIOThread, cached_position));
+  NotifyGeoposition(cached_position);
   WaitForJSPrompt();
   CheckGeoposition(cached_position);
 
@@ -588,8 +585,7 @@ IN_PROC_BROWSER_TEST_F(GeolocationBrowserTest, TwoWatchesInOneFrame) {
 
   // The second watch will now have cancelled. Ensure an update still makes
   // its way through to the first watcher.
-  ChromeThread::PostTask(ChromeThread::IO, FROM_HERE, NewRunnableFunction(
-      &NotifyGeopositionOnIOThread, final_position));
+  NotifyGeoposition(final_position);
   WaitForJSPrompt();
   CheckGeoposition(final_position);
 }
diff --git a/chrome/browser/geolocation/geolocation_dispatcher_host.cc b/chrome/browser/geolocation/geolocation_dispatcher_host.cc
index 5e4dc40..b4d380b 100644
--- a/chrome/browser/geolocation/geolocation_dispatcher_host.cc
+++ b/chrome/browser/geolocation/geolocation_dispatcher_host.cc
@@ -10,7 +10,7 @@
 
 #include "chrome/common/geoposition.h"
 #include "chrome/browser/geolocation/geolocation_permission_context.h"
-#include "chrome/browser/geolocation/location_arbitrator.h"
+#include "chrome/browser/geolocation/geolocation_provider.h"
 #include "chrome/browser/renderer_host/render_process_host.h"
 #include "chrome/browser/renderer_host/render_view_host.h"
 #include "chrome/browser/renderer_host/render_view_host_notification_task.h"
@@ -20,7 +20,7 @@
 
 namespace {
 class GeolocationDispatcherHostImpl : public GeolocationDispatcherHost,
-                                      public GeolocationArbitrator::Delegate {
+                                      public GeolocationObserver {
  public:
   GeolocationDispatcherHostImpl(
       int resource_message_filter_process_id,
@@ -53,7 +53,7 @@ class GeolocationDispatcherHostImpl : public GeolocationDispatcherHost,
 
   // Updates the |location_arbitrator_| with the currently required update
   // options, based on |bridge_update_options_|.
-  void RefreshUpdateOptions();
+  void RefreshGeolocationObserverOptions();
 
   int resource_message_filter_process_id_;
   scoped_refptr<GeolocationPermissionContext> geolocation_permission_context_;
@@ -65,10 +65,10 @@ class GeolocationDispatcherHostImpl : public GeolocationDispatcherHost,
   std::set<int> geolocation_renderer_ids_;
   // Maps <renderer_id, bridge_id> to the location arbitrator update options
   // that correspond to this particular bridge.
-  std::map<std::pair<int, int>, GeolocationArbitrator::UpdateOptions>
+  std::map<std::pair<int, int>, GeolocationObserverOptions>
       bridge_update_options_;
   // Only set whilst we are registered with the arbitrator.
-  scoped_refptr<GeolocationArbitrator> location_arbitrator_;
+  GeolocationProvider* location_provider_;
 
   DISALLOW_COPY_AND_ASSIGN(GeolocationDispatcherHostImpl);
 };
@@ -77,15 +77,16 @@ GeolocationDispatcherHostImpl::GeolocationDispatcherHostImpl(
     int resource_message_filter_process_id,
     GeolocationPermissionContext* geolocation_permission_context)
     : resource_message_filter_process_id_(resource_message_filter_process_id),
-      geolocation_permission_context_(geolocation_permission_context) {
+      geolocation_permission_context_(geolocation_permission_context),
+      location_provider_(NULL) {
   // This is initialized by ResourceMessageFilter. Do not add any non-trivial
   // initialization here, defer to OnRegisterBridge which is triggered whenever
   // a javascript geolocation object is actually initialized.
 }
 
 GeolocationDispatcherHostImpl::~GeolocationDispatcherHostImpl() {
-  if (location_arbitrator_)
-    location_arbitrator_->RemoveObserver(this);
+  if (location_provider_)
+    location_provider_->RemoveObserver(this);
 }
 
 bool GeolocationDispatcherHostImpl::OnMessageReceived(
@@ -169,12 +170,11 @@ void GeolocationDispatcherHostImpl::OnStartUpdating(
   DLOG(INFO) << __FUNCTION__ << " " << resource_message_filter_process_id_
              << ":" << render_view_id << ":" << bridge_id;
   bridge_update_options_[std::make_pair(render_view_id, bridge_id)] =
-      GeolocationArbitrator::UpdateOptions(enable_high_accuracy);
-  location_arbitrator_ =
-      geolocation_permission_context_->StartUpdatingRequested(
-          resource_message_filter_process_id_, render_view_id, bridge_id,
-          requesting_frame);
-  RefreshUpdateOptions();
+      GeolocationObserverOptions(enable_high_accuracy);
+  geolocation_permission_context_->StartUpdatingRequested(
+      resource_message_filter_process_id_, render_view_id, bridge_id,
+      requesting_frame);
+  RefreshGeolocationObserverOptions();
 }
 
 void GeolocationDispatcherHostImpl::OnStopUpdating(
@@ -183,7 +183,7 @@ void GeolocationDispatcherHostImpl::OnStopUpdating(
   DLOG(INFO) << __FUNCTION__ << " " << resource_message_filter_process_id_
              << ":" << render_view_id << ":" << bridge_id;
   if (bridge_update_options_.erase(std::make_pair(render_view_id, bridge_id)))
-    RefreshUpdateOptions();
+    RefreshGeolocationObserverOptions();
   geolocation_permission_context_->StopUpdatingRequested(
       resource_message_filter_process_id_, render_view_id, bridge_id);
 }
@@ -204,17 +204,20 @@ void GeolocationDispatcherHostImpl::OnResume(
   // TODO(bulach): connect this with GeolocationArbitrator.
 }
 
-void GeolocationDispatcherHostImpl::RefreshUpdateOptions() {
+void GeolocationDispatcherHostImpl::RefreshGeolocationObserverOptions() {
   DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO));
-  DCHECK(location_arbitrator_);
   if (bridge_update_options_.empty()) {
-    location_arbitrator_->RemoveObserver(this);
-    location_arbitrator_ = NULL;
+    if (location_provider_) {
+      location_provider_->RemoveObserver(this);
+      location_provider_ = NULL;
+    }
   } else {
-    location_arbitrator_->AddObserver(
-        this,
-        GeolocationArbitrator::UpdateOptions::Collapse(
-            bridge_update_options_));
+    if (!location_provider_)
+      location_provider_ = GeolocationProvider::GetInstance();
+    // Re-add to re-establish our options, in case they changed.
+    location_provider_->AddObserver(this,
+                                    GeolocationObserverOptions::Collapse(
+                                        bridge_update_options_));
   }
 }
 }  // namespace
diff --git a/chrome/browser/geolocation/geolocation_observer.h b/chrome/browser/geolocation/geolocation_observer.h
new file mode 100755
index 0000000..72ff0ac
--- /dev/null
+++ b/chrome/browser/geolocation/geolocation_observer.h
@@ -0,0 +1,49 @@
+// Copyright (c) 2010 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_GEOLOCATION_GEOLOCATION_OBSERVER_H_
+#define CHROME_BROWSER_GEOLOCATION_GEOLOCATION_OBSERVER_H_
+#pragma once
+
+struct Geoposition;
+
+// This interface is implemented by observers of GeolocationProvider as
+// well as GeolocationProvider itself as an observer of GeolocationArbitrator.
+class GeolocationObserver {
+ public:
+  // This will be called whenever the 'best available' location is updated,
+  // or when an error is encountered meaning no location data will be
+  // available in the forseeable future.
+  virtual void OnLocationUpdate(const Geoposition& position) = 0;
+
+ protected:
+  GeolocationObserver() {}
+  virtual ~GeolocationObserver() {}
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(GeolocationObserver);
+};
+
+struct GeolocationObserverOptions {
+  GeolocationObserverOptions() : use_high_accuracy(false) {}
+  explicit GeolocationObserverOptions(bool high_accuracy)
+      : use_high_accuracy(high_accuracy) {}
+
+  // Given a map<ANYTHING, GeolocationObserverOptions> this function will
+  // iterate the map and collapse all the options found to a single instance
+  // that satisfies them all.
+  template <typename MAP>
+  static GeolocationObserverOptions Collapse(const MAP& options_map) {
+    for (typename MAP::const_iterator it = options_map.begin();
+         it != options_map.end(); ++it) {
+      if (it->second.use_high_accuracy)
+        return GeolocationObserverOptions(true);
+    }
+    return GeolocationObserverOptions(false);
+  }
+
+  bool use_high_accuracy;
+};
+
+#endif  // CHROME_BROWSER_GEOLOCATION_GEOLOCATION_OBSERVER_H_
diff --git a/chrome/browser/geolocation/geolocation_permission_context.cc b/chrome/browser/geolocation/geolocation_permission_context.cc
index 7d75f84..dd63132 100644
--- a/chrome/browser/geolocation/geolocation_permission_context.cc
+++ b/chrome/browser/geolocation/geolocation_permission_context.cc
@@ -12,7 +12,7 @@
 #include "chrome/browser/extensions/extensions_service.h"
 #include "chrome/browser/geolocation/geolocation_content_settings_map.h"
 #include "chrome/browser/geolocation/geolocation_dispatcher_host.h"
-#include "chrome/browser/geolocation/location_arbitrator.h"
+#include "chrome/browser/geolocation/geolocation_provider.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/browser/profile.h"
 #include "chrome/browser/renderer_host/render_process_host.h"
@@ -420,23 +420,22 @@ void GeolocationPermissionContext::CancelGeolocationPermissionRequest(
   CancelPendingInfoBarRequest(render_process_id, render_view_id, bridge_id);
 }
 
-GeolocationArbitrator* GeolocationPermissionContext::StartUpdatingRequested(
+void GeolocationPermissionContext::StartUpdatingRequested(
     int render_process_id, int render_view_id, int bridge_id,
     const GURL& requesting_frame) {
   DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO));
   // Note we cannot store the arbitrator as a member as it is not thread safe.
-  GeolocationArbitrator* arbitrator = GeolocationArbitrator::GetInstance();
+  GeolocationProvider* provider = GeolocationProvider::GetInstance();
 
   // WebKit will not request permission until it has received a valid
   // location, but the google network location provider will not give a
   // valid location until the user has granted permission. So we cut the Gordian
   // Knot by reusing the the 'start updating' request to also trigger
   // a 'permission request' should the provider still be awaiting permission.
-  if (!arbitrator->HasPermissionBeenGranted()) {
+  if (!provider->HasPermissionBeenGranted()) {
     RequestGeolocationPermission(render_process_id, render_view_id, bridge_id,
                                  requesting_frame);
   }
-  return arbitrator;
 }
 
 void GeolocationPermissionContext::StopUpdatingRequested(
@@ -477,7 +476,7 @@ void GeolocationPermissionContext::NotifyPermissionSet(
 void GeolocationPermissionContext::NotifyArbitratorPermissionGranted(
     const GURL& requesting_frame) {
   DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO));
-  GeolocationArbitrator::GetInstance()->OnPermissionGranted(requesting_frame);
+  GeolocationProvider::GetInstance()->OnPermissionGranted(requesting_frame);
 }
 
 void GeolocationPermissionContext::CancelPendingInfoBarRequest(
diff --git a/chrome/browser/geolocation/geolocation_permission_context.h b/chrome/browser/geolocation/geolocation_permission_context.h
index c93fec3..2fc7da1 100644
--- a/chrome/browser/geolocation/geolocation_permission_context.h
+++ b/chrome/browser/geolocation/geolocation_permission_context.h
@@ -14,7 +14,6 @@ class GeolocationDispatcherHost;
 class GeolocationInfoBarQueueController;
 class GeolocationPermissionContext;
 class GURL;
-class GeolocationArbitrator;
 class InfoBarDelegate;
 class Profile;
 class RenderViewHost;
@@ -47,12 +46,10 @@ class GeolocationPermissionContext
       const GURL& requesting_frame, bool allowed);
 
   // Called when a geolocation object wants to start receiving location updates.
-  // Returns the location arbitrator that the caller (namely, the dispatcher
-  // host) will use to receive these updates. The arbitrator is ref counted.
   // This also applies global policy around which location providers may be
   // enbaled at a given time (e.g. prior to the user agreeing to any geolocation
   // permission requests).
-  GeolocationArbitrator* StartUpdatingRequested(
+  void StartUpdatingRequested(
       int render_process_id, int render_view_id, int bridge_id,
       const GURL& requesting_frame);
 
diff --git a/chrome/browser/geolocation/geolocation_provider.cc b/chrome/browser/geolocation/geolocation_provider.cc
new file mode 100755
index 0000000..5104d6a
--- /dev/null
+++ b/chrome/browser/geolocation/geolocation_provider.cc
@@ -0,0 +1,125 @@
+// Copyright (c) 2010 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/geolocation/geolocation_provider.h"
+
+#include "base/singleton.h"
+#include "chrome/browser/geolocation/location_arbitrator.h"
+
+// This class is guaranteed to outlive its internal thread, so ref counting
+// is not required.
+DISABLE_RUNNABLE_METHOD_REFCOUNT(GeolocationProvider);
+
+GeolocationProvider* GeolocationProvider::GetInstance() {
+  return Singleton<GeolocationProvider>::get();
+}
+
+GeolocationProvider::GeolocationProvider()
+    : base::Thread("Geolocation"),
+      client_loop_(base::MessageLoopProxy::CreateForCurrentThread()),
+      arbitrator_(NULL) {
+}
+
+GeolocationProvider::~GeolocationProvider() {
+  DCHECK(observers_.empty());  // observers must unregister.
+  DCHECK(!IsRunning());
+  Stop();
+  DCHECK(!arbitrator_);
+}
+
+void GeolocationProvider::AddObserver(GeolocationObserver* observer,
+    const GeolocationObserverOptions& update_options) {
+  DCHECK(OnClientThread());
+  observers_[observer] = update_options;
+  OnObserversChanged();
+  if (position_.IsInitialized())
+    observer->OnLocationUpdate(position_);
+}
+
+bool GeolocationProvider::RemoveObserver(GeolocationObserver* observer) {
+  DCHECK(OnClientThread());
+  size_t remove = observers_.erase(observer);
+  OnObserversChanged();
+  return remove > 0;
+}
+
+void GeolocationProvider::OnObserversChanged() {
+  DCHECK(OnClientThread());
+  if (observers_.empty()) {
+    Stop();
+  } else {
+    if (!IsRunning())
+      Start();
+    // The high accuracy requirement may have changed.
+    Task* task = NewRunnableMethod(this,
+        &GeolocationProvider::SetObserverOptions,
+            GeolocationObserverOptions::Collapse(observers_));
+    message_loop()->PostTask(FROM_HERE, task);
+  }
+}
+
+void GeolocationProvider::NotifyObservers(const Geoposition& position) {
+  DCHECK(OnClientThread());
+  DCHECK(position.IsInitialized());
+  position_ = position;
+  ObserverMap::const_iterator it = observers_.begin();
+  while (it != observers_.end()) {
+    // Advance iterator before callback to guard against synchronous unregister.
+    GeolocationObserver* observer = it->first;
+    ++it;
+    observer->OnLocationUpdate(position_);
+  }
+}
+
+void GeolocationProvider::SetObserverOptions(
+    const GeolocationObserverOptions& options) {
+  DCHECK(OnGeolocationThread());
+  if (!arbitrator_)
+    arbitrator_ = GeolocationArbitrator::Create(this);
+  arbitrator_->SetObserverOptions(options);
+}
+
+void GeolocationProvider::OnPermissionGranted(const GURL& requesting_frame) {
+  if (!OnGeolocationThread()) {
+    DCHECK(OnClientThread());
+    most_recent_authorized_frame_ = requesting_frame;
+    Task* task = NewRunnableMethod(this,
+        &GeolocationProvider::OnPermissionGranted, requesting_frame);
+    message_loop()->PostTask(FROM_HERE, task);
+    return;
+  }
+  DCHECK(OnGeolocationThread());
+  arbitrator_->OnPermissionGranted(requesting_frame);
+}
+
+void GeolocationProvider::Init() {
+  DCHECK(OnGeolocationThread());
+}
+
+void GeolocationProvider::CleanUp() {
+  DCHECK(OnGeolocationThread());
+  delete arbitrator_;
+  arbitrator_ = NULL;
+}
+
+void GeolocationProvider::OnLocationUpdate(const Geoposition& position) {
+  DCHECK(OnGeolocationThread());
+  Task* task = NewRunnableMethod(this,
+                                 &GeolocationProvider::NotifyObservers,
+                                 position);
+  client_loop_->PostTask(FROM_HERE, task);
+}
+
+bool GeolocationProvider::HasPermissionBeenGranted() const {
+  DCHECK(OnClientThread());
+  return most_recent_authorized_frame_.is_valid();
+}
+
+bool GeolocationProvider::OnClientThread() const {
+  return client_loop_->BelongsToCurrentThread();
+}
+
+bool GeolocationProvider::OnGeolocationThread() const {
+  return MessageLoop::current() == message_loop();
+}
diff --git a/chrome/browser/geolocation/geolocation_provider.h b/chrome/browser/geolocation/geolocation_provider.h
new file mode 100755
index 0000000..ae10093
--- /dev/null
+++ b/chrome/browser/geolocation/geolocation_provider.h
@@ -0,0 +1,85 @@
+// Copyright (c) 2010 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_GEOLOCATION_GEOLOCATION_PROVIDER_H_
+#define CHROME_BROWSER_GEOLOCATION_GEOLOCATION_PROVIDER_H_
+#pragma once
+
+#include <map>
+
+#include "base/thread.h"
+#include "chrome/browser/geolocation/geolocation_observer.h"
+#include "chrome/common/geoposition.h"
+#include "googleurl/src/gurl.h"
+
+class GeolocationArbitrator;
+
+template<typename Type>
+struct DefaultSingletonTraits;
+
+// This is the main API to the geolocaiton subsystem. The application
+// will hold a single instance of this class, and can register multiple
+// observers which will be notified of location updates. Underlying location
+// arbitrator will only be enabled whilst there is at least one observer
+// registered.
+class GeolocationProvider : public base::Thread, public GeolocationObserver {
+ public:
+  GeolocationProvider();
+
+  // Must be called from the same thread as the GeolocationProvider was created
+  // on. The GeolocationObserverOptions passed are used as a 'hint' for the
+  // provider preferences for this particular observer, however the observer
+  // could receive callbacks for best available locations from any active
+  // provider whilst it is registered.
+  // If an existing observer is added a second time it's options are updated
+  // but only a single call to RemoveObserver() is required to remove it.
+  void AddObserver(GeolocationObserver* delegate,
+                   const GeolocationObserverOptions& update_options);
+  // Remove a previously registered observer. No-op if not previously registered
+  // via AddObserver(). Returns true if the observer was removed.
+  bool RemoveObserver(GeolocationObserver* delegate);
+  void OnPermissionGranted(const GURL& requesting_frame);
+  bool HasPermissionBeenGranted() const;
+
+  // GeolocationObserver
+  virtual void OnLocationUpdate(const Geoposition& position);
+
+  // Gets a pointer to the singleton instance of the location relayer, which
+  // is in turn bound to the browser's global context objects. Ownership is NOT
+  // returned.
+  static GeolocationProvider* GetInstance();
+
+  typedef std::map<GeolocationObserver*, GeolocationObserverOptions>
+      ObserverMap;
+
+ private:
+  friend struct DefaultSingletonTraits<GeolocationProvider>;
+  ~GeolocationProvider();
+
+  bool OnClientThread() const;
+  bool OnGeolocationThread() const;
+  void OnObserversChanged();
+  // Passes the observers' geolocation options through to the arbitrator.
+  void SetObserverOptions(const GeolocationObserverOptions& options);
+  // Notifies observers when a new position fix is available.
+  void NotifyObservers(const Geoposition& position);
+
+  // Thread
+  virtual void Init();
+  virtual void CleanUp();
+
+  scoped_refptr<base::MessageLoopProxy> client_loop_;
+
+  // Only used on client thread
+  ObserverMap observers_;
+  GURL most_recent_authorized_frame_;
+  Geoposition position_;
+
+  // Only to be used on the geolocation thread.
+  GeolocationArbitrator* arbitrator_;
+
+  DISALLOW_COPY_AND_ASSIGN(GeolocationProvider);
+};
+
+#endif  // CHROME_BROWSER_GEOLOCATION_GEOLOCATION_PROVIDER_H_
diff --git a/chrome/browser/geolocation/location_arbitrator.cc b/chrome/browser/geolocation/location_arbitrator.cc
index bf47ed7..5b5d08a 100644
--- a/chrome/browser/geolocation/location_arbitrator.cc
+++ b/chrome/browser/geolocation/location_arbitrator.cc
@@ -6,94 +6,19 @@
 
 #include <map>
 
-#include "base/logging.h"
-#include "base/non_thread_safe.h"
-#include "base/ref_counted.h"
-#include "base/scoped_ptr.h"
-#include "base/string_util.h"
-#include "base/scoped_vector.h"
-#include "chrome/browser/geolocation/access_token_store.h"
-#include "chrome/browser/geolocation/location_provider.h"
 #include "chrome/browser/profile.h"
-#include "chrome/common/geoposition.h"
-#include "chrome/common/net/url_request_context_getter.h"
-#include "googleurl/src/gurl.h"
 
 namespace {
 const char* kDefaultNetworkProviderUrl = "https://www.google.com/loc/json";
-GeolocationArbitrator* g_instance_ = NULL;
 // TODO(joth): Remove this global function pointer and update all tests to use
 // an injected ProviderFactory class instead.
 GeolocationArbitrator::LocationProviderFactoryFunction
     g_provider_factory_function_for_test = NULL;
 }  // namespace
 
-// 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 =
-    11 * base::Time::kMillisecondsPerSecond;
-
-class GeolocationArbitratorImpl
-    : public GeolocationArbitrator,
-      public LocationProviderBase::ListenerInterface,
-      public NonThreadSafe {
- public:
-  GeolocationArbitratorImpl(AccessTokenStore* access_token_store,
-                            URLRequestContextGetter* context_getter,
-                            GetTimeNow get_time_now,
-                            ProviderFactory* provider_factory);
-  virtual ~GeolocationArbitratorImpl();
-
-  // GeolocationArbitrator
-  virtual void AddObserver(GeolocationArbitrator::Delegate* delegate,
-                           const UpdateOptions& update_options);
-  virtual bool RemoveObserver(GeolocationArbitrator::Delegate* delegate);
-  virtual Geoposition GetCurrentPosition();
-  virtual void OnPermissionGranted(const GURL& requesting_frame);
-  virtual bool HasPermissionBeenGranted() const;
-
-  // ListenerInterface
-  virtual void LocationUpdateAvailable(LocationProviderBase* provider);
-
-  void OnAccessTokenStoresLoaded(
-      AccessTokenStore::AccessTokenSet access_token_store);
-
- private:
-  // Takes ownership of |provider| on entry; it will either be added to
-  // |provider_vector| or deleted on error (e.g. it fails to start).
-  void RegisterProvider(LocationProviderBase* provider,
-                        ScopedVector<LocationProviderBase>* provider_vector);
-  void StartProviders();
-
-  // 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_;
-  scoped_refptr<URLRequestContextGetter> context_getter_;
-  GetTimeNow get_time_now_;
-  scoped_refptr<ProviderFactory> provider_factory_;
-
-  ScopedVector<LocationProviderBase> providers_;
-
-  typedef std::map<GeolocationArbitrator::Delegate*, UpdateOptions> DelegateMap;
-  DelegateMap observers_;
-
-  // The current best estimate of our position.
-  Geoposition position_;
-  // The provider which supplied the current |position_|
-  const LocationProviderBase* position_provider_;
-
-  GURL most_recent_authorized_frame_;
-
-  CancelableRequestConsumer request_consumer_;
-};
 
 class DefaultLocationProviderFactory
-    : public GeolocationArbitratorImpl::ProviderFactory {
+    : public GeolocationArbitrator::ProviderFactory {
  public:
   virtual LocationProviderBase* NewNetworkLocationProvider(
       AccessTokenStore* access_token_store,
@@ -112,66 +37,48 @@ class DefaultLocationProviderFactory
   }
 };
 
-GeolocationArbitratorImpl::GeolocationArbitratorImpl(
+// 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 =
+    11 * base::Time::kMillisecondsPerSecond;
+
+GeolocationArbitrator::GeolocationArbitrator(
     AccessTokenStore* access_token_store,
     URLRequestContextGetter* context_getter,
     GetTimeNow get_time_now,
+    GeolocationObserver* observer,
     ProviderFactory* provider_factory)
     : access_token_store_(access_token_store),
       context_getter_(context_getter),
       get_time_now_(get_time_now),
+      observer_(observer),
       provider_factory_(provider_factory),
       position_provider_(NULL) {
-  DCHECK(NULL == g_instance_);
   DCHECK(GURL(kDefaultNetworkProviderUrl).is_valid());
-  g_instance_ = this;
   access_token_store_->LoadAccessTokens(
       &request_consumer_,
       NewCallback(this,
-                  &GeolocationArbitratorImpl::OnAccessTokenStoresLoaded));
-}
-
-GeolocationArbitratorImpl::~GeolocationArbitratorImpl() {
-  DCHECK(CalledOnValidThread());
-  DCHECK(observers_.empty()) << "Not all observers have unregistered";
-  DCHECK(this == g_instance_);
-  g_instance_ = NULL;
+                  &GeolocationArbitrator::OnAccessTokenStoresLoaded));
 }
 
-void GeolocationArbitratorImpl::AddObserver(
-    GeolocationArbitrator::Delegate* delegate,
-    const UpdateOptions& update_options) {
-  DCHECK(CalledOnValidThread());
-  observers_[delegate] = update_options;
-  StartProviders();
-  if (position_.IsInitialized()) {
-    delegate->OnLocationUpdate(position_);
-  }
-}
-
-bool GeolocationArbitratorImpl::RemoveObserver(
-    GeolocationArbitrator::Delegate* delegate) {
-  DCHECK(CalledOnValidThread());
-  size_t remove = observers_.erase(delegate);
-  if (observers_.empty()) {
-    // TODO(joth): Delayed callback to linger before stopping providers.
-    for (ScopedVector<LocationProviderBase>::iterator i = providers_.begin();
-        i != providers_.end(); ++i) {
-      (*i)->StopProvider();
-    }
-  } else {  // The high accuracy requirement may have changed.
-    StartProviders();
-  }
-  return remove > 0;
-}
-
-Geoposition GeolocationArbitratorImpl::GetCurrentPosition() {
-  return position_;
+GeolocationArbitrator::~GeolocationArbitrator() {
 }
 
-void GeolocationArbitratorImpl::OnPermissionGranted(
+GeolocationArbitrator* GeolocationArbitrator::Create(
+    GeolocationObserver* observer) {
+  // Construct the arbitrator using default token store and url context. We
+  // get the url context getter from the default profile as it's not
+  // particularly important which profile it is attached to: the network
+  // request implementation disables cookies anyhow.
+  return new GeolocationArbitrator(NewChromePrefsAccessTokenStore(),
+                                   Profile::GetDefaultRequestContext(),
+                                   base::Time::Now,
+                                   observer,
+                                   new DefaultLocationProviderFactory);
+}
+
+void GeolocationArbitrator::OnPermissionGranted(
     const GURL& requesting_frame) {
-  DCHECK(CalledOnValidThread());
   most_recent_authorized_frame_ = requesting_frame;
   for (ScopedVector<LocationProviderBase>::iterator i = providers_.begin();
       i != providers_.end(); ++i) {
@@ -179,33 +86,22 @@ void GeolocationArbitratorImpl::OnPermissionGranted(
   }
 }
 
-bool GeolocationArbitratorImpl::HasPermissionBeenGranted() const {
-  DCHECK(CalledOnValidThread());
-  return most_recent_authorized_frame_.is_valid();
+void GeolocationArbitrator::SetObserverOptions(
+    const GeolocationObserverOptions& options) {
+  // Stash options incase OnAccessTokenStoresLoaded has not yet been called
+  // (in which case |providers_| will be empty).
+  current_provider_options_ = options;
+  StartProviders();
 }
 
-void GeolocationArbitratorImpl::LocationUpdateAvailable(
-    LocationProviderBase* provider) {
-  DCHECK(CalledOnValidThread());
-  DCHECK(provider);
-  Geoposition new_position;
-  provider->GetPosition(&new_position);
-  DCHECK(new_position.IsInitialized());
-  if (!IsNewPositionBetter(position_, new_position,
-                           provider == position_provider_))
-    return;
-  position_ = new_position;
-  position_provider_ = provider;
-  DelegateMap::const_iterator it = observers_.begin();
-  while (it != observers_.end()) {
-    // Advance iterator before callback to guard against synchronous unregister.
-    Delegate* delegate = it->first;
-    ++it;
-    delegate->OnLocationUpdate(position_);
+void GeolocationArbitrator::StartProviders() {
+  for (ScopedVector<LocationProviderBase>::iterator i = providers_.begin();
+       i != providers_.end(); ++i) {
+    (*i)->StartProvider(current_provider_options_.use_high_accuracy);
   }
 }
 
-void GeolocationArbitratorImpl::OnAccessTokenStoresLoaded(
+void GeolocationArbitrator::OnAccessTokenStoresLoaded(
     AccessTokenStore::AccessTokenSet access_token_set) {
   DCHECK(providers_.empty())
       << "OnAccessTokenStoresLoaded : has existing location "
@@ -219,37 +115,37 @@ void GeolocationArbitratorImpl::OnAccessTokenStoresLoaded(
     RegisterProvider(
         provider_factory_->NewNetworkLocationProvider(
             access_token_store_.get(), context_getter_.get(),
-            i->first, i->second),
-        &providers_);
+            i->first, i->second));
   }
-  RegisterProvider(provider_factory_->NewSystemLocationProvider(),
-                   &providers_);
+  RegisterProvider(provider_factory_->NewSystemLocationProvider());
   StartProviders();
 }
 
-void GeolocationArbitratorImpl::RegisterProvider(
-    LocationProviderBase* provider,
-    ScopedVector<LocationProviderBase>* provider_vector) {
-  DCHECK(provider_vector);
+void GeolocationArbitrator::RegisterProvider(
+    LocationProviderBase* provider) {
   if (!provider)
     return;
   provider->RegisterListener(this);
   if (most_recent_authorized_frame_.is_valid())
     provider->OnPermissionGranted(most_recent_authorized_frame_);
-  provider_vector->push_back(provider);
+  providers_->push_back(provider);
 }
 
-void GeolocationArbitratorImpl::StartProviders() {
-  DCHECK(CalledOnValidThread());
-  const bool high_accuracy_required =
-      UpdateOptions::Collapse(observers_).use_high_accuracy;
-  for (ScopedVector<LocationProviderBase>::iterator i = providers_.begin();
-      i != providers_.end(); ++i) {
-    (*i)->StartProvider(high_accuracy_required);
-  }
+void GeolocationArbitrator::LocationUpdateAvailable(
+    LocationProviderBase* provider) {
+  DCHECK(provider);
+  Geoposition new_position;
+  provider->GetPosition(&new_position);
+  DCHECK(new_position.IsInitialized());
+  if (!IsNewPositionBetter(position_, new_position,
+                           provider == position_provider_))
+    return;
+  position_provider_ = provider;
+  position_ = new_position;
+  observer_->OnLocationUpdate(position_);
 }
 
-bool GeolocationArbitratorImpl::IsNewPositionBetter(
+bool GeolocationArbitrator::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,
@@ -275,40 +171,14 @@ bool GeolocationArbitratorImpl::IsNewPositionBetter(
   return false;
 }
 
-GeolocationArbitrator* GeolocationArbitrator::Create(
-    AccessTokenStore* access_token_store,
-    URLRequestContextGetter* context_getter,
-    GetTimeNow get_time_now,
-    ProviderFactory* provider_factory) {
-  return new GeolocationArbitratorImpl(access_token_store, context_getter,
-                                       get_time_now, provider_factory);
+bool GeolocationArbitrator::HasPermissionBeenGranted() const {
+  return most_recent_authorized_frame_.is_valid();
 }
 
-GeolocationArbitrator* GeolocationArbitrator::GetInstance() {
-  if (!g_instance_) {
-    // Construct the arbitrator using default token store and url context. We
-    // get the url context getter from the default profile as it's not
-    // particularly important which profile it is attached to: the network
-    // request implementation disables cookies anyhow.
-    Create(NewChromePrefsAccessTokenStore(),
-           Profile::GetDefaultRequestContext(),
-           base::Time::Now,
-           new DefaultLocationProviderFactory);
-    DCHECK(g_instance_);
-  }
-  return g_instance_;
+GeolocationArbitrator::ProviderFactory::~ProviderFactory() {
 }
 
 void GeolocationArbitrator::SetProviderFactoryForTest(
       LocationProviderFactoryFunction factory_function) {
   g_provider_factory_function_for_test = factory_function;
 }
-
-GeolocationArbitrator::GeolocationArbitrator() {
-}
-
-GeolocationArbitrator::~GeolocationArbitrator() {
-}
-
-GeolocationArbitrator::ProviderFactory::~ProviderFactory() {
-}
diff --git a/chrome/browser/geolocation/location_arbitrator.h b/chrome/browser/geolocation/location_arbitrator.h
index a4d166f..590264f 100644
--- a/chrome/browser/geolocation/location_arbitrator.h
+++ b/chrome/browser/geolocation/location_arbitrator.h
@@ -7,8 +7,14 @@
 #pragma once
 
 #include "base/string16.h"
+#include "base/scoped_vector.h"
 #include "base/time.h"
-#include "base/ref_counted.h"
+#include "chrome/browser/geolocation/access_token_store.h"
+#include "chrome/browser/geolocation/location_provider.h"
+#include "chrome/browser/geolocation/geolocation_observer.h"
+#include "chrome/common/geoposition.h"
+#include "chrome/common/net/url_request_context_getter.h"
+#include "googleurl/src/gurl.h"
 
 class AccessTokenStore;
 class GURL;
@@ -16,15 +22,11 @@ class LocationProviderBase;
 class URLRequestContextGetter;
 struct Geoposition;
 
-// This is the main API to the geolocaiton subsystem. Typically the application
-// will hold a single instance of this class, and can register multiple
-// observers which will be notified of location updates. Underlying location
-// provider(s) will only be enabled whilst there is at least one observer
-// registered.
+
 // 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 GeolocationArbitrator : public base::RefCounted<GeolocationArbitrator> {
+class 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
@@ -50,82 +52,27 @@ class GeolocationArbitrator : public base::RefCounted<GeolocationArbitrator> {
     virtual ~ProviderFactory();
   };
 
-  // Creates and returns a new instance of the location arbitrator. Allows
-  // injection of dependencies, for testing.
-  static GeolocationArbitrator* Create(
-      AccessTokenStore* access_token_store,
-      URLRequestContextGetter* context_getter,
-      GetTimeNow get_time_now,
-      ProviderFactory* provider_factory);
+  GeolocationArbitrator(AccessTokenStore* access_token_store,
+                        URLRequestContextGetter* context_getter,
+                        GetTimeNow get_time_now,
+                        GeolocationObserver* observer,
+                        ProviderFactory* provider_factory);
+  ~GeolocationArbitrator();
 
-  // Gets a pointer to the singleton instance of the location arbitrator, which
-  // is in turn bound to the browser's global context objects. Ownership is NOT
-  // returned.
-  static GeolocationArbitrator* GetInstance();
+  static GeolocationArbitrator* Create(GeolocationObserver* observer);
 
-  class Delegate {
-   public:
-    // This will be called whenever the 'best available' location is updated,
-    // or when an error is encountered meaning no location data will be
-    // available in the forseeable future.
-    virtual void OnLocationUpdate(const Geoposition& position) = 0;
-
-   protected:
-    Delegate() {}
-    virtual ~Delegate() {}
-
-   private:
-    DISALLOW_COPY_AND_ASSIGN(Delegate);
-  };
-  struct UpdateOptions {
-    UpdateOptions() : use_high_accuracy(false) {}
-    explicit UpdateOptions(bool high_accuracy)
-        : use_high_accuracy(high_accuracy) {}
-
-    // Given a map<ANYTHING, UpdateOptions> this function will iterate the map
-    // and collapse all the options found to a single instance that satisfies
-    // them all.
-    template <typename MAP>
-    static UpdateOptions Collapse(const MAP& options_map) {
-      for (typename MAP::const_iterator it = options_map.begin();
-           it != options_map.end(); ++it) {
-        if (it->second.use_high_accuracy)
-          return UpdateOptions(true);
-      }
-      return UpdateOptions(false);
-    }
-
-    bool use_high_accuracy;
-  };
-
-  // Must be called from the same thread as the arbitrator was created on.
-  // The update options passed are used as a 'hint' for the provider preferences
-  // for this particular observer, however the delegate could receive callbacks
-  // for best available locations from any active provider whilst it is
-  // registered.
-  // If an existing delegate is added a second time it's options are updated
-  // but only a single call to RemoveObserver() is required to remove it.
-  virtual void AddObserver(Delegate* delegate,
-                           const UpdateOptions& update_options) = 0;
-  // Remove a previously registered observer. No-op if not previously registered
-  // via AddObserver(). Returns true if the observer was removed.
-  virtual bool RemoveObserver(Delegate* delegate) = 0;
-
-  // Returns the current position estimate, or an uninitialized position
-  // if none is yet available. Once initialized, this will always match
-  // the most recent observer notification (via Delegate::OnLocationUpdate()).
-  virtual Geoposition GetCurrentPosition() = 0;
+  void SetObserverOptions(const GeolocationObserverOptions& options);
 
   // 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.
-  virtual void OnPermissionGranted(const GURL& requesting_frame) = 0;
+  void OnPermissionGranted(const GURL& requesting_frame);
 
   // Returns true if this arbitrator has received at least one call to
   // OnPermissionGranted().
-  virtual bool HasPermissionBeenGranted() const = 0;
+  bool HasPermissionBeenGranted() const;
 
   // For testing, a factory function can be set which will be used to create
   // a specified test provider. Pass NULL to reset to the default behavior.
@@ -135,13 +82,37 @@ class GeolocationArbitrator : public base::RefCounted<GeolocationArbitrator> {
   static void SetProviderFactoryForTest(
       LocationProviderFactoryFunction factory_function);
 
- protected:
-  friend class base::RefCounted<GeolocationArbitrator>;
-  GeolocationArbitrator();
-  // RefCounted object; no not delete directly.
-  virtual ~GeolocationArbitrator();
+  // ListenerInterface
+  virtual void LocationUpdateAvailable(LocationProviderBase* provider);
 
  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);
+  void StartProviders();
+  // 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_;
+  scoped_refptr<URLRequestContextGetter> context_getter_;
+  GetTimeNow get_time_now_;
+  GeolocationObserver* observer_;
+  scoped_refptr<ProviderFactory> provider_factory_;
+  ScopedVector<LocationProviderBase> providers_;
+  GeolocationObserverOptions current_provider_options_;
+  // The provider which supplied the current |position_|
+  const LocationProviderBase* position_provider_;
+  GURL most_recent_authorized_frame_;
+  CancelableRequestConsumer request_consumer_;
+  // The current best estimate of our position.
+  Geoposition position_;
+
   DISALLOW_COPY_AND_ASSIGN(GeolocationArbitrator);
 };
 
diff --git a/chrome/browser/geolocation/location_arbitrator_unittest.cc b/chrome/browser/geolocation/location_arbitrator_unittest.cc
index 76351ac..888254c 100644
--- a/chrome/browser/geolocation/location_arbitrator_unittest.cc
+++ b/chrome/browser/geolocation/location_arbitrator_unittest.cc
@@ -6,6 +6,7 @@
 
 #include "base/scoped_ptr.h"
 #include "chrome/browser/geolocation/fake_access_token_store.h"
+#include "chrome/browser/geolocation/geolocation_observer.h"
 #include "chrome/browser/geolocation/location_provider.h"
 #include "chrome/browser/geolocation/mock_location_provider.h"
 #include "chrome/common/geoposition.h"
@@ -13,7 +14,7 @@
 
 namespace {
 
-class MockLocationObserver : public GeolocationArbitrator::Delegate {
+class MockLocationObserver : public GeolocationObserver {
  public:
   void InvalidateLastPosition() {
     last_position_.latitude = 100;
@@ -85,18 +86,23 @@ class GeolocationLocationArbitratorTest : public testing::Test {
  protected:
   virtual void SetUp() {
     access_token_store_ = new FakeAccessTokenStore;
+    observer_.reset(new MockLocationObserver());
     providers_ = new MockProviderFactory();
-    arbitrator_ = GeolocationArbitrator::Create(access_token_store_.get(),
-                                                NULL, GetTimeNow, providers_);
+    arbitrator_.reset(new GeolocationArbitrator(access_token_store_.get(),
+                                                NULL,
+                                                GetTimeNow,
+                                                observer_.get(),
+                                                providers_));
   }
 
   virtual void TearDown() {
+    providers_ = NULL;
   }
 
   void CheckLastPositionInfo(double latitude,
                              double longitude,
                              double accuracy) {
-    Geoposition geoposition = arbitrator_->GetCurrentPosition();
+    Geoposition geoposition = observer_->last_position_;
     EXPECT_TRUE(geoposition.IsValidFix());
     EXPECT_DOUBLE_EQ(latitude, geoposition.latitude);
     EXPECT_DOUBLE_EQ(longitude, geoposition.longitude);
@@ -111,13 +117,15 @@ class GeolocationLocationArbitratorTest : public testing::Test {
 
   scoped_refptr<FakeAccessTokenStore> access_token_store_;
   scoped_refptr<MockProviderFactory> providers_;
-  scoped_refptr<GeolocationArbitrator> arbitrator_;
+  scoped_ptr<MockLocationObserver> observer_;
+  scoped_ptr<GeolocationArbitrator> arbitrator_;
+  MessageLoop loop_;
 };
 
 TEST_F(GeolocationLocationArbitratorTest, CreateDestroy) {
   EXPECT_TRUE(access_token_store_);
   EXPECT_TRUE(arbitrator_ != NULL);
-  arbitrator_ = NULL;
+  arbitrator_.reset();
   SUCCEED();
 }
 
@@ -137,9 +145,6 @@ TEST_F(GeolocationLocationArbitratorTest, NormalUsage) {
 
   EXPECT_TRUE(access_token_store_->access_token_set_.empty());
   EXPECT_TRUE(access_token_store_->request_);
-  MockLocationObserver observer;
-  arbitrator_->AddObserver(&observer, GeolocationArbitrator::UpdateOptions());
-
   EXPECT_TRUE(access_token_store_->access_token_set_.empty());
   ASSERT_TRUE(access_token_store_->request_);
 
@@ -151,13 +156,14 @@ TEST_F(GeolocationLocationArbitratorTest, NormalUsage) {
   EXPECT_TRUE(providers_->cell_->has_listeners());
   EXPECT_EQ(MockLocationProvider::LOW_ACCURACY, providers_->cell_->state_);
   EXPECT_EQ(MockLocationProvider::LOW_ACCURACY, providers_->gps_->state_);
-  EXPECT_FALSE(observer.last_position_.IsInitialized());
+  EXPECT_FALSE(observer_->last_position_.IsInitialized());
 
   SetReferencePosition(&providers_->cell_->position_);
-  providers_->cell_->UpdateListeners();
-  EXPECT_TRUE(observer.last_position_.IsInitialized());
+  providers_->cell_->HandlePositionChanged();
+
+  EXPECT_TRUE(observer_->last_position_.IsInitialized());
   EXPECT_EQ(providers_->cell_->position_.latitude,
-            observer.last_position_.latitude);
+            observer_->last_position_.latitude);
 
   EXPECT_FALSE(
       providers_->cell_->permission_granted_url_.is_valid());
@@ -169,120 +175,62 @@ TEST_F(GeolocationLocationArbitratorTest, NormalUsage) {
       providers_->cell_->permission_granted_url_.is_valid());
   EXPECT_EQ(frame_url,
             providers_->cell_->permission_granted_url_);
-
-  EXPECT_TRUE(arbitrator_->RemoveObserver(&observer));
-}
-
-TEST_F(GeolocationLocationArbitratorTest, MultipleListener) {
-  MockLocationObserver observer1;
-  arbitrator_->AddObserver(&observer1, GeolocationArbitrator::UpdateOptions());
-  MockLocationObserver observer2;
-  arbitrator_->AddObserver(&observer2, GeolocationArbitrator::UpdateOptions());
-
-  access_token_store_->NotifyDelegateTokensLoaded();
-  ASSERT_TRUE(providers_->cell_);
-  EXPECT_FALSE(observer1.last_position_.IsInitialized());
-  EXPECT_FALSE(observer2.last_position_.IsInitialized());
-
-  SetReferencePosition(&providers_->cell_->position_);
-  providers_->cell_->UpdateListeners();
-  EXPECT_TRUE(observer1.last_position_.IsInitialized());
-  EXPECT_TRUE(observer2.last_position_.IsInitialized());
-
-  // Add third observer, and remove the first.
-  MockLocationObserver observer3;
-  arbitrator_->AddObserver(&observer3, GeolocationArbitrator::UpdateOptions());
-  EXPECT_TRUE(arbitrator_->RemoveObserver(&observer1));
-  observer1.InvalidateLastPosition();
-  observer2.InvalidateLastPosition();
-  observer3.InvalidateLastPosition();
-
-  providers_->cell_->UpdateListeners();
-  EXPECT_FALSE(observer1.last_position_.IsInitialized());
-  EXPECT_TRUE(observer2.last_position_.IsInitialized());
-  EXPECT_TRUE(observer3.last_position_.IsInitialized());
-
-  EXPECT_TRUE(arbitrator_->RemoveObserver(&observer2));
-  EXPECT_TRUE(arbitrator_->RemoveObserver(&observer3));
 }
 
-TEST_F(GeolocationLocationArbitratorTest,
-       MultipleAddObserverCallsFromSameListener) {
-  MockLocationObserver observer;
-  arbitrator_->AddObserver(
-      &observer, GeolocationArbitrator::UpdateOptions(false));
+TEST_F(GeolocationLocationArbitratorTest, SetObserverOptions) {
   access_token_store_->NotifyDelegateTokensLoaded();
   ASSERT_TRUE(providers_->cell_);
   ASSERT_TRUE(providers_->gps_);
-  EXPECT_EQ(MockLocationProvider::LOW_ACCURACY, providers_->cell_->state_);
-  EXPECT_EQ(MockLocationProvider::LOW_ACCURACY, providers_->gps_->state_);
-  arbitrator_->AddObserver(
-      &observer, GeolocationArbitrator::UpdateOptions(true));
+  arbitrator_->SetObserverOptions(GeolocationObserverOptions(true));
   EXPECT_EQ(MockLocationProvider::HIGH_ACCURACY, providers_->cell_->state_);
   EXPECT_EQ(MockLocationProvider::HIGH_ACCURACY, providers_->gps_->state_);
-  arbitrator_->AddObserver(
-      &observer, GeolocationArbitrator::UpdateOptions(false));
+  arbitrator_->SetObserverOptions(GeolocationObserverOptions(false));
   EXPECT_EQ(MockLocationProvider::LOW_ACCURACY, providers_->cell_->state_);
   EXPECT_EQ(MockLocationProvider::LOW_ACCURACY, providers_->gps_->state_);
-  EXPECT_TRUE(arbitrator_->RemoveObserver(&observer));
-  EXPECT_EQ(MockLocationProvider::STOPPED, providers_->cell_->state_);
-  EXPECT_EQ(MockLocationProvider::STOPPED, providers_->gps_->state_);
-  EXPECT_FALSE(arbitrator_->RemoveObserver(&observer));
 }
 
-TEST_F(GeolocationLocationArbitratorTest, RegistrationAfterFixArrives) {
-  MockLocationObserver observer1;
-  arbitrator_->AddObserver(&observer1, GeolocationArbitrator::UpdateOptions());
-
+TEST_F(GeolocationLocationArbitratorTest, SetObserverOptionsAfterFixArrives) {
   access_token_store_->NotifyDelegateTokensLoaded();
   ASSERT_TRUE(providers_->cell_);
-  EXPECT_FALSE(observer1.last_position_.IsInitialized());
+  ASSERT_TRUE(providers_->gps_);
+  EXPECT_EQ(MockLocationProvider::LOW_ACCURACY, providers_->cell_->state_);
+  EXPECT_EQ(MockLocationProvider::LOW_ACCURACY, providers_->gps_->state_);
   SetReferencePosition(&providers_->cell_->position_);
-  providers_->cell_->UpdateListeners();
-  EXPECT_TRUE(observer1.last_position_.IsValidFix());
-
-  MockLocationObserver observer2;
-  EXPECT_FALSE(observer2.last_position_.IsValidFix());
-  arbitrator_->AddObserver(&observer2, GeolocationArbitrator::UpdateOptions());
-  EXPECT_TRUE(observer2.last_position_.IsValidFix());
-
-  EXPECT_TRUE(arbitrator_->RemoveObserver(&observer1));
-  EXPECT_TRUE(arbitrator_->RemoveObserver(&observer2));
+  providers_->cell_->HandlePositionChanged();
+  EXPECT_EQ(MockLocationProvider::LOW_ACCURACY, providers_->cell_->state_);
+  EXPECT_EQ(MockLocationProvider::LOW_ACCURACY, providers_->gps_->state_);
+  arbitrator_->SetObserverOptions(GeolocationObserverOptions(true));
+  EXPECT_EQ(MockLocationProvider::HIGH_ACCURACY, providers_->cell_->state_);
+  EXPECT_EQ(MockLocationProvider::HIGH_ACCURACY, providers_->gps_->state_);
 }
 
-
 TEST_F(GeolocationLocationArbitratorTest, Arbitration) {
-  // No position so far
-  EXPECT_FALSE(arbitrator_->GetCurrentPosition().IsInitialized());
-  MockLocationObserver observer;
-  arbitrator_->AddObserver(&observer,
-                           GeolocationArbitrator::UpdateOptions(true));
   access_token_store_->NotifyDelegateTokensLoaded();
   ASSERT_TRUE(providers_->cell_);
   ASSERT_TRUE(providers_->gps_);
 
   SetPositionFix(&providers_->cell_->position_, 1, 2, 150, GetTimeNow());
-  providers_->cell_->UpdateListeners();
+  providers_->cell_->HandlePositionChanged();
 
   // First position available
-  EXPECT_TRUE(arbitrator_->GetCurrentPosition().IsValidFix());
+  EXPECT_TRUE(observer_->last_position_.IsValidFix());
   CheckLastPositionInfo(1, 2, 150);
 
   SetPositionFix(&providers_->gps_->position_, 3, 4, 50, GetTimeNow());
-  providers_->gps_->UpdateListeners();
+  providers_->gps_->HandlePositionChanged();
 
   // More accurate fix available
   CheckLastPositionInfo(3, 4, 50);
 
   SetPositionFix(&providers_->cell_->position_, 5, 6, 150, GetTimeNow());
-  providers_->cell_->UpdateListeners();
+  providers_->cell_->HandlePositionChanged();
 
   // New fix is available but it's less accurate, older fix should be kept.
   CheckLastPositionInfo(3, 4, 50);
 
   // Advance time, and notify once again
   AdvanceTimeNow(SwitchOnFreshnessCliff());
-  providers_->cell_->UpdateListeners();
+  providers_->cell_->HandlePositionChanged();
 
   // New fix is available, less accurate but fresher
   CheckLastPositionInfo(5, 6, 150);
@@ -291,14 +239,14 @@ TEST_F(GeolocationLocationArbitratorTest, Arbitration) {
   AdvanceTimeNow(SwitchOnFreshnessCliff());
   SetPositionFix(&providers_->cell_->position_, 5.676731, 139.629385, 1000,
                  GetTimeNow());
-  providers_->cell_->UpdateListeners();
+  providers_->cell_->HandlePositionChanged();
   CheckLastPositionInfo(5.676731, 139.629385, 1000);
 
   // 15 secs later, step outside. Switches to gps signal.
   AdvanceTimeNow(base::TimeDelta::FromSeconds(15));
   SetPositionFix(&providers_->gps_->position_, 3.5676457, 139.629198, 50,
                  GetTimeNow());
-  providers_->gps_->UpdateListeners();
+  providers_->gps_->HandlePositionChanged();
   CheckLastPositionInfo(3.5676457, 139.629198, 50);
 
   // 5 mins later switch cells while walking. Stay on gps.
@@ -307,15 +255,15 @@ TEST_F(GeolocationLocationArbitratorTest, Arbitration) {
                  GetTimeNow());
   SetPositionFix(&providers_->gps_->position_, 3.5677675, 139.632314, 50,
                  GetTimeNow());
-  providers_->cell_->UpdateListeners();
-  providers_->gps_->UpdateListeners();
+  providers_->cell_->HandlePositionChanged();
+  providers_->gps_->HandlePositionChanged();
   CheckLastPositionInfo(3.5677675, 139.632314, 50);
 
   // Ride train and gps signal degrades slightly. Stay on fresher gps
   AdvanceTimeNow(base::TimeDelta::FromMinutes(5));
   SetPositionFix(&providers_->gps_->position_, 3.5679026, 139.634777, 300,
                  GetTimeNow());
-  providers_->gps_->UpdateListeners();
+  providers_->gps_->HandlePositionChanged();
   CheckLastPositionInfo(3.5679026, 139.634777, 300);
 
   // 14 minutes later
@@ -325,12 +273,12 @@ TEST_F(GeolocationLocationArbitratorTest, Arbitration) {
   // oscillating.
   SetPositionFix(&providers_->gps_->position_, 3.5659005, 139.682579, 300,
                  GetTimeNow());
-  providers_->gps_->UpdateListeners();
+  providers_->gps_->HandlePositionChanged();
 
   AdvanceTimeNow(base::TimeDelta::FromSeconds(7));
   SetPositionFix(&providers_->cell_->position_, 3.5689579, 139.691420, 1000,
                  GetTimeNow());
-  providers_->cell_->UpdateListeners();
+  providers_->cell_->HandlePositionChanged();
   CheckLastPositionInfo(3.5659005, 139.682579, 300);
 
   // 1 minute later
@@ -339,10 +287,10 @@ TEST_F(GeolocationLocationArbitratorTest, Arbitration) {
   // Enter tunnel. Stay on fresher gps for a moment.
   SetPositionFix(&providers_->cell_->position_, 3.5657078, 139.68922, 300,
                  GetTimeNow());
-  providers_->cell_->UpdateListeners();
+  providers_->cell_->HandlePositionChanged();
   SetPositionFix(&providers_->gps_->position_, 3.5657104, 139.690341, 300,
                  GetTimeNow());
-  providers_->gps_->UpdateListeners();
+  providers_->gps_->HandlePositionChanged();
   CheckLastPositionInfo(3.5657104, 139.690341, 300);
 
   // 2 minutes later
@@ -350,10 +298,8 @@ TEST_F(GeolocationLocationArbitratorTest, Arbitration) {
   // Arrive in station. Cell moves but GPS is stale. Switch to fresher cell.
   SetPositionFix(&providers_->cell_->position_, 3.5658700, 139.069979, 1000,
                  GetTimeNow());
-  providers_->cell_->UpdateListeners();
+  providers_->cell_->HandlePositionChanged();
   CheckLastPositionInfo(3.5658700, 139.069979, 1000);
-
-  EXPECT_TRUE(arbitrator_->RemoveObserver(&observer));
 }
 
 }  // namespace
diff --git a/chrome/browser/geolocation/mock_location_provider.cc b/chrome/browser/geolocation/mock_location_provider.cc
index baa653b..8d3a73f 100644
--- a/chrome/browser/geolocation/mock_location_provider.cc
+++ b/chrome/browser/geolocation/mock_location_provider.cc
@@ -10,13 +10,19 @@
 #include "base/compiler_specific.h"
 #include "base/logging.h"
 #include "base/message_loop.h"
+#include "base/message_loop_proxy.h"
 #include "base/task.h"
 
+// The provider will always be destroyed before the thread it runs on, so ref
+// counting is not required.
+DISABLE_RUNNABLE_METHOD_REFCOUNT(MockLocationProvider);
+
 MockLocationProvider* MockLocationProvider::instance_ = NULL;
 
 MockLocationProvider::MockLocationProvider(MockLocationProvider** self_ref)
     : state_(STOPPED),
-      self_ref_(self_ref) {
+      self_ref_(self_ref),
+      provider_loop_(base::MessageLoopProxy::CreateForCurrentThread()) {
   CHECK(self_ref_);
   CHECK(*self_ref_ == NULL);
   *self_ref_ = this;
@@ -27,6 +33,18 @@ MockLocationProvider::~MockLocationProvider() {
   *self_ref_ = NULL;
 }
 
+void MockLocationProvider::HandlePositionChanged() {
+  if (provider_loop_->BelongsToCurrentThread()) {
+    // The location arbitrator unit tests rely on this method running
+    // synchronously.
+    UpdateListeners();
+  } else {
+    Task* task = NewRunnableMethod(
+        this, &MockLocationProvider::HandlePositionChanged);
+    provider_loop_->PostTask(FROM_HERE, task);
+  }
+}
+
 bool MockLocationProvider::StartProvider(bool high_accuracy) {
   state_ = high_accuracy ? HIGH_ACCURACY : LOW_ACCURACY;
   return true;
@@ -84,7 +102,7 @@ class AutoMockLocationProvider : public MockLocationProvider {
       listeners_updated_ = true;
       MessageLoop::current()->PostTask(
           FROM_HERE, task_factory_.NewRunnableMethod(
-              &MockLocationProvider::UpdateListeners));
+          &MockLocationProvider::HandlePositionChanged));
     }
   }
 
diff --git a/chrome/browser/geolocation/mock_location_provider.h b/chrome/browser/geolocation/mock_location_provider.h
index 1fcad60..ced5d89 100644
--- a/chrome/browser/geolocation/mock_location_provider.h
+++ b/chrome/browser/geolocation/mock_location_provider.h
@@ -6,6 +6,10 @@
 #define CHROME_BROWSER_GEOLOCATION_MOCK_LOCATION_PROVIDER_H_
 #pragma once
 
+
+#include "base/ref_counted.h"
+#include "base/scoped_ptr.h"
+#include "base/thread.h"
 #include "chrome/browser/geolocation/location_provider.h"
 #include "chrome/common/geoposition.h"
 #include "googleurl/src/gurl.h"
@@ -18,7 +22,8 @@ class MockLocationProvider : public LocationProviderBase {
   explicit MockLocationProvider(MockLocationProvider** self_ref);
   ~MockLocationProvider();
 
-  using LocationProviderBase::UpdateListeners;
+  // Updates listeners with the new position.
+  void HandlePositionChanged();
 
   // LocationProviderBase implementation.
   virtual bool StartProvider(bool high_accuracy);
@@ -31,6 +36,8 @@ class MockLocationProvider : public LocationProviderBase {
   GURL permission_granted_url_;
   MockLocationProvider** self_ref_;
 
+  scoped_refptr<base::MessageLoopProxy> provider_loop_;
+
   // Set when an instance of the mock is created via a factory function.
   static MockLocationProvider* instance_;
 
diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi
index b28ecb5..c5a1f11 100644
--- a/chrome/chrome_browser.gypi
+++ b/chrome/chrome_browser.gypi
@@ -1595,16 +1595,19 @@
         'browser/geolocation/gateway_data_provider_common.h',
         'browser/geolocation/gateway_data_provider_win.cc',
         'browser/geolocation/gateway_data_provider_win.h',
+        'browser/geolocation/geolocation_content_settings_map.cc',
+        'browser/geolocation/geolocation_content_settings_map.h',
         'browser/geolocation/geolocation_dispatcher_host.cc',
         'browser/geolocation/geolocation_dispatcher_host.h',
+        'browser/geolocation/geolocation_exceptions_table_model.cc',
+        'browser/geolocation/geolocation_exceptions_table_model.h',
+        'browser/geolocation/geolocation_observer.h',
         'browser/geolocation/geolocation_permission_context.cc',
         'browser/geolocation/geolocation_permission_context.h',
         'browser/geolocation/geolocation_prefs.cc',
         'browser/geolocation/geolocation_prefs.h',
-        'browser/geolocation/geolocation_content_settings_map.cc',
-        'browser/geolocation/geolocation_content_settings_map.h',
-        'browser/geolocation/geolocation_exceptions_table_model.cc',
-        'browser/geolocation/geolocation_exceptions_table_model.h',
+        'browser/geolocation/geolocation_provider.cc',
+        'browser/geolocation/geolocation_provider.h',
         'browser/geolocation/geolocation_settings_state.cc',
         'browser/geolocation/geolocation_settings_state.h',
         'browser/geolocation/gps_location_provider_linux.cc',