sync: Loop committing items without downloading updates (v2)

sync/sessions/session_state.h

 // 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.
 
 // The 'sessions' namespace comprises all the pieces of state that are
 // combined to form a SyncSession instance. In that way, it can be thought of
 // as an extension of the SyncSession type itself. Session scoping gives
 // context to things like "conflict progress", "update progress", etc, and the
 // separation this file provides allows clients to only include the parts they
 // need rather than the entire session stack.
 
 #ifndef SYNC_SESSIONS_SESSION_STATE_H_
 #define SYNC_SESSIONS_SESSION_STATE_H_
 #pragma once
 
 #include <map>
 #include <set>
 #include <string>
 #include <utility>
 #include <vector>
 
 #include "base/basictypes.h"
 #include "sync/engine/syncer_types.h"
 #include "sync/engine/syncproto.h"
 #include "sync/protocol/sync_protocol_error.h"
-#include "sync/sessions/ordered_commit_set.h"
 #include "sync/syncable/model_type.h"
 #include "sync/syncable/model_type_payload_map.h"
 #include "sync/syncable/syncable.h"
 
 namespace base {
 class DictionaryValue;
 }
 
 namespace browser_sync {
 namespace sessions {
@@ skipping 70 matching lines @@
   SyncSessionSnapshot();
   SyncSessionSnapshot(
       const SyncerStatus& syncer_status,
       const ErrorCounters& errors,
       int64 num_server_changes_remaining,
       bool is_share_usable,
       syncable::ModelTypeSet initial_sync_ended,
       const syncable::ModelTypePayloadMap& download_progress_markers,
       bool more_to_sync,
       bool is_silenced,
-      int64 unsynced_count,
       int num_encryption_conflicts,
       int num_hierarchy_conflicts,
       int num_simple_conflicts,
       int num_server_conflicts,
-      bool did_commit_items,

[tim (not reviewing), 2012/05/11 18:42:16]

Where is this captured, again?

[rlarocque, 2012/05/14 23:10:43]

It was based on control_params.items_comitted, which was set at the end of
PostCommitMessageCommand.  This makes it much less useful than its name implies;
it gives us no indication as to whether or not the server accepted our commits.

       const SyncSourceInfo& source,
       bool notifications_enabled,
       size_t num_entries,
       base::Time sync_start_time,
       bool retry_scheduled);
   ~SyncSessionSnapshot();
 
   // Caller takes ownership of the returned dictionary.
   base::DictionaryValue* ToValue() const;
 
@@ skipping 21 matching lines @@
 
  private:
   SyncerStatus syncer_status_;
   ErrorCounters errors_;
   int64 num_server_changes_remaining_;
   bool is_share_usable_;
   syncable::ModelTypeSet initial_sync_ended_;
   syncable::ModelTypePayloadMap download_progress_markers_;
   bool has_more_to_sync_;
   bool is_silenced_;
   int64 unsynced_count_;

[tim (not reviewing), 2012/05/11 18:42:16]

You left this here, but removed it everywhere else (it seems).

How do we tell now from about:sync how many items were unsynced at the start of
the last cycle, and whether any of those were committed?

[rlarocque, 2012/05/14 23:10:43]

Oops.  I meant to remove it here, too.

This CL leaves about:sync in rough shape.  I'll try to fix it in follow-up CLs. 
Nicolas's work to expose a fresher unsynced_count will help.  I'd also like to
add some strictly increasing counts of number of items committed.

It's important to note that we only trigger a refresh of about:sync after a sync
cycle is completed.  For cycles that commit many items, we will need to refresh
the page manually.  Previously, it would refresh itself with every commit batch.
 

[tim (not reviewing), 2012/05/15 22:25:00]

OK.  Can you make sure there are bugs documenting the gaps / regressions?

   int num_encryption_conflicts_;
   int num_hierarchy_conflicts_;
   int num_simple_conflicts_;
   int num_server_conflicts_;
-  bool did_commit_items_;
   SyncSourceInfo source_;
   bool notifications_enabled_;
   size_t num_entries_;
   base::Time sync_start_time_;
   bool retry_scheduled_;
 };
 
 // Tracks progress of conflicts and their resolutions.
 class ConflictProgress {
  public:
@@ skipping 134 matching lines @@
 // of state are dirty-on-write and should incur dirty bit updates if changed.
 
 // Grouping of all state that applies to all model types.  Note that some
 // components of the global grouping can internally implement finer grained
 // scope control (such as OrderedCommitSet), but the top level entity is still
 // a singleton with respect to model types.
 struct AllModelTypeState {
   explicit AllModelTypeState(bool* dirty_flag);
   ~AllModelTypeState();
 
-  // Commits for all model types are bundled together into a single message.
-  ClientToServerMessage commit_message;
-  ClientToServerResponse commit_response;
   // We GetUpdates for some combination of types at once.
   // requested_update_types stores the set of types which were requested.
   syncable::ModelTypeSet updates_request_types;
   ClientToServerResponse updates_response;
   // Used to build the shared commit message.
-  DirtyOnWrite<std::vector<int64> > unsynced_handles;
   DirtyOnWrite<SyncerStatus> syncer_status;
   DirtyOnWrite<ErrorCounters> error;
   SyncCycleControlParameters control_params;
   DirtyOnWrite<int64> num_server_changes_remaining;
-  OrderedCommitSet commit_set;
 };
 
 // Grouping of all state that applies to a single ModelSafeGroup.
 struct PerModelSafeGroupState {
   explicit PerModelSafeGroupState(bool* dirty_flag);
   ~PerModelSafeGroupState();
 
   UpdateProgress update_progress;
   ConflictProgress conflict_progress;
 };
 
 }  // namespace sessions
 }  // namespace browser_sync
 
 #endif  // SYNC_SESSIONS_SESSION_STATE_H_