sync: Follow-up to conflict resolution refactor

sync/sessions/status_controller.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.
 
 // StatusController handles all counter and status related number crunching and
-// state tracking on behalf of a SyncSession.  It 'controls' the model data
-// defined in session_state.h.  The most important feature of StatusController
-// is the ScopedModelSafetyRestriction. When one of these is active, the
-// underlying data set exposed via accessors is swapped out to the appropriate
-// set for the restricted ModelSafeGroup behind the scenes.  For example, if
-// GROUP_UI is set, then accessors such as conflict_progress() and commit_ids()
-// are implicitly restricted to returning only data pertaining to GROUP_UI.
-// You can see which parts of status fall into this "restricted" category, or
-// the global "shared" category for all model types, by looking at the struct
-// declarations in session_state.h. If these accessors are invoked without a
-// restriction in place, this is a violation and will cause debug assertions
-// to surface improper use of the API in development.  Likewise for
-// invocation of "shared" accessors when a restriction is in place; for
-// safety's sake, an assertion will fire.
+// state tracking on behalf of a SyncSession.
+//
+// The most important feature of StatusController is the
+// ScopedModelSafeGroupRestriction.  Some of its functions expose per-thread
+// state, and can be called only when the SafetyRestriction is in effect.  Other

[Nicolas Zea, 2012/10/26 21:14:30]

I think the more indepth description that was here previously is still useful
right?

[rlarocque, 2012/10/26 21:41:46]

The only remaining use of the ModelSafeGroupRestriction is
commit_id_projection().  The examples of conflict_progress() and commit_ids()
are no longer valid.  The comments about session_state.h are no longer valid,
since there is no longer any PerModelSafeGroupState.  Much of the "shared" state
are strictly increasing counters that are incremented on different threads, so
the comment about requiring a lack of group restriction no longer applies.

I'd be open to re-wording this comment if you think it leaves out some valuable
information or is less clear than the original.  However, much of the original
is out of date and had to be modified, if not deleted.  

[Nicolas Zea, 2012/10/26 22:03:27]

"per-thread" is kind of implicit, the more explicit description for me is per
model safe group. I'd mainly just like to see the example provided previously
(i.e. using GROUP_UI) kept and just updated to use an appropriate field (e.g.
commit_id_projection). You're right, the rest of it isn't really applicable
anymore.

+// parts of its state are global, and do not require the restriction.
 //
 // NOTE: There is no concurrent access protection provided by this class. It
 // assumes one single thread is accessing this class for each unique
 // ModelSafeGroup, and also only one single thread (in practice, the
 // SyncerThread) responsible for all "shared" access when no restriction is in
 // place. Thus, every bit of data is to be accessed mutually exclusively with
 // respect to threads.
 //
 // StatusController can also track if changes occur to certain parts of state
 // so that various parts of the sync engine can avoid broadcasting
 // notifications if no changes occurred.
 
 #ifndef SYNC_SESSIONS_STATUS_CONTROLLER_H_
 #define SYNC_SESSIONS_STATUS_CONTROLLER_H_
 
 #include <map>
 #include <vector>
 
 #include "base/logging.h"
 #include "base/stl_util.h"
 #include "base/time.h"
 #include "sync/internal_api/public/sessions/model_neutral_state.h"
 #include "sync/sessions/ordered_commit_set.h"
