[Device Info] Implement gc on startup

This CL implements a garbage collection routine for device info. This
routine is run on every startup of sync. This is okay from performance
point of view as all data is anyway in memory at this stage and number
of entities we iterate over is small.

Bug: 1007233
Change-Id: Icd6165a532a67218bcdf1d4d275b5b12cddbfaf9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1821785
Commit-Queue: Jan Krcal <jkrcal@chromium.org>
Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#699791}

components/sync_device_info/device_info_sync_bridge.cc

@@ -43,6 +43,8 @@ using ClientIdToSpecifics =
 
 namespace {
 
+constexpr base::TimeDelta kExpirationThreshold = base::TimeDelta::FromDays(56);
+
 // Find the timestamp for the last time this |device_info| was edited.
 Time GetLastUpdateTime(const DeviceInfoSpecifics& specifics) {
   if (specifics.has_last_updated_timestamp()) {
@@ -209,6 +211,7 @@ base::Optional<ModelError> DeviceInfoSyncBridge::ApplySyncChanges(
     }
 
     if (change->type() == EntityChange::ACTION_DELETE) {
+      // This should never get exercised as no client issues tombstones.
       has_changes |= DeleteSpecifics(guid, batch.get());
     } else {
       const DeviceInfoSpecifics& specifics =
@@ -444,6 +447,7 @@ void DeviceInfoSyncBridge::OnReadAllMetadata(
   // This probably isn't strictly needed, but in case the cache_guid has changed
   // we save the new one to prefs.
   device_info_prefs_->AddLocalCacheGuid(local_cache_guid_);
+  ExpireOldEntries();
   ReconcileLocalAndStored();
 }
 
@@ -556,4 +560,31 @@ int DeviceInfoSyncBridge::CountActiveDevices(const Time now) const {
   return max_overlapping_sum;
 }
 
+void DeviceInfoSyncBridge::ExpireOldEntries() {
+  const base::Time expiration_threshold =
+      base::Time::Now() - kExpirationThreshold;
+  std::unordered_set<std::string> cache_guids_to_expire;
+  // Just collecting cache guids to expire to avoid modifying |all_data_| via
+  // DeleteSpecifics() while iterating over it.
+  for (const auto& pair : all_data_) {
+    const std::string& cache_guid = pair.first;
+    if (cache_guid != local_cache_guid_ &&
+        GetLastUpdateTime(*pair.second) < expiration_threshold) {
+      cache_guids_to_expire.insert(cache_guid);
+    }
+  }
+
+  if (cache_guids_to_expire.empty()) {
+    return;
+  }
+
+  std::unique_ptr<WriteBatch> batch = store_->CreateWriteBatch();
+  for (const std::string& cache_guid : cache_guids_to_expire) {
+    DeleteSpecifics(cache_guid, batch.get());
+    batch->GetMetadataChangeList()->ClearMetadata(cache_guid);
+    change_processor()->UntrackEntityForStorageKey(cache_guid);
+  }
+  CommitAndNotify(std::move(batch), /*should_notify=*/true);
+}
+
 }  // namespace syncer

components/sync_device_info/device_info_sync_bridge.h

@@ -127,6 +127,9 @@ class DeviceInfoSyncBridge : public ModelTypeSyncBridge,
   // allow unit tests to control expected results.
   int CountActiveDevices(const base::Time now) const;
 
+  // Deletes locally old data and metadata entries without issuing tombstones.
+  void ExpireOldEntries();
+
   const std::unique_ptr<MutableLocalDeviceInfoProvider>
       local_device_info_provider_;
 

components/sync_device_info/device_info_sync_bridge_unittest.cc

@@ -950,6 +950,35 @@ TEST_F(DeviceInfoSyncBridgeTest, ApplyStopSyncChangesWithKeepData) {
   EXPECT_TRUE(bridge()->IsPulseTimerRunningForTest());
 }
 
+TEST_F(DeviceInfoSyncBridgeTest, ExpireOldEntriesUponStartup) {
+  InitializeAndMergeInitialData();
+  ASSERT_EQ(1u, bridge()->GetAllDeviceInfo().size());
+  ASSERT_EQ(1, change_count());
+  ASSERT_FALSE(ReadAllFromStore().empty());
+
+  const DeviceInfoSpecifics specifics_old =
+      CreateSpecifics(1, base::Time::Now() - base::TimeDelta::FromDays(57));
+  const DeviceInfoSpecifics specifics_fresh =
+      CreateSpecifics(1, base::Time::Now() - base::TimeDelta::FromDays(55));
+  auto error = bridge()->ApplySyncChanges(
+      bridge()->CreateMetadataChangeList(),
+      EntityAddList({specifics_old, specifics_fresh}));
+
+  ASSERT_FALSE(error);
+  ASSERT_EQ(2u, bridge()->GetAllDeviceInfo().size());
+  ASSERT_EQ(2, change_count());
+
+  // Reloading from storage should expire the old remote entity (but keep the
+  // fresh one).
+  RestartBridge();
+  EXPECT_EQ(2u, bridge()->GetAllDeviceInfo().size());
+  // Make sure this is well persisted to the DB store.
+  EXPECT_THAT(ReadAllFromStore(),
+              UnorderedElementsAre(
+                  Pair(local_device()->GetLocalDeviceInfo()->guid(), _),
+                  Pair(specifics_fresh.cache_guid(), _)));
+}
+
 }  // namespace
 
 }  // namespace syncer