[SendTabToSelf] remove expired sync entries after 10 days.

Bug: 910390
Change-Id: Iad49c4b2d7d45374e1959006fb1f88bd34322fd0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1474791
Commit-Queue: Jeffrey Cohen <jeffreycohen@chromium.org>
Reviewed-by: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#642050}

chrome/browser/sync/test/integration/single_client_send_tab_to_self_sync_test.cc

@@ -2,6 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "base/time/time.h"
 #include "chrome/browser/sync/send_tab_to_self_sync_service_factory.h"
 #include "chrome/browser/sync/test/integration/send_tab_to_self_helper.h"
 #include "chrome/browser/sync/test/integration/sync_test.h"
@@ -29,17 +30,19 @@ IN_PROC_BROWSER_TEST_F(SingleClientSendTabToSelfSyncTest,
                        DownloadWhenSyncEnabled) {
   const std::string kUrl("https://www.example.com");
   const std::string kGuid("kGuid");
-
   sync_pb::EntitySpecifics specifics;
   sync_pb::SendTabToSelfSpecifics* send_tab_to_self =
       specifics.mutable_send_tab_to_self();
   send_tab_to_self->set_url(kUrl);
   send_tab_to_self->set_guid(kGuid);
+  send_tab_to_self->set_shared_time_usec(
+      base::Time::Now().ToDeltaSinceWindowsEpoch().InMicroseconds());
 
   fake_server_->InjectEntity(
       syncer::PersistentUniqueClientEntity::CreateFromSpecificsForTesting(
-          "non_unique_name", kGuid, specifics, /*creation_time=*/0,
-          /*last_modified_time=*/0));
+          "non_unique_name", kGuid, specifics,
+          /*creation_time=*/base::Time::Now().ToTimeT(),
+          /*last_modified_time=*/base::Time::Now().ToTimeT()));
 
   ASSERT_TRUE(SetupSync());
 

components/send_tab_to_self/send_tab_to_self_bridge.cc

@@ -112,13 +112,18 @@ base::Optional<syncer::ModelError> SendTabToSelfBridge::ApplySyncChanges(
 
       std::unique_ptr<SendTabToSelfEntry> entry =
           SendTabToSelfEntry::FromProto(specifics, clock_->Now());
-      // This entry is new. Add it to the model.
-      added.push_back(entry.get());
-      SendTabToSelfLocal entry_pb = entry->AsLocalProto();
-      entries_[entry->GetGUID()] = std::move(entry);
-
-      // Write to the store.
-      batch->WriteData(guid, entry_pb.SerializeAsString());
+      // This entry is new. Add it to the model if it hasn't expired.
+      if (entry->IsExpired(clock_->Now())) {
+        // Remove expired data from server.
+        change_processor()->Delete(guid, batch->GetMetadataChangeList());
+      } else {
+        added.push_back(entry.get());
+        SendTabToSelfLocal entry_pb = entry->AsLocalProto();
+        entries_[entry->GetGUID()] = std::move(entry);
+
+        // Write to the store.
+        batch->WriteData(guid, entry_pb.SerializeAsString());
+      }
     }
   }
 
@@ -395,6 +400,8 @@ void SendTabToSelfBridge::OnReadAllMetadata(
   }
   change_processor()->ModelReadyToSync(std::move(metadata_batch));
   NotifySendTabToSelfModelLoaded();
+
+  DoGarbageCollection();
 }
 
 void SendTabToSelfBridge::OnCommit(
@@ -420,4 +427,21 @@ SendTabToSelfEntry* SendTabToSelfBridge::GetMutableEntryByGUID(
   return it->second.get();
 }
 
+void SendTabToSelfBridge::DoGarbageCollection() {
+  std::vector<std::string> removed;
+
+  auto entry = entries_.begin();
+  while (entry != entries_.end()) {
+    DCHECK_EQ(entry->first, entry->second->GetGUID());
+    std::string guid = entry->first;
+    bool expired = entry->second->IsExpired(clock_->Now());
+    entry++;
+    if (expired) {
+      DeleteEntry(guid);
+      removed.push_back(guid);
+    }
+  }
+  NotifyRemoteSendTabToSelfEntryDeleted(removed);
+}
+
 }  // namespace send_tab_to_self

components/send_tab_to_self/send_tab_to_self_bridge.h

@@ -123,6 +123,9 @@ class SendTabToSelfBridge : public syncer::ModelTypeSyncBridge,
   // exist.
   SendTabToSelfEntry* GetMutableEntryByGUID(const std::string& guid) const;
 
+  // Delete expired entries.
+  void DoGarbageCollection();
+
   // |entries_| is keyed by GUIDs.
   SendTabToSelfEntries entries_;
 

components/send_tab_to_self/send_tab_to_self_bridge_unittest.cc

@@ -97,8 +97,9 @@ class SendTabToSelfBridgeTest : public testing::Test {
     base::RunLoop().RunUntilIdle();
   }
 