-#include "sync/sessions/session_state.h"
 
 namespace syncer {
 namespace sessions {
 
 class StatusController {
  public:
   explicit StatusController(const ModelSafeRoutingInfo& routes);
   ~StatusController();
 
-  // Progress counters.  All const methods may return NULL if the
-  // progress structure doesn't exist, but all non-const methods
-  // auto-create.
-  const std::set<syncable::Id>* simple_conflict_ids() const;
-  std::set<syncable::Id>* mutable_simple_conflict_ids();
-  const std::set<syncable::Id>* GetUnrestrictedSimpleConflictIds(
-      ModelSafeGroup group) const;
-
   // ClientToServer messages.
   const ModelTypeSet updates_request_types() const {
     return model_neutral_.updates_request_types;
   }
   void set_updates_request_types(ModelTypeSet value) {
     model_neutral_.updates_request_types = value;
   }
   const sync_pb::ClientToServerResponse& updates_response() const {
     return model_neutral_.updates_response;
   }
   sync_pb::ClientToServerResponse* mutable_updates_response() {
     return &model_neutral_.updates_response;
   }
 
   // Changelog related state.
   int64 num_server_changes_remaining() const {
     return model_neutral_.num_server_changes_remaining;
   }
 
   const OrderedCommitSet::Projection& commit_id_projection(
       const sessions::OrderedCommitSet &commit_set) {
     DCHECK(group_restriction_in_effect_)
         << "No group restriction for projection.";
     return commit_set.GetCommitIdProjection(group_restriction_);
   }
 
-  // Control parameters for sync cycles.
-  bool conflicts_resolved() const {
-    return model_neutral_.conflicts_resolved;
-  }
-
-  // If a GetUpdates for any data type resulted in downloading an update that
-  // is in conflict, this method returns true.
-  // Note: this includes unresolvable conflicts.
-  bool HasConflictingUpdates() const;
-
   // Various conflict counters.
   int num_encryption_conflicts() const;
   int num_hierarchy_conflicts() const;
   int num_server_conflicts() const;
 
-  int num_simple_conflicts() const;
-
   // Aggregate sum of all conflicting items over all conflict types.
   int TotalNumConflictingItems() const;
 
   // Number of successfully applied updates.
   int num_updates_applied() const;
 
   int num_server_overwrites() const;
 
   // Returns the number of updates received from the sync server.
   int64 CountUpdates() const;
@@ skipping 39 matching lines @@
   void increment_num_reflected_updates_downloaded_by(int value);
 
   // Update application and conflict resolution counters.
   void increment_num_updates_applied_by(int value);
   void increment_num_encryption_conflicts_by(int value);
   void increment_num_hierarchy_conflicts_by(int value);
   void increment_num_server_conflicts();
   void increment_num_local_overwrites();
   void increment_num_server_overwrites();
 
-  // TODO(rlarocque): Remove these after conflict resolution refactor.
-  void update_conflicts_resolved(bool resolved);
-  void reset_conflicts_resolved();
-
   // Commit counters.
   void increment_num_successful_commits();
   void increment_num_successful_bookmark_commits();
   void set_num_successful_bookmark_commits(int value);
 
   // Server communication status tracking.
   void set_sync_protocol_error(const SyncProtocolError& error);
   void set_last_get_key_result(const SyncerError result);
   void set_last_download_updates_result(const SyncerError result);
   void set_commit_result(const SyncerError result);
@@ skipping 14 matching lines @@
   // restriction.
   bool ActiveGroupRestrictionIncludesModel(ModelType model) const {
     if (!group_restriction_in_effect_)
       return true;
     ModelSafeRoutingInfo::const_iterator it = routing_info_.find(model);
     if (it == routing_info_.end())
       return false;
     return group_restriction() == it->second;
   }
 
-  // Returns the state, if it exists, or NULL otherwise.
-  const PerModelSafeGroupState* GetModelSafeGroupState(
-      bool restrict, ModelSafeGroup group) const;
-
-  // Helper to lazily create objects for per-ModelSafeGroup state.
-  PerModelSafeGroupState* GetOrCreateModelSafeGroupState(
-      bool restrict, ModelSafeGroup group);
-
   ModelNeutralState model_neutral_;
-  std::map<ModelSafeGroup, PerModelSafeGroupState*> per_model_group_;
-
-  STLValueDeleter<std::map<ModelSafeGroup, PerModelSafeGroupState*> >
-      per_model_group_deleter_;
 
   // Used to fail read/write operations on state that don't obey the current
   // active ModelSafeWorker contract.
   bool group_restriction_in_effect_;
   ModelSafeGroup group_restriction_;
 
   const ModelSafeRoutingInfo routing_info_;
 
   base::Time sync_start_time_;
 
@@ skipping 17 matching lines @@
   }
  private:
   StatusController* status_;
   DISALLOW_COPY_AND_ASSIGN(ScopedModelSafeGroupRestriction);
 };
 
 }  // namespace sessions
 }  // namespace syncer
 
 #endif  // SYNC_SESSIONS_STATUS_CONTROLLER_H_