[Sync] Maintain a global_id mapping to update UserEvents that references navigations

components/sync/user_events/user_event_sync_bridge_unittest.cc

Index: components/sync/user_events/user_event_sync_bridge_unittest.cc
diff --git a/components/sync/user_events/user_event_sync_bridge_unittest.cc b/components/sync/user_events/user_event_sync_bridge_unittest.cc
index 33443cce12afc73d3b5f5367037ccf3e80195bf6..4dbf8be3cb317a790dda7c5be18a08b347b2d751 100644
--- a/components/sync/user_events/user_event_sync_bridge_unittest.cc
+++ b/components/sync/user_events/user_event_sync_bridge_unittest.cc
@@ -6,6 +6,7 @@
 
 #include <map>
 #include <set>
+#include <utility>
 
 #include "base/bind.h"
 #include "base/memory/ptr_util.h"
@@ -76,20 +77,44 @@ std::unique_ptr<UserEventSpecifics> SpecificsUniquePtr(int64_t event_time_usec,
       CreateSpecifics(event_time_usec, navigation_id, session_id));
 }
 
+class TestGlobalIdMapper : public GlobalIdMapper {
+ public:
+  void AddGlobalIdChangeObserver(GlobalIdChange callback) override {
+    callback_ = std::move(callback);
+  }
+
+  int64_t GetLatestGlobalId(int64_t global_id) override {
+    auto iter = id_map_.find(global_id);
+    return iter == id_map_.end() ? global_id : iter->second;
+  }
+
+  void ChangeId(int64_t old_id, int64_t new_id) {
+    id_map_[old_id] = new_id;
+    callback_.Run(old_id, new_id);
+  }
+
+ private:
+  GlobalIdChange callback_;
+  std::map<int64_t, int64_t> id_map_;
+};
+
 class UserEventSyncBridgeTest : public testing::Test {
  protected:
   UserEventSyncBridgeTest() {
     bridge_ = base::MakeUnique<UserEventSyncBridge>(
         ModelTypeStoreTestUtil::FactoryForInMemoryStoreForTest(),
-        RecordingModelTypeChangeProcessor::FactoryForBridgeTest(&processor_));
+        RecordingModelTypeChangeProcessor::FactoryForBridgeTest(&processor_),
+        &test_global_id_mapper_);
   }
 
   UserEventSyncBridge* bridge() { return bridge_.get(); }
   const RecordingModelTypeChangeProcessor& processor() { return *processor_; }
+  TestGlobalIdMapper* mapper() { return &test_global_id_mapper_; }
 
  private:
   std::unique_ptr<UserEventSyncBridge> bridge_;
   RecordingModelTypeChangeProcessor* processor_;
+  TestGlobalIdMapper test_global_id_mapper_;
   base::MessageLoop message_loop_;
 };
 
@@ -141,6 +166,35 @@ TEST_F(UserEventSyncBridgeTest, ApplySyncChanges) {
   bridge()->GetAllData(base::Bind(&VerifyDataBatchCount, 1));
 }
 
+TEST_F(UserEventSyncBridgeTest, HandleGlobalIdChange) {
+  const UserEventSpecifics specifics1(CreateSpecifics(1u, 2u, 3u));
+  const UserEventSpecifics specifics2(CreateSpecifics(1u, 4u, 3u));
+  const UserEventSpecifics specifics3(CreateSpecifics(1u, 5u, 3u));
+
+  mapper()->ChangeId(2u, 4u);
+  bridge()->RecordUserEvent(base::MakeUnique<UserEventSpecifics>(specifics1));
+  const std::string storage_key = processor().put_multimap().begin()->first;
+  EXPECT_EQ(1u, processor().put_multimap().size());
+  bridge()->GetAllData(VerifyCallback({{storage_key, specifics2}}));
+
+  // This id update is done while the event is "in flight", and should result in
+  // it being updated and re-sent to sync.
+  mapper()->ChangeId(4u, 5u);
+  EXPECT_EQ(2u, processor().put_multimap().size());
+  bridge()->GetAllData(VerifyCallback({{storage_key, specifics3}}));

[Patrick Noland, 2017/06/28 22:44:37]

It's kind of hard to follow the different timestamps/ids through this test. Can
you make them variables with names(and rename the specifics they belong to) so
that it's apparent what you're expecting? e.g.

uint64_t first_id = 2u;
uint64_t second_id = 4u;
const UserEventSpecifics specifics_with_second_id(CreateSpecifics(1u, second_id,
3u));

mapper()->ChangeId(first_id, second_id) 
...
bridge()->GetAllData(VerifyCallback({{storage_key, specifics_with_second_id}}))

[skym, 2017/07/05 19:14:28]

Done.

+  auto error_on_delete =
+      bridge()->ApplySyncChanges(bridge()->CreateMetadataChangeList(),
+                                 {EntityChange::CreateDelete(storage_key)});
+  EXPECT_FALSE(error_on_delete);
+  bridge()->GetAllData(base::Bind(&VerifyDataBatchCount, 0));
+
+  // This id update should be ignored, since we received commit confirmation
+  // above.
+  mapper()->ChangeId(5u, 6u);
+  EXPECT_EQ(2u, processor().put_multimap().size());
+  bridge()->GetAllData(base::Bind(&VerifyDataBatchCount, 0));
+}
+
 }  // namespace
 
 }  // namespace syncer