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

components/sync/user_events/user_event_sync_bridge_unittest.cc

 // Copyright 2017 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/sync/user_events/user_event_sync_bridge.h"
 
 #include <map>
 #include <set>
+#include <utility>
 
 #include "base/bind.h"
 #include "base/memory/ptr_util.h"
 #include "base/message_loop/message_loop.h"
 #include "base/run_loop.h"
 #include "components/sync/driver/fake_sync_service.h"
 #include "components/sync/model/data_batch.h"
 #include "components/sync/model/model_type_store_test_util.h"
 #include "components/sync/model/recording_model_type_change_processor.h"
 #include "components/sync/protocol/sync.pb.h"
@@ skipping 50 matching lines @@
   return specifics;
 }
 
 std::unique_ptr<UserEventSpecifics> SpecificsUniquePtr(int64_t event_time_usec,
                                                        int64_t navigation_id,
                                                        uint64_t session_id) {
   return base::MakeUnique<UserEventSpecifics>(
       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_;
 };
 
 TEST_F(UserEventSyncBridgeTest, MetadataIsInitialized) {
   base::RunLoop().RunUntilIdle();
   EXPECT_TRUE(processor().metadata()->GetModelTypeState().initial_sync_done());
 }
 
 TEST_F(UserEventSyncBridgeTest, SingleRecord) {
   const UserEventSpecifics specifics(CreateSpecifics(1u, 2u, 3u));
@@ skipping 31 matching lines @@
   bridge()->GetAllData(base::Bind(&VerifyDataBatchCount, 2));
 
   const std::string storage_key = processor().put_multimap().begin()->first;
   auto error_on_delete =
       bridge()->ApplySyncChanges(bridge()->CreateMetadataChangeList(),
                                  {EntityChange::CreateDelete(storage_key)});
   EXPECT_FALSE(error_on_delete);
   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