Commit 459c7c57 authored by skym's avatar skym Committed by Commit bot

[Sync] Stop updating local device info time stamp on merge.

When merging, if we're given specifics that correspond to the local
guid, we try to access specifics object for the local guid in
|all_data_|. The problem with this, is that there isn't always an entry
there if reconcile has not happened yet. This only happens when the
initial sync round trip finishes before metadata loads from disk, but
this race is happening to users, see the bug.

The fix could be to check that |all_data_| actually contains an entry
for the local guid, but it really doesn't matter. Local data completely
cleared when sync is disabled, so reconcile will always create a local
entry with a fresh time stamp around the time merge is called. There's
no reason to complicate the logic inside of merge for no real benefit,
so I've simply removed the set_last_updated_timestamp call.

BUG=668938

Review-Url: https://codereview.chromium.org/2536043002
Cr-Commit-Position: refs/heads/master@{#435011}
parent 27fd4897
......@@ -142,11 +142,6 @@ SyncError DeviceInfoSyncBridge::MergeSyncData(
// Don't Put local data if it's the same as the remote copy.
if (local_info->Equals(*SpecificsToModel(specifics))) {
local_guids_to_put.erase(local_guid);
} else {
// This device is valid right now and this entry is about to be
// committed, use this as an opportunity to refresh the timestamp.
all_data_[local_guid]->set_last_updated_timestamp(
TimeToProtoTime(Time::Now()));
}
} else {
// Remote data wins conflicts.
......
......@@ -663,6 +663,21 @@ TEST_F(DeviceInfoSyncBridgeTest, MergeLocalGuid) {
EXPECT_TRUE(processor()->put_multimap().empty());
}
TEST_F(DeviceInfoSyncBridgeTest, MergeLocalGuidBeforeReconcile) {
InitializeBridge();
const DeviceInfoSpecifics specifics = CreateSpecifics(kDefaultLocalSuffix);
// The message loop is never pumped, which means local data/metadata is never
// loaded, and thus reconcile is never called. The bridge should ignore this
// EntityData because its cache guid is the same the local device's.
const SyncError error = bridge()->MergeSyncData(
bridge()->CreateMetadataChangeList(),
{{specifics.cache_guid(), SpecificsToEntity(specifics)}});
EXPECT_FALSE(error.IsSet());
EXPECT_EQ(0, change_count());
EXPECT_EQ(0u, bridge()->GetAllDeviceInfo().size());
}
TEST_F(DeviceInfoSyncBridgeTest, CountActiveDevices) {
InitializeAndPump();
EXPECT_EQ(1, bridge()->CountActiveDevices());
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment