Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(3)

Side by Side Diff: sync/engine/syncer_util.cc

Issue 1145453004: Sync: Remove VerifyLocalIdToUpdate (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Created 5 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch
« no previous file with comments | « no previous file | no next file » | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
1 // Copyright 2012 The Chromium Authors. All rights reserved. 1 // Copyright 2012 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "sync/engine/syncer_util.h" 5 #include "sync/engine/syncer_util.h"
6 6
7 #include <algorithm> 7 #include <algorithm>
8 #include <set> 8 #include <set>
9 #include <string> 9 #include <string>
10 #include <vector> 10 #include <vector>
(...skipping 57 matching lines...) Expand 10 before | Expand all | Expand 10 after
68 using syncable::SERVER_UNIQUE_POSITION; 68 using syncable::SERVER_UNIQUE_POSITION;
69 using syncable::SERVER_VERSION; 69 using syncable::SERVER_VERSION;
70 using syncable::SPECIFICS; 70 using syncable::SPECIFICS;
71 using syncable::SYNCER; 71 using syncable::SYNCER;
72 using syncable::UNIQUE_BOOKMARK_TAG; 72 using syncable::UNIQUE_BOOKMARK_TAG;
73 using syncable::UNIQUE_CLIENT_TAG; 73 using syncable::UNIQUE_CLIENT_TAG;
74 using syncable::UNIQUE_POSITION; 74 using syncable::UNIQUE_POSITION;
75 using syncable::UNIQUE_SERVER_TAG; 75 using syncable::UNIQUE_SERVER_TAG;
76 using syncable::WriteTransaction; 76 using syncable::WriteTransaction;
77 77
78 // TODO (stanisc): crbug.com/362467: remove this function once
79 // issue 362467 is fixed.
80 // Validates that the local ID picked by FindLocalIdToUpdate doesn't
81 // conflict with already existing item with update_id.
82 void VerifyLocalIdToUpdate(syncable::BaseTransaction* trans,
83 const syncable::Id& local_id,
84 const syncable::Id& update_id,
85 bool local_deleted,
86 bool deleted_in_update) {
87 if (local_id == update_id) {
88 // ID matches, everything is good.
89 return;
90 }
91
92 // If the ID doesn't match, it means that an entry with |local_id| has been
93 // picked and an entry with |update_id| isn't supposed to exist.
94 syncable::Entry update_entry(trans, GET_BY_ID, update_id);
95 if (!update_entry.good())
96 return;
97
98 // Fail early so that the crash dump indicates which of the cases below
99 // has triggered the issue.
100 // Crash dumps don't always preserve data. The 2 separate cases below are
101 // to make it easy to see the the state of item with |update_id| in the
102 // crash dump.
103 if (update_entry.GetIsDel()) {
104 LOG(FATAL) << "VerifyLocalIdToUpdate: existing deleted entry " << update_id
105 << " conflicts with local entry " << local_id
106 << " picked by an update.\n"
107 << "Local item deleted: " << local_deleted
108 << ", deleted flag in update: " << deleted_in_update;
109 } else {
110 LOG(FATAL) << "VerifyLocalIdToUpdate: existing entry " << update_id
111 << " conflicts with local entry " << local_id
112 << " picked by an update.\n"
113 << "Local item deleted: " << local_deleted
114 << ", deleted flag in update: " << deleted_in_update;
115 }
116 }
117
118 syncable::Id FindLocalIdToUpdate( 78 syncable::Id FindLocalIdToUpdate(
119 syncable::BaseTransaction* trans, 79 syncable::BaseTransaction* trans,
120 const sync_pb::SyncEntity& update) { 80 const sync_pb::SyncEntity& update) {
121 // Expected entry points of this function: 81 // Expected entry points of this function:
122 // SyncEntity has NOT been applied to SERVER fields. 82 // SyncEntity has NOT been applied to SERVER fields.
123 // SyncEntity has NOT been applied to LOCAL fields. 83 // SyncEntity has NOT been applied to LOCAL fields.
124 // DB has not yet been modified, no entries created for this update. 84 // DB has not yet been modified, no entries created for this update.
125 85
126 const std::string& client_id = trans->directory()->cache_guid(); 86 const std::string& client_id = trans->directory()->cache_guid();
127 const syncable::Id& update_id = SyncableIdFromProto(update.id_string()); 87 const syncable::Id& update_id = SyncableIdFromProto(update.id_string());
(...skipping 32 matching lines...) Expand 10 before | Expand all | Expand 10 after
160 // Signal an error; drop this update on the floor. Note that 120 // Signal an error; drop this update on the floor. Note that
161 // we don't server delete the item, because we don't allow it to 121 // we don't server delete the item, because we don't allow it to
162 // exist locally at all. So the item will remain orphaned on 122 // exist locally at all. So the item will remain orphaned on
163 // the server, and we won't pay attention to it. 123 // the server, and we won't pay attention to it.
164 return syncable::Id(); 124 return syncable::Id();
165 } 125 }
166 } 126 }
167 // Target this change to the existing local entry; later, 127 // Target this change to the existing local entry; later,
168 // we'll change the ID of the local entry to update_id 128 // we'll change the ID of the local entry to update_id
169 // if needed. 129 // if needed.
170 VerifyLocalIdToUpdate(trans, local_entry.GetId(), update_id,
171 local_entry.GetIsDel(), update.deleted());
172 return local_entry.GetId(); 130 return local_entry.GetId();
173 } else { 131 } else {
174 // Case 3: We have a local entry with the same client tag. 132 // Case 3: We have a local entry with the same client tag.
175 // We should change the ID of the local entry to the server entry. 133 // We should change the ID of the local entry to the server entry.
176 // This will result in an server ID with base version == 0, but that's 134 // This will result in an server ID with base version == 0, but that's
177 // a legal state for an item with a client tag. By changing the ID, 135 // a legal state for an item with a client tag. By changing the ID,
178 // update will now be applied to local_entry. 136 // update will now be applied to local_entry.
179 DCHECK(0 == local_entry.GetBaseVersion() || 137 DCHECK(0 == local_entry.GetBaseVersion() ||
180 CHANGES_VERSION == local_entry.GetBaseVersion()); 138 CHANGES_VERSION == local_entry.GetBaseVersion());
181 VerifyLocalIdToUpdate(trans, local_entry.GetId(), update_id,
182 local_entry.GetIsDel(), update.deleted());
183 return local_entry.GetId(); 139 return local_entry.GetId();
184 } 140 }
185 } 141 }
186 } else if (update.has_originator_cache_guid() && 142 } else if (update.has_originator_cache_guid() &&
187 update.originator_cache_guid() == client_id) { 143 update.originator_cache_guid() == client_id) {
188 // If a commit succeeds, but the response does not come back fast enough 144 // If a commit succeeds, but the response does not come back fast enough
189 // then the syncer might assume that it was never committed. 145 // then the syncer might assume that it was never committed.
190 // The server will track the client that sent up the original commit and 146 // The server will track the client that sent up the original commit and
191 // return this in a get updates response. When this matches a local 147 // return this in a get updates response. When this matches a local
192 // uncommitted item, we must mutate our local item and version to pick up 148 // uncommitted item, we must mutate our local item and version to pick up
(...skipping 26 matching lines...) Expand all
219 // An entry should never be version 0 and SYNCED. 175 // An entry should never be version 0 and SYNCED.
220 DCHECK(local_entry.GetIsUnsynced()); 176 DCHECK(local_entry.GetIsUnsynced());
221 177
222 // Just a quick sanity check. 178 // Just a quick sanity check.
223 DCHECK(!local_entry.GetId().ServerKnows()); 179 DCHECK(!local_entry.GetId().ServerKnows());
224 180
225 DVLOG(1) << "Reuniting lost commit response IDs. server id: " 181 DVLOG(1) << "Reuniting lost commit response IDs. server id: "
226 << update_id << " local id: " << local_entry.GetId() 182 << update_id << " local id: " << local_entry.GetId()
227 << " new version: " << new_version; 183 << " new version: " << new_version;
228 184
229 VerifyLocalIdToUpdate(trans, local_entry.GetId(), update_id,
230 local_entry.GetIsDel(), update.deleted());
231 return local_entry.GetId(); 185 return local_entry.GetId();
232 } 186 }
233 } else if (update.has_server_defined_unique_tag() && 187 } else if (update.has_server_defined_unique_tag() &&
234 !update.server_defined_unique_tag().empty()) { 188 !update.server_defined_unique_tag().empty()) {
235 // The client creates type root folders with a local ID on demand when a 189 // The client creates type root folders with a local ID on demand when a
236 // progress marker for the given type is initially set. 190 // progress marker for the given type is initially set.
237 // The server might also attempt to send a type root folder for the same 191 // The server might also attempt to send a type root folder for the same
238 // type (during the transition period until support for root folders is 192 // type (during the transition period until support for root folders is
239 // removed for new client versions). 193 // removed for new client versions).
240 // TODO(stanisc): crbug.com/438313: remove this once the transition to 194 // TODO(stanisc): crbug.com/438313: remove this once the transition to
241 // implicit root folders on the server is done. 195 // implicit root folders on the server is done.
242 syncable::Entry local_entry(trans, syncable::GET_BY_SERVER_TAG, 196 syncable::Entry local_entry(trans, syncable::GET_BY_SERVER_TAG,
243 update.server_defined_unique_tag()); 197 update.server_defined_unique_tag());
244 if (local_entry.good() && !local_entry.GetId().ServerKnows()) { 198 if (local_entry.good() && !local_entry.GetId().ServerKnows()) {
245 DCHECK(local_entry.GetId() != update_id); 199 DCHECK(local_entry.GetId() != update_id);
246 VerifyLocalIdToUpdate(trans, local_entry.GetId(), update_id,
247 local_entry.GetIsDel(), update.deleted());
248 return local_entry.GetId(); 200 return local_entry.GetId();
249 } 201 }
250 } 202 }
251 203
252 // Fallback: target an entry having the server ID, creating one if needed. 204 // Fallback: target an entry having the server ID, creating one if needed.
253 return update_id; 205 return update_id;
254 } 206 }
255 207
256 UpdateAttemptResponse AttemptToUpdateEntry( 208 UpdateAttemptResponse AttemptToUpdateEntry(
257 syncable::WriteTransaction* const trans, 209 syncable::WriteTransaction* const trans,
(...skipping 459 matching lines...) Expand 10 before | Expand all | Expand 10 after
717 if (update.version() < target->GetServerVersion()) { 669 if (update.version() < target->GetServerVersion()) {
718 LOG(WARNING) << "Update older than current server version for " 670 LOG(WARNING) << "Update older than current server version for "
719 << *target << " Update:" 671 << *target << " Update:"
720 << SyncerProtoUtil::SyncEntityDebugString(update); 672 << SyncerProtoUtil::SyncEntityDebugString(update);
721 return VERIFY_SUCCESS; // Expected in new sync protocol. 673 return VERIFY_SUCCESS; // Expected in new sync protocol.
722 } 674 }
723 return VERIFY_UNDECIDED; 675 return VERIFY_UNDECIDED;
724 } 676 }
725 677
726 } // namespace syncer 678 } // namespace syncer
OLDNEW
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698