geolocation: Add LocationProvider::FillDiagnostics

Introduce mojom::GeolocationDiagnostics and add a method to
LocationProvider to populate it with details about the provider's
internal state.

At first, mojom::GeolocationDiagnostics will contain an enum
value indicating whether the provider is stopped, started in high
resolution mode, started in low resolution mode, or blocked by a
system permission.

Bug: 1110995
Change-Id: I07b6446b5e3fa78f738c4f75c3a35ed50fdf2d06
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4506614
Commit-Queue: Matt Reynolds <mattreynolds@chromium.org>
Reviewed-by: Alvin Ji <alvinji@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1158481}

services/device/BUILD.gn

@@ -44,6 +44,7 @@ source_set("lib") {
     "//services/device/geolocation",
     "//services/device/power_monitor",
     "//services/device/public/cpp:device_features",
+    "//services/device/public/mojom:geolocation_internals",
     "//services/device/public/mojom:usb",
     "//services/device/public/mojom:usb_test",
     "//services/device/screen_orientation",

services/device/geolocation/core_location_provider.cc

@@ -33,6 +33,11 @@ CoreLocationProvider::~CoreLocationProvider() {
   StopProvider();
 }
 
+void CoreLocationProvider::FillDiagnostics(
+    mojom::GeolocationDiagnostics& diagnostics) {
+  diagnostics.provider_state = state_;
+}
+
 void CoreLocationProvider::SetUpdateCallback(
     const LocationProviderUpdateCallback& callback) {
   callback_ = callback;
@@ -47,16 +52,22 @@ void CoreLocationProvider::StartProvider(bool high_accuracy) {
   if (has_permission_) {
     StartWatching();
   } else {
+    state_ = mojom::GeolocationDiagnostics::ProviderState::
+        kBlockedBySystemPermission;
     provider_start_attemped_ = true;
   }
 }
 
 void CoreLocationProvider::StartWatching() {
+  state_ = high_accuracy_
+               ? mojom::GeolocationDiagnostics::ProviderState::kHighAccuracy
+               : mojom::GeolocationDiagnostics::ProviderState::kLowAccuracy;
   position_observers_->AddObserver(this);
   geolocation_manager_->StartWatchingPosition(high_accuracy_);
 }
 
 void CoreLocationProvider::StopProvider() {
+  state_ = mojom::GeolocationDiagnostics::ProviderState::kStopped;
   position_observers_->RemoveObserver(this);
   geolocation_manager_->StopWatchingPosition();
 }

services/device/geolocation/core_location_provider.h

@@ -9,6 +9,7 @@
 #include "base/task/single_thread_task_runner.h"
 #include "services/device/public/cpp/geolocation/geolocation_manager.h"
 #include "services/device/public/cpp/geolocation/location_provider.h"
+#include "services/device/public/mojom/geolocation_internals.mojom.h"
 #include "services/device/public/mojom/geoposition.mojom.h"
 
 namespace device {
@@ -26,6 +27,7 @@ class CoreLocationProvider : public LocationProvider,
   ~CoreLocationProvider() override;
 
   // LocationProvider implementation.
+  void FillDiagnostics(mojom::GeolocationDiagnostics& diagnostics) override;
   void SetUpdateCallback(
       const LocationProviderUpdateCallback& callback) override;
   void StartProvider(bool high_accuracy) override;
@@ -44,6 +46,8 @@ class CoreLocationProvider : public LocationProvider,
   void OnSystemPermissionUpdated(
       LocationSystemPermissionStatus new_status) override;
 
+  mojom::GeolocationDiagnostics::ProviderState state_ =
+      mojom::GeolocationDiagnostics::ProviderState::kStopped;
   raw_ptr<GeolocationManager> geolocation_manager_;
   // References to the observer lists are kept to ensure their lifetime as the
   // BrowserProcess may destroy its reference on the UI Thread before we

services/device/geolocation/core_location_provider_unittest.mm

@@ -12,6 +12,7 @@
 #include "base/time/time.h"
 #include "services/device/public/cpp/geolocation/geoposition.h"
 #include "services/device/public/cpp/test/fake_geolocation_manager.h"
+#include "services/device/public/mojom/geolocation_internals.mojom.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 #if !defined(__has_feature) || !__has_feature(objc_arc)
@@ -48,6 +49,12 @@ void FakeUpdatePosition(const mojom::Geoposition& result) {
     return provider_->GetPosition();
   }
 
+  mojom::GeolocationDiagnostics::ProviderState GetProviderState() {
+    mojom::GeolocationDiagnostics diagnostics;
+    provider_->FillDiagnostics(diagnostics);
+    return diagnostics.provider_state;
+  }
+
   base::test::TaskEnvironment task_environment_;
   const LocationProvider::LocationProviderUpdateCallback callback_;
   std::unique_ptr<FakeGeolocationManager> fake_geolocation_manager_;
@@ -66,8 +73,12 @@ void FakeUpdatePosition(const mojom::Geoposition& result) {
   base::RunLoop().RunUntilIdle();
   provider_->StartProvider(/*high_accuracy=*/true);
   EXPECT_TRUE(IsUpdating());
+  EXPECT_EQ(GetProviderState(),
+            mojom::GeolocationDiagnostics::ProviderState::kHighAccuracy);
   provider_->StopProvider();
   EXPECT_FALSE(IsUpdating());
+  EXPECT_EQ(GetProviderState(),
+            mojom::GeolocationDiagnostics::ProviderState::kStopped);
   provider_.reset();
 }
 
@@ -78,21 +89,32 @@ void FakeUpdatePosition(const mojom::Geoposition& result) {
   base::RunLoop().RunUntilIdle();
   provider_->StartProvider(/*high_accuracy=*/true);
   EXPECT_FALSE(IsUpdating());
+  EXPECT_EQ(
+      GetProviderState(),
+      mojom::GeolocationDiagnostics::ProviderState::kBlockedBySystemPermission);
   provider_.reset();
 }
 
 TEST_F(CoreLocationProviderTest, DontStartUpdatingUntilPermissionGranted) {
   InitializeProvider();
   provider_->StartProvider(/*high_accuracy=*/true);
   EXPECT_FALSE(IsUpdating());
+  EXPECT_EQ(
+      GetProviderState(),
+      mojom::GeolocationDiagnostics::ProviderState::kBlockedBySystemPermission);
   fake_geolocation_manager_->SetSystemPermission(
       LocationSystemPermissionStatus::kDenied);
   base::RunLoop().RunUntilIdle();
   EXPECT_FALSE(IsUpdating());
+  EXPECT_EQ(
+      GetProviderState(),
+      mojom::GeolocationDiagnostics::ProviderState::kBlockedBySystemPermission);
   fake_geolocation_manager_->SetSystemPermission(
       LocationSystemPermissionStatus::kAllowed);
   base::RunLoop().RunUntilIdle();
   EXPECT_TRUE(IsUpdating());
+  EXPECT_EQ(GetProviderState(),
+            mojom::GeolocationDiagnostics::ProviderState::kHighAccuracy);
   provider_.reset();
 }
 
@@ -103,6 +125,8 @@ void FakeUpdatePosition(const mojom::Geoposition& result) {
   base::RunLoop().RunUntilIdle();
   provider_->StartProvider(/*high_accuracy=*/true);
   EXPECT_TRUE(IsUpdating());
+  EXPECT_EQ(GetProviderState(),
+            mojom::GeolocationDiagnostics::ProviderState::kHighAccuracy);
 
   // test info
   double latitude = 147.147;
@@ -133,6 +157,8 @@ void FakeUpdatePosition(const mojom::Geoposition& result) {
 
   provider_->StopProvider();
   EXPECT_FALSE(IsUpdating());
+  EXPECT_EQ(GetProviderState(),
+            mojom::GeolocationDiagnostics::ProviderState::kStopped);
   provider_.reset();
 }
 

services/device/geolocation/fake_location_provider.cc

@@ -39,17 +39,24 @@ void FakeLocationProvider::HandlePositionChanged(
   }
 }
 
+void FakeLocationProvider::FillDiagnostics(
+    mojom::GeolocationDiagnostics& diagnostics) {
+  diagnostics.provider_state = state_;
+}
+
 void FakeLocationProvider::SetUpdateCallback(
     const LocationProviderUpdateCallback& callback) {
   callback_ = callback;
 }
 
 void FakeLocationProvider::StartProvider(bool high_accuracy) {
-  state_ = high_accuracy ? HIGH_ACCURACY : LOW_ACCURACY;
+  state_ = high_accuracy
+               ? mojom::GeolocationDiagnostics::ProviderState::kHighAccuracy
+               : mojom::GeolocationDiagnostics::ProviderState::kLowAccuracy;
 }
 
 void FakeLocationProvider::StopProvider() {
-  state_ = STOPPED;
+  state_ = mojom::GeolocationDiagnostics::ProviderState::kStopped;
 }
 
 const mojom::GeopositionResult* FakeLocationProvider::GetPosition() {

services/device/geolocation/fake_location_provider.h

@@ -10,15 +10,14 @@
 #include "base/task/single_thread_task_runner.h"
 #include "base/threading/thread.h"
 #include "services/device/public/cpp/geolocation/location_provider.h"
+#include "services/device/public/mojom/geolocation_internals.mojom.h"
 #include "services/device/public/mojom/geoposition.mojom.h"
 
 namespace device {
 
 // Fake implementation of a location provider for testing.
 class FakeLocationProvider : public LocationProvider {
  public:
-  enum State { STOPPED, LOW_ACCURACY, HIGH_ACCURACY } state_ = STOPPED;
-
   FakeLocationProvider();
 
   FakeLocationProvider(const FakeLocationProvider&) = delete;
@@ -29,10 +28,11 @@ class FakeLocationProvider : public LocationProvider {
   // Updates listeners with the new position.
   void HandlePositionChanged(mojom::GeopositionResultPtr result);
 
-  State state() const { return state_; }
+  mojom::GeolocationDiagnostics::ProviderState state() const { return state_; }
   bool is_permission_granted() const { return is_permission_granted_; }
 
   // LocationProvider implementation.
+  void FillDiagnostics(mojom::GeolocationDiagnostics& diagnostics) override;
   void SetUpdateCallback(
       const LocationProviderUpdateCallback& callback) override;
   void StartProvider(bool high_accuracy) override;
@@ -43,6 +43,8 @@ class FakeLocationProvider : public LocationProvider {
   scoped_refptr<base::SingleThreadTaskRunner> provider_task_runner_;
 
  private:
+  mojom::GeolocationDiagnostics::ProviderState state_ =
+      mojom::GeolocationDiagnostics::ProviderState::kStopped;
   bool is_permission_granted_ = false;
   mojom::GeopositionResultPtr result_;
   LocationProviderUpdateCallback callback_;

services/device/geolocation/geolocation_provider_impl_unittest.cc

@@ -152,7 +152,8 @@ bool GeolocationProviderTest::ProvidersStarted() {
 
 void GeolocationProviderTest::GetProvidersStarted() {
   DCHECK(provider()->task_runner()->BelongsToCurrentThread());
-  is_started_ = arbitrator()->state() != FakeLocationProvider::STOPPED;
+  is_started_ = arbitrator()->state() !=
+                mojom::GeolocationDiagnostics::ProviderState::kStopped;
 }
 
 void GeolocationProviderTest::SendMockLocation(

services/device/geolocation/location_arbitrator.cc

@@ -139,6 +139,18 @@ const mojom::GeopositionResult* LocationArbitrator::GetPosition() {
   return result_.get();
 }
 
+void LocationArbitrator::FillDiagnostics(
+    mojom::GeolocationDiagnostics& diagnostics) {
+  if (!is_running_ || providers_.empty()) {
+    diagnostics.provider_state =
+        mojom::GeolocationDiagnostics::ProviderState::kStopped;
+    return;
+  }
+  for (auto& provider : providers_) {
+    provider->FillDiagnostics(diagnostics);
+  }
+}
+
 void LocationArbitrator::SetUpdateCallback(
     const LocationProviderUpdateCallback& callback) {
   DCHECK(!callback.is_null());

services/device/geolocation/location_arbitrator.h

@@ -19,6 +19,7 @@
 #include "services/device/geolocation/network_location_provider.h"
 #include "services/device/geolocation/position_cache.h"
 #include "services/device/public/cpp/geolocation/location_provider.h"
+#include "services/device/public/mojom/geolocation_internals.mojom.h"
 #include "services/device/public/mojom/geoposition.mojom.h"
 #include "url/gurl.h"
 
@@ -61,6 +62,7 @@ class LocationArbitrator : public LocationProvider {
   bool HasPermissionBeenGrantedForTest() const;
 
   // LocationProvider implementation.
+  void FillDiagnostics(mojom::GeolocationDiagnostics& diagnostics) override;
   void SetUpdateCallback(
       const LocationProviderUpdateCallback& callback) override;
   void StartProvider(bool enable_high_accuracy) override;

services/device/geolocation/location_arbitrator_unittest.cc

@@ -113,6 +113,12 @@ class TestingLocationArbitrator : public LocationArbitrator {
     return base::WrapUnique(system_location_provider_.get());
   }
 
+  mojom::GeolocationDiagnostics::ProviderState state() {
+    mojom::GeolocationDiagnostics diagnostics;
+    FillDiagnostics(diagnostics);
+    return diagnostics.provider_state;
+  }
+
   raw_ptr<FakeLocationProvider, DanglingUntriaged> network_location_provider_ =
       nullptr;
   raw_ptr<FakeLocationProvider> system_location_provider_ = nullptr;
@@ -180,6 +186,8 @@ TEST_F(GeolocationLocationArbitratorTest, CreateDestroy) {
   InitializeArbitrator(
       base::BindRepeating(&GetCustomLocationProviderForTest, nullptr), nullptr);
   EXPECT_TRUE(arbitrator_);
+  EXPECT_EQ(arbitrator_->state(),
+            mojom::GeolocationDiagnostics::ProviderState::kStopped);
   arbitrator_.reset();
   SUCCEED();
 }
@@ -210,8 +218,8 @@ TEST_F(GeolocationLocationArbitratorTest, NormalUsageNetwork) {
 
   ASSERT_TRUE(network_location_provider());
   EXPECT_FALSE(system_location_provider());
-  EXPECT_EQ(FakeLocationProvider::LOW_ACCURACY,
-            network_location_provider()->state_);
+  EXPECT_EQ(mojom::GeolocationDiagnostics::ProviderState::kLowAccuracy,
+            network_location_provider()->state());
   EXPECT_FALSE(observer_->last_result());
 
   SetReferencePosition(network_location_provider());
@@ -244,8 +252,8 @@ TEST_F(GeolocationLocationArbitratorTest, NormalUsageSystem) {
 
   EXPECT_FALSE(network_location_provider());
   ASSERT_TRUE(system_location_provider());
-  EXPECT_EQ(FakeLocationProvider::LOW_ACCURACY,
-            system_location_provider()->state_);
+  EXPECT_EQ(mojom::GeolocationDiagnostics::ProviderState::kLowAccuracy,
+            system_location_provider()->state());
   EXPECT_FALSE(observer_->last_result());
 
   SetReferencePosition(system_location_provider());
@@ -281,7 +289,8 @@ TEST_F(GeolocationLocationArbitratorTest, CustomSystemProviderOnly) {
 
   EXPECT_FALSE(network_location_provider());
   EXPECT_FALSE(system_location_provider());
-  EXPECT_EQ(FakeLocationProvider::LOW_ACCURACY, fake_location_provider->state_);
+  EXPECT_EQ(mojom::GeolocationDiagnostics::ProviderState::kLowAccuracy,
+            fake_location_provider->state());
   EXPECT_FALSE(observer_->last_result());
 
   SetReferencePosition(fake_location_provider);
@@ -309,14 +318,14 @@ TEST_F(GeolocationLocationArbitratorTest, SetObserverOptions) {
   arbitrator_->StartProvider(false);
   ASSERT_TRUE(network_location_provider());
   EXPECT_FALSE(system_location_provider());
-  EXPECT_EQ(FakeLocationProvider::LOW_ACCURACY,
-            network_location_provider()->state_);
+  EXPECT_EQ(mojom::GeolocationDiagnostics::ProviderState::kLowAccuracy,
+            network_location_provider()->state());
   SetReferencePosition(network_location_provider());
-  EXPECT_EQ(FakeLocationProvider::LOW_ACCURACY,
-            network_location_provider()->state_);
+  EXPECT_EQ(mojom::GeolocationDiagnostics::ProviderState::kLowAccuracy,
+            network_location_provider()->state());
   arbitrator_->StartProvider(true);
-  EXPECT_EQ(FakeLocationProvider::HIGH_ACCURACY,
-            network_location_provider()->state_);
+  EXPECT_EQ(mojom::GeolocationDiagnostics::ProviderState::kHighAccuracy,
+            network_location_provider()->state());
 }
 
 // Verifies that the arbitrator doesn't retain pointers to old providers after

services/device/geolocation/location_provider_android.cc

@@ -29,6 +29,11 @@ void LocationProviderAndroid::NotifyNewGeoposition(
     callback_.Run(this, last_result_.Clone());
 }
 
+void LocationProviderAndroid::FillDiagnostics(
+    mojom::GeolocationDiagnostics& diagnostics) {
+  diagnostics.provider_state = state_;
+}
+
 void LocationProviderAndroid::SetUpdateCallback(
     const LocationProviderUpdateCallback& callback) {
   DCHECK(thread_checker_.CalledOnValidThread());
@@ -37,6 +42,9 @@ void LocationProviderAndroid::SetUpdateCallback(
 
 void LocationProviderAndroid::StartProvider(bool high_accuracy) {
   DCHECK(thread_checker_.CalledOnValidThread());
+  state_ = high_accuracy
+               ? mojom::GeolocationDiagnostics::ProviderState::kHighAccuracy
+               : mojom::GeolocationDiagnostics::ProviderState::kLowAccuracy;
   LocationApiAdapterAndroid::GetInstance()->Start(
       base::BindRepeating(&LocationProviderAndroid::NotifyNewGeoposition,
                           weak_ptr_factory_.GetWeakPtr()),
@@ -45,6 +53,7 @@ void LocationProviderAndroid::StartProvider(bool high_accuracy) {
 
 void LocationProviderAndroid::StopProvider() {
   DCHECK(thread_checker_.CalledOnValidThread());
+  state_ = mojom::GeolocationDiagnostics::ProviderState::kStopped;
   LocationApiAdapterAndroid::GetInstance()->Stop();
 }
 

services/device/geolocation/location_provider_android.h

@@ -9,6 +9,7 @@
 #include "base/memory/weak_ptr.h"
 #include "base/threading/thread_checker.h"
 #include "services/device/public/cpp/geolocation/location_provider.h"
+#include "services/device/public/mojom/geolocation_internals.mojom.h"
 #include "services/device/public/mojom/geoposition.mojom.h"
 
 namespace device {
@@ -23,6 +24,7 @@ class LocationProviderAndroid : public LocationProvider {
   void NotifyNewGeoposition(mojom::GeopositionResultPtr result);
 
   // LocationProvider implementation.
+  void FillDiagnostics(mojom::GeolocationDiagnostics& diagnostics) override;
   void SetUpdateCallback(
       const LocationProviderUpdateCallback& callback) override;
   void StartProvider(bool high_accuracy) override;
@@ -33,6 +35,8 @@ class LocationProviderAndroid : public LocationProvider {
  private:
   base::ThreadChecker thread_checker_;
 
+  mojom::GeolocationDiagnostics::ProviderState state_ =
+      mojom::GeolocationDiagnostics::ProviderState::kStopped;
   mojom::GeopositionResultPtr last_result_;
   LocationProviderUpdateCallback callback_;