Commit c51d1eb9 authored by stanisc's avatar stanisc Committed by Commit bot

Sync: Remove VerifyLocalIdToUpdate.

Now that 362467 is fixed, VerifyLocalIdToUpdate function is no
longer needed and should be removed. This function was added two
months ago to help diagnose the root cause.

I've verified that there are no new crash dumps with
VerifyLocalIdToUpdate since 44.0.2401.0.

BUG=362467

Review URL: https://codereview.chromium.org/1145453004

Cr-Commit-Position: refs/heads/master@{#330455}
parent cd48f4c5
...@@ -75,46 +75,6 @@ using syncable::UNIQUE_POSITION; ...@@ -75,46 +75,6 @@ using syncable::UNIQUE_POSITION;
using syncable::UNIQUE_SERVER_TAG; using syncable::UNIQUE_SERVER_TAG;
using syncable::WriteTransaction; using syncable::WriteTransaction;
// TODO (stanisc): crbug.com/362467: remove this function once
// issue 362467 is fixed.
// Validates that the local ID picked by FindLocalIdToUpdate doesn't
// conflict with already existing item with update_id.
void VerifyLocalIdToUpdate(syncable::BaseTransaction* trans,
const syncable::Id& local_id,
const syncable::Id& update_id,
bool local_deleted,
bool deleted_in_update) {
if (local_id == update_id) {
// ID matches, everything is good.
return;
}
// If the ID doesn't match, it means that an entry with |local_id| has been
// picked and an entry with |update_id| isn't supposed to exist.
syncable::Entry update_entry(trans, GET_BY_ID, update_id);
if (!update_entry.good())
return;
// Fail early so that the crash dump indicates which of the cases below
// has triggered the issue.
// Crash dumps don't always preserve data. The 2 separate cases below are
// to make it easy to see the the state of item with |update_id| in the
// crash dump.
if (update_entry.GetIsDel()) {
LOG(FATAL) << "VerifyLocalIdToUpdate: existing deleted entry " << update_id
<< " conflicts with local entry " << local_id
<< " picked by an update.\n"
<< "Local item deleted: " << local_deleted
<< ", deleted flag in update: " << deleted_in_update;
} else {
LOG(FATAL) << "VerifyLocalIdToUpdate: existing entry " << update_id
<< " conflicts with local entry " << local_id
<< " picked by an update.\n"
<< "Local item deleted: " << local_deleted
<< ", deleted flag in update: " << deleted_in_update;
}
}
syncable::Id FindLocalIdToUpdate( syncable::Id FindLocalIdToUpdate(
syncable::BaseTransaction* trans, syncable::BaseTransaction* trans,
const sync_pb::SyncEntity& update) { const sync_pb::SyncEntity& update) {
...@@ -167,8 +127,6 @@ syncable::Id FindLocalIdToUpdate( ...@@ -167,8 +127,6 @@ syncable::Id FindLocalIdToUpdate(
// Target this change to the existing local entry; later, // Target this change to the existing local entry; later,
// we'll change the ID of the local entry to update_id // we'll change the ID of the local entry to update_id
// if needed. // if needed.
VerifyLocalIdToUpdate(trans, local_entry.GetId(), update_id,
local_entry.GetIsDel(), update.deleted());
return local_entry.GetId(); return local_entry.GetId();
} else { } else {
// Case 3: We have a local entry with the same client tag. // Case 3: We have a local entry with the same client tag.
...@@ -178,8 +136,6 @@ syncable::Id FindLocalIdToUpdate( ...@@ -178,8 +136,6 @@ syncable::Id FindLocalIdToUpdate(
// update will now be applied to local_entry. // update will now be applied to local_entry.
DCHECK(0 == local_entry.GetBaseVersion() || DCHECK(0 == local_entry.GetBaseVersion() ||
CHANGES_VERSION == local_entry.GetBaseVersion()); CHANGES_VERSION == local_entry.GetBaseVersion());
VerifyLocalIdToUpdate(trans, local_entry.GetId(), update_id,
local_entry.GetIsDel(), update.deleted());
return local_entry.GetId(); return local_entry.GetId();
} }
} }
...@@ -226,8 +182,6 @@ syncable::Id FindLocalIdToUpdate( ...@@ -226,8 +182,6 @@ syncable::Id FindLocalIdToUpdate(
<< update_id << " local id: " << local_entry.GetId() << update_id << " local id: " << local_entry.GetId()
<< " new version: " << new_version; << " new version: " << new_version;
VerifyLocalIdToUpdate(trans, local_entry.GetId(), update_id,
local_entry.GetIsDel(), update.deleted());
return local_entry.GetId(); return local_entry.GetId();
} }
} else if (update.has_server_defined_unique_tag() && } else if (update.has_server_defined_unique_tag() &&
...@@ -243,8 +197,6 @@ syncable::Id FindLocalIdToUpdate( ...@@ -243,8 +197,6 @@ syncable::Id FindLocalIdToUpdate(
update.server_defined_unique_tag()); update.server_defined_unique_tag());
if (local_entry.good() && !local_entry.GetId().ServerKnows()) { if (local_entry.good() && !local_entry.GetId().ServerKnows()) {
DCHECK(local_entry.GetId() != update_id); DCHECK(local_entry.GetId() != update_id);
VerifyLocalIdToUpdate(trans, local_entry.GetId(), update_id,
local_entry.GetIsDel(), update.deleted());
return local_entry.GetId(); return local_entry.GetId();
} }
} }
......
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