Remove context-based dependencies on SyncerSession from ApplyUpdatesCommand.

This helps clarify UpdateApplicator and removes SyncerSession from SyncerUtil.

Part of work to separate sync session "context" like the ConflictResolver, ServerConnectionManager,
ModelSafeWorker, etc from progress state.

BUG=25266
TEST=sync unit tests

Review URL: http://codereview.chromium.org/351026

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@30868 0039d316-1c4b-4281-b951-d872f2087c98

8 files changed, 48 insertions, 46 deletions

diff --git a/chrome/browser/sync/engine/apply_updates_command.cc b/chrome/browser/sync/engine/apply_updates_command.cc
index 2d9f3e5..7b8e715 100644
--- a/chrome/browser/sync/engine/apply_updates_command.cc
+++ b/chrome/browser/sync/engine/apply_updates_command.cc
@@ -25,10 +25,10 @@ void ApplyUpdatesCommand::ModelChangingExecuteImpl(SyncerSession *session) {
   syncable::Directory::UnappliedUpdateMetaHandles handles;
   dir->GetUnappliedUpdateMetaHandles(&trans, &handles);
 
-  UpdateApplicator applicator(session, handles.begin(), handles.end());
-  while (applicator.AttemptOneApplication(&trans)) {
-  }
-  applicator.SaveProgressIntoSessionState();
+  UpdateApplicator applicator(session->resolver(), handles.begin(),
+                              handles.end());
+  while (applicator.AttemptOneApplication(&trans)) {}
+  applicator.SaveProgressIntoSessionState(session);
 }
 
 }  // namespace browser_sync
diff --git a/chrome/browser/sync/engine/apply_updates_command.h b/chrome/browser/sync/engine/apply_updates_command.h
index 7588f4b..67a68c8 100644
--- a/chrome/browser/sync/engine/apply_updates_command.h
+++ b/chrome/browser/sync/engine/apply_updates_command.h
@@ -6,7 +6,6 @@
 #define CHROME_BROWSER_SYNC_ENGINE_APPLY_UPDATES_COMMAND_H_
 
 #include "chrome/browser/sync/engine/model_changing_syncer_command.h"
-#include "chrome/browser/sync/engine/syncer_session.h"
 #include "chrome/browser/sync/util/sync_types.h"
 
 namespace syncable {
@@ -17,6 +16,8 @@ class Id;
 
 namespace browser_sync {
 
+class SyncerSession;
+
 class ApplyUpdatesCommand : public ModelChangingSyncerCommand {
  public:
   ApplyUpdatesCommand();
diff --git a/chrome/browser/sync/engine/build_and_process_conflict_sets_command.cc b/chrome/browser/sync/engine/build_and_process_conflict_sets_command.cc
index a57f9ed..0cc1ac6 100644
--- a/chrome/browser/sync/engine/build_and_process_conflict_sets_command.cc
+++ b/chrome/browser/sync/engine/build_and_process_conflict_sets_command.cc
@@ -218,7 +218,8 @@ bool BuildAndProcessConflictSetsCommand::ApplyUpdatesTransactionally(
 
   // 5. Use the usual apply updates from the special start state we've just
   // prepared.
-  UpdateApplicator applicator(session, handles.begin(), handles.end());
+  UpdateApplicator applicator(session->resolver(), handles.begin(),
+                              handles.end());
   while (applicator.AttemptOneApplication(trans)) {
     // Keep going till all updates are applied.
   }
@@ -236,7 +237,7 @@ bool BuildAndProcessConflictSetsCommand::ApplyUpdatesTransactionally(
     }
     return false;  // Don't save progress -- we just undid it.
   }
-  applicator.SaveProgressIntoSessionState();
+  applicator.SaveProgressIntoSessionState(session);
   return true;
 }
 
diff --git a/chrome/browser/sync/engine/conflict_resolver.cc b/chrome/browser/sync/engine/conflict_resolver.cc
index 469d682..32a65fa 100644
--- a/chrome/browser/sync/engine/conflict_resolver.cc
+++ b/chrome/browser/sync/engine/conflict_resolver.cc
@@ -312,7 +312,7 @@ bool ConflictResolver::AttemptItemMerge(WriteTransaction* trans,
   server_named->Put(syncable::BASE_VERSION, 0);
   CHECK(SUCCESS ==
         SyncerUtil::AttemptToUpdateEntryWithoutMerge(
-            trans, locally_named, NULL, NULL));
+            trans, locally_named, NULL));
   return true;
 }
 
