sync: Process 'control' data separately

sync/engine/syncer.cc

 // Copyright (c) 2012 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 "sync/engine/syncer.h"
 
 #include "base/debug/trace_event.h"
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/message_loop.h"
 #include "base/time.h"
 #include "build/build_config.h"
+#include "sync/engine/apply_metadata_updates.h"
 #include "sync/engine/apply_updates_command.h"
 #include "sync/engine/build_commit_command.h"
 #include "sync/engine/cleanup_disabled_types_command.h"
 #include "sync/engine/commit.h"
 #include "sync/engine/conflict_resolver.h"
 #include "sync/engine/download_updates_command.h"
 #include "sync/engine/net/server_connection_manager.h"
 #include "sync/engine/process_commit_response_command.h"
 #include "sync/engine/process_updates_command.h"
 #include "sync/engine/resolve_conflicts_command.h"
@@ skipping 136 matching lines @@
           DVLOG(1) << "Aborting sync cycle due to download updates failure";
         } else if (!session->status_controller()
                        .ServerSaysNothingMoreToDownload()) {
           next_step = DOWNLOAD_UPDATES;
         } else {
           next_step = APPLY_UPDATES;
         }
         break;
       }
       case APPLY_UPDATES: {
+        // These include encryption updates that should be applied early.
+        // FIXME: Shoud this use the same transaction as ApplyUpdatesCommand?
+        ApplyMetadataUpdates(session->context()->directory());

[Nicolas Zea, 2012/06/21 02:58:36]

We can enforce the early application by just making sure that the the
passive/internal model worker is the first in the list. It might be cleaner to
have this method be part of ApplyUpdatesCommand's ModelChangeExecuteImpl, and
condition whether we should call this or the normal functionality by checking
which worker we're on?

[rlarocque, 2012/06/21 18:23:27]

I'd rather not enforce an ordering constraint on the list.  I think it's
currently stored as a set, and many of the functions that work with sets of
model safe groups don't necessarily respect ordering.  

The ModelSafeWorker functions are supposed to be unaware of which group they're
on.  See crbug.com/121303.  

[tim (not reviewing), 2012/06/22 16:34:15]

Yeah, we shouldn't try to force ordering within the command.  If this needs to
happen before ApplyUpdatesCommand for all "real" types, then it should happen
before the apply updates step, in its own step.  

+
         ApplyUpdatesCommand apply_updates;
         apply_updates.Execute(session);
         if (last_step == APPLY_UPDATES) {
           // We're in configuration mode, but we still need to run the
           // SYNCER_END step.
           last_step = SYNCER_END;
           next_step = SYNCER_END;
         } else {
           next_step = COMMIT;
         }
@@ skipping 109 matching lines @@
   entry->Put(SERVER_CTIME, Time());
   entry->Put(SERVER_VERSION, 0);
   entry->Put(SERVER_IS_DIR, false);
   entry->Put(SERVER_IS_DEL, false);
   entry->Put(IS_UNAPPLIED_UPDATE, false);
   entry->Put(SERVER_SPECIFICS, sync_pb::EntitySpecifics::default_instance());
   entry->Put(SERVER_POSITION_IN_PARENT, 0);
 }
 
 }  // namespace browser_sync