sync: Loop committing items without downloading updates

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"
@@ skipping 47 matching lines @@
 {
   switch (step) {
     ENUM_CASE(SYNCER_BEGIN);
     ENUM_CASE(CLEANUP_DISABLED_TYPES);
     ENUM_CASE(DOWNLOAD_UPDATES);
     ENUM_CASE(PROCESS_CLIENT_COMMAND);
     ENUM_CASE(VERIFY_UPDATES);
     ENUM_CASE(PROCESS_UPDATES);
     ENUM_CASE(STORE_TIMESTAMPS);
     ENUM_CASE(APPLY_UPDATES);
-    ENUM_CASE(BUILD_COMMIT_REQUEST);
-    ENUM_CASE(POST_COMMIT_MESSAGE);
-    ENUM_CASE(PROCESS_COMMIT_RESPONSE);
+    ENUM_CASE(COMMIT);
     ENUM_CASE(RESOLVE_CONFLICTS);
     ENUM_CASE(APPLY_UPDATES_TO_RESOLVE_CONFLICTS);
     ENUM_CASE(CLEAR_PRIVATE_DATA);
     ENUM_CASE(SYNCER_END);
   }
   NOTREACHED();
   return "";
 }
 #undef ENUM_CASE
 
@@ skipping 97 matching lines @@
       }
       case APPLY_UPDATES: {
         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 = BUILD_COMMIT_REQUEST;
+          next_step = COMMIT;
         }
         break;
       }
-      // These two steps are combined since they are executed within the same
-      // write transaction.
-      case BUILD_COMMIT_REQUEST: {
+
+      case COMMIT: {

[rlarocque, 2012/04/13 02:02:22]

I intend to move this block of code into a function.  This revision of the CL is
intended to keep the diff size manageable, so I haven't moved it yet.

+        StatusController* status_controller =
+            session->mutable_status_controller();
         syncable::Directory* dir = session->context()->directory();
-        WriteTransaction trans(FROM_HERE, SYNCER, dir);
-        sessions::ScopedSetSessionWriteTransaction set_trans(session, &trans);
+        size_t batch_size = session->context()->max_commit_batch_size();

[rlarocque, 2012/04/13 02:02:22]

Nick warned me about this.  He suggested that the server might want to change
the commit batch size in response to certain client behaviours.  Obviously this
line prevents us from responding to those requests.

It turns out this is not a regression.  The syncer will currently respond to the
server's request to set the commit batch size only if the ClientCommand came in
on a GetUpdates response.  

Adding support for handling ClientCommands on all response messages is something
that makes sense to me, but we would have to start by turning
ProcessClientCommand into a function.  That's not something I want to tackle in
this CL.

Let me know if it makes sense to add support for this.  I will open a bug and
try to get it fixed by M20.

 
-        DVLOG(1) << "Getting the Commit IDs";
-        GetCommitIdsCommand get_commit_ids_command(
-            session->context()->max_commit_batch_size());
-        get_commit_ids_command.Execute(session);
+        for ( ; ; ) {
+          sessions::OrderedCommitSet commit_set(session->routing_info());
+          ClientToServerMessage commit_message;
+          ClientToServerResponse commit_response;
 
-        if (!session->status_controller().commit_ids().empty()) {
-          DVLOG(1) << "Building a commit message";
-          BuildCommitCommand build_commit_command;
-          build_commit_command.Execute(session);
+          {
+            WriteTransaction trans(FROM_HERE, SYNCER, dir);
+            sessions::ScopedSetSessionWriteTransaction set_trans(session,
+                                                                 &trans);
 
-          next_step = POST_COMMIT_MESSAGE;
-        } else {
-          next_step = RESOLVE_CONFLICTS;
+            DVLOG(1) << "Getting the Commit IDs";
+            GetCommitIdsCommand get_commit_ids_command(batch_size, &commit_set);
+            get_commit_ids_command.Execute(session);
+            if (commit_set.Empty())
+              break;
+
+            BuildCommitCommand build_commit_command(commit_set,
+                                                    &commit_message);
+            build_commit_command.Execute(session);
+          } // Time for some network IO.  Release the lock.
+
+
+          DVLOG(1) << "Sending a commit message";
+          PostCommitMessageCommand post_commit_command(commit_message,
+                                                       &commit_response);
+          status_controller->set_last_post_commit_result(
+              post_commit_command.Execute(session));
+
+          ProcessCommitResponseCommand process_response_command(
+              commit_set, commit_message, commit_response);
+          status_controller->set_last_process_commit_response_result(
+                  process_response_command.Execute(session));
+
+          if (status_controller->last_post_commit_result() != SYNCER_OK
+              || (status_controller->last_process_commit_response_result()
+                  != SYNCER_OK)) {
+            break; // break out of for loop.
+          }
         }
 
-        break;
-      }
-      case POST_COMMIT_MESSAGE: {
-        PostCommitMessageCommand post_commit_command;
-        session->mutable_status_controller()->set_last_post_commit_result(
-            post_commit_command.Execute(session));
-        next_step = PROCESS_COMMIT_RESPONSE;
-        break;
-      }
-      case PROCESS_COMMIT_RESPONSE: {
-        ProcessCommitResponseCommand process_response_command;
-        session->mutable_status_controller()->
-            set_last_process_commit_response_result(
-                process_response_command.Execute(session));
         next_step = RESOLVE_CONFLICTS;
         break;
       }
       case RESOLVE_CONFLICTS: {
         StatusController* status = session->mutable_status_controller();
         status->reset_conflicts_resolved();
         ResolveConflictsCommand resolve_conflicts_command;
         resolve_conflicts_command.Execute(session);
 
         // Has ConflictingUpdates includes both resolvable and unresolvable
@@ skipping 98 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