sync: Use a single transaction in ProcessUpdates rather than one per entry.

chrome/browser/sync/engine/process_updates_command.cc

Index: chrome/browser/sync/engine/process_updates_command.cc
diff --git a/chrome/browser/sync/engine/process_updates_command.cc b/chrome/browser/sync/engine/process_updates_command.cc
index b0aa54b0962ebcb2b9aea922ce64f7fd82884052..bceaf167809d4737f356faa1638f4c031fbdc022 100644
--- a/chrome/browser/sync/engine/process_updates_command.cc
+++ b/chrome/browser/sync/engine/process_updates_command.cc
@@ -45,6 +45,7 @@ void ProcessUpdatesCommand::ModelChangingExecuteImpl(SyncSession* session) {
 
   StatusController* status = session->status_controller();
 
+  syncable::WriteTransaction trans(FROM_HERE, syncable::SYNCER, dir);
   const sessions::UpdateProgress& progress(status->update_progress());
   vector<sessions::VerifiedUpdate>::const_iterator it;
   for (it = progress.VerifiedUpdatesBegin();
@@ -54,7 +55,7 @@ void ProcessUpdatesCommand::ModelChangingExecuteImpl(SyncSession* session) {
 
     if (it->first != VERIFY_SUCCESS && it->first != VERIFY_UNDELETE)
       continue;
-    switch (ProcessUpdate(dir, update)) {
+    switch (ProcessUpdate(dir, update, &trans)) {
       case SUCCESS_PROCESSED:
       case SUCCESS_STORED:
         break;
@@ -91,32 +92,31 @@ bool ReverifyEntry(syncable::WriteTransaction* trans, const SyncEntity& entry,
 // Process a single update. Will avoid touching global state.
 ServerUpdateProcessingResult ProcessUpdatesCommand::ProcessUpdate(
     const syncable::ScopedDirLookup& dir,
-    const sync_pb::SyncEntity& proto_update) {
+    const sync_pb::SyncEntity& proto_update,
+    syncable::WriteTransaction* const trans) {

[rlarocque, 2011/11/15 00:52:23]

Was there a reason you decided to use a pointer here instead of a reference?

Since the function seems to assume that it's non-NULL, I'd prefer to see a
reference here.

 
   const SyncEntity& update = *static_cast<const SyncEntity*>(&proto_update);
   syncable::Id server_id = update.id();
   const std::string name = SyncerProtoUtil::NameFromSyncEntity(update);
 
-  syncable::WriteTransaction trans(FROM_HERE, syncable::SYNCER, dir);
-
   // Look to see if there's a local item that should recieve this update,
   // maybe due to a duplicate client tag or a lost commit response.
-  syncable::Id local_id = SyncerUtil::FindLocalIdToUpdate(&trans, update);
+  syncable::Id local_id = SyncerUtil::FindLocalIdToUpdate(trans, update);
 
   // FindLocalEntryToUpdate has veto power.
   if (local_id.IsNull()) {
     return SUCCESS_PROCESSED;  // The entry has become irrelevant.
   }
 
-  SyncerUtil::CreateNewEntry(&trans, local_id);
+  SyncerUtil::CreateNewEntry(trans, local_id);
 
   // We take a two step approach. First we store the entries data in the
   // server fields of a local entry and then move the data to the local fields
-  syncable::MutableEntry target_entry(&trans, syncable::GET_BY_ID, local_id);
+  syncable::MutableEntry target_entry(trans, syncable::GET_BY_ID, local_id);
 
   // We need to run the Verify checks again; the world could have changed
   // since VerifyUpdatesCommand.
-  if (!ReverifyEntry(&trans, update, &target_entry)) {
+  if (!ReverifyEntry(trans, update, &target_entry)) {
     return SUCCESS_PROCESSED;  // The entry has become irrelevant.
   }
 
@@ -124,7 +124,7 @@ ServerUpdateProcessingResult ProcessUpdatesCommand::ProcessUpdate(
   // change the ID now, after we're sure that the update can succeed.
   if (local_id != server_id) {
     DCHECK(!update.deleted());
-    SyncerUtil::ChangeEntryIDAndUpdateChildren(&trans, &target_entry,
+    SyncerUtil::ChangeEntryIDAndUpdateChildren(trans, &target_entry,
         server_id);
     // When IDs change, versions become irrelevant.  Forcing BASE_VERSION
     // to zero would ensure that this update gets applied, but would indicate