Revert "fallback to Network Provider if wifi is turned off"

This reverts commit e13a069e.

Reason for revert:

Findit (https://goo.gl/kROfz5) identified CL at revision 829053 as the
culprit for flakes in the build cycles as shown on:
https://analysis.chromium.org/p/chromium/flake-portal/analysis/culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyQwsSDEZsYWtlQ3VscHJpdCIxY2hyb21pdW0vZTEzYTA2OWUyZmNmZmU3OWM4MmRlNTAzMmEwMzVhMDRjNDIwMmZkMgw

Sample Failed Build: https://ci.chromium.org/b/8863210065181297712

Sample Failed Step: services_unittests on (none) GPU on Mac on Mac-10.13.6

Sample Flaky Test: CoreLocationProviderTest.ShouldUseCallbackTest

Original change's description:
> fallback to Network Provider if wifi is turned off
> 
> If Wifi is disabled Core Location does not work. In this case we will
> fallback to the Network Location Provider which can use the IP address
> to get an much less precise location.
> 
> Bug: 1135766
> Change-Id: I3f5fa8e78fbb34bcec36dbd6eeb0e6adc06c64c6
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2453437
> Reviewed-by: Reilly Grant <reillyg@chromium.org>
> Commit-Queue: James Hollyer <jameshollyer@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#829053}


Change-Id: Iecd1cf5b4e3a36fdac2b11d5c52754fab8772c31
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1135766
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2549832
Cr-Commit-Position: refs/heads/master@{#829193}

services/device/geolocation/BUILD.gn

@@ -39,7 +39,6 @@ source_set("geolocation") {
     "public_ip_address_geolocator.h",
     "public_ip_address_location_notifier.cc",
     "public_ip_address_location_notifier.h",
-    "system_location_provider.h",
     "wifi_data.cc",
     "wifi_data.h",
     "wifi_data_provider.cc",

services/device/geolocation/core_location_provider.h

@@ -8,7 +8,6 @@
 #import <CoreLocation/CoreLocation.h>
 
 #include "base/mac/scoped_nsobject.h"
-#include "services/device/geolocation/system_location_provider.h"
 #include "services/device/public/cpp/geolocation/location_provider.h"
 #include "services/device/public/mojom/geoposition.mojom.h"
 
@@ -17,7 +16,7 @@
 namespace device {
 
 // Location provider for macOS using the platform's Core Location API.
-class CoreLocationProvider : public SystemLocationProvider {
+class CoreLocationProvider : public LocationProvider {
  public:
   CoreLocationProvider();
   ~CoreLocationProvider() override;
@@ -30,23 +29,16 @@ class CoreLocationProvider : public SystemLocationProvider {
   const mojom::Geoposition& GetPosition() override;
   void OnPermissionGranted() override;
 
-  // SystemLocationProvider implementation
-  void SetShouldUseSystemProviderCallback(
-      const ShouldUseCallback& callback) override;
-
   void SystemLocationPermissionGranted();
   void SystemLocationPermissionDenied();
   void DidUpdatePosition(CLLocation* location);
   void SetManagerForTesting(CLLocationManager* location_manager);
-  void StopUsing();
 
  private:
   base::scoped_nsobject<CLLocationManager> location_manager_;
   base::scoped_nsobject<LocationDelegate> delegate_;
   mojom::Geoposition last_position_;
   LocationProviderUpdateCallback callback_;
-  ShouldUseCallback should_use_callback_;
-  bool is_being_used_ = false;
   bool has_permission_ = false;
   bool provider_start_attemped_ = false;
   base::WeakPtrFactory<CoreLocationProvider> weak_ptr_factory_{this};

services/device/geolocation/core_location_provider.mm

@@ -15,8 +15,6 @@
      didUpdateLocations:(NSArray*)locations;
 - (void)locationManager:(CLLocationManager*)manager
     didChangeAuthorizationStatus:(CLAuthorizationStatus)status;
-- (void)locationManager:(CLLocationManager*)manager
-       didFailWithError:(NSError*)error;
 
 @end
 
@@ -47,17 +45,6 @@
   }
 }
 
-- (void)locationManager:(CLLocationManager*)manager
-       didFailWithError:(NSError*)error {
-  [error localizedDescription];
-  if ([error code] == kCLErrorLocationUnknown) {
-    // Fallback to network provider because Core Location is not able to
-    // determine location. This is likely due to wifi adapter being turned off.
-    if (provider_)
-      provider_->StopUsing();
-  }
-}
-
 - (void)locationManager:(CLLocationManager*)manager
      didUpdateLocations:(NSArray*)locations {
   if (provider_)
@@ -115,11 +102,6 @@ void CoreLocationProvider::OnPermissionGranted() {
   // Nothing to do here.
 }
 
-void CoreLocationProvider::SetShouldUseSystemProviderCallback(
-    const ShouldUseCallback& callback) {
-  should_use_callback_ = callback;
-}
-
 void CoreLocationProvider::SystemLocationPermissionGranted() {
   has_permission_ = true;
   if (provider_start_attemped_) {
@@ -132,17 +114,7 @@ void CoreLocationProvider::SystemLocationPermissionDenied() {
   has_permission_ = false;
 }
 
-void CoreLocationProvider::StopUsing() {
-  if (should_use_callback_)
-    should_use_callback_.Run(/*should_use=*/false);
-  is_being_used_ = false;
-}
-
 void CoreLocationProvider::DidUpdatePosition(CLLocation* location) {
-  if (!is_being_used_ && should_use_callback_) {
-    should_use_callback_.Run(/*should_use=*/true);
-    is_being_used_ = true;
-  }
   // The error values in CLLocation correlate exactly to our error values.
   last_position_.latitude = location.coordinate.latitude;
   last_position_.longitude = location.coordinate.longitude;
@@ -153,6 +125,7 @@ void CoreLocationProvider::DidUpdatePosition(CLLocation* location) {
   last_position_.altitude_accuracy = location.verticalAccuracy;
   last_position_.speed = location.speed;
   last_position_.heading = location.course;
+
   callback_.Run(this, last_position_);
 }
 

services/device/geolocation/core_location_provider_unittest.mm

@@ -25,7 +25,6 @@
 // Utility functions.
 - (void)fakeUpdatePosition:(CLLocation*)test_location;
 - (void)fakeUpdatePermission:(CLAuthorizationStatus)status;
-- (void)fakeTurnOffWifi;
 @end
 
 @implementation FakeCLLocationManager
@@ -57,16 +56,6 @@
   [_delegate locationManager:(id)self didChangeAuthorizationStatus:status];
 }
 
-- (void)fakeTurnOffWifi {
-  [_delegate locationManager:(id)self
-            didFailWithError:[NSError errorWithDomain:NSURLErrorDomain
-                                                 code:kCLErrorLocationUnknown
-                                             userInfo:@{
-                                               @"Error reason" :
-                                                   @"Test wifi turning off"
-                                             }]];
-}
-
 @end
 
 namespace device {
@@ -75,8 +64,6 @@ class CoreLocationProviderTest : public testing::Test {
  public:
   std::unique_ptr<CoreLocationProvider> provider_;
 
-  void ShouldUseCallback(bool should_use) { should_use_tracker_ = should_use; }
-
  protected:
   CoreLocationProviderTest() {}
 
@@ -88,8 +75,6 @@ class CoreLocationProviderTest : public testing::Test {
 
   bool IsUpdating() { return [fake_location_manager_ updating]; }
 
-  bool ShouldUse() { return should_use_tracker_; }
-
   // updates the position synchronously
   void FakeUpdatePosition(CLLocation* location) {
     [fake_location_manager_ fakeUpdatePosition:location];
@@ -102,7 +87,6 @@ class CoreLocationProviderTest : public testing::Test {
   base::test::TaskEnvironment task_environment_;
   const LocationProvider::LocationProviderUpdateCallback callback_;
   FakeCLLocationManager* fake_location_manager_;
-  bool should_use_tracker_ = false;
 };
 
 TEST_F(CoreLocationProviderTest, CreateDestroy) {
@@ -209,32 +193,4 @@ TEST_F(CoreLocationProviderTest, GetPositionUpdates) {
   provider_.reset();
 }
 
-TEST_F(CoreLocationProviderTest, ShouldUseCallbackTest) {
-  InitializeProvider();
-
-  provider_->StartProvider(/*high_accuracy=*/true);
-  provider_->SetShouldUseSystemProviderCallback(base::BindRepeating(
-      &CoreLocationProviderTest::ShouldUseCallback, base::Unretained(this)));
-
-  CLLocationCoordinate2D coors;
-  coors.latitude = 147.147;
-  coors.longitude = 101.101;
-  CLLocation* test_mac_location =
-      [[CLLocation alloc] initWithCoordinate:coors
-                                    altitude:417.417
-                          horizontalAccuracy:10.5
-                            verticalAccuracy:15.5
-                                   timestamp:[NSDate date]];
-
-  provider_->SetUpdateCallback(
-      base::BindLambdaForTesting([&](const LocationProvider* provider,
-                                     const mojom::Geoposition& position) {}));
-  EXPECT_FALSE(ShouldUse());
-  FakeUpdatePosition(test_mac_location);
-  EXPECT_TRUE(ShouldUse());
-
-  [fake_location_manager_ fakeTurnOffWifi];
-  EXPECT_FALSE(ShouldUse());
-}
-
 }  // namespace device

services/device/geolocation/fake_location_provider.cc

@@ -21,10 +21,7 @@ namespace device {
 FakeLocationProvider::FakeLocationProvider()
     : provider_task_runner_(base::ThreadTaskRunnerHandle::Get()) {}
 
-FakeLocationProvider::~FakeLocationProvider() {
-  if (destructor_callback_)
-    destructor_callback_.Run();
-}
+FakeLocationProvider::~FakeLocationProvider() = default;
 
 void FakeLocationProvider::HandlePositionChanged(
     const mojom::Geoposition& position) {
@@ -62,18 +59,4 @@ void FakeLocationProvider::OnPermissionGranted() {
   is_permission_granted_ = true;
 }
 
-void FakeLocationProvider::SetShouldUseSystemProviderCallback(
-    const ShouldUseCallback& callback) {
-  should_use_system_provider_callback_ = callback;
-}
-
-void FakeLocationProvider::CallShouldUseSystemProvider(bool should_use) {
-  should_use_system_provider_callback_.Run(should_use);
-}
-
-void FakeLocationProvider::SetProviderDestroyedCallback(
-    const base::RepeatingClosure& callback) {
-  destructor_callback_ = callback;
-}
-
 }  // namespace device

services/device/geolocation/fake_location_provider.h

@@ -10,14 +10,13 @@
 #include "base/memory/ref_counted.h"
 #include "base/single_thread_task_runner.h"
 #include "base/threading/thread.h"
-#include "services/device/geolocation/system_location_provider.h"
 #include "services/device/public/cpp/geolocation/location_provider.h"
 #include "services/device/public/mojom/geoposition.mojom.h"
 
 namespace device {
 
 // Fake implementation of a location provider for testing.
-class FakeLocationProvider : public SystemLocationProvider {
+class FakeLocationProvider : public LocationProvider {
  public:
   enum State { STOPPED, LOW_ACCURACY, HIGH_ACCURACY } state_ = STOPPED;
 
@@ -38,22 +37,12 @@ class FakeLocationProvider : public SystemLocationProvider {
   const mojom::Geoposition& GetPosition() override;
   void OnPermissionGranted() override;
 
-  // SystemLocationProvider implementation
-  void SetShouldUseSystemProviderCallback(
-      const ShouldUseCallback& callback) override;
-
-  void CallShouldUseSystemProvider(bool should_use);
-
-  void SetProviderDestroyedCallback(const base::RepeatingClosure& callback);
-
   scoped_refptr<base::SingleThreadTaskRunner> provider_task_runner_;
 
  private:
   bool is_permission_granted_ = false;
   mojom::Geoposition position_;
   LocationProviderUpdateCallback callback_;
-  ShouldUseCallback should_use_system_provider_callback_;
-  base::RepeatingClosure destructor_callback_;
 
   DISALLOW_COPY_AND_ASSIGN(FakeLocationProvider);
 };

services/device/geolocation/location_arbitrator.cc

@@ -46,46 +46,24 @@ bool LocationArbitrator::HasPermissionBeenGrantedForTest() const {
   return is_permission_granted_;
 }
 
-void LocationArbitrator::ShouldUseSystemProvider(bool should_use) {
-  force_ignore_system_location_ = !should_use;
-  if (!should_use && !network_location_provider_) {
-    network_location_provider_ =
-        NewNetworkLocationProvider(url_loader_factory_, api_key_);
-    RegisterProvider(network_location_provider_.get());
-  }
-
-  if (should_use && network_location_provider_)
-    network_location_provider_.reset();
-
-  DoStartProviders();
-}
-
 void LocationArbitrator::OnPermissionGranted() {
   is_permission_granted_ = true;
-
-#if defined(OS_MAC)
-  // On macOS we always want to keep the |system_location_provider_| in the most
-  // up to date state because it is the preferred provider if it is returning
-  // position data.
-  if (system_location_provider_ && force_ignore_system_location_)
-    system_location_provider_->OnPermissionGranted();
-#endif
-  auto* current_provider = GetProvider();
-  if (current_provider)
-    current_provider->OnPermissionGranted();
+  for (const auto& provider : providers_)
+    provider->OnPermissionGranted();
 }
 
 void LocationArbitrator::StartProvider(bool enable_high_accuracy) {
   is_running_ = true;
   enable_high_accuracy_ = enable_high_accuracy;
 
-  if (!HasProvider())
+  if (providers_.empty()) {
     RegisterProviders();
+  }
   DoStartProviders();
 }
 
 void LocationArbitrator::DoStartProviders() {
-  if (!HasProvider()) {
+  if (providers_.empty()) {
     // If no providers are available, we report an error to avoid
     // callers waiting indefinitely for a reply.
     mojom::Geoposition position;
@@ -93,16 +71,9 @@ void LocationArbitrator::DoStartProviders() {
     arbitrator_update_callback_.Run(this, position);
     return;
   }
-  // On macOS we always want to start the |system_location_provider_| even if it
-  // is currently being ignored because we want to switch to it if it starts
-  // producing valid data.
-#if defined(OS_MAC)
-  if (system_location_provider_ && force_ignore_system_location_)
-    system_location_provider_->StartProvider(enable_high_accuracy_);
-#endif
-  auto* current_provider = GetProvider();
-  if (current_provider)
-    current_provider->StartProvider(enable_high_accuracy_);
+  for (const auto& provider : providers_) {
+    provider->StartProvider(enable_high_accuracy_);
+  }
 }
 
 void LocationArbitrator::StopProvider() {
@@ -112,51 +83,38 @@ void LocationArbitrator::StopProvider() {
   position_provider_ = nullptr;
   position_ = mojom::Geoposition();
 
-  custom_location_provider_.reset();
-  if (system_location_provider_)
-    system_location_provider_->StopProvider();
-  network_location_provider_.reset();
+  providers_.clear();
   is_running_ = false;
 }
 
-void LocationArbitrator::RegisterProvider(LocationProvider* provider) {
+void LocationArbitrator::RegisterProvider(
+    std::unique_ptr<LocationProvider> provider) {
   if (!provider)
     return;
-  // Using base::Unretained is safe here because the |provider| is owned by
-  // this and therefore will be destroyed before this is.
   provider->SetUpdateCallback(base::BindRepeating(
       &LocationArbitrator::OnLocationUpdate, base::Unretained(this)));
   if (is_permission_granted_)
     provider->OnPermissionGranted();
+  providers_.push_back(std::move(provider));
 }
 
 void LocationArbitrator::RegisterProviders() {
   if (custom_location_provider_getter_) {
     auto custom_provider = custom_location_provider_getter_.Run();
     if (custom_provider) {
-      RegisterProvider(custom_provider.get());
-      custom_location_provider_ = std::move(custom_provider);
+      RegisterProvider(std::move(custom_provider));
       return;
     }
   }
 
   auto system_provider = NewSystemLocationProvider();
   if (system_provider) {
-    RegisterProvider(system_provider.get());
-    system_location_provider_ = std::move(system_provider);
-    // Using base::Unretained is safe here because the
-    // |system_location_provider_| is owned by this and therefore will be
-    // destroyed before this is.
-    system_location_provider_->SetShouldUseSystemProviderCallback(
-        base::BindRepeating(&LocationArbitrator::ShouldUseSystemProvider,
-                            base::Unretained(this)));
+    RegisterProvider(std::move(system_provider));
+    return;
   }
 
-  if (url_loader_factory_) {
-    network_location_provider_ =
-        NewNetworkLocationProvider(url_loader_factory_, api_key_);
-    RegisterProvider(network_location_provider_.get());
-  }
+  if (url_loader_factory_)
+    RegisterProvider(NewNetworkLocationProvider(url_loader_factory_, api_key_));
 }
 
 void LocationArbitrator::OnLocationUpdate(
@@ -196,7 +154,7 @@ LocationArbitrator::NewNetworkLocationProvider(
 #endif
 }
 
-std::unique_ptr<SystemLocationProvider>
+std::unique_ptr<LocationProvider>
 LocationArbitrator::NewSystemLocationProvider() {
 #if defined(OS_LINUX) || defined(OS_CHROMEOS) || defined(OS_FUCHSIA)
   return nullptr;
@@ -236,23 +194,4 @@ bool LocationArbitrator::IsNewPositionBetter(
   return false;
 }
 
-bool LocationArbitrator::HasProvider() {
-  return custom_location_provider_ ||
-         (system_location_provider_ && !force_ignore_system_location_) ||
-         network_location_provider_;
-}
-
-LocationProvider* LocationArbitrator::GetProvider() {
-  if (custom_location_provider_)
-    return custom_location_provider_.get();
-
-  if (system_location_provider_ && !force_ignore_system_location_)
-    return system_location_provider_.get();
-
-  if (network_location_provider_)
-    return network_location_provider_.get();
-
-  return nullptr;
-}
-
 }  // namespace device

services/device/geolocation/location_arbitrator.h

@@ -18,7 +18,6 @@
 #include "services/device/geolocation/geolocation_provider_impl.h"
 #include "services/device/geolocation/network_location_provider.h"
 #include "services/device/geolocation/position_cache.h"
-#include "services/device/geolocation/system_location_provider.h"
 #include "services/device/public/cpp/geolocation/location_provider.h"
 #include "services/device/public/mojom/geoposition.mojom.h"
 #include "url/gurl.h"
@@ -50,7 +49,6 @@ class LocationArbitrator : public LocationProvider {
 
   static GURL DefaultNetworkProviderURL();
   bool HasPermissionBeenGrantedForTest() const;
-  void ShouldUseSystemProvider(bool should_use);
 
   // LocationProvider implementation.
   void SetUpdateCallback(
@@ -66,7 +64,7 @@ class LocationArbitrator : public LocationProvider {
   virtual std::unique_ptr<LocationProvider> NewNetworkLocationProvider(
       scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
       const std::string& api_key);
-  virtual std::unique_ptr<SystemLocationProvider> NewSystemLocationProvider();
+  virtual std::unique_ptr<LocationProvider> NewSystemLocationProvider();
   virtual base::Time GetTimeNow() const;
 
  private:
@@ -74,7 +72,7 @@ class LocationArbitrator : public LocationProvider {
 
   // Provider will either be added to |providers_| or
   // deleted on error (e.g. it fails to start).
-  void RegisterProvider(LocationProvider* provider);
+  void RegisterProvider(std::unique_ptr<LocationProvider> provider);
   void RegisterProviders();
 
   // Tells all registered providers to start.
@@ -94,19 +92,13 @@ class LocationArbitrator : public LocationProvider {
                            const mojom::Geoposition& new_position,
                            bool from_same_provider) const;
 
-  bool HasProvider();
-  LocationProvider* GetProvider();
-
   const CustomLocationProviderCallback custom_location_provider_getter_;
   const scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory_;
   const std::string api_key_;
 
   LocationProvider::LocationProviderUpdateCallback arbitrator_update_callback_;
 
-  std::unique_ptr<LocationProvider> custom_location_provider_;
-  std::unique_ptr<LocationProvider> network_location_provider_;
-  std::unique_ptr<SystemLocationProvider> system_location_provider_;
-  bool force_ignore_system_location_ = true;
+  std::vector<std::unique_ptr<LocationProvider>> providers_;
   bool enable_high_accuracy_;
   // The provider which supplied the current |position_|
   const LocationProvider* position_provider_;
@@ -124,7 +116,7 @@ class LocationArbitrator : public LocationProvider {
 
 // Factory functions for the various types of location provider to abstract
 // over the platform-dependent implementations.
-std::unique_ptr<SystemLocationProvider> NewSystemLocationProvider();
+std::unique_ptr<LocationProvider> NewSystemLocationProvider();
 
 }  // namespace device
 

services/device/geolocation/location_arbitrator_unittest.cc

@@ -10,7 +10,6 @@
 #include "base/bind.h"
 #include "base/memory/ptr_util.h"
 #include "base/test/task_environment.h"
-#include "build/build_config.h"
 #include "services/device/geolocation/fake_location_provider.h"
 #include "services/device/geolocation/fake_position_cache.h"
 #include "services/device/public/cpp/geolocation/geoposition.h"
@@ -103,14 +102,10 @@ class TestingLocationArbitrator : public LocationArbitrator {
       scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
       const std::string& api_key) override {
     network_location_provider_ = new FakeLocationProvider;
-    network_location_provider_->SetProviderDestroyedCallback(
-        base::BindRepeating(
-            &TestingLocationArbitrator::FakeLocationProviderDestructorCallback,
-            base::Unretained(this)));
     return base::WrapUnique(network_location_provider_);
   }
 
-  std::unique_ptr<SystemLocationProvider> NewSystemLocationProvider() override {
+  std::unique_ptr<LocationProvider> NewSystemLocationProvider() override {
     if (!should_use_system_location_provider_) {
       return nullptr;
     }
@@ -119,10 +114,6 @@ class TestingLocationArbitrator : public LocationArbitrator {
     return base::WrapUnique(system_location_provider_);
   }
 
-  void FakeLocationProviderDestructorCallback() {
-    network_location_provider_ = nullptr;
-  }
-
   FakeLocationProvider* network_location_provider_ = nullptr;
   FakeLocationProvider* system_location_provider_ = nullptr;
   bool should_use_system_location_provider_;
@@ -248,31 +239,27 @@ TEST_F(GeolocationLocationArbitratorTest, NormalUsageSystem) {
   EXPECT_FALSE(system_location_provider());
   arbitrator_->StartProvider(false);
 
-  auto* location_provider = network_location_provider();
-  EXPECT_TRUE(location_provider);
-
-#if defined(OS_MAC)
-  location_provider = system_location_provider();
-  EXPECT_TRUE(location_provider);
-#endif
-  EXPECT_EQ(FakeLocationProvider::LOW_ACCURACY, location_provider->state_);
+  EXPECT_FALSE(network_location_provider());
+  ASSERT_TRUE(system_location_provider());
+  EXPECT_EQ(FakeLocationProvider::LOW_ACCURACY,
+            system_location_provider()->state_);
   EXPECT_FALSE(ValidateGeoposition(observer_->last_position()));
   EXPECT_EQ(mojom::Geoposition::ErrorCode::NONE,
             observer_->last_position().error_code);
 
-  SetReferencePosition(location_provider);
+  SetReferencePosition(system_location_provider());
 
   EXPECT_TRUE(ValidateGeoposition(observer_->last_position()) ||
               observer_->last_position().error_code !=
                   mojom::Geoposition::ErrorCode::NONE);
-  EXPECT_EQ(location_provider->GetPosition().latitude,
+  EXPECT_EQ(system_location_provider()->GetPosition().latitude,
             observer_->last_position().latitude);
 
-  EXPECT_FALSE(location_provider->is_permission_granted());
+  EXPECT_FALSE(system_location_provider()->is_permission_granted());
   EXPECT_FALSE(arbitrator_->HasPermissionBeenGrantedForTest());
   arbitrator_->OnPermissionGranted();
   EXPECT_TRUE(arbitrator_->HasPermissionBeenGrantedForTest());
-  EXPECT_TRUE(location_provider->is_permission_granted());
+  EXPECT_TRUE(system_location_provider()->is_permission_granted());
 }
 
 // Tests basic operation (single position fix) with no network location
@@ -362,48 +349,4 @@ TEST_F(GeolocationLocationArbitratorTest, TwoOneShotsIsNewPositionBetter) {
   CheckLastPositionInfo(3, 139, 150);
 }
 
-#if defined(OS_MAC)
-TEST_F(GeolocationLocationArbitratorTest, NetworkProviderFallback) {
-  InitializeArbitrator(
-      base::BindRepeating(&GetCustomLocationProviderForTest, nullptr),
-      url_loader_factory_, /*should_use_system_location_provider=*/true);
-  ASSERT_TRUE(arbitrator_);
-
-  arbitrator_->StartProvider(false);
-
-  EXPECT_EQ(network_location_provider()->state(),
-            FakeLocationProvider::State::LOW_ACCURACY);
-  EXPECT_EQ(system_location_provider()->state(),
-            FakeLocationProvider::State::LOW_ACCURACY);
-
-  system_location_provider()->CallShouldUseSystemProvider(/*should_use=*/true);
-
-  EXPECT_FALSE(network_location_provider());
-  EXPECT_EQ(system_location_provider()->state(),
-            FakeLocationProvider::State::LOW_ACCURACY);
-
-  arbitrator_->StopProvider();
-  arbitrator_->StartProvider(/*high_accuracy=*/false);
-
-  EXPECT_FALSE(network_location_provider());
-  EXPECT_EQ(system_location_provider()->state(),
-            FakeLocationProvider::State::LOW_ACCURACY);
-
-  system_location_provider()->CallShouldUseSystemProvider(/*should_use=*/false);
-
-  EXPECT_EQ(network_location_provider()->state(),
-            FakeLocationProvider::State::LOW_ACCURACY);
-  EXPECT_EQ(system_location_provider()->state(),
-            FakeLocationProvider::State::LOW_ACCURACY);
-
-  arbitrator_->StopProvider();
-  arbitrator_->StartProvider(/*high_accuracy=*/false);
-
-  EXPECT_EQ(network_location_provider()->state(),
-            FakeLocationProvider::State::LOW_ACCURACY);
-  EXPECT_EQ(system_location_provider()->state(),
-            FakeLocationProvider::State::LOW_ACCURACY);
-}
-#endif
-
 }  // namespace device

services/device/geolocation/location_provider_android.cc

@@ -57,14 +57,8 @@ void LocationProviderAndroid::OnPermissionGranted() {
   // Nothing to do here.
 }
 
-void LocationProviderAndroid::SetShouldUseSystemProviderCallback(
-    const ShouldUseCallback& callback) {
-  // Androids's location provider should always be used.
-  callback.Run(/*should_use=*/true);
-}
-
 // static
-std::unique_ptr<SystemLocationProvider> NewSystemLocationProvider() {
+std::unique_ptr<LocationProvider> NewSystemLocationProvider() {
   return base::WrapUnique(new LocationProviderAndroid);
 }
 

services/device/geolocation/location_provider_android.h

@@ -8,14 +8,13 @@
 #include "base/compiler_specific.h"
 #include "base/memory/weak_ptr.h"
 #include "base/threading/thread_checker.h"
-#include "services/device/geolocation/system_location_provider.h"
 #include "services/device/public/cpp/geolocation/location_provider.h"
 #include "services/device/public/mojom/geoposition.mojom.h"
 
 namespace device {
 
 // Location provider for Android using the platform provider over JNI.
-class LocationProviderAndroid : public SystemLocationProvider {
+class LocationProviderAndroid : public LocationProvider {
  public:
   LocationProviderAndroid();
   ~LocationProviderAndroid() override;
@@ -31,10 +30,6 @@ class LocationProviderAndroid : public SystemLocationProvider {
   const mojom::Geoposition& GetPosition() override;
   void OnPermissionGranted() override;
 
-  // SystemLocationProvider implementation.
-  void SetShouldUseSystemProviderCallback(
-      const ShouldUseCallback& callback) override;
-
  private:
   base::ThreadChecker thread_checker_;
 

services/device/geolocation/system_location_provider.h

-// Copyright 2020 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 SERVICES_DEVICE_GEOLOCATION_SYSTEM_LOCATION_PROVIDER_H_
-#define SERVICES_DEVICE_GEOLOCATION_SYSTEM_LOCATION_PROVIDER_H_
-
-#include "services/device/public/cpp/geolocation/location_provider.h"
-
-namespace device {
-
-class SystemLocationProvider : public LocationProvider {
- public:
-  using ShouldUseCallback = base::RepeatingCallback<void(bool)>;
-
-  // SystemLocationProviders are sometimes flaky (so far this is only used for
-  // macOS). Therefore the LocationArbitrator will default to using the
-  // NetworkLocationProvider until the SystemLocationProvider determines it is
-  // working properly. At this time the provided callback should be called with
-  // |should_use|=true. If at any time the SystemLocationProvider stops working
-  // this callback should again be used with |should_use|=false to notify the
-  // LocationArbitrator that it can no longer rely on the
-  // SystemLocationProvider. For example the macOS SystemLocationProvider no
-  // longer works when the WiFi adapter is turned off.
-  virtual void SetShouldUseSystemProviderCallback(
-      const ShouldUseCallback& callback) {}
-};
-
-}  // namespace device
-
-#endif  // SERVICES_DEVICE_GEOLOCATION_SYSTEM_LOCATION_PROVIDER_H_
\ No newline at end of file

services/device/geolocation/wifi_data_provider_mac.mm

@@ -14,7 +14,6 @@
 #include "base/strings/sys_string_conversions.h"
 #include "services/device/geolocation/wifi_data_provider_common.h"
 #include "services/device/geolocation/wifi_data_provider_manager.h"
-#include "services/device/public/cpp/device_features.h"
 
 namespace device {
 
@@ -102,12 +101,6 @@ WifiDataProviderMac::~WifiDataProviderMac() {}
 
 std::unique_ptr<WifiDataProviderMac::WlanApiInterface>
 WifiDataProviderMac::CreateWlanApi() {
-  // With the Core Location backend enabled the NetworkLocationProvider is only
-  // needed when WiFi is turned off in order to provide a fallback location
-  // using the device IP address. Disable the WlanApi so avoid collecting more
-  // data than is necessary.
-  if (base::FeatureList::IsEnabled(features::kMacCoreLocationBackend))
-    return nullptr;
   return std::make_unique<CoreWlanApi>();
 }
 

services/device/geolocation/win/location_provider_winrt.cc

@@ -456,14 +456,8 @@ HRESULT LocationProviderWinrt::GetGeolocator(IGeolocator** geo_locator) {
   return hr;
 }
 
-void LocationProviderWinrt::SetShouldUseSystemProviderCallback(
-    const ShouldUseCallback& callback) {
-  // WinRT's location provider should always be used if available.
-  callback.Run(/*should_use=*/true);
-}
-
 // static
-std::unique_ptr<SystemLocationProvider> NewSystemLocationProvider() {
+std::unique_ptr<LocationProvider> NewSystemLocationProvider() {
   if (!base::FeatureList::IsEnabled(
           features::kWinrtGeolocationImplementation) ||
       !IsWinRTSupported() || !IsSystemLocationSettingEnabled()) {

services/device/geolocation/win/location_provider_winrt.h

@@ -9,14 +9,13 @@
 #include <wrl/client.h>
 
 #include "base/threading/thread_checker.h"
-#include "services/device/geolocation/system_location_provider.h"
 #include "services/device/public/cpp/geolocation/location_provider.h"
 #include "services/device/public/mojom/geoposition.mojom.h"
 
 namespace device {
 
 // Location provider for Windows 8/10 using the WinRT platform apis
-class LocationProviderWinrt : public SystemLocationProvider {
+class LocationProviderWinrt : public LocationProvider {
  public:
   LocationProviderWinrt();
   ~LocationProviderWinrt() override;
@@ -29,10 +28,6 @@ class LocationProviderWinrt : public SystemLocationProvider {
   const mojom::Geoposition& GetPosition() override;
   void OnPermissionGranted() override;
 
-  // SystemLocationProvider implementation
-  void SetShouldUseSystemProviderCallback(
-      const ShouldUseCallback& callback) override;
-
  protected:
   virtual HRESULT GetGeolocator(
       ABI::Windows::Devices::Geolocation::IGeolocator** geo_locator);