diff --git a/chrome/browser/sync/engine/syncer_util.cc b/chrome/browser/sync/engine/syncer_util.cc
index 8e32838..4074cd6 100644
--- a/chrome/browser/sync/engine/syncer_util.cc
+++ b/chrome/browser/sync/engine/syncer_util.cc
@@ -10,7 +10,6 @@
 
 #include "chrome/browser/sync/engine/conflict_resolver.h"
 #include "chrome/browser/sync/engine/syncer_proto_util.h"
-#include "chrome/browser/sync/engine/syncer_session.h"
 #include "chrome/browser/sync/engine/syncer_types.h"
 #include "chrome/browser/sync/engine/syncproto.h"
 #include "chrome/browser/sync/syncable/directory_manager.h"
@@ -220,20 +219,17 @@ void SyncerUtil::AttemptReuniteLostCommitResponses(
 UpdateAttemptResponse SyncerUtil::AttemptToUpdateEntry(
     syncable::WriteTransaction* const trans,
     syncable::MutableEntry* const entry,
-    SyncerSession* const session) {
+    ConflictResolver* resolver) {
 
   syncable::Id conflicting_id;
   UpdateAttemptResponse result =
-     AttemptToUpdateEntryWithoutMerge(trans, entry, session,
-                                      &conflicting_id);
+     AttemptToUpdateEntryWithoutMerge(trans, entry, &conflicting_id);
   if (result != NAME_CONFLICT) {
     return result;
   }
   syncable::MutableEntry same_path(trans, syncable::GET_BY_ID, conflicting_id);
   CHECK(same_path.good());
 
-  ConflictResolver* resolver = session->resolver();
-
   if (resolver &&
       resolver->AttemptItemMerge(trans, &same_path, entry)) {
     return SUCCESS;
@@ -247,7 +243,7 @@ UpdateAttemptResponse SyncerUtil::AttemptToUpdateEntry(
 UpdateAttemptResponse SyncerUtil::AttemptToUpdateEntryWithoutMerge(
     syncable::WriteTransaction* const trans,
     syncable::MutableEntry* const entry,
-    SyncerSession* const session, syncable::Id* const conflicting_id) {
+    syncable::Id* const conflicting_id) {
 
   CHECK(entry->good());
   if (!entry->Get(IS_UNAPPLIED_UPDATE))
diff --git a/chrome/browser/sync/engine/syncer_util.h b/chrome/browser/sync/engine/syncer_util.h
index 2a8fdb3..cf9d9c7 100644
--- a/chrome/browser/sync/engine/syncer_util.h
+++ b/chrome/browser/sync/engine/syncer_util.h
@@ -22,7 +22,6 @@
 
 namespace browser_sync {
 
-class SyncerSession;
 class SyncEntity;
 
 class SyncerUtil {
@@ -55,12 +54,12 @@ class SyncerUtil {
   static UpdateAttemptResponse AttemptToUpdateEntry(
       syncable::WriteTransaction* const trans,
       syncable::MutableEntry* const entry,
-      SyncerSession* const session);
+      ConflictResolver* resolver);
 
   static UpdateAttemptResponse AttemptToUpdateEntryWithoutMerge(
       syncable::WriteTransaction* const trans,
       syncable::MutableEntry* const entry,
-      SyncerSession* const session, syncable::Id* const conflicting_id);
+      syncable::Id* const conflicting_id);
 
   // Pass in name to avoid redundant UTF8 conversion.
   static void UpdateServerFieldsFromUpdate(
diff --git a/chrome/browser/sync/engine/update_applicator.cc b/chrome/browser/sync/engine/update_applicator.cc
index 6001c4e..8c5db40 100644
--- a/chrome/browser/sync/engine/update_applicator.cc
+++ b/chrome/browser/sync/engine/update_applicator.cc
@@ -15,10 +15,13 @@ using std::vector;
 
 namespace browser_sync {
 
-UpdateApplicator::UpdateApplicator(SyncerSession* session,
-                                   const vi64iter& begin,
-                                   const vi64iter& end)
-    : session_(session), begin_(begin), end_(end), pointer_(begin),
+UpdateApplicator::UpdateApplicator(ConflictResolver* resolver,
+                                   const UpdateIterator& begin,
+                                   const UpdateIterator& end)
+    : resolver_(resolver),
+      begin_(begin),
+      end_(end),
+      pointer_(begin),
       progress_(false) {
     size_t item_count = end - begin;
     LOG(INFO) << "UpdateApplicator created for " << item_count << " items.";
@@ -45,7 +48,7 @@ bool UpdateApplicator::AttemptOneApplication(
   }
   syncable::MutableEntry entry(trans, syncable::GET_BY_HANDLE, *pointer_);
   UpdateAttemptResponse updateResponse =
-      SyncerUtil::AttemptToUpdateEntry(trans, &entry, session_);
+      SyncerUtil::AttemptToUpdateEntry(trans, &entry, resolver_);
   switch (updateResponse) {
     case SUCCESS:
       --end_;
@@ -76,25 +79,25 @@ bool UpdateApplicator::AllUpdatesApplied() const {
          begin_ == end_;
 }
 
-void UpdateApplicator::SaveProgressIntoSessionState() {
+void UpdateApplicator::SaveProgressIntoSessionState(SyncerSession* session) {
   DCHECK(begin_ == end_ || ((pointer_ == end_) && !progress_))
       << "SaveProgress called before updates exhausted.";
 
   vector<syncable::Id>::const_iterator i;
   for (i = conflicting_ids_.begin(); i != conflicting_ids_.end(); ++i) {
-    session_->EraseBlockedItem(*i);
-    session_->AddCommitConflict(*i);
-    session_->AddAppliedUpdate(CONFLICT, *i);
+    session->EraseBlockedItem(*i);
+    session->AddCommitConflict(*i);
+    session->AddAppliedUpdate(CONFLICT, *i);
   }
   for (i = blocked_ids_.begin(); i != blocked_ids_.end(); ++i) {
-    session_->AddBlockedItem(*i);
-    session_->EraseCommitConflict(*i);
-    session_->AddAppliedUpdate(BLOCKED, *i);
+    session->AddBlockedItem(*i);
+    session->EraseCommitConflict(*i);
+    session->AddAppliedUpdate(BLOCKED, *i);
   }
   for (i = successful_ids_.begin(); i != successful_ids_.end(); ++i) {
-    session_->EraseCommitConflict(*i);
-    session_->EraseBlockedItem(*i);
-    session_->AddAppliedUpdate(SUCCESS, *i);
+    session->EraseCommitConflict(*i);
+    session->EraseBlockedItem(*i);
+    session->AddAppliedUpdate(SUCCESS, *i);
   }
 }
 
diff --git a/chrome/browser/sync/engine/update_applicator.h b/chrome/browser/sync/engine/update_applicator.h
index 8675b81..dd65639 100644
--- a/chrome/browser/sync/engine/update_applicator.h
+++ b/chrome/browser/sync/engine/update_applicator.h
@@ -16,22 +16,22 @@
 
 #include "base/basictypes.h"
 #include "base/port.h"
-
-namespace syncable {
-class Id;
-class WriteTransaction;
-}  // namespace syncable
+#include "chrome/browser/sync/syncable/syncable.h"
 
 namespace browser_sync {
 
+class ConflictResolver;
 class SyncerSession;
 
 class UpdateApplicator {
  public:
-  typedef std::vector<int64>::iterator vi64iter;
+  typedef syncable::Directory::UnappliedUpdateMetaHandles::iterator
+      UpdateIterator;
+
+  UpdateApplicator(ConflictResolver* resolver,
+                   const UpdateIterator& begin,
+                   const UpdateIterator& end);
 
-  UpdateApplicator(SyncerSession* session, const vi64iter& begin,
-                   const vi64iter& end);
   // returns true if there's more we can do.
   bool AttemptOneApplication(syncable::WriteTransaction* trans);
   // return true if we've applied all updates.
@@ -41,13 +41,15 @@ class UpdateApplicator {
   // SyncerSession -- to get that to happen, call this method after update
   // application is finished (i.e., when AttemptOneAllocation stops returning
   // true).
-  void SaveProgressIntoSessionState();
+  void SaveProgressIntoSessionState(SyncerSession* session);
 
  private:
-  SyncerSession* const session_;
-  vi64iter const begin_;
-  vi64iter end_;
-  vi64iter pointer_;
+  // Used to resolve conflicts when trying to apply updates.
+  ConflictResolver* const resolver_;
+
+  UpdateIterator const begin_;
+  UpdateIterator end_;
+  UpdateIterator pointer_;
   bool progress_;
 
   // Track the result of the various items.