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

device/geolocation/network_location_provider.cc

Index: device/geolocation/network_location_provider.cc
diff --git a/device/geolocation/network_location_provider.cc b/device/geolocation/network_location_provider.cc
index 46206a158fb0018e47dfd75d64245e5681dc8f0e..a542c03bf188bb0513a4e60d029c4034c055cfea 100644
--- a/device/geolocation/network_location_provider.cc
+++ b/device/geolocation/network_location_provider.cc
@@ -4,6 +4,8 @@
 
 #include "device/geolocation/network_location_provider.h"
 
+#include <utility>
+
 #include "base/bind.h"
 #include "base/location.h"
 #include "base/single_thread_task_runner.h"
@@ -128,26 +130,15 @@ NetworkLocationProvider::~NetworkLocationProvider() {
 }
 
 // LocationProvider implementation
-void NetworkLocationProvider::GetPosition(Geoposition* position) {
-  DCHECK(position);
-  *position = position_;
-}
-
-void NetworkLocationProvider::RequestRefresh() {
-  // TODO(joth): When called via the public (base class) interface, this should
-  // poke each data provider to get them to expedite their next scan.
-  // Whilst in the delayed start, only send request if all data is ready.
-  // TODO(mcasas): consider not using HasWeakPtrs() https://crbug.com/629158.
-  if (!weak_factory_.HasWeakPtrs() || is_wifi_data_complete_) {
-    RequestPosition();
-  }
+const Geoposition& NetworkLocationProvider::GetPosition() {
+  return position_;
 }
 
 void NetworkLocationProvider::OnPermissionGranted() {
   const bool was_permission_granted = is_permission_granted_;
   is_permission_granted_ = true;
   if (!was_permission_granted && IsStarted()) {
-    RequestRefresh();
+    RequestPosition();
   }
 }
 
@@ -212,7 +203,7 @@ void NetworkLocationProvider::OnWifiDataUpdated() {
   wifi_timestamp_ = base::Time::Now();
 
   is_new_data_available_ = is_wifi_data_complete_;
-  RequestRefresh();
+  RequestPosition();
 }
 
 void NetworkLocationProvider::StopProvider() {
@@ -226,42 +217,48 @@ void NetworkLocationProvider::StopProvider() {
 
 // Other methods
 void NetworkLocationProvider::RequestPosition() {
-  DCHECK(CalledOnValidThread());
-  if (!is_new_data_available_)
-    return;
-
-  const Geoposition* cached_position =
-      position_cache_->FindPosition(wifi_data_);
-  DCHECK(!wifi_timestamp_.is_null())
-      << "Timestamp must be set before looking up position";
-  if (cached_position) {
-    DCHECK(cached_position->Validate());
-    // Record the position and update its timestamp.
-    position_ = *cached_position;
-    // The timestamp of a position fix is determined by the timestamp
-    // of the source data update. (The value of position_.timestamp from
-    // the cache could be from weeks ago!)
-    position_.timestamp = wifi_timestamp_;
+  // TODO(mcasas): consider not using HasWeakPtrs() https://crbug.com/629158.
+  if (!weak_factory_.HasWeakPtrs() || is_wifi_data_complete_) {

[Wez, 2016/08/24 23:20:36]

Let's use an early-exit for this (like |is_new_data_available_| below) rather
than indent the whole rest of the function!

[CJ, 2016/08/24 23:35:28]

Done.

+    DCHECK(CalledOnValidThread());

[Wez, 2016/08/24 23:20:36]

This DCHECK ensures we're called on the correct thread, so belongs at the start
of the function.

[CJ, 2016/08/24 23:35:28]

Done.

+    if (!is_new_data_available_)
+      return;
+
+    const Geoposition* cached_position =
+        position_cache_->FindPosition(wifi_data_);
+    DCHECK(!wifi_timestamp_.is_null())
+        << "Timestamp must be set before looking up position";
+    if (cached_position) {
+      DCHECK(cached_position->Validate());
+      // Record the position and update its timestamp.
+      position_ = *cached_position;
+
+      // The timestamp of a position fix is determined by the timestamp
+      // of the source data update. (The value of position_.timestamp from
+      // the cache could be from weeks ago!)
+      position_.timestamp = wifi_timestamp_;
+      is_new_data_available_ = false;
+
+      // Let listeners know that we now have a position available.
+      NotifyCallback(position_);
+      return;
+    }
+    // Don't send network requests until authorized. http://crbug.com/39171
+    if (!is_permission_granted_)
+      return;
+
+    weak_factory_.InvalidateWeakPtrs();
     is_new_data_available_ = false;
-    // Let listeners know that we now have a position available.
-    NotifyCallback(position_);
-    return;
-  }
-  // Don't send network requests until authorized. http://crbug.com/39171
-  if (!is_permission_granted_)
-    return;
 
-  weak_factory_.InvalidateWeakPtrs();
-  is_new_data_available_ = false;
-
-  // TODO(joth): Rather than cancel pending requests, we should create a new
-  // NetworkLocationRequest for each and hold a set of pending requests.
-  if (request_->is_request_pending()) {
-    DVLOG(1) << "NetworkLocationProvider - pre-empting pending network request "
-                "with new data. Wifi APs: "
-             << wifi_data_.access_point_data.size();
+    // TODO(joth): Rather than cancel pending requests, we should create a new
+    // // NetworkLocationRequest for each and hold a set of pending requests.
+    if (request_->is_request_pending()) {
+      DVLOG(1)
+          << "NetworkLocationProvider - pre-empting pending network request "
+             "with new data. Wifi APs: "
+          << wifi_data_.access_point_data.size();
+    }
+    request_->MakeRequest(access_token_, wifi_data_, wifi_timestamp_);
   }
-  request_->MakeRequest(access_token_, wifi_data_, wifi_timestamp_);
 }
 
 bool NetworkLocationProvider::IsStarted() const {