Gets rid of the LocationArbitrator interface, in preference for LocationProvider.

device/geolocation/location_arbitrator_impl.cc

Index: device/geolocation/location_arbitrator_impl.cc
diff --git a/device/geolocation/location_arbitrator_impl.cc b/device/geolocation/location_arbitrator_impl.cc
index a61a4a3ab22a867fba5d272f7d9961b12776bf23..866f4b8f3b40d61f173b539ed410b995d7222528 100644
--- a/device/geolocation/location_arbitrator_impl.cc
+++ b/device/geolocation/location_arbitrator_impl.cc
@@ -5,6 +5,8 @@
 #include "device/geolocation/location_arbitrator_impl.h"
 
 #include <map>
+#include <memory>
+#include <utility>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
@@ -51,7 +53,7 @@ void LocationArbitratorImpl::OnPermissionGranted() {
     provider->OnPermissionGranted();
 }
 
-void LocationArbitratorImpl::StartProviders(bool enable_high_accuracy) {
+bool LocationArbitratorImpl::StartProvider(bool enable_high_accuracy) {
   // Stash options as OnAccessTokenStoresLoaded has not yet been called.
   is_running_ = true;
   enable_high_accuracy_ = enable_high_accuracy;
@@ -67,26 +69,27 @@ void LocationArbitratorImpl::StartProviders(bool enable_high_accuracy) {
           base::Bind(&LocationArbitratorImpl::OnAccessTokenStoresLoaded,
                      base::Unretained(this)));
       access_token_store->LoadAccessTokens(token_store_callback_.callback());
-      return;
+      return true;
     }
   }
-  DoStartProviders();
+  return DoStartProviders();
 }
 
-void LocationArbitratorImpl::DoStartProviders() {
+bool LocationArbitratorImpl::DoStartProviders() {
   if (providers_.empty()) {
     // If no providers are available, we report an error to avoid
     // callers waiting indefinitely for a reply.
     Geoposition position;
     position.error_code = Geoposition::ERROR_CODE_PERMISSION_DENIED;
     arbitrator_update_callback_.Run(position);
-    return;
+    return false;
   }
   for (const auto& provider : providers_)
     provider->StartProvider(enable_high_accuracy_);

[Kevin M, 2016/08/15 19:12:46]

This is throwing away the return value of StartProvider... intentional?

[CJ, 2016/08/15 20:11:51]

Done.

+  return true;
 }
 
-void LocationArbitratorImpl::StopProviders() {
+void LocationArbitratorImpl::StopProvider() {
   // Reset the reference location state (provider+position)
   // so that future starts use fresh locations from
   // the newly constructed providers.
@@ -140,6 +143,15 @@ void LocationArbitratorImpl::OnLocationUpdate(const LocationProvider* provider,
   arbitrator_update_callback_.Run(position_);
 }
 
+void LocationArbitratorImpl::GetPosition(Geoposition* position) {

[Kevin M, 2016/08/15 19:12:46]

Change this to return a const Geoposition& and just return *position_ directly.
This is more syntactically pleasant to read, plus it gives the caller the
opportunity to avoid making a copy by just accessing fields from the reference.

[CJ, 2016/08/15 20:11:51]

Yeah I was wondering if I should refactor this. I wasn't sure if there was a
good reason to do it the way it is now.

+  *position = position_;
+}
+
+void LocationArbitratorImpl::SetUpdateCallback(
+    const LocationProviderUpdateCallback& callback) {

[Kevin M, 2016/08/15 19:12:46]

DCHECK(!callback.is_null())

[CJ, 2016/08/15 20:11:51]

Done.

+  provider_update_callback_ = callback;
+}
+
 scoped_refptr<AccessTokenStore> LocationArbitratorImpl::NewAccessTokenStore() {
   return delegate_->CreateAccessTokenStore();
 }
@@ -205,7 +217,7 @@ bool LocationArbitratorImpl::IsNewPositionBetter(
   return false;
 }
 
-bool LocationArbitratorImpl::HasPermissionBeenGranted() const {
+bool LocationArbitratorImpl::HasPermissionBeenGrantedForTest() const {
   return is_permission_granted_;
 }