-  base::Time AdvanceAndGetTime() {
-    clock_.Advance(base::TimeDelta::FromMilliseconds(10));
+  base::Time AdvanceAndGetTime(
+      base::TimeDelta delta = base::TimeDelta::FromMilliseconds(10)) {
+    clock_.Advance(delta);
     return clock_.Now();
   }
 
@@ -356,6 +357,68 @@ TEST_F(SendTabToSelfBridgeTest, PreserveDissmissalAfterRestartBridge) {
   EXPECT_TRUE(bridge()->GetEntryByGUID(guids[0])->GetNotificationDismissed());
 }
 
+TEST_F(SendTabToSelfBridgeTest, ExpireEntryDuringInit) {
+  InitializeBridge();
+
+  const sync_pb::SendTabToSelfSpecifics expired_specifics =
+      CreateSpecifics(1, AdvanceAndGetTime(), AdvanceAndGetTime());
+
+  AdvanceAndGetTime(kExpiryTime / 2.0);
+
+  const sync_pb::SendTabToSelfSpecifics not_expired_specifics =
+      CreateSpecifics(2, AdvanceAndGetTime(), AdvanceAndGetTime());
+
+  sync_pb::ModelTypeState state = StateWithEncryption("ekn");
+  std::unique_ptr<syncer::MetadataChangeList> metadata_changes =
+      bridge()->CreateMetadataChangeList();
+  metadata_changes->UpdateModelTypeState(state);
+
+  auto error = bridge()->ApplySyncChanges(
+      std::move(metadata_changes),
+      EntityAddList({expired_specifics, not_expired_specifics}));
+  ASSERT_FALSE(error);
+
+  AdvanceAndGetTime(kExpiryTime / 2.0);
+
+  EXPECT_CALL(*mock_observer(), EntriesRemovedRemotely(SizeIs(1)));
+
+  ShutdownBridge();
+  InitializeBridge();
+
+  std::vector<std::string> guids = bridge()->GetAllGuids();
+  EXPECT_EQ(1ul, guids.size());
+  EXPECT_EQ(not_expired_specifics.url(),
+            bridge()->GetEntryByGUID(guids[0])->GetURL().spec());
+}
+
+TEST_F(SendTabToSelfBridgeTest, AddExpiredEntry) {
+  InitializeBridge();
+
+  sync_pb::ModelTypeState state = StateWithEncryption("ekn");
+  std::unique_ptr<syncer::MetadataChangeList> metadata_changes =
+      bridge()->CreateMetadataChangeList();
+  metadata_changes->UpdateModelTypeState(state);
+
+  const sync_pb::SendTabToSelfSpecifics expired_specifics =
+      CreateSpecifics(1, AdvanceAndGetTime(), AdvanceAndGetTime());
+
+  AdvanceAndGetTime(kExpiryTime);
+
+  const sync_pb::SendTabToSelfSpecifics not_expired_specifics =
+      CreateSpecifics(2, AdvanceAndGetTime(), AdvanceAndGetTime());
+
+  auto error = bridge()->ApplySyncChanges(
+      std::move(metadata_changes),
+      EntityAddList({expired_specifics, not_expired_specifics}));
+
+  ASSERT_FALSE(error);
+
+  std::vector<std::string> guids = bridge()->GetAllGuids();
+  EXPECT_EQ(1ul, guids.size());
+  EXPECT_EQ(not_expired_specifics.url(),
+            bridge()->GetEntryByGUID(guids[0])->GetURL().spec());
+}
+
 }  // namespace
 
 }  // namespace send_tab_to_self

components/send_tab_to_self/send_tab_to_self_entry.cc

@@ -131,4 +131,10 @@ std::unique_ptr<SendTabToSelfEntry> SendTabToSelfEntry::FromLocalProto(
   return to_return;
 }
 
+bool SendTabToSelfEntry::IsExpired(base::Time current_time) const {
+  return (current_time.ToDeltaSinceWindowsEpoch() -
+              GetSharedTime().ToDeltaSinceWindowsEpoch() >=
+          kExpiryTime);
+}
+
 }  // namespace send_tab_to_self

components/send_tab_to_self/send_tab_to_self_entry.h

@@ -17,6 +17,8 @@ class SendTabToSelfSpecifics;
 
 namespace send_tab_to_self {
 
+constexpr base::TimeDelta kExpiryTime = base::TimeDelta::FromDays(10);
+
 class SendTabToSelfLocal;
 
 // A tab that is being shared. The URL is a unique identifier for an entry, as
@@ -71,6 +73,9 @@ class SendTabToSelfEntry {
       const SendTabToSelfLocal& pb_entry,
       base::Time now);
 
+  // Returns if the Entry has expired based on the |current_time|.
+  bool IsExpired(base::Time current_time) const;
+
  private:
   std::string guid_;
   GURL url_;

components/send_tab_to_self/send_tab_to_self_entry_unittest.cc

@@ -91,6 +91,17 @@ TEST(SendTabToSelfEntry, FromProto) {
   EXPECT_TRUE(IsEqualForTesting(*entry, *pb_entry));
 }
 
+// Tests that the send tab to self entry expiry works as expected
+TEST(SendTabToSelfEntry, IsExpired) {
+  SendTabToSelfEntry entry("1", GURL("http://example.com"), "bar",
+                           base::Time::FromTimeT(10), base::Time::FromTimeT(10),
+                           "device");
+
+  EXPECT_TRUE(entry.IsExpired(base::Time::FromTimeT(11) +
+                              base::TimeDelta::FromDays(10)));
+  EXPECT_FALSE(entry.IsExpired(base::Time::FromTimeT(11)));
+}
+
 }  // namespace
 
 }  // namespace send_tab_to_self