Log metrics for the geolocation position cache

When the network location provider is in use, Chrome periodically
scans for nearby WiFi access points. The MAC addresses of these
access points are sent to a Google service, which returns a position
estimate. To limit network traffic when a device is not moving, a
cache of recent position estimates is maintained. If the set of MAC
addresses exactly matches a previous query, the cached estimate is
used instead.

This adds metrics for recording whether a cached value was used. It
also records how many items were in the cache when the cache is
queried.

BUG=944320

Change-Id: I385e1f76099207dd9276edc258a958959945058f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1532255
Commit-Queue: Matt Reynolds <mattreynolds@chromium.org>
Reviewed-by: Brian White <bcwhite@chromium.org>
Reviewed-by: Miguel Casas <mcasas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#650536}

services/device/geolocation/fake_position_cache.cc

@@ -44,6 +44,10 @@ const mojom::Geoposition* FakePositionCache::FindPosition(
   return it == data.end() ? nullptr : &(it->second);
 }
 
+size_t FakePositionCache::GetPositionCacheSize() const {
+  return data.size();
+}
+
 const mojom::Geoposition& FakePositionCache::GetLastUsedNetworkPosition()
     const {
   return last_used_position;

services/device/geolocation/fake_position_cache.h

@@ -23,6 +23,7 @@ class FakePositionCache : public PositionCache {
                      const mojom::Geoposition& position) override;
   const mojom::Geoposition* FindPosition(
       const WifiData& wifi_data) const override;
+  size_t GetPositionCacheSize() const override;
   const mojom::Geoposition& GetLastUsedNetworkPosition() const override;
   void SetLastUsedNetworkPosition(const mojom::Geoposition& position) override;
 

services/device/geolocation/network_location_provider.cc

@@ -8,6 +8,8 @@
 
 #include "base/bind.h"
 #include "base/location.h"
+#include "base/metrics/histogram_macros.h"
+#include "base/numerics/ranges.h"
 #include "base/single_thread_task_runner.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/threading/thread_task_runner_handle.h"
@@ -179,6 +181,12 @@ void NetworkLocationProvider::RequestPosition() {
 
   const mojom::Geoposition* cached_position =
       position_cache_->FindPosition(wifi_data_);
+
+  UMA_HISTOGRAM_BOOLEAN("Geolocation.PositionCache.CacheHit",
+                        cached_position != nullptr);
+  UMA_HISTOGRAM_COUNTS_100("Geolocation.PositionCache.CacheSize",
+                           position_cache_->GetPositionCacheSize());
+
   if (cached_position) {
     mojom::Geoposition position(*cached_position);
     DCHECK(ValidateGeoposition(position));

services/device/geolocation/position_cache.h

@@ -31,6 +31,9 @@ class PositionCache {
   virtual const mojom::Geoposition* FindPosition(
       const WifiData& wifi_data) const = 0;
 
+  // Returns the number of cached position responses stored in the cache.
+  virtual size_t GetPositionCacheSize() const = 0;
+
   // Returns most recently used position, or an invalid Geoposition if
   // SetLastUsedNetworkPosition wasn't called yet.
   virtual const mojom::Geoposition& GetLastUsedNetworkPosition() const = 0;

services/device/geolocation/position_cache_impl.cc

@@ -81,6 +81,10 @@ const mojom::Geoposition* PositionCacheImpl::FindPosition(
   return it == data_.end() ? nullptr : (it->position());
 }
 
+size_t PositionCacheImpl::GetPositionCacheSize() const {
+  return data_.size();
+}
+
 const mojom::Geoposition& PositionCacheImpl::GetLastUsedNetworkPosition()
     const {
   return last_used_position_;

services/device/geolocation/position_cache_impl.h

@@ -44,6 +44,8 @@ class PositionCacheImpl
   const mojom::Geoposition* FindPosition(
       const WifiData& wifi_data) const override;
 
+  size_t GetPositionCacheSize() const override;
+
   const mojom::Geoposition& GetLastUsedNetworkPosition() const override;
   void SetLastUsedNetworkPosition(const mojom::Geoposition& position) override;
 

services/device/geolocation/position_cache_impl_unittest.cc

@@ -34,6 +34,7 @@ class PositionCacheImplTest : public ::testing::Test {
 TEST_F(PositionCacheImplTest, EmptyCacheReturnsNoLocations) {
   WifiData empty_wifi_data;
   EXPECT_EQ(nullptr, cache_.FindPosition(empty_wifi_data));
+  EXPECT_EQ(0U, cache_.GetPositionCacheSize());
 }
 
 TEST_F(PositionCacheImplTest, CanAddEmptyWifiData) {
@@ -45,6 +46,7 @@ TEST_F(PositionCacheImplTest, CanAddEmptyWifiData) {
       cache_.FindPosition(empty_wifi_data);
   ASSERT_NE(nullptr, found_position);
   EXPECT_TRUE(geoposition.Equals(*found_position));
+  EXPECT_EQ(1U, cache_.GetPositionCacheSize());
 }
 
 TEST_F(PositionCacheImplTest, FirstAddedWifiDataReturned) {
@@ -55,6 +57,7 @@ TEST_F(PositionCacheImplTest, FirstAddedWifiDataReturned) {
   const mojom::Geoposition* found_position = cache_.FindPosition(wifi_data);
   ASSERT_NE(nullptr, found_position);
   EXPECT_TRUE(geoposition.Equals(*found_position));
+  EXPECT_EQ(1U, cache_.GetPositionCacheSize());
 }
 
 TEST_F(PositionCacheImplTest, LastAddedWifiDataReturned) {
@@ -71,6 +74,7 @@ TEST_F(PositionCacheImplTest, LastAddedWifiDataReturned) {
       cache_.FindPosition(final_wifi_data);
   ASSERT_NE(nullptr, found_position);
   EXPECT_TRUE(final_geoposition.Equals(*found_position));
+  EXPECT_EQ(3U, cache_.GetPositionCacheSize());
 }
 
 TEST_F(PositionCacheImplTest, MaxPositionsFound) {
@@ -84,6 +88,7 @@ TEST_F(PositionCacheImplTest, MaxPositionsFound) {
   for (const auto& test_data_pair : test_data) {
     cache_.CachePosition(test_data_pair.first, test_data_pair.second);
   }
+  EXPECT_EQ(PositionCacheImpl::kMaximumSize, cache_.GetPositionCacheSize());
   // Make sure all elements are cached.
   for (const auto& test_data_pair : test_data) {
     const mojom::Geoposition* found_position =
@@ -97,6 +102,7 @@ TEST_F(PositionCacheImplTest, Eviction) {
   WifiData initial_wifi_data = testing::CreateDefaultUniqueWifiData();
   mojom::Geoposition initial_geoposition = testing::CreateGeoposition(1);
   cache_.CachePosition(initial_wifi_data, initial_geoposition);
+  EXPECT_EQ(1U, cache_.GetPositionCacheSize());
 
   // Add as many entries as the cache's size limit, which should evict
   // |initial_wifi_data|.
@@ -104,6 +110,7 @@ TEST_F(PositionCacheImplTest, Eviction) {
     cache_.CachePosition(testing::CreateDefaultUniqueWifiData(),
                          testing::CreateGeoposition(i));
   }
+  EXPECT_EQ(PositionCacheImpl::kMaximumSize, cache_.GetPositionCacheSize());
 
   // |initial_wifi_data| can no longer be found in cache_.
   ASSERT_EQ(nullptr, cache_.FindPosition(initial_wifi_data));
@@ -128,17 +135,20 @@ TEST_F(PositionCacheImplTest, EntryEvictedAfterMaxLifetimeReached) {
       cache_.FindPosition(initial_wifi_data);
   ASSERT_NE(nullptr, found_position);
   EXPECT_TRUE(initial_geoposition.Equals(*found_position));
+  EXPECT_EQ(1U, cache_.GetPositionCacheSize());
 
   task_environment_.FastForwardBy(PositionCacheImpl::kMaximumLifetime);
 
   // Position was evicted.
   EXPECT_EQ(nullptr, cache_.FindPosition(initial_wifi_data));
+  EXPECT_EQ(0U, cache_.GetPositionCacheSize());
 }
 
 TEST_F(PositionCacheImplTest, OnlyOldEntriesEvicted) {
   WifiData older_wifi_data = testing::CreateDefaultUniqueWifiData();
   mojom::Geoposition older_geoposition = testing::CreateGeoposition(1);
   cache_.CachePosition(older_wifi_data, older_geoposition);
+  EXPECT_EQ(1U, cache_.GetPositionCacheSize());
 
   // Some time passes, but less than kMaximumLifetime
   task_environment_.FastForwardBy(PositionCacheImpl::kMaximumLifetime * 0.5);
@@ -148,11 +158,13 @@ TEST_F(PositionCacheImplTest, OnlyOldEntriesEvicted) {
       cache_.FindPosition(older_wifi_data);
   ASSERT_NE(nullptr, found_position);
   EXPECT_TRUE(older_geoposition.Equals(*found_position));
+  EXPECT_EQ(1U, cache_.GetPositionCacheSize());
 
   // New position is added.
   WifiData newer_wifi_data = testing::CreateDefaultUniqueWifiData();
   mojom::Geoposition newer_geoposition = testing::CreateGeoposition(2);
   cache_.CachePosition(newer_wifi_data, newer_geoposition);
+  EXPECT_EQ(2U, cache_.GetPositionCacheSize());
 
   // Enough time passes to evict the older entry, but not enough to evict the
   // newer one.
@@ -164,6 +176,7 @@ TEST_F(PositionCacheImplTest, OnlyOldEntriesEvicted) {
       cache_.FindPosition(newer_wifi_data);
   ASSERT_NE(nullptr, found_newer_position);
   EXPECT_TRUE(newer_geoposition.Equals(*found_newer_position));
+  EXPECT_EQ(1U, cache_.GetPositionCacheSize());
 }
 
 TEST_F(PositionCacheImplTest, NetworkChangeClearsEmptyWifiDataPosition) {
@@ -171,11 +184,13 @@ TEST_F(PositionCacheImplTest, NetworkChangeClearsEmptyWifiDataPosition) {
   WifiData initial_wifi_data = testing::CreateDefaultUniqueWifiData();
   mojom::Geoposition initial_geoposition = testing::CreateGeoposition(1);
   cache_.CachePosition(initial_wifi_data, initial_geoposition);
+  EXPECT_EQ(1U, cache_.GetPositionCacheSize());
 
   // Cache a position for empty WifiData (wired network).
   WifiData empty_wifi_data;
   mojom::Geoposition empty_data_geoposition = testing::CreateGeoposition(2);
   cache_.CachePosition(empty_wifi_data, empty_data_geoposition);
+  EXPECT_EQ(2U, cache_.GetPositionCacheSize());
 
   cache_.SetLastUsedNetworkPosition(initial_geoposition);
 
@@ -197,5 +212,6 @@ TEST_F(PositionCacheImplTest, NetworkChangeClearsEmptyWifiDataPosition) {
       cache_.FindPosition(initial_wifi_data);
   ASSERT_NE(nullptr, found_position);
   EXPECT_TRUE(initial_geoposition.Equals(*found_position));
+  EXPECT_EQ(1U, cache_.GetPositionCacheSize());
 }
 }  // namespace device

tools/metrics/histograms/histograms.xml

@@ -41036,6 +41036,26 @@ uploading your change for review.
   <summary>Http response codes in NetworkLocationRequest.</summary>
 </histogram>
 
+<histogram name="Geolocation.PositionCache.CacheHit" units="BooleanCacheHit"
+    expires_after="2019-09-20">
+  <owner>mattreynolds@chromium.org</owner>
+  <owner>deviceapi-team@google.com</owner>
+  <summary>
+    For each query into the position cache used by the network location
+    provider, records whether a position was returned from the cache.
+  </summary>
+</histogram>
+
+<histogram name="Geolocation.PositionCache.CacheSize" units="count"
+    expires_after="2019-09-20">
+  <owner>mattreynolds@chromium.org</owner>
+  <owner>deviceapi-team@google.com</owner>
+  <summary>
+    For each query into the position cache used by the network location
+    provider, records the number of items in the cache.
+  </summary>
+</histogram>
+
 <histogram base="true" name="Geolocation.SettingsDialog.AcceptEvent"
     enum="GeolocationSettingsDialogBackOff">
   <owner>benwells@chromium.org</owner>