sync: Loop committing items without downloading updates

sync/engine/syncer.cc

Index: sync/engine/syncer.cc
diff --git a/sync/engine/syncer.cc b/sync/engine/syncer.cc
index 62c7af893ff2d6dc91d74c53980c7a50f4eef564..a4e3bcad129adb8f0ac4e2834d78a799ff429c90 100644
--- a/sync/engine/syncer.cc
+++ b/sync/engine/syncer.cc
@@ -65,9 +65,7 @@ const char* SyncerStepToString(const SyncerStep step)
     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);
@@ -185,46 +183,57 @@ void Syncer::SyncShare(sessions::SyncSession* session,
           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;
       }