geolocation: Add metrics for Geolocation accuracy, mode, and source

This CL adds new metrics to track accuracy value, the mode and provider source of LocationProviderManager of a valid Geoposition. Bug: 366493546 Change-Id: I75b67ed06f14f3fe31fd7d3854c7b1d10329157e Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5855586 Commit-Queue: Jack Hsieh <[email protected]> Reviewed-by: Reilly Grant <[email protected]> Reviewed-by: Tom Sepez <[email protected]> Cr-Commit-Position: refs/heads/main@{#1357353}
azx1573 · Sep 18, 2024 · 6aa5080 · 6aa5080
1 parent 11263b1
commit 6aa5080
Show file tree

Hide file tree

Showing 4 changed files with 76 additions and 5 deletions.
diff --git a/services/device/geolocation/location_provider_manager.cc b/services/device/geolocation/location_provider_manager.cc
@@ -12,6 +12,7 @@
 #include "base/functional/bind.h"
 #include "base/functional/callback_helpers.h"
 #include "base/memory/ptr_util.h"
+#include "base/metrics/histogram_functions.h"
 #include "base/task/single_thread_task_runner.h"
 #include "build/build_config.h"
 #include "services/device/geolocation/network_location_provider.h"
@@ -29,6 +30,19 @@ using ::device::mojom::LocationProviderManagerMode::kHybridPlatform;
 using ::device::mojom::LocationProviderManagerMode::kNetworkOnly;
 using ::device::mojom::LocationProviderManagerMode::kPlatformOnly;
 
+enum class LocationProviderManagerSource {
+  // NOTE: Do not renumber these as that would confuse interpretation of
+  // previously logged data. When making changes, also update the enum list
+  // in tools/metrics/histograms/metadata/geolocation/enums.xml to keep it in
+  // sync.
+  kNetworkProvider = 0,
+  kPlatformProvider = 1,
+  kCustomProvider = 2,
+
+  // NOTE: Add entries only immediately above this line.
+  kMaxValue = kCustomProvider,
+};
+
 LocationProviderManager::LocationProviderManager(
     CustomLocationProviderCallback custom_location_provider_getter,
     GeolocationSystemPermissionManager* geolocation_system_permission_manager,
@@ -223,6 +237,24 @@ void LocationProviderManager::OnLocationUpdate(
       break;
   }
 
+  if (new_result->is_position()) {
+    base::UmaHistogramEnumeration("Geolocation.LocationProviderManager.Mode",
+                                  provider_manager_mode_);
+
+    LocationProviderManagerSource source;
+    if (provider == network_location_provider_.get()) {
+      source = LocationProviderManagerSource::kNetworkProvider;
+    } else if (provider == platform_location_provider_.get()) {
+      source = LocationProviderManagerSource::kPlatformProvider;
+    } else if (provider == custom_location_provider_.get()) {
+      source = LocationProviderManagerSource::kCustomProvider;
+    } else {
+      NOTREACHED();
+    }
+    base::UmaHistogramEnumeration("Geolocation.LocationProviderManager.Source",
+                                  source);
+  }
+
   location_update_callback_.Run(this, std::move(new_result));
 }
 

diff --git a/services/device/public/mojom/geolocation_internals.mojom b/services/device/public/mojom/geolocation_internals.mojom
@@ -137,11 +137,16 @@ struct NetworkLocationResponse {
 // Determines which type of location provider LocationProviderManager should
 // create and use for geolocation.
 enum LocationProviderManagerMode {
-  kNetworkOnly,
-  kPlatformOnly,
-  kCustomOnly,
-  kHybridPlatform,
-  kHybridFallbackNetwork,
+  kNetworkOnly = 0,
+  kPlatformOnly = 1,
+  kCustomOnly = 2,
+  kHybridPlatform = 3,
+  kHybridFallbackNetwork = 4,
+
+  // NOTE: Do not renumber these as that would confuse interpretation of
+  // previously logged data. When making changes, also update the enum list
+  // in tools/metrics/histograms/metadata/geolocation/enums.xml to keep it in
+  // sync.
 };
 
 // Interface for chrome://location-internals to be notified when the debug data

diff --git a/tools/metrics/histograms/metadata/geolocation/enums.xml b/tools/metrics/histograms/metadata/geolocation/enums.xml
@@ -74,6 +74,20 @@ [email protected].
   <int value="4" label="Authorized when in-use"/>
 </enum>
 
+<enum name="LocationProviderManagerMode">
+  <int value="0" label="kNetworkOnly"/>
+  <int value="1" label="kPlatformOnly"/>
+  <int value="2" label="kCustomOnly"/>
+  <int value="3" label="kHybridPlatform"/>
+  <int value="4" label="kHybridFallbackNetwork"/>
+</enum>
+
+<enum name="LocationProviderManagerSource">
+  <int value="0" label="kNetworkProvider"/>
+  <int value="1" label="kPlatformProvider"/>
+  <int value="2" label="kCustomProvider"/>
+</enum>
+
 <enum name="LocationSystemPermissionStatus">
   <int value="0" label="kNotDetermined"/>
   <int value="1" label="kDenied"/>

diff --git a/tools/metrics/histograms/metadata/geolocation/histograms.xml b/tools/metrics/histograms/metadata/geolocation/histograms.xml
@@ -72,6 +72,26 @@ [email protected].
   <summary>iOS location permission state, recorded at startup.</summary>
 </histogram>
 
+<histogram name="Geolocation.LocationProviderManager.Mode"
+    enum="LocationProviderManagerMode" expires_after="2026-09-10">
+  <owner>[email protected]</owner>
+  <owner>[email protected]</owner>
+  <summary>
+    Records the mode of the LocationProviderManager that acquires a valid
+    Geoposition.
+  </summary>
+</histogram>
+
+<histogram name="Geolocation.LocationProviderManager.Source"
+    enum="LocationProviderManagerSource" expires_after="2026-09-10">
+  <owner>[email protected]</owner>
+  <owner>[email protected]</owner>
+  <summary>
+    Records the location provider source of the LocationProviderManager that
+    acquires a valid Geoposition.
+  </summary>
+</histogram>
+
 <histogram name="Geolocation.LocationProviderWinrt.CreateGeoposition"
     enum="Hresult" expires_after="2026-08-05">
   <owner>[email protected]</owner>