diff options
author | tim@chromium.org <tim@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2009-11-21 03:29:52 +0000 |
---|---|---|
committer | tim@chromium.org <tim@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2009-11-21 03:29:52 +0000 |
commit | 9ec27852a63c2d8fbcf4942629532af171f0a312 (patch) | |
tree | 5f22093d68d1699073439b40043383df8434da4f /chrome/browser/sync/engine | |
parent | fb6be8f376c29a67ca134c71f3a4e3c414dd4c47 (diff) | |
download | chromium_src-9ec27852a63c2d8fbcf4942629532af171f0a312.zip chromium_src-9ec27852a63c2d8fbcf4942629532af171f0a312.tar.gz chromium_src-9ec27852a63c2d8fbcf4942629532af171f0a312.tar.bz2 |
Add browser_sync 'sessions' to relieve SyncCycleState, SyncProcessState, SyncerSession,
SyncerStatus, and ConflictResolutionView of duty.
Main impact is factors all status munging to 'StatusController', adds SyncSessionContext
to wrap various engine parts needed by different components, removes duplicated methods by a
factor of ~3 making it easier to reason about, and adds a 'Controller' to the session object to
give a way to delegate session-global (i.e affecting any session) occurrences such as throttling.
Also adds testing for 'HasMoreToSync' and other session related code.
BUG=25266
TEST=SyncSessionTest(added), StatusControllerTest(added)
various sync_unit_tests in this CL
Review URL: http://codereview.chromium.org/386030
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@32732 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome/browser/sync/engine')
54 files changed, 949 insertions, 2880 deletions
diff --git a/chrome/browser/sync/engine/all_status.cc b/chrome/browser/sync/engine/all_status.cc index fb1ef44f..a9bd051 100644 --- a/chrome/browser/sync/engine/all_status.cc +++ b/chrome/browser/sync/engine/all_status.cc @@ -17,6 +17,7 @@ #include "chrome/browser/sync/engine/syncproto.h" #include "chrome/browser/sync/notifier/listener/talk_mediator.h" #include "chrome/browser/sync/protocol/service_constants.h" +#include "chrome/browser/sync/sessions/session_state.h" #include "chrome/browser/sync/syncable/directory_manager.h" #include "chrome/browser/sync/util/event_sys-inl.h" @@ -72,7 +73,7 @@ void AllStatus::WatchAuthWatcher(AuthWatcher* auth_watcher) { void AllStatus::WatchSyncerThread(SyncerThread* syncer_thread) { syncer_thread_hookup_.reset( - NewEventListenerHookup(syncer_thread->channel(), this, + NewEventListenerHookup(syncer_thread->relay_channel(), this, &AllStatus::HandleSyncerEvent)); } @@ -92,28 +93,29 @@ AllStatus::Status AllStatus::CreateBlankStatus() const { AllStatus::Status AllStatus::CalcSyncing(const SyncerEvent &event) const { Status status = CreateBlankStatus(); - SyncerStatus syncerStatus(event.last_session); - status.unsynced_count += static_cast<int>(syncerStatus.unsynced_count()); - status.conflicting_count += syncerStatus.conflicting_commits(); + const sessions::SyncSessionSnapshot* snapshot = event.snapshot; + status.unsynced_count += static_cast<int>(snapshot->unsynced_count); + status.conflicting_count += snapshot->errors.num_conflicting_commits; // The syncer may not be done yet, which could cause conflicting updates. // But this is only used for status, so it is better to have visibility. - status.conflicting_count += syncerStatus.conflicting_updates(); + status.conflicting_count += snapshot->num_conflicting_updates; - status.syncing |= syncerStatus.syncing(); - // Show a syncer as syncing if it's got stalled updates. - status.syncing = event.last_session->HasMoreToSync() && - event.last_session->silenced_until().is_null(); - status.initial_sync_ended |= syncerStatus.IsShareUsable(); - status.syncer_stuck |= syncerStatus.syncer_stuck(); - if (syncerStatus.consecutive_errors() > status.max_consecutive_errors) - status.max_consecutive_errors = syncerStatus.consecutive_errors(); + status.syncing |= snapshot->syncer_status.syncing; + status.syncing = snapshot->has_more_to_sync && snapshot->is_silenced; + status.initial_sync_ended |= snapshot->is_share_usable; + status.syncer_stuck |= snapshot->syncer_status.syncer_stuck; + + const sessions::ErrorCounters& errors(snapshot->errors); + if (errors.consecutive_errors > status.max_consecutive_errors) + status.max_consecutive_errors = errors.consecutive_errors; // 100 is an arbitrary limit. - if (syncerStatus.consecutive_transient_error_commits() > 100) + if (errors.consecutive_transient_error_commits > 100) status.server_broken = true; - status.updates_available += syncerStatus.num_server_changes_remaining(); - status.updates_received += syncerStatus.current_sync_timestamp(); + const sessions::ChangelogProgress& progress(snapshot->changelog_progress); + status.updates_available += progress.num_server_changes_remaining; + status.updates_received += progress.current_sync_timestamp; return status; } diff --git a/chrome/browser/sync/engine/all_status.h b/chrome/browser/sync/engine/all_status.h index 29691ec..c529390 100644 --- a/chrome/browser/sync/engine/all_status.h +++ b/chrome/browser/sync/engine/all_status.h @@ -13,7 +13,6 @@ #include "base/atomicops.h" #include "base/lock.h" #include "base/scoped_ptr.h" -#include "chrome/browser/sync/engine/syncer_status.h" #include "chrome/browser/sync/util/event_sys.h" namespace browser_sync { diff --git a/chrome/browser/sync/engine/apply_updates_command.cc b/chrome/browser/sync/engine/apply_updates_command.cc index 7b8e715..dda4e64 100644 --- a/chrome/browser/sync/engine/apply_updates_command.cc +++ b/chrome/browser/sync/engine/apply_updates_command.cc @@ -4,19 +4,22 @@ #include "chrome/browser/sync/engine/apply_updates_command.h" -#include "chrome/browser/sync/engine/syncer_session.h" #include "chrome/browser/sync/engine/update_applicator.h" +#include "chrome/browser/sync/sessions/sync_session.h" #include "chrome/browser/sync/syncable/directory_manager.h" #include "chrome/browser/sync/syncable/syncable.h" #include "chrome/browser/sync/util/sync_types.h" namespace browser_sync { +using sessions::SyncSession; + ApplyUpdatesCommand::ApplyUpdatesCommand() {} ApplyUpdatesCommand::~ApplyUpdatesCommand() {} -void ApplyUpdatesCommand::ModelChangingExecuteImpl(SyncerSession *session) { - syncable::ScopedDirLookup dir(session->dirman(), session->account_name()); +void ApplyUpdatesCommand::ModelChangingExecuteImpl(SyncSession* session) { + syncable::ScopedDirLookup dir(session->context()->directory_manager(), + session->context()->account_name()); if (!dir.good()) { LOG(ERROR) << "Scoped dir lookup failed!"; return; @@ -25,10 +28,12 @@ void ApplyUpdatesCommand::ModelChangingExecuteImpl(SyncerSession *session) { syncable::Directory::UnappliedUpdateMetaHandles handles; dir->GetUnappliedUpdateMetaHandles(&trans, &handles); - UpdateApplicator applicator(session->resolver(), handles.begin(), + UpdateApplicator applicator(session->context()->resolver(), handles.begin(), handles.end()); while (applicator.AttemptOneApplication(&trans)) {} - applicator.SaveProgressIntoSessionState(session); + applicator.SaveProgressIntoSessionState( + session->status_controller()->mutable_conflict_progress(), + session->status_controller()->mutable_update_progress()); } } // namespace browser_sync diff --git a/chrome/browser/sync/engine/apply_updates_command.h b/chrome/browser/sync/engine/apply_updates_command.h index 67a68c8..c70a517 100644 --- a/chrome/browser/sync/engine/apply_updates_command.h +++ b/chrome/browser/sync/engine/apply_updates_command.h @@ -16,14 +16,13 @@ class Id; namespace browser_sync { -class SyncerSession; - class ApplyUpdatesCommand : public ModelChangingSyncerCommand { public: ApplyUpdatesCommand(); virtual ~ApplyUpdatesCommand(); - virtual void ModelChangingExecuteImpl(SyncerSession* session); + // ModelChangingSyncerCommand implementation. + virtual void ModelChangingExecuteImpl(sessions::SyncSession* session); private: DISALLOW_COPY_AND_ASSIGN(ApplyUpdatesCommand); diff --git a/chrome/browser/sync/engine/apply_updates_command_unittest.cc b/chrome/browser/sync/engine/apply_updates_command_unittest.cc index 4076bcb..c9f7c99 100755 --- a/chrome/browser/sync/engine/apply_updates_command_unittest.cc +++ b/chrome/browser/sync/engine/apply_updates_command_unittest.cc @@ -3,9 +3,7 @@ // found in the LICENSE file. #include "chrome/browser/sync/engine/apply_updates_command.h" -#include "chrome/browser/sync/engine/sync_cycle_state.h" -#include "chrome/browser/sync/engine/sync_process_state.h" -#include "chrome/browser/sync/engine/syncer_session.h" +#include "chrome/browser/sync/sessions/sync_session.h" #include "chrome/browser/sync/syncable/directory_manager.h" #include "chrome/browser/sync/syncable/syncable.h" #include "chrome/browser/sync/syncable/syncable_id.h" @@ -22,20 +20,44 @@ using syncable::Id; using syncable::UNITTEST; namespace browser_sync { +using sessions::SyncSessionContext; +using sessions::SyncSession; // A test fixture for tests exercising ApplyUpdatesCommand. -class ApplyUpdatesCommandTest : public testing::Test { +class ApplyUpdatesCommandTest : public testing::Test, + public SyncSession::Delegate { + public: + // SyncSession::Delegate implementation. + virtual void OnSilencedUntil(const base::TimeTicks& silenced_until) { + FAIL() << "Should not get silenced."; + } + virtual bool IsSyncingCurrentlySilenced() { + ADD_FAILURE() << "No requests for silenced state should be made."; + return false; + } + virtual void OnReceivedLongPollIntervalUpdate( + const base::TimeDelta& new_interval) { + FAIL() << "Should not get poll interval update."; + } + virtual void OnReceivedShortPollIntervalUpdate( + const base::TimeDelta& new_interval) { + FAIL() << "Should not get poll interval update."; + } + protected: ApplyUpdatesCommandTest() : next_revision_(1) {} virtual ~ApplyUpdatesCommandTest() {} virtual void SetUp() { syncdb_.SetUp(); + context_.reset(new SyncSessionContext(NULL, syncdb_.manager(), NULL)); + context_->set_account_name(syncdb_.name()); } virtual void TearDown() { syncdb_.TearDown(); } protected: + // Create a new unapplied update. void CreateUnappliedNewItemWithParent(const string& item_id, const string& parent_id) { @@ -55,7 +77,7 @@ class ApplyUpdatesCommandTest : public testing::Test { TestDirectorySetterUpper syncdb_; ApplyUpdatesCommand apply_updates_command_; - + scoped_ptr<SyncSessionContext> context_; private: int64 next_revision_; DISALLOW_COPY_AND_ASSIGN(ApplyUpdatesCommandTest); @@ -66,18 +88,15 @@ TEST_F(ApplyUpdatesCommandTest, Simple) { CreateUnappliedNewItemWithParent("parent", root_server_id); CreateUnappliedNewItemWithParent("child", "parent"); - SyncCycleState cycle_state; - SyncProcessState process_state(syncdb_.manager(), syncdb_.name(), - NULL, NULL, NULL, NULL); - SyncerSession session(&cycle_state, &process_state); - + SyncSession session(context_.get(), this); apply_updates_command_.ModelChangingExecuteImpl(&session); - EXPECT_EQ(2, cycle_state.AppliedUpdatesSize()) + sessions::StatusController* status = session.status_controller(); + EXPECT_EQ(2, status->update_progress().AppliedUpdatesSize()) << "All updates should have been attempted"; - EXPECT_EQ(0, process_state.ConflictingItemsSize()) + EXPECT_EQ(0, status->conflict_progress()->ConflictingItemsSize()) << "Simple update shouldn't result in conflicts"; - EXPECT_EQ(2, cycle_state.SuccessfullyAppliedUpdateCount()) + EXPECT_EQ(2, status->update_progress().SuccessfullyAppliedUpdateCount()) << "All items should have been successfully applied"; } @@ -91,18 +110,15 @@ TEST_F(ApplyUpdatesCommandTest, UpdateWithChildrenBeforeParents) { CreateUnappliedNewItemWithParent("a_child_created_second", "parent"); CreateUnappliedNewItemWithParent("x_child_created_second", "parent"); - SyncCycleState cycle_state; - SyncProcessState process_state(syncdb_.manager(), syncdb_.name(), - NULL, NULL, NULL, NULL); - SyncerSession session(&cycle_state, &process_state); - + SyncSession session(context_.get(), this); apply_updates_command_.ModelChangingExecuteImpl(&session); - EXPECT_EQ(5, cycle_state.AppliedUpdatesSize()) + sessions::StatusController* status = session.status_controller(); + EXPECT_EQ(5, status->update_progress().AppliedUpdatesSize()) << "All updates should have been attempted"; - EXPECT_EQ(0, process_state.ConflictingItemsSize()) + EXPECT_EQ(0, status->conflict_progress()->ConflictingItemsSize()) << "Simple update shouldn't result in conflicts, even if out-of-order"; - EXPECT_EQ(5, cycle_state.SuccessfullyAppliedUpdateCount()) + EXPECT_EQ(5, status->update_progress().SuccessfullyAppliedUpdateCount()) << "All updates should have been successfully applied"; } @@ -111,18 +127,15 @@ TEST_F(ApplyUpdatesCommandTest, NestedItemsWithUnknownParent) { CreateUnappliedNewItemWithParent("some_item", "unknown_parent"); CreateUnappliedNewItemWithParent("some_other_item", "some_item"); - SyncCycleState cycle_state; - SyncProcessState process_state(syncdb_.manager(), syncdb_.name(), - NULL, NULL, NULL, NULL); - SyncerSession session(&cycle_state, &process_state); - + SyncSession session(context_.get(), this); apply_updates_command_.ModelChangingExecuteImpl(&session); - EXPECT_EQ(2, cycle_state.AppliedUpdatesSize()) + sessions::StatusController* status = session.status_controller(); + EXPECT_EQ(2, status->update_progress().AppliedUpdatesSize()) << "All updates should have been attempted"; - EXPECT_EQ(2, process_state.ConflictingItemsSize()) + EXPECT_EQ(2, status->conflict_progress()->ConflictingItemsSize()) << "All updates with an unknown ancestors should be in conflict"; - EXPECT_EQ(0, cycle_state.SuccessfullyAppliedUpdateCount()) + EXPECT_EQ(0, status->update_progress().SuccessfullyAppliedUpdateCount()) << "No item with an unknown ancestor should be applied"; } @@ -136,18 +149,15 @@ TEST_F(ApplyUpdatesCommandTest, ItemsBothKnownAndUnknown) { CreateUnappliedNewItemWithParent("third_known_item", "fourth_known_item"); CreateUnappliedNewItemWithParent("fourth_known_item", root_server_id); - SyncCycleState cycle_state; - SyncProcessState process_state(syncdb_.manager(), syncdb_.name(), - NULL, NULL, NULL, NULL); - SyncerSession session(&cycle_state, &process_state); - + SyncSession session(context_.get(), this); apply_updates_command_.ModelChangingExecuteImpl(&session); - EXPECT_EQ(6, cycle_state.AppliedUpdatesSize()) + sessions::StatusController* status = session.status_controller(); + EXPECT_EQ(6, status->update_progress().AppliedUpdatesSize()) << "All updates should have been attempted"; - EXPECT_EQ(2, process_state.ConflictingItemsSize()) + EXPECT_EQ(2, status->conflict_progress()->ConflictingItemsSize()) << "The updates with unknown ancestors should be in conflict"; - EXPECT_EQ(4, cycle_state.SuccessfullyAppliedUpdateCount()) + EXPECT_EQ(4, status->update_progress().SuccessfullyAppliedUpdateCount()) << "The updates with known ancestors should be successfully applied"; } 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 edbd601..41988f5 100755 --- a/chrome/browser/sync/engine/build_and_process_conflict_sets_command.cc +++ b/chrome/browser/sync/engine/build_and_process_conflict_sets_command.cc @@ -11,13 +11,17 @@ #include "base/basictypes.h" #include "base/format_macros.h" #include "base/rand_util.h" -#include "chrome/browser/sync/engine/conflict_resolution_view.h" #include "chrome/browser/sync/engine/syncer_util.h" #include "chrome/browser/sync/engine/update_applicator.h" +#include "chrome/browser/sync/sessions/sync_session.h" #include "chrome/browser/sync/syncable/directory_manager.h" namespace browser_sync { +using sessions::ConflictProgress; +using sessions::StatusController; +using sessions::SyncSession; +using sessions::UpdateProgress; using std::set; using std::string; using std::vector; @@ -26,22 +30,24 @@ BuildAndProcessConflictSetsCommand::BuildAndProcessConflictSetsCommand() {} BuildAndProcessConflictSetsCommand::~BuildAndProcessConflictSetsCommand() {} void BuildAndProcessConflictSetsCommand::ModelChangingExecuteImpl( - SyncerSession* session) { - session->set_conflict_sets_built(BuildAndProcessConflictSets(session)); + SyncSession* session) { + session->status_controller()->set_conflict_sets_built( + BuildAndProcessConflictSets(session)); } bool BuildAndProcessConflictSetsCommand::BuildAndProcessConflictSets( - SyncerSession* session) { - syncable::ScopedDirLookup dir(session->dirman(), session->account_name()); + SyncSession* session) { + syncable::ScopedDirLookup dir(session->context()->directory_manager(), + session->context()->account_name()); if (!dir.good()) return false; bool had_single_direction_sets = false; { // Scope for transaction. syncable::WriteTransaction trans(dir, syncable::SYNCER, __FILE__, __LINE__); - ConflictResolutionView conflict_view(session); - BuildConflictSets(&trans, &conflict_view); - had_single_direction_sets = - ProcessSingleDirectionConflictSets(&trans, session); + BuildConflictSets(&trans, + session->status_controller()->mutable_conflict_progress()); + had_single_direction_sets = ProcessSingleDirectionConflictSets(&trans, + session->context()->resolver(), session->status_controller()); // We applied some updates transactionally, lets try syncing again. if (had_single_direction_sets) return true; @@ -50,12 +56,12 @@ bool BuildAndProcessConflictSetsCommand::BuildAndProcessConflictSets( } bool BuildAndProcessConflictSetsCommand::ProcessSingleDirectionConflictSets( - syncable::WriteTransaction* trans, SyncerSession* const session) { + syncable::WriteTransaction* trans, ConflictResolver* resolver, + StatusController* status) { bool rv = false; - ConflictResolutionView conflict_view(session); set<ConflictSet*>::const_iterator all_sets_iterator; - for (all_sets_iterator = conflict_view.ConflictSetsBegin(); - all_sets_iterator != conflict_view.ConflictSetsEnd(); ) { + for (all_sets_iterator = status->conflict_progress()->ConflictSetsBegin(); + all_sets_iterator != status->conflict_progress()->ConflictSetsEnd();) { const ConflictSet* conflict_set = *all_sets_iterator; CHECK(conflict_set->size() >= 2); // We scan the set to see if it consists of changes of only one type. @@ -71,9 +77,8 @@ bool BuildAndProcessConflictSetsCommand::ProcessSingleDirectionConflictSets( } if (conflict_set->size() == unsynced_count && 0 == unapplied_count) { LOG(INFO) << "Skipped transactional commit attempt."; - } else if (conflict_set->size() == unapplied_count && - 0 == unsynced_count && - ApplyUpdatesTransactionally(trans, conflict_set, session)) { + } else if (conflict_set->size() == unapplied_count && 0 == unsynced_count && + ApplyUpdatesTransactionally(trans, conflict_set, resolver, status)) { rv = true; } ++all_sets_iterator; @@ -125,20 +130,20 @@ bool RollbackEntry(syncable::WriteTransaction* trans, } void PlaceEntriesAtRoot(syncable::WriteTransaction* trans, - const vector<syncable::Id>* ids) { - vector<syncable::Id>::const_iterator it; - for (it = ids->begin(); it != ids->end(); ++it) { - syncable::MutableEntry entry(trans, syncable::GET_BY_ID, *it); + const vector<syncable::Id>* ids) { + vector<syncable::Id>::const_iterator it; + for (it = ids->begin(); it != ids->end(); ++it) { + syncable::MutableEntry entry(trans, syncable::GET_BY_ID, *it); entry.Put(syncable::PARENT_ID, trans->root_id()); + } } -} } // namespace bool BuildAndProcessConflictSetsCommand::ApplyUpdatesTransactionally( syncable::WriteTransaction* trans, const vector<syncable::Id>* const update_set, - SyncerSession* const session) { + ConflictResolver* resolver, StatusController* status) { // The handles in the |update_set| order. vector<int64> handles; @@ -183,8 +188,7 @@ bool BuildAndProcessConflictSetsCommand::ApplyUpdatesTransactionally( // 5. Use the usual apply updates from the special start state we've just // prepared. - UpdateApplicator applicator(session->resolver(), handles.begin(), - handles.end()); + UpdateApplicator applicator(resolver, handles.begin(), handles.end()); while (applicator.AttemptOneApplication(trans)) { // Keep going till all updates are applied. } @@ -202,16 +206,17 @@ bool BuildAndProcessConflictSetsCommand::ApplyUpdatesTransactionally( } return false; // Don't save progress -- we just undid it. } - applicator.SaveProgressIntoSessionState(session); + applicator.SaveProgressIntoSessionState(status->mutable_conflict_progress(), + status->mutable_update_progress()); return true; } void BuildAndProcessConflictSetsCommand::BuildConflictSets( syncable::BaseTransaction* trans, - ConflictResolutionView* view) { - view->CleanupSets(); - set<syncable::Id>::iterator i = view->CommitConflictsBegin(); - while (i != view->CommitConflictsEnd()) { + ConflictProgress* conflict_progress) { + conflict_progress->CleanupSets(); + set<syncable::Id>::iterator i = conflict_progress->ConflictingItemsBegin(); + while (i != conflict_progress->ConflictingItemsEnd()) { syncable::Entry entry(trans, syncable::GET_BY_ID, *i); CHECK(entry.good()); if (!entry.Get(syncable::IS_UNSYNCED) && @@ -219,7 +224,7 @@ void BuildAndProcessConflictSetsCommand::BuildConflictSets( // This can happen very rarely. It means we had a simply conflicting item // that randomly committed. We drop the entry as it's no longer // conflicting. - view->EraseCommitConflict(i++); + conflict_progress->EraseConflictingItemById(*(i++)); continue; } if (entry.ExistsOnClientBecauseNameIsNonEmpty() && @@ -231,15 +236,15 @@ void BuildAndProcessConflictSetsCommand::BuildConflictSets( bool new_parent = entry.Get(syncable::PARENT_ID) != entry.Get(syncable::SERVER_PARENT_ID); if (new_parent) - MergeSetsForIntroducedLoops(trans, &entry, view); - MergeSetsForNonEmptyDirectories(trans, &entry, view); + MergeSetsForIntroducedLoops(trans, &entry, conflict_progress); + MergeSetsForNonEmptyDirectories(trans, &entry, conflict_progress); ++i; } } void BuildAndProcessConflictSetsCommand::MergeSetsForIntroducedLoops( syncable::BaseTransaction* trans, syncable::Entry* entry, - ConflictResolutionView* view) { + ConflictProgress* conflict_progress) { // This code crawls up from the item in question until it gets to the root // or itself. If it gets to the root it does nothing. If it finds a loop all // moved unsynced entries in the list of crawled entries have their sets @@ -273,7 +278,8 @@ void BuildAndProcessConflictSetsCommand::MergeSetsForIntroducedLoops( if (parent_id.IsRoot()) return; for (size_t i = 0; i < conflicting_entries.size(); i++) { - view->MergeSets(entry->Get(syncable::ID), conflicting_entries[i]); + conflict_progress->MergeSets(entry->Get(syncable::ID), + conflicting_entries[i]); } } @@ -336,7 +342,7 @@ class LocallyDeletedPathChecker { template <typename Checker> void CrawlDeletedTreeMergingSets(syncable::BaseTransaction* trans, const syncable::Entry& entry, - ConflictResolutionView* view, + ConflictProgress* conflict_progress, Checker checker) { syncable::Id parent_id = entry.Get(syncable::PARENT_ID); syncable::Id double_step_parent_id = parent_id; @@ -355,10 +361,12 @@ void CrawlDeletedTreeMergingSets(syncable::BaseTransaction* trans, trans, double_step_parent_id, parent_id, entry); } syncable::Entry parent(trans, syncable::GET_BY_ID, parent_id); - if (checker.CausingConflict(parent, entry)) - view->MergeSets(entry.Get(syncable::ID), parent.Get(syncable::ID)); - else + if (checker.CausingConflict(parent, entry)) { + conflict_progress->MergeSets(entry.Get(syncable::ID), + parent.Get(syncable::ID)); + } else { break; + } parent_id = parent.Get(syncable::PARENT_ID); } } @@ -367,10 +375,10 @@ void CrawlDeletedTreeMergingSets(syncable::BaseTransaction* trans, void BuildAndProcessConflictSetsCommand::MergeSetsForNonEmptyDirectories( syncable::BaseTransaction* trans, syncable::Entry* entry, - ConflictResolutionView* view) { + ConflictProgress* conflict_progress) { if (entry->Get(syncable::IS_UNSYNCED) && !entry->Get(syncable::IS_DEL)) { ServerDeletedPathChecker checker; - CrawlDeletedTreeMergingSets(trans, *entry, view, checker); + CrawlDeletedTreeMergingSets(trans, *entry, conflict_progress, checker); } if (entry->Get(syncable::IS_UNAPPLIED_UPDATE) && !entry->Get(syncable::SERVER_IS_DEL)) { @@ -382,8 +390,9 @@ void BuildAndProcessConflictSetsCommand::MergeSetsForNonEmptyDirectories( LocallyDeletedPathChecker checker; if (!checker.CausingConflict(parent, *entry)) return; - view->MergeSets(entry->Get(syncable::ID), parent.Get(syncable::ID)); - CrawlDeletedTreeMergingSets(trans, parent, view, checker); + conflict_progress->MergeSets(entry->Get(syncable::ID), + parent.Get(syncable::ID)); + CrawlDeletedTreeMergingSets(trans, parent, conflict_progress, checker); } } diff --git a/chrome/browser/sync/engine/build_and_process_conflict_sets_command.h b/chrome/browser/sync/engine/build_and_process_conflict_sets_command.h index e0fed74..4a408c7 100644 --- a/chrome/browser/sync/engine/build_and_process_conflict_sets_command.h +++ b/chrome/browser/sync/engine/build_and_process_conflict_sets_command.h @@ -20,40 +20,47 @@ class WriteTransaction; namespace browser_sync { -class ConflictResolutionView; -class SyncerSession; +class ConflictResolver; + +namespace sessions { +class ConflictProgress; +class StatusController; +} class BuildAndProcessConflictSetsCommand : public ModelChangingSyncerCommand { public: BuildAndProcessConflictSetsCommand(); virtual ~BuildAndProcessConflictSetsCommand(); - virtual void ModelChangingExecuteImpl(SyncerSession* session); + // ModelChangingSyncerCommand implementation. + virtual void ModelChangingExecuteImpl(sessions::SyncSession* session); private: - bool BuildAndProcessConflictSets(SyncerSession* session); + bool BuildAndProcessConflictSets(sessions::SyncSession* session); bool ProcessSingleDirectionConflictSets( - syncable::WriteTransaction* trans, SyncerSession* const session); + syncable::WriteTransaction* trans, ConflictResolver* resolver, + sessions::StatusController* status); bool ApplyUpdatesTransactionally( syncable::WriteTransaction* trans, const std::vector<syncable::Id>* const update_set, - SyncerSession* const session); + ConflictResolver* resolver, + sessions::StatusController* status); void BuildConflictSets(syncable::BaseTransaction* trans, - ConflictResolutionView* view); + sessions::ConflictProgress* conflict_progress); void MergeSetsForNameClash(syncable::BaseTransaction* trans, syncable::Entry* entry, - ConflictResolutionView* view); + sessions::ConflictProgress* conflict_progress); void MergeSetsForIntroducedLoops(syncable::BaseTransaction* trans, - syncable::Entry* entry, - ConflictResolutionView* view); + syncable::Entry* entry, + sessions::ConflictProgress* conflict_progress); void MergeSetsForNonEmptyDirectories(syncable::BaseTransaction* trans, - syncable::Entry* entry, - ConflictResolutionView* view); + syncable::Entry* entry, + sessions::ConflictProgress* conflict_progress); void MergeSetsForPositionUpdate(syncable::BaseTransaction* trans, - syncable::Entry* entry, - ConflictResolutionView* view); + syncable::Entry* entry, + sessions::ConflictProgress* conflict_progress); DISALLOW_COPY_AND_ASSIGN(BuildAndProcessConflictSetsCommand); }; diff --git a/chrome/browser/sync/engine/build_commit_command.cc b/chrome/browser/sync/engine/build_commit_command.cc index 637ab39..3e4e01a 100644 --- a/chrome/browser/sync/engine/build_commit_command.cc +++ b/chrome/browser/sync/engine/build_commit_command.cc @@ -9,9 +9,8 @@ #include <vector> #include "chrome/browser/sync/engine/syncer_proto_util.h" -#include "chrome/browser/sync/engine/syncer_session.h" #include "chrome/browser/sync/engine/syncer_util.h" -#include "chrome/browser/sync/engine/syncproto.h" +#include "chrome/browser/sync/sessions/sync_session.h" #include "chrome/browser/sync/syncable/syncable.h" #include "chrome/browser/sync/syncable/syncable_changes_version.h" #include "chrome/browser/sync/util/sync_types.h" @@ -25,11 +24,13 @@ using syncable::MutableEntry; namespace browser_sync { +using sessions::SyncSession; + BuildCommitCommand::BuildCommitCommand() {} BuildCommitCommand::~BuildCommitCommand() {} void BuildCommitCommand::AddExtensionsActivityToMessage( - SyncerSession* session, CommitMessage* message) { + SyncSession* session, CommitMessage* message) { const ExtensionsActivityMonitor::Records& records = session->extensions_activity(); for (ExtensionsActivityMonitor::Records::const_iterator it = records.begin(); @@ -42,9 +43,9 @@ void BuildCommitCommand::AddExtensionsActivityToMessage( } } -void BuildCommitCommand::ExecuteImpl(SyncerSession* session) { +void BuildCommitCommand::ExecuteImpl(SyncSession* session) { ClientToServerMessage message; - message.set_share(session->account_name()); + message.set_share(session->context()->account_name()); message.set_message_contents(ClientToServerMessage::COMMIT); CommitMessage* commit_message = message.mutable_commit(); @@ -52,7 +53,7 @@ void BuildCommitCommand::ExecuteImpl(SyncerSession* session) { session->write_transaction()->directory()->cache_guid()); AddExtensionsActivityToMessage(session, commit_message); - const vector<Id>& commit_ids = session->commit_ids(); + const vector<Id>& commit_ids = session->status_controller()->commit_ids(); for (size_t i = 0; i < commit_ids.size(); i++) { Id id = commit_ids[i]; SyncEntity* sync_entry = @@ -147,7 +148,7 @@ void BuildCommitCommand::ExecuteImpl(SyncerSession* session) { } } } - session->set_commit_message(message); + session->status_controller()->mutable_commit_message()->CopyFrom(message); } } // namespace browser_sync diff --git a/chrome/browser/sync/engine/build_commit_command.h b/chrome/browser/sync/engine/build_commit_command.h index 1c29590..f2f4516 100644 --- a/chrome/browser/sync/engine/build_commit_command.h +++ b/chrome/browser/sync/engine/build_commit_command.h @@ -7,7 +7,7 @@ #include "base/basictypes.h" #include "chrome/browser/sync/engine/syncer_command.h" -#include "chrome/browser/sync/engine/syncer_session.h" +#include "chrome/browser/sync/engine/syncproto.h" namespace browser_sync { @@ -16,10 +16,11 @@ class BuildCommitCommand : public SyncerCommand { BuildCommitCommand(); virtual ~BuildCommitCommand(); - virtual void ExecuteImpl(SyncerSession *session); + // SyncerCommand implementation. + virtual void ExecuteImpl(sessions::SyncSession* session); private: - void AddExtensionsActivityToMessage(SyncerSession* session, + void AddExtensionsActivityToMessage(sessions::SyncSession* session, CommitMessage* message); DISALLOW_COPY_AND_ASSIGN(BuildCommitCommand); }; diff --git a/chrome/browser/sync/engine/client_command_channel.h b/chrome/browser/sync/engine/client_command_channel.h deleted file mode 100644 index 4d94998..0000000 --- a/chrome/browser/sync/engine/client_command_channel.h +++ /dev/null @@ -1,31 +0,0 @@ -// Copyright (c) 2006-2009 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. - -#ifndef CHROME_BROWSER_SYNC_ENGINE_CLIENT_COMMAND_CHANNEL_H_ -#define CHROME_BROWSER_SYNC_ENGINE_CLIENT_COMMAND_CHANNEL_H_ - -#include "chrome/browser/sync/protocol/sync.pb.h" -#include "chrome/browser/sync/util/event_sys.h" - -namespace browser_sync { - -// Commands for the client come back in sync responses, which is kind of -// inconvenient because some services (like the bandwidth throttler) want to -// know about them. So to avoid explicit dependencies on this protocol -// behavior, the syncer dumps all client commands onto a shared client command -// channel. - -struct ClientCommandChannelTraits { - typedef const sync_pb::ClientCommand* EventType; - static inline bool IsChannelShutdownEvent(const EventType &event) { - return 0 == event; - } -}; - -typedef EventChannel<ClientCommandChannelTraits, Lock> - ClientCommandChannel; - -} // namespace browser_sync - -#endif // CHROME_BROWSER_SYNC_ENGINE_CLIENT_COMMAND_CHANNEL_H_ diff --git a/chrome/browser/sync/engine/conflict_resolution_view.cc b/chrome/browser/sync/engine/conflict_resolution_view.cc deleted file mode 100644 index e4e8620..0000000 --- a/chrome/browser/sync/engine/conflict_resolution_view.cc +++ /dev/null @@ -1,128 +0,0 @@ -// Copyright (c) 2006-2009 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. -// -// THIS CLASS PROVIDES NO SYNCHRONIZATION GUARANTEES. - -#include "chrome/browser/sync/engine/conflict_resolution_view.h" - -#include <map> -#include <set> - -#include "chrome/browser/sync/engine/sync_process_state.h" -#include "chrome/browser/sync/engine/syncer_session.h" - -using std::map; -using std::set; - -namespace browser_sync { - -ConflictResolutionView::ConflictResolutionView(SyncerSession* session) - : process_state_(session->sync_process_state_) {} - -int ConflictResolutionView::conflicting_updates() const { - return process_state_->conflicting_updates(); -} - -int ConflictResolutionView::conflicting_commits() const { - return process_state_->conflicting_commits(); -} - -void ConflictResolutionView::set_conflicting_commits(const int val) { - process_state_->set_conflicting_commits(val); -} - -int64 ConflictResolutionView::current_sync_timestamp() const { - return process_state_->current_sync_timestamp(); -} - -int64 ConflictResolutionView::num_server_changes_remaining() const { - return process_state_->num_server_changes_remaining(); -} - -// True iff we're stuck. User should contact support. -bool ConflictResolutionView::syncer_stuck() const { - return process_state_->syncer_stuck(); -} - -void ConflictResolutionView::set_syncer_stuck(const bool val) { - process_state_->set_syncer_stuck(val); -} - -IdToConflictSetMap::const_iterator ConflictResolutionView::IdToConflictSetFind( - const syncable::Id& the_id) const { - return process_state_->IdToConflictSetFind(the_id); -} - -IdToConflictSetMap::const_iterator - ConflictResolutionView::IdToConflictSetBegin() const { - return process_state_->IdToConflictSetBegin(); -} - -IdToConflictSetMap::const_iterator - ConflictResolutionView::IdToConflictSetEnd() const { - return process_state_->IdToConflictSetEnd(); -} - -IdToConflictSetMap::size_type - ConflictResolutionView::IdToConflictSetSize() const { - return process_state_->IdToConflictSetSize(); -} - -const ConflictSet* - ConflictResolutionView::IdToConflictSetGet(const syncable::Id& the_id) { - return process_state_->IdToConflictSetGet(the_id); -} - -set<ConflictSet*>::const_iterator - ConflictResolutionView::ConflictSetsBegin() const { - return process_state_->ConflictSetsBegin(); -} - -set<ConflictSet*>::const_iterator - ConflictResolutionView::ConflictSetsEnd() const { - return process_state_->ConflictSetsEnd(); -} - -set<ConflictSet*>::size_type - ConflictResolutionView::ConflictSetsSize() const { - return process_state_->ConflictSetsSize(); -} - -void ConflictResolutionView::MergeSets(const syncable::Id& set1, - const syncable::Id& set2) { - process_state_->MergeSets(set1, set2); -} - -void ConflictResolutionView::CleanupSets() { - process_state_->CleanupSets(); -} - -bool ConflictResolutionView::HasCommitConflicts() const { - return process_state_->HasConflictingItems(); -} - -int ConflictResolutionView::CommitConflictsSize() const { - return process_state_->ConflictingItemsSize(); -} - -void ConflictResolutionView::AddCommitConflict(const syncable::Id& the_id) { - process_state_->AddConflictingItem(the_id); -} - -void ConflictResolutionView::EraseCommitConflict( - set<syncable::Id>::iterator it) { - process_state_->EraseConflictingItem(it); -} - -set<syncable::Id>::iterator -ConflictResolutionView::CommitConflictsBegin() const { - return process_state_->ConflictingItemsBegin(); -} - -set<syncable::Id>::iterator -ConflictResolutionView::CommitConflictsEnd() const { - return process_state_->ConflictingItemsEnd(); -} - -} // namespace browser_sync diff --git a/chrome/browser/sync/engine/conflict_resolution_view.h b/chrome/browser/sync/engine/conflict_resolution_view.h deleted file mode 100755 index e349d3e..0000000 --- a/chrome/browser/sync/engine/conflict_resolution_view.h +++ /dev/null @@ -1,105 +0,0 @@ -// Copyright (c) 2009 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. -// -// Conflict resolution view is intended to provide a restricted view of the -// sync cycle state for the conflict resolver. Since the resolver doesn't get -// to see all of the SyncProcess, we can allow it to operate on a subsection of -// the data. - -#ifndef CHROME_BROWSER_SYNC_ENGINE_CONFLICT_RESOLUTION_VIEW_H_ -#define CHROME_BROWSER_SYNC_ENGINE_CONFLICT_RESOLUTION_VIEW_H_ - -#include <map> -#include <set> -#include <vector> - -#include "base/basictypes.h" -#include "chrome/browser/sync/engine/syncer_types.h" - -namespace syncable { -class Id; -} - -namespace browser_sync { - -class SyncCycleState; -class SyncProcessState; -class SyncerSession; - -class ConflictResolutionView { - // THIS CLASS PROVIDES NO SYNCHRONIZATION GUARANTEES. - public: - - explicit ConflictResolutionView(SyncProcessState* state) - : process_state_(state) { - } - - explicit ConflictResolutionView(SyncerSession* session); - - ~ConflictResolutionView() {} - - int conflicting_updates() const; - - // TODO(sync) can successful commit go in session? - int successful_commits() const; - - void increment_successful_commits(); - - void zero_successful_commits(); - - int conflicting_commits() const; - - void set_conflicting_commits(const int val); - - // True iff we're stuck. Something has gone wrong with the syncer. - bool syncer_stuck() const; - - void set_syncer_stuck(const bool val); - - int64 current_sync_timestamp() const; - - int64 num_server_changes_remaining() const; - - IdToConflictSetMap::const_iterator IdToConflictSetFind( - const syncable::Id& the_id) const; - - IdToConflictSetMap::const_iterator IdToConflictSetBegin() const; - - IdToConflictSetMap::const_iterator IdToConflictSetEnd() const; - - IdToConflictSetMap::size_type IdToConflictSetSize() const; - - const ConflictSet* IdToConflictSetGet(const syncable::Id& the_id); - - std::set<ConflictSet*>::const_iterator ConflictSetsBegin() const; - - std::set<ConflictSet*>::const_iterator ConflictSetsEnd() const; - - std::set<ConflictSet*>::size_type ConflictSetsSize() const; - - void MergeSets(const syncable::Id& set1, const syncable::Id& set2); - - void CleanupSets(); - - bool HasCommitConflicts() const; - - int CommitConflictsSize() const; - - void AddCommitConflict(const syncable::Id& the_id); - - void EraseCommitConflict(std::set<syncable::Id>::iterator it); - - std::set<syncable::Id>::iterator CommitConflictsBegin() const; - - std::set<syncable::Id>::iterator CommitConflictsEnd() const; - - private: - SyncProcessState* process_state_; - - DISALLOW_COPY_AND_ASSIGN(ConflictResolutionView); -}; - -} // namespace browser_sync - -#endif // CHROME_BROWSER_SYNC_ENGINE_CONFLICT_RESOLUTION_VIEW_H_ diff --git a/chrome/browser/sync/engine/conflict_resolver.cc b/chrome/browser/sync/engine/conflict_resolver.cc index 1f7896e..11aeff2 100755 --- a/chrome/browser/sync/engine/conflict_resolver.cc +++ b/chrome/browser/sync/engine/conflict_resolver.cc @@ -10,6 +10,7 @@ #include "chrome/browser/sync/engine/syncer.h" #include "chrome/browser/sync/engine/syncer_util.h" #include "chrome/browser/sync/protocol/service_constants.h" +#include "chrome/browser/sync/sessions/status_controller.h" #include "chrome/browser/sync/syncable/directory_manager.h" #include "chrome/browser/sync/syncable/syncable.h" #include "chrome/browser/sync/util/character_set_converters.h" @@ -28,6 +29,9 @@ using syncable::WriteTransaction; namespace browser_sync { +using sessions::ConflictProgress; +using sessions::StatusController; + const int SYNC_CYCLES_BEFORE_ADMITTING_DEFEAT = 8; ConflictResolver::ConflictResolver() { @@ -60,8 +64,7 @@ void ConflictResolver::OverwriteServerChanges(WriteTransaction* trans, ConflictResolver::ProcessSimpleConflictResult ConflictResolver::ProcessSimpleConflict(WriteTransaction* trans, - Id id, - SyncerSession* session) { + const Id& id) { MutableEntry entry(trans, syncable::GET_BY_ID, id); // Must be good as the entry won't have been cleaned up. CHECK(entry.good()); @@ -345,8 +348,7 @@ bool AttemptToFixRemovedDirectoriesWithContent(WriteTransaction* trans, // TODO(sync): Eliminate conflict sets. They're not necessary. bool ConflictResolver::ProcessConflictSet(WriteTransaction* trans, ConflictSet* conflict_set, - int conflict_count, - SyncerSession* session) { + int conflict_count) { int set_size = conflict_set->size(); if (set_size < 2) { LOG(WARNING) << "Skipping conflict set because it has size " << set_size; @@ -379,13 +381,13 @@ bool ConflictResolver::LogAndSignalIfConflictStuck( int attempt_count, InputIt begin, InputIt end, - ConflictResolutionView* view) { + StatusController* status) { if (attempt_count < SYNC_CYCLES_BEFORE_ADMITTING_DEFEAT) { return false; } // Don't signal stuck if we're not up to date. - if (view->num_server_changes_remaining() > 0) { + if (status->change_progress().num_server_changes_remaining > 0) { return false; } @@ -399,7 +401,7 @@ bool ConflictResolver::LogAndSignalIfConflictStuck( LOG(ERROR) << " Bad ID:" << *i; } - view->set_syncer_stuck(true); + status->set_syncer_stuck(true); return true; // TODO(sync): If we're stuck for a while we need to alert the user, clear @@ -408,27 +410,27 @@ bool ConflictResolver::LogAndSignalIfConflictStuck( } bool ConflictResolver::ResolveSimpleConflicts(const ScopedDirLookup& dir, - ConflictResolutionView* view, - SyncerSession* session) { + StatusController* status) { WriteTransaction trans(dir, syncable::SYNCER, __FILE__, __LINE__); bool forward_progress = false; + ConflictProgress const* progress = status->conflict_progress(); // First iterate over simple conflict items (those that belong to no set). set<Id>::const_iterator conflicting_item_it; - for (conflicting_item_it = view->CommitConflictsBegin(); - conflicting_item_it != view->CommitConflictsEnd(); + for (conflicting_item_it = progress->ConflictingItemsBeginConst(); + conflicting_item_it != progress->ConflictingItemsEnd(); ++conflicting_item_it) { Id id = *conflicting_item_it; map<Id, ConflictSet*>::const_iterator item_set_it = - view->IdToConflictSetFind(id); - if (item_set_it == view->IdToConflictSetEnd() || + progress->IdToConflictSetFind(id); + if (item_set_it == progress->IdToConflictSetEnd() || 0 == item_set_it->second) { // We have a simple conflict. - switch (ProcessSimpleConflict(&trans, id, session)) { + switch (ProcessSimpleConflict(&trans, id)) { case NO_SYNC_PROGRESS: { int conflict_count = (simple_conflict_count_map_[id] += 2); LogAndSignalIfConflictStuck(&trans, conflict_count, - &id, &id + 1, view); + &id, &id + 1, status); break; } case SYNC_PROGRESS: @@ -449,17 +451,17 @@ bool ConflictResolver::ResolveSimpleConflicts(const ScopedDirLookup& dir, } bool ConflictResolver::ResolveConflicts(const ScopedDirLookup& dir, - ConflictResolutionView* view, - SyncerSession* session) { + StatusController* status) { + ConflictProgress const* progress = status->conflict_progress(); bool rv = false; - if (ResolveSimpleConflicts(dir, view, session)) + if (ResolveSimpleConflicts(dir, status)) rv = true; WriteTransaction trans(dir, syncable::SYNCER, __FILE__, __LINE__); set<Id> children_of_dirs_merged_last_round; std::swap(children_of_merged_dirs_, children_of_dirs_merged_last_round); set<ConflictSet*>::const_iterator set_it; - for (set_it = view->ConflictSetsBegin(); - set_it != view->ConflictSetsEnd(); + for (set_it = progress->ConflictSetsBegin(); + set_it != progress->ConflictSetsEnd(); set_it++) { ConflictSet* conflict_set = *set_it; ConflictSetCountMapKey key = GetSetKey(conflict_set); @@ -478,12 +480,12 @@ bool ConflictResolver::ResolveConflicts(const ScopedDirLookup& dir, conflict_count += 2; } // See if we should process this set. - if (ProcessConflictSet(&trans, conflict_set, conflict_count, session)) { + if (ProcessConflictSet(&trans, conflict_set, conflict_count)) { rv = true; } LogAndSignalIfConflictStuck(&trans, conflict_count, conflict_set->begin(), - conflict_set->end(), view); + conflict_set->end(), status); } if (rv) { // This code means we don't signal that syncing is stuck when any conflict diff --git a/chrome/browser/sync/engine/conflict_resolver.h b/chrome/browser/sync/engine/conflict_resolver.h index 779e123..a680c2a 100755 --- a/chrome/browser/sync/engine/conflict_resolver.h +++ b/chrome/browser/sync/engine/conflict_resolver.h @@ -8,13 +8,9 @@ #ifndef CHROME_BROWSER_SYNC_ENGINE_CONFLICT_RESOLVER_H_ #define CHROME_BROWSER_SYNC_ENGINE_CONFLICT_RESOLVER_H_ -#include <list> -#include <vector> +#include <set> #include "base/basictypes.h" -#include "chrome/browser/sync/engine/conflict_resolution_view.h" -#include "chrome/browser/sync/engine/syncer_session.h" -#include "chrome/browser/sync/engine/syncer_status.h" #include "chrome/browser/sync/engine/syncer_types.h" #include "chrome/browser/sync/util/event_sys.h" #include "testing/gtest/include/gtest/gtest_prod.h" // For FRIEND_TEST @@ -28,6 +24,9 @@ class WriteTransaction; } // namespace syncable namespace browser_sync { +namespace sessions { +class StatusController; +} class ConflictResolver { friend class SyncerTest; @@ -38,8 +37,7 @@ class ConflictResolver { // Called by the syncer at the end of a update/commit cycle. // Returns true if the syncer should try to apply its updates again. bool ResolveConflicts(const syncable::ScopedDirLookup& dir, - ConflictResolutionView* view, - SyncerSession* session); + sessions::StatusController* status); private: // We keep a map to record how often we've seen each conflict set. We use this @@ -65,24 +63,21 @@ class ConflictResolver { ProcessSimpleConflictResult ProcessSimpleConflict( syncable::WriteTransaction* trans, - syncable::Id id, - SyncerSession* session); + const syncable::Id& id); bool ResolveSimpleConflicts(const syncable::ScopedDirLookup& dir, - ConflictResolutionView* view, - SyncerSession* session); + sessions::StatusController* status); bool ProcessConflictSet(syncable::WriteTransaction* trans, ConflictSet* conflict_set, - int conflict_count, - SyncerSession* session); + int conflict_count); // Returns true if we're stuck. template <typename InputIt> bool LogAndSignalIfConflictStuck(syncable::BaseTransaction* trans, int attempt_count, InputIt start, InputIt end, - ConflictResolutionView* view); + sessions::StatusController* status); ConflictSetCountMap conflict_set_count_map_; SimpleConflictCountMap simple_conflict_count_map_; diff --git a/chrome/browser/sync/engine/download_updates_command.cc b/chrome/browser/sync/engine/download_updates_command.cc index 1883e31..9b75ec2 100644 --- a/chrome/browser/sync/engine/download_updates_command.cc +++ b/chrome/browser/sync/engine/download_updates_command.cc @@ -9,29 +9,32 @@ #include "chrome/browser/sync/engine/syncer.h" #include "chrome/browser/sync/engine/syncer_proto_util.h" #include "chrome/browser/sync/engine/syncproto.h" +#include "chrome/browser/sync/sessions/sync_session.h" #include "chrome/browser/sync/syncable/directory_manager.h" #include "chrome/browser/sync/util/sync_types.h" using syncable::ScopedDirLookup; namespace browser_sync { - +using sessions::StatusController; +using sessions::SyncSession; using std::string; DownloadUpdatesCommand::DownloadUpdatesCommand() {} DownloadUpdatesCommand::~DownloadUpdatesCommand() {} -void DownloadUpdatesCommand::ExecuteImpl(SyncerSession* session) { +void DownloadUpdatesCommand::ExecuteImpl(SyncSession* session) { ClientToServerMessage client_to_server_message; ClientToServerResponse update_response; - client_to_server_message.set_share(session->account_name()); + client_to_server_message.set_share(session->context()->account_name()); client_to_server_message.set_message_contents( ClientToServerMessage::GET_UPDATES); GetUpdatesMessage* get_updates = client_to_server_message.mutable_get_updates(); - ScopedDirLookup dir(session->dirman(), session->account_name()); + ScopedDirLookup dir(session->context()->directory_manager(), + session->context()->account_name()); if (!dir.good()) { LOG(ERROR) << "Scoped dir lookup failed!"; return; @@ -42,21 +45,21 @@ void DownloadUpdatesCommand::ExecuteImpl(SyncerSession* session) { // Set GetUpdatesMessage.GetUpdatesCallerInfo information. get_updates->mutable_caller_info()->set_source(session->TestAndSetSource()); get_updates->mutable_caller_info()->set_notifications_enabled( - session->notifications_enabled()); + session->context()->notifications_enabled()); bool ok = SyncerProtoUtil::PostClientToServerMessage( &client_to_server_message, &update_response, session); + StatusController* status = session->status_controller(); if (!ok) { - SyncerStatus status(session); - status.increment_consecutive_problem_get_updates(); - status.increment_consecutive_errors(); + status->increment_num_consecutive_problem_get_updates(); + status->increment_num_consecutive_errors(); LOG(ERROR) << "PostClientToServerMessage() failed"; return; } - session->set_update_response(update_response); + status->mutable_updates_response()->CopyFrom(update_response); } } // namespace browser_sync diff --git a/chrome/browser/sync/engine/download_updates_command.h b/chrome/browser/sync/engine/download_updates_command.h index efd5468..6169bb7 100644 --- a/chrome/browser/sync/engine/download_updates_command.h +++ b/chrome/browser/sync/engine/download_updates_command.h @@ -7,16 +7,17 @@ #include "base/basictypes.h" #include "chrome/browser/sync/engine/syncer_command.h" -#include "chrome/browser/sync/engine/syncer_session.h" namespace browser_sync { -// Downloads updates from the server and places them in the SyncerSession. +// Downloads updates from the server and places them in the SyncSession. class DownloadUpdatesCommand : public SyncerCommand { public: DownloadUpdatesCommand(); virtual ~DownloadUpdatesCommand(); - virtual void ExecuteImpl(SyncerSession* session); + + // SyncerCommand implementation. + virtual void ExecuteImpl(sessions::SyncSession* session); private: DISALLOW_COPY_AND_ASSIGN(DownloadUpdatesCommand); diff --git a/chrome/browser/sync/engine/get_commit_ids_command.cc b/chrome/browser/sync/engine/get_commit_ids_command.cc index 5c1f015..a827e91 100644 --- a/chrome/browser/sync/engine/get_commit_ids_command.cc +++ b/chrome/browser/sync/engine/get_commit_ids_command.cc @@ -8,7 +8,6 @@ #include <utility> #include <vector> -#include "chrome/browser/sync/engine/syncer_session.h" #include "chrome/browser/sync/engine/syncer_util.h" #include "chrome/browser/sync/syncable/syncable.h" #include "chrome/browser/sync/util/sync_types.h" @@ -18,20 +17,24 @@ using std::vector; namespace browser_sync { +using sessions::SyncSession; +using sessions::StatusController; + GetCommitIdsCommand::GetCommitIdsCommand(int commit_batch_size) : requested_commit_batch_size_(commit_batch_size) {} GetCommitIdsCommand::~GetCommitIdsCommand() {} -void GetCommitIdsCommand::ExecuteImpl(SyncerSession* session) { +void GetCommitIdsCommand::ExecuteImpl(SyncSession* session) { // Gather the full set of unsynced items and store it in the session. They // are not in the correct order for commit. syncable::Directory::UnsyncedMetaHandles all_unsynced_handles; SyncerUtil::GetUnsyncedEntries(session->write_transaction(), - &all_unsynced_handles); - session->set_unsynced_handles(all_unsynced_handles); + &all_unsynced_handles); + StatusController* status = session->status_controller(); + status->set_unsynced_handles(all_unsynced_handles); - BuildCommitIds(session); + BuildCommitIds(status->unsynced_handles(), session->write_transaction()); const vector<syncable::Id>& verified_commit_ids = ordered_commit_set_.GetCommitIds(); @@ -39,7 +42,7 @@ void GetCommitIdsCommand::ExecuteImpl(SyncerSession* session) { for (size_t i = 0; i < verified_commit_ids.size(); i++) LOG(INFO) << "Debug commit batch result:" << verified_commit_ids[i]; - session->set_commit_ids(verified_commit_ids); + status->set_commit_ids(verified_commit_ids); } void GetCommitIdsCommand::AddUncommittedParentsAndTheirPredecessors( @@ -117,21 +120,23 @@ bool GetCommitIdsCommand::IsCommitBatchFull() { return ordered_commit_set_.Size() >= requested_commit_batch_size_; } -void GetCommitIdsCommand::AddCreatesAndMoves(SyncerSession* session) { +void GetCommitIdsCommand::AddCreatesAndMoves( + const vector<int64>& unsynced_handles, + syncable::WriteTransaction* write_transaction) { // Add moves and creates, and prepend their uncommitted parents. - for (CommitMetahandleIterator iterator(session, &ordered_commit_set_); - !IsCommitBatchFull() && iterator.Valid(); - iterator.Increment()) { + for (CommitMetahandleIterator iterator(unsynced_handles, write_transaction, + &ordered_commit_set_); + !IsCommitBatchFull() && iterator.Valid(); + iterator.Increment()) { int64 metahandle = iterator.Current(); - syncable::Entry entry(session->write_transaction(), + syncable::Entry entry(write_transaction, syncable::GET_BY_HANDLE, metahandle); if (!entry.Get(syncable::IS_DEL)) { - AddUncommittedParentsAndTheirPredecessors( - session->write_transaction(), entry.Get(syncable::PARENT_ID)); - AddPredecessorsThenItem(session->write_transaction(), &entry, - syncable::IS_UNSYNCED); + AddUncommittedParentsAndTheirPredecessors(write_transaction, + entry.Get(syncable::PARENT_ID)); + AddPredecessorsThenItem(write_transaction, &entry, syncable::IS_UNSYNCED); } } @@ -140,21 +145,21 @@ void GetCommitIdsCommand::AddCreatesAndMoves(SyncerSession* session) { ordered_commit_set_.Truncate(requested_commit_batch_size_); } -void GetCommitIdsCommand::AddDeletes(SyncerSession* session) { +void GetCommitIdsCommand::AddDeletes(const vector<int64>& unsynced_handles, + syncable::WriteTransaction* write_transaction) { set<syncable::Id> legal_delete_parents; - for (CommitMetahandleIterator iterator(session, &ordered_commit_set_); - !IsCommitBatchFull() && iterator.Valid(); - iterator.Increment()) { + for (CommitMetahandleIterator iterator(unsynced_handles, write_transaction, + &ordered_commit_set_); + !IsCommitBatchFull() && iterator.Valid(); + iterator.Increment()) { int64 metahandle = iterator.Current(); - syncable::Entry entry(session->write_transaction(), - syncable::GET_BY_HANDLE, + syncable::Entry entry(write_transaction, syncable::GET_BY_HANDLE, metahandle); if (entry.Get(syncable::IS_DEL)) { - syncable::Entry parent(session->write_transaction(), - syncable::GET_BY_ID, + syncable::Entry parent(write_transaction, syncable::GET_BY_ID, entry.Get(syncable::PARENT_ID)); // If the parent is deleted and unsynced, then any children of that // parent don't need to be added to the delete queue. @@ -206,12 +211,12 @@ void GetCommitIdsCommand::AddDeletes(SyncerSession* session) { // parent did expect at least one old deleted child // parent was not deleted - for (CommitMetahandleIterator iterator(session, &ordered_commit_set_); + for (CommitMetahandleIterator iterator(unsynced_handles, write_transaction, + &ordered_commit_set_); !IsCommitBatchFull() && iterator.Valid(); iterator.Increment()) { int64 metahandle = iterator.Current(); - syncable::MutableEntry entry(session->write_transaction(), - syncable::GET_BY_HANDLE, + syncable::MutableEntry entry(write_transaction, syncable::GET_BY_HANDLE, metahandle); if (entry.Get(syncable::IS_DEL)) { syncable::Id parent_id = entry.Get(syncable::PARENT_ID); @@ -222,7 +227,8 @@ void GetCommitIdsCommand::AddDeletes(SyncerSession* session) { } } -void GetCommitIdsCommand::BuildCommitIds(SyncerSession* session) { +void GetCommitIdsCommand::BuildCommitIds(const vector<int64>& unsynced_handles, + syncable::WriteTransaction* write_transaction) { // Commits follow these rules: // 1. Moves or creates are preceded by needed folder creates, from // root to leaf. For folders whose contents are ordered, moves @@ -233,10 +239,10 @@ void GetCommitIdsCommand::BuildCommitIds(SyncerSession* session) { // delete trees. // Add moves and creates, and prepend their uncommitted parents. - AddCreatesAndMoves(session); + AddCreatesAndMoves(unsynced_handles, write_transaction); // Add all deletes. - AddDeletes(session); + AddDeletes(unsynced_handles, write_transaction); } } // namespace browser_sync diff --git a/chrome/browser/sync/engine/get_commit_ids_command.h b/chrome/browser/sync/engine/get_commit_ids_command.h index 3abb975..3003d54 100644 --- a/chrome/browser/sync/engine/get_commit_ids_command.h +++ b/chrome/browser/sync/engine/get_commit_ids_command.h @@ -9,8 +9,8 @@ #include <vector> #include "chrome/browser/sync/engine/syncer_command.h" -#include "chrome/browser/sync/engine/syncer_session.h" #include "chrome/browser/sync/engine/syncer_util.h" +#include "chrome/browser/sync/sessions/sync_session.h" #include "chrome/browser/sync/util/sync_types.h" using std::pair; @@ -25,10 +25,12 @@ class GetCommitIdsCommand : public SyncerCommand { explicit GetCommitIdsCommand(int commit_batch_size); virtual ~GetCommitIdsCommand(); - virtual void ExecuteImpl(SyncerSession* session); + // SyncerCommand implementation. + virtual void ExecuteImpl(sessions::SyncSession* session); - // Returns a vector of IDs that should be committed. - void BuildCommitIds(SyncerSession *session); + // Builds a vector of IDs that should be committed. + void BuildCommitIds(const vector<int64>& unsynced_handles, + syncable::WriteTransaction* write_transaction); // These classes are public for testing. // TODO(ncarter): This code is more generic than just Commit and can @@ -101,14 +103,16 @@ class GetCommitIdsCommand : public SyncerCommand { public: // TODO(chron): Cache ValidateCommitEntry responses across iterators to save // UTF8 conversion and filename checking - CommitMetahandleIterator(SyncerSession* session, + CommitMetahandleIterator(const vector<int64>& unsynced_handles, + syncable::WriteTransaction* write_transaction, OrderedCommitSet* commit_set) - : session_(session), + : write_transaction_(write_transaction), + handle_iterator_(unsynced_handles.begin()), + unsynced_handles_end_(unsynced_handles.end()), commit_set_(commit_set) { - handle_iterator_ = session->unsynced_handles().begin(); // TODO(chron): Remove writes from this iterator. - DCHECK(session->has_open_write_transaction()); + DCHECK(write_transaction_); if (Valid() && !ValidateMetahandleForCommit(*handle_iterator_)) Increment(); @@ -125,7 +129,7 @@ class GetCommitIdsCommand : public SyncerCommand { return false; for (++handle_iterator_; - handle_iterator_ != session_->unsynced_handles().end(); + handle_iterator_ != unsynced_handles_end_; ++handle_iterator_) { if (ValidateMetahandleForCommit(*handle_iterator_)) return true; @@ -135,7 +139,7 @@ class GetCommitIdsCommand : public SyncerCommand { } bool Valid() const { - return !(handle_iterator_ == session_->unsynced_handles().end()); + return !(handle_iterator_ == unsynced_handles_end_); } private: @@ -144,9 +148,8 @@ class GetCommitIdsCommand : public SyncerCommand { return false; // We should really not WRITE in this iterator, but we can fix that - // later. ValidateCommitEntry writes to the DB, and we add the blocked - // items. We should move that somewhere else later. - syncable::MutableEntry entry(session_->write_transaction(), + // later. We should move that somewhere else later. + syncable::MutableEntry entry(write_transaction_, syncable::GET_BY_HANDLE, metahandle); VerifyCommitResult verify_result = SyncerUtil::ValidateCommitEntry(&entry); @@ -157,8 +160,9 @@ class GetCommitIdsCommand : public SyncerCommand { return verify_result == VERIFY_OK; } - SyncerSession* session_; + syncable::WriteTransaction* const write_transaction_; vector<int64>::const_iterator handle_iterator_; + vector<int64>::const_iterator unsynced_handles_end_; OrderedCommitSet* commit_set_; DISALLOW_COPY_AND_ASSIGN(CommitMetahandleIterator); @@ -183,9 +187,11 @@ class GetCommitIdsCommand : public SyncerCommand { bool IsCommitBatchFull(); - void AddCreatesAndMoves(SyncerSession* session); + void AddCreatesAndMoves(const vector<int64>& unsynced_handles, + syncable::WriteTransaction* write_transaction); - void AddDeletes(SyncerSession* session); + void AddDeletes(const vector<int64>& unsynced_handles, + syncable::WriteTransaction* write_transaction); OrderedCommitSet ordered_commit_set_; diff --git a/chrome/browser/sync/engine/model_changing_syncer_command.cc b/chrome/browser/sync/engine/model_changing_syncer_command.cc index f549c89..2283917 100644 --- a/chrome/browser/sync/engine/model_changing_syncer_command.cc +++ b/chrome/browser/sync/engine/model_changing_syncer_command.cc @@ -5,14 +5,14 @@ #include "chrome/browser/sync/engine/model_changing_syncer_command.h" #include "chrome/browser/sync/engine/model_safe_worker.h" -#include "chrome/browser/sync/engine/syncer_session.h" +#include "chrome/browser/sync/sessions/sync_session.h" #include "chrome/browser/sync/util/closure.h" namespace browser_sync { -void ModelChangingSyncerCommand::ExecuteImpl(SyncerSession* session) { +void ModelChangingSyncerCommand::ExecuteImpl(sessions::SyncSession* session) { work_session_ = session; - session->model_safe_worker()->DoWorkAndWaitUntilDone( + session->context()->model_safe_worker()->DoWorkAndWaitUntilDone( NewCallback(this, &ModelChangingSyncerCommand::StartChangingModel)); } diff --git a/chrome/browser/sync/engine/model_changing_syncer_command.h b/chrome/browser/sync/engine/model_changing_syncer_command.h index 32361090..b2ebee1 100644 --- a/chrome/browser/sync/engine/model_changing_syncer_command.h +++ b/chrome/browser/sync/engine/model_changing_syncer_command.h @@ -8,6 +8,9 @@ #include "chrome/browser/sync/engine/syncer_command.h" namespace browser_sync { +namespace sessions { +class SyncSession; +} // An abstract SyncerCommand which dispatches its Execute step to the // model-safe worker thread. Classes derived from ModelChangingSyncerCommand @@ -25,7 +28,7 @@ class ModelChangingSyncerCommand : public SyncerCommand { virtual ~ModelChangingSyncerCommand() { } // SyncerCommand implementation. Sets work_session to session. - virtual void ExecuteImpl(SyncerSession* session); + virtual void ExecuteImpl(sessions::SyncSession* session); // wrapper so implementations don't worry about storing work_session void StartChangingModel() { @@ -33,14 +36,14 @@ class ModelChangingSyncerCommand : public SyncerCommand { } // Abstract method to be implemented by subclasses. - virtual void ModelChangingExecuteImpl(SyncerSession* session) = 0; + virtual void ModelChangingExecuteImpl(sessions::SyncSession* session) = 0; private: // ExecuteImpl is expected to be run by SyncerCommand to set work_session. // StartChangingModel is called to start this command running. // Implementations will implement ModelChangingExecuteImpl and not // worry about storing the session or setting it. They are given work_session. - SyncerSession* work_session_; + sessions::SyncSession* work_session_; DISALLOW_COPY_AND_ASSIGN(ModelChangingSyncerCommand); }; diff --git a/chrome/browser/sync/engine/post_commit_message_command.cc b/chrome/browser/sync/engine/post_commit_message_command.cc index 02aac9f..1db6078 100644 --- a/chrome/browser/sync/engine/post_commit_message_command.cc +++ b/chrome/browser/sync/engine/post_commit_message_command.cc @@ -7,8 +7,8 @@ #include <vector> #include "chrome/browser/sync/engine/syncer_proto_util.h" -#include "chrome/browser/sync/engine/syncer_session.h" #include "chrome/browser/sync/engine/syncproto.h" +#include "chrome/browser/sync/sessions/sync_session.h" #include "chrome/browser/sync/syncable/directory_manager.h" #include "chrome/browser/sync/util/sync_types.h" @@ -19,32 +19,33 @@ namespace browser_sync { PostCommitMessageCommand::PostCommitMessageCommand() {} PostCommitMessageCommand::~PostCommitMessageCommand() {} -void PostCommitMessageCommand::ExecuteImpl(SyncerSession* session) { - if (session->commit_ids_empty()) +void PostCommitMessageCommand::ExecuteImpl(sessions::SyncSession* session) { + if (session->status_controller()->commit_ids().empty()) return; // Nothing to commit. ClientToServerResponse response; - syncable::ScopedDirLookup dir(session->dirman(), session->account_name()); + syncable::ScopedDirLookup dir(session->context()->directory_manager(), + session->context()->account_name()); if (!dir.good()) return; - if (!SyncerProtoUtil::PostClientToServerMessage(session->commit_message(), - &response, session)) { + sessions::StatusController* status = session->status_controller(); + if (!SyncerProtoUtil::PostClientToServerMessage( + status->mutable_commit_message(), &response, session)) { // None of our changes got through, let's clear sync flags and wait for // another list update. - SyncerStatus status(session); - status.increment_consecutive_problem_commits(); - status.increment_consecutive_errors(); + status->increment_num_consecutive_problem_commits(); + status->increment_num_consecutive_errors(); syncable::WriteTransaction trans(dir, syncable::SYNCER, __FILE__, __LINE__); // TODO(sync): why set this flag, it seems like a bad side-effect? - const vector<syncable::Id>& commit_ids = session->commit_ids(); + const vector<syncable::Id>& commit_ids = status->commit_ids(); for (size_t i = 0; i < commit_ids.size(); i++) { syncable::MutableEntry entry(&trans, syncable::GET_BY_ID, commit_ids[i]); entry.Put(syncable::SYNCING, false); } return; } else { - session->set_item_committed(); + status->set_items_committed(true); } - session->set_commit_response(response); + status->mutable_commit_response()->CopyFrom(response); } } // namespace browser_sync diff --git a/chrome/browser/sync/engine/post_commit_message_command.h b/chrome/browser/sync/engine/post_commit_message_command.h index 85e9f71..4f49894 100644 --- a/chrome/browser/sync/engine/post_commit_message_command.h +++ b/chrome/browser/sync/engine/post_commit_message_command.h @@ -6,7 +6,6 @@ #define CHROME_BROWSER_SYNC_ENGINE_POST_COMMIT_MESSAGE_COMMAND_H_ #include "chrome/browser/sync/engine/syncer_command.h" -#include "chrome/browser/sync/engine/syncer_session.h" #include "chrome/browser/sync/util/sync_types.h" namespace browser_sync { @@ -16,7 +15,8 @@ class PostCommitMessageCommand : public SyncerCommand { PostCommitMessageCommand(); virtual ~PostCommitMessageCommand(); - virtual void ExecuteImpl(SyncerSession* session); + // SyncerCommand implementation. + virtual void ExecuteImpl(sessions::SyncSession* session); private: DISALLOW_COPY_AND_ASSIGN(PostCommitMessageCommand); diff --git a/chrome/browser/sync/engine/process_commit_response_command.cc b/chrome/browser/sync/engine/process_commit_response_command.cc index f0fe413..4cc9912 100755 --- a/chrome/browser/sync/engine/process_commit_response_command.cc +++ b/chrome/browser/sync/engine/process_commit_response_command.cc @@ -9,10 +9,9 @@ #include "base/basictypes.h" #include "chrome/browser/sync/engine/syncer_proto_util.h" -#include "chrome/browser/sync/engine/syncer_session.h" -#include "chrome/browser/sync/engine/syncer_status.h" #include "chrome/browser/sync/engine/syncer_util.h" #include "chrome/browser/sync/engine/syncproto.h" +#include "chrome/browser/sync/sessions/sync_session.h" #include "chrome/browser/sync/syncable/directory_manager.h" #include "chrome/browser/sync/syncable/syncable.h" @@ -39,40 +38,44 @@ using syncable::SYNCING; namespace browser_sync { -void IncrementErrorCounters(SyncerStatus status) { - status.increment_consecutive_problem_commits(); - status.increment_consecutive_errors(); +using sessions::StatusController; +using sessions::SyncSession; +using sessions::ConflictProgress; + +void IncrementErrorCounters(StatusController* status) { + status->increment_num_consecutive_problem_commits(); + status->increment_num_consecutive_errors(); } -void ResetErrorCounters(SyncerStatus status) { - status.zero_consecutive_problem_commits(); - status.zero_consecutive_errors(); +void ResetErrorCounters(StatusController* status) { + status->set_num_consecutive_problem_commits(0); + status->set_num_consecutive_errors(0); } -ProcessCommitResponseCommand::ProcessCommitResponseCommand( - ExtensionsActivityMonitor* monitor) : extensions_monitor_(monitor) {} +ProcessCommitResponseCommand::ProcessCommitResponseCommand() {} ProcessCommitResponseCommand::~ProcessCommitResponseCommand() {} void ProcessCommitResponseCommand::ModelChangingExecuteImpl( - SyncerSession* session) { + SyncSession* session) { ProcessCommitResponse(session); - if (!session->HadSuccessfulCommits()) - extensions_monitor_->PutRecords(session->extensions_activity()); + ExtensionsActivityMonitor* monitor = session->context()->extensions_monitor(); + if (session->status_controller()->syncer_status().num_successful_commits == 0) + monitor->PutRecords(session->extensions_activity()); } void ProcessCommitResponseCommand::ProcessCommitResponse( - SyncerSession* session) { + SyncSession* session) { // TODO(sync): This function returns if it sees problems. We probably want // to flag the need for an update or similar. - ScopedDirLookup dir(session->dirman(), session->account_name()); + ScopedDirLookup dir(session->context()->directory_manager(), + session->context()->account_name()); if (!dir.good()) { LOG(ERROR) << "Scoped dir lookup failed!"; return; } - const ClientToServerResponse& response = session->commit_response(); - const vector<syncable::Id>& commit_ids = session->commit_ids(); - // TODO(sync): move counters out of here. - SyncerStatus status(session); + StatusController* status = session->status_controller(); + const ClientToServerResponse& response(status->commit_response()); + const vector<syncable::Id>& commit_ids(status->commit_ids()); if (!response.has_commit()) { // TODO(sync): What if we didn't try to commit anything? @@ -107,6 +110,7 @@ void ProcessCommitResponseCommand::ProcessCommitResponse( bool over_quota = false; set<syncable::Id> conflicting_new_folder_ids; set<syncable::Id> deleted_folders; + ConflictProgress* conflict_progress = status->mutable_conflict_progress(); { // Scope for WriteTransaction. WriteTransaction trans(dir, SYNCER, __FILE__, __LINE__); for (int i = 0; i < cr.entryresponse_size(); i++) { @@ -114,7 +118,7 @@ void ProcessCommitResponseCommand::ProcessCommitResponse( ProcessSingleCommitResponse(&trans, cr.entryresponse(i), commit_ids[i], &conflicting_new_folder_ids, - &deleted_folders, session); + &deleted_folders); switch (response_type) { case CommitResponse::INVALID_MESSAGE: ++error_commits; @@ -122,18 +126,22 @@ void ProcessCommitResponseCommand::ProcessCommitResponse( case CommitResponse::CONFLICT: ++conflicting_commits; // This is important to activate conflict resolution. - session->AddCommitConflict(commit_ids[i]); + conflict_progress->AddConflictingItemById(commit_ids[i]); break; case CommitResponse::SUCCESS: // TODO(sync): worry about sync_rate_ rate calc? ++successes; - status.increment_successful_commits(); + status->increment_num_successful_commits(); break; case CommitResponse::OVER_QUOTA: over_quota = true; // We handle over quota like a retry, which is same as transient. case CommitResponse::RETRY: case CommitResponse::TRANSIENT_ERROR: + // TODO(tim): Now that we have SyncSession::Delegate, we + // should plumb this directly for exponential backoff purposes rather + // than trying to infer from HasMoreToSync(). See + // SyncerThread::CalculatePollingWaitTime. ++transient_error_commits; break; default: @@ -143,21 +151,21 @@ void ProcessCommitResponseCommand::ProcessCommitResponse( } // TODO(sync): move status reporting elsewhere. - status.set_conflicting_commits(conflicting_commits); + status->set_num_conflicting_commits(conflicting_commits); if (0 == successes) { - status.increment_consecutive_transient_error_commits_by( + status->increment_num_consecutive_transient_error_commits_by( transient_error_commits); - status.increment_consecutive_errors_by(transient_error_commits); + status->increment_num_consecutive_errors_by(transient_error_commits); } else { - status.zero_consecutive_transient_error_commits(); - status.zero_consecutive_errors(); + status->set_num_consecutive_transient_error_commits(0); + status->set_num_consecutive_errors(0); } if (commit_count != (conflicting_commits + error_commits + transient_error_commits)) { ResetErrorCounters(status); } SyncerUtil::MarkDeletedChildrenSynced(dir, &deleted_folders); - session->set_over_quota(over_quota); + status->set_over_quota(over_quota); return; } @@ -175,8 +183,7 @@ ProcessCommitResponseCommand::ProcessSingleCommitResponse( const sync_pb::CommitResponse_EntryResponse& pb_server_entry, const syncable::Id& pre_commit_id, std::set<syncable::Id>* conflicting_new_folder_ids, - set<syncable::Id>* deleted_folders, - SyncerSession* const session) { + set<syncable::Id>* deleted_folders) { const CommitResponse_EntryResponse& server_entry = *static_cast<const CommitResponse_EntryResponse*>(&pb_server_entry); @@ -241,7 +248,7 @@ ProcessCommitResponseCommand::ProcessSingleCommitResponse( ProcessSuccessfulCommitResponse(trans, server_entry, pre_commit_id, &local_entry, syncing_was_set, - deleted_folders, session); + deleted_folders); return response; } @@ -249,8 +256,7 @@ void ProcessCommitResponseCommand::ProcessSuccessfulCommitResponse( syncable::WriteTransaction* trans, const CommitResponse_EntryResponse& server_entry, const syncable::Id& pre_commit_id, syncable::MutableEntry* local_entry, - bool syncing_was_set, set<syncable::Id>* deleted_folders, - SyncerSession* const session) { + bool syncing_was_set, set<syncable::Id>* deleted_folders) { int64 old_version = local_entry->Get(BASE_VERSION); int64 new_version = server_entry.version(); bool bad_commit_version = false; diff --git a/chrome/browser/sync/engine/process_commit_response_command.h b/chrome/browser/sync/engine/process_commit_response_command.h index 9950ba4..1eae175 100644 --- a/chrome/browser/sync/engine/process_commit_response_command.h +++ b/chrome/browser/sync/engine/process_commit_response_command.h @@ -9,7 +9,6 @@ #include "base/basictypes.h" #include "chrome/browser/sync/engine/model_changing_syncer_command.h" -#include "chrome/browser/sync/engine/syncer_session.h" #include "chrome/browser/sync/engine/syncproto.h" namespace syncable { @@ -23,37 +22,33 @@ namespace browser_sync { class ProcessCommitResponseCommand : public ModelChangingSyncerCommand { public: - explicit ProcessCommitResponseCommand(ExtensionsActivityMonitor* monitor); + ProcessCommitResponseCommand(); virtual ~ProcessCommitResponseCommand(); - virtual void ModelChangingExecuteImpl(SyncerSession* session); + // ModelChangingSyncerCommand implementation. + virtual void ModelChangingExecuteImpl(sessions::SyncSession* session); + private: CommitResponse::RESPONSE_TYPE ProcessSingleCommitResponse( syncable::WriteTransaction* trans, const sync_pb::CommitResponse_EntryResponse& pb_server_entry, const syncable::Id& pre_commit_id, std::set<syncable::Id>* conflicting_new_directory_ids, - std::set<syncable::Id>* deleted_folders, - SyncerSession* const session); + std::set<syncable::Id>* deleted_folders); // Actually does the work of execute. - void ProcessCommitResponse(SyncerSession* session); + void ProcessCommitResponse(sessions::SyncSession* session); void ProcessSuccessfulCommitResponse(syncable::WriteTransaction* trans, const CommitResponse_EntryResponse& server_entry, const syncable::Id& pre_commit_id, syncable::MutableEntry* local_entry, - bool syncing_was_set, std::set<syncable::Id>* deleted_folders, - SyncerSession* const session); + bool syncing_was_set, std::set<syncable::Id>* deleted_folders); void PerformCommitTimeNameAside( syncable::WriteTransaction* trans, const CommitResponse_EntryResponse& server_entry, syncable::MutableEntry* local_entry); - // We may need to update this with records from a commit attempt if the - // attempt failed. - ExtensionsActivityMonitor* extensions_monitor_; - DISALLOW_COPY_AND_ASSIGN(ProcessCommitResponseCommand); }; diff --git a/chrome/browser/sync/engine/process_updates_command.cc b/chrome/browser/sync/engine/process_updates_command.cc index 468a342..cb98368 100755 --- a/chrome/browser/sync/engine/process_updates_command.cc +++ b/chrome/browser/sync/engine/process_updates_command.cc @@ -9,9 +9,9 @@ #include "base/basictypes.h" #include "chrome/browser/sync/engine/syncer.h" #include "chrome/browser/sync/engine/syncer_proto_util.h" -#include "chrome/browser/sync/engine/syncer_session.h" #include "chrome/browser/sync/engine/syncer_util.h" #include "chrome/browser/sync/engine/syncproto.h" +#include "chrome/browser/sync/sessions/sync_session.h" #include "chrome/browser/sync/syncable/directory_manager.h" #include "chrome/browser/sync/syncable/syncable.h" @@ -19,27 +19,32 @@ using std::vector; namespace browser_sync { +using sessions::SyncSession; +using sessions::StatusController; + ProcessUpdatesCommand::ProcessUpdatesCommand() {} ProcessUpdatesCommand::~ProcessUpdatesCommand() {} -void ProcessUpdatesCommand::ModelChangingExecuteImpl(SyncerSession* session) { - syncable::ScopedDirLookup dir(session->dirman(), session->account_name()); +void ProcessUpdatesCommand::ModelChangingExecuteImpl(SyncSession* session) { + syncable::ScopedDirLookup dir(session->context()->directory_manager(), + session->context()->account_name()); if (!dir.good()) { LOG(ERROR) << "Scoped dir lookup failed!"; return; } - SyncerStatus status(session); - const GetUpdatesResponse updates = session->update_response().get_updates(); + const GetUpdatesResponse& updates = + session->status_controller()->updates_response().get_updates(); const int update_count = updates.entries_size(); LOG(INFO) << "Get updates from ts " << dir->last_sync_timestamp() << " returned " << update_count << " updates."; + StatusController* status = session->status_controller(); if (updates.has_changes_remaining()) { int64 changes_left = updates.changes_remaining(); LOG(INFO) << "Changes remaining:" << changes_left; - status.set_num_server_changes_remaining(changes_left); + status->set_num_server_changes_remaining(changes_left); } int64 new_timestamp = 0; @@ -49,7 +54,7 @@ void ProcessUpdatesCommand::ModelChangingExecuteImpl(SyncerSession* session) { if (0 == update_count) { if (new_timestamp > dir->last_sync_timestamp()) { dir->set_last_sync_timestamp(new_timestamp); - session->set_timestamp_dirty(); + status->set_timestamp_dirty(true); } return; } @@ -60,11 +65,12 @@ void ProcessUpdatesCommand::ModelChangingExecuteImpl(SyncerSession* session) { // be skipped and we DON'T step past them, we will sync forever. int64 latest_skip_timestamp = 0; bool any_non_skip_results = false; - vector<VerifiedUpdate>::iterator it; - for (it = session->VerifiedUpdatesBegin(); - it < session->VerifiedUpdatesEnd(); + const sessions::UpdateProgress& progress(status->update_progress()); + vector<sessions::VerifiedUpdate>::const_iterator it; + for (it = progress.VerifiedUpdatesBegin(); + it != progress.VerifiedUpdatesEnd(); ++it) { - const sync_pb::SyncEntity update = it->second; + const sync_pb::SyncEntity& update = it->second; any_non_skip_results = (it->first != VERIFY_SKIP); if (!any_non_skip_results) { @@ -98,13 +104,13 @@ void ProcessUpdatesCommand::ModelChangingExecuteImpl(SyncerSession* session) { if (new_timestamp > dir->last_sync_timestamp()) { dir->set_last_sync_timestamp(new_timestamp); - session->set_timestamp_dirty(); + status->set_timestamp_dirty(true); } - status.zero_consecutive_problem_get_updates(); - status.zero_consecutive_errors(); - status.set_current_sync_timestamp(dir->last_sync_timestamp()); - status.set_syncing(true); + status->set_num_consecutive_problem_get_updates(0); + status->set_num_consecutive_errors(0); + status->set_current_sync_timestamp(dir->last_sync_timestamp()); + status->set_syncing(true); return; } diff --git a/chrome/browser/sync/engine/process_updates_command.h b/chrome/browser/sync/engine/process_updates_command.h index a6cee34..27f3814 100644 --- a/chrome/browser/sync/engine/process_updates_command.h +++ b/chrome/browser/sync/engine/process_updates_command.h @@ -18,8 +18,6 @@ class SyncEntity; namespace browser_sync { -class SyncerSession; - // A syncer command for processing updates. // // Preconditions - updates in the SyncerSesssion have been downloaded @@ -32,7 +30,8 @@ class ProcessUpdatesCommand : public ModelChangingSyncerCommand { ProcessUpdatesCommand(); virtual ~ProcessUpdatesCommand(); - virtual void ModelChangingExecuteImpl(SyncerSession* session); + // ModelChangingSyncerCommand implementation. + virtual void ModelChangingExecuteImpl(sessions::SyncSession* session); ServerUpdateProcessingResult ProcessUpdate( const syncable::ScopedDirLookup& dir, const sync_pb::SyncEntity& pb_entry); diff --git a/chrome/browser/sync/engine/resolve_conflicts_command.cc b/chrome/browser/sync/engine/resolve_conflicts_command.cc index 6caf9b4..df74d2bd 100644 --- a/chrome/browser/sync/engine/resolve_conflicts_command.cc +++ b/chrome/browser/sync/engine/resolve_conflicts_command.cc @@ -5,7 +5,7 @@ #include "chrome/browser/sync/engine/resolve_conflicts_command.h" #include "chrome/browser/sync/engine/conflict_resolver.h" -#include "chrome/browser/sync/engine/syncer_session.h" +#include "chrome/browser/sync/sessions/sync_session.h" #include "chrome/browser/sync/syncable/directory_manager.h" namespace browser_sync { @@ -14,15 +14,18 @@ ResolveConflictsCommand::ResolveConflictsCommand() {} ResolveConflictsCommand::~ResolveConflictsCommand() {} void ResolveConflictsCommand::ModelChangingExecuteImpl( - SyncerSession* session) { - if (!session->resolver()) + sessions::SyncSession* session) { + ConflictResolver* resolver = session->context()->resolver(); + DCHECK(resolver); + if (!resolver) return; - syncable::ScopedDirLookup dir(session->dirman(), session->account_name()); + + syncable::ScopedDirLookup dir(session->context()->directory_manager(), + session->context()->account_name()); if (!dir.good()) return; - ConflictResolutionView conflict_view(session); - session->set_conflicts_resolved( - session->resolver()->ResolveConflicts(dir, &conflict_view, session)); + sessions::StatusController* status = session->status_controller(); + status->set_conflicts_resolved(resolver->ResolveConflicts(dir, status)); } } // namespace browser_sync diff --git a/chrome/browser/sync/engine/resolve_conflicts_command.h b/chrome/browser/sync/engine/resolve_conflicts_command.h index 776326b..3ac3207 100644 --- a/chrome/browser/sync/engine/resolve_conflicts_command.h +++ b/chrome/browser/sync/engine/resolve_conflicts_command.h @@ -16,14 +16,13 @@ class Id; namespace browser_sync { -class SyncerSession; - class ResolveConflictsCommand : public ModelChangingSyncerCommand { public: ResolveConflictsCommand(); virtual ~ResolveConflictsCommand(); - virtual void ModelChangingExecuteImpl(SyncerSession* session); + // ModelChangingSyncerCommand implementation. + virtual void ModelChangingExecuteImpl(sessions::SyncSession* session); private: DISALLOW_COPY_AND_ASSIGN(ResolveConflictsCommand); diff --git a/chrome/browser/sync/engine/sync_cycle_state.h b/chrome/browser/sync/engine/sync_cycle_state.h deleted file mode 100644 index db82228..0000000 --- a/chrome/browser/sync/engine/sync_cycle_state.h +++ /dev/null @@ -1,247 +0,0 @@ -// Copyright (c) 2006-2009 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 sync process consists of a sequence of sync cycles, each of which -// (hopefully) moves the client into closer synchronization with the server. -// This class holds state that is pertinent to a single sync cycle. -// -// THIS CLASS PROVIDES NO SYNCHRONIZATION GUARANTEES. - -#ifndef CHROME_BROWSER_SYNC_ENGINE_SYNC_CYCLE_STATE_H_ -#define CHROME_BROWSER_SYNC_ENGINE_SYNC_CYCLE_STATE_H_ - -#include <utility> -#include <vector> - -#include "base/basictypes.h" -#include "chrome/browser/sync/engine/syncer_types.h" -#include "chrome/browser/sync/engine/syncproto.h" -#include "chrome/browser/sync/util/event_sys.h" - -namespace syncable { -class WriteTransaction; -class Id; -} // namespace syncable - -namespace browser_sync { - -typedef std::pair<VerifyResult, sync_pb::SyncEntity> VerifiedUpdate; -typedef std::pair<UpdateAttemptResponse, syncable::Id> AppliedUpdate; - -// This is the type declaration for the eventsys channel that the syncer uses -// to send events to other system components. -struct SyncerEvent; - -// SyncCycleState holds the entire state of a single sync cycle; -// GetUpdates, Commit, and Conflict Resolution. After said cycle, the -// State may contain items that were unable to be processed because of -// errors. -class SyncCycleState { - public: - SyncCycleState() - : write_transaction_(NULL), - conflict_sets_built_(false), - conflicts_resolved_(false), - items_committed_(false), - over_quota_(false), - timestamp_dirty_(false), - dirty_(true) {} - - void set_update_response(const ClientToServerResponse& update_response) { - update_response_.CopyFrom(update_response); - } - - const ClientToServerResponse& update_response() const { - return update_response_; - } - - void set_commit_response(const ClientToServerResponse& commit_response) { - commit_response_.CopyFrom(commit_response); - } - - const ClientToServerResponse& commit_response() const { - return commit_response_; - } - - void AddVerifyResult(const VerifyResult& verify_result, - const sync_pb::SyncEntity& entity) { - verified_updates_.push_back(std::make_pair(verify_result, entity)); - } - - bool HasVerifiedUpdates() const { - return !verified_updates_.empty(); - } - - // Log a successful or failing update attempt. - void AddAppliedUpdate(const UpdateAttemptResponse& response, - const syncable::Id& id) { - applied_updates_.push_back(std::make_pair(response, id)); - } - - bool HasAppliedUpdates() const { - return !applied_updates_.empty(); - } - - std::vector<AppliedUpdate>::iterator AppliedUpdatesBegin() { - return applied_updates_.begin(); - } - - std::vector<VerifiedUpdate>::iterator VerifiedUpdatesBegin() { - return verified_updates_.begin(); - } - - std::vector<AppliedUpdate>::iterator AppliedUpdatesEnd() { - return applied_updates_.end(); - } - - std::vector<VerifiedUpdate>::iterator VerifiedUpdatesEnd() { - return verified_updates_.end(); - } - - // Returns the number of update application attempts. This includes both - // failures and successes. - int AppliedUpdatesSize() const { - return applied_updates_.size(); - } - - // Count the number of successful update applications that have happend this - // cycle. Note that if an item is successfully applied twice, it will be - // double counted here. - int SuccessfullyAppliedUpdateCount() const { - int count = 0; - for (std::vector<AppliedUpdate>::const_iterator it = - applied_updates_.begin(); - it != applied_updates_.end(); - ++it) { - if (it->first == SUCCESS) - count++; - } - return count; - } - - int VerifiedUpdatesSize() const { - return verified_updates_.size(); - } - - const std::vector<int64>& unsynced_handles() const { - return unsynced_handles_; - } - - void set_unsynced_handles(const std::vector<int64>& unsynced_handles) { - UpdateDirty(unsynced_handles != unsynced_handles_); - unsynced_handles_ = unsynced_handles; - } - - int64 unsynced_count() const { return unsynced_handles_.size(); } - - const std::vector<syncable::Id>& commit_ids() const { return commit_ids_; } - - void set_commit_ids(const std::vector<syncable::Id>& commit_ids) { - commit_ids_ = commit_ids; - } - - bool commit_ids_empty() const { return commit_ids_.empty(); } - - // The write transaction must be deleted by the caller of this function. - void set_write_transaction(syncable::WriteTransaction* write_transaction) { - DCHECK(!write_transaction_) << "Forgot to clear the write transaction."; - write_transaction_ = write_transaction; - } - - syncable::WriteTransaction* write_transaction() const { - return write_transaction_; - } - - bool has_open_write_transaction() { return write_transaction_ != NULL; } - - // Sets the write transaction to null, but doesn't free the memory. - void ClearWriteTransaction() { write_transaction_ = NULL; } - - ClientToServerMessage* commit_message() { return &commit_message_; } - - void set_commit_message(ClientToServerMessage message) { - commit_message_ = message; - } - - void set_conflict_sets_built(bool b) { - conflict_sets_built_ = b; - } - - bool conflict_sets_built() const { - return conflict_sets_built_; - } - - void set_conflicts_resolved(bool b) { - conflicts_resolved_ = b; - } - - bool conflicts_resolved() const { - return conflicts_resolved_; - } - - void set_over_quota(bool b) { - UpdateDirty(b != over_quota_); - over_quota_ = b; - } - - bool over_quota() const { - return over_quota_; - } - - void set_item_committed() { items_committed_ |= true; } - - bool items_committed() const { return items_committed_; } - - // Returns true if this object has been modified since last SetClean() call. - bool IsDirty() const { return dirty_; } - - // Call to tell this status object that its new state has been seen. - void SetClean() { dirty_ = false; } - - // Indicate that we've made a change to directory timestamp. - void set_timestamp_dirty() { - timestamp_dirty_ = true; - } - - bool is_timestamp_dirty() const { - return timestamp_dirty_; - } - - private: - void UpdateDirty(bool new_info) { dirty_ |= new_info; } - - // Download updates supplies: - ClientToServerResponse update_response_; - ClientToServerResponse commit_response_; - ClientToServerMessage commit_message_; - - syncable::WriteTransaction* write_transaction_; - std::vector<int64> unsynced_handles_; - std::vector<syncable::Id> commit_ids_; - - // At a certain point during the sync process we'll want to build the - // conflict sets. This variable tracks whether or not that has happened. - bool conflict_sets_built_; - bool conflicts_resolved_; - bool items_committed_; - bool over_quota_; - - // If we've set the timestamp to a new value during this cycle. - bool timestamp_dirty_; - - bool dirty_; - - // Some container for updates that failed verification. - std::vector<VerifiedUpdate> verified_updates_; - - // Stores the result of the various ApplyUpdate attempts we've made. - // May contain duplicate entries. - std::vector<AppliedUpdate> applied_updates_; - - DISALLOW_COPY_AND_ASSIGN(SyncCycleState); -}; - -} // namespace browser_sync - -#endif // CHROME_BROWSER_SYNC_ENGINE_SYNC_CYCLE_STATE_H_ diff --git a/chrome/browser/sync/engine/sync_process_state.cc b/chrome/browser/sync/engine/sync_process_state.cc deleted file mode 100644 index b842f47..0000000 --- a/chrome/browser/sync/engine/sync_process_state.cc +++ /dev/null @@ -1,287 +0,0 @@ -// Copyright (c) 2006-2009 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. - -// -// THIS CLASS PROVIDES NO SYNCHRONIZATION GUARANTEES. - -#include "chrome/browser/sync/engine/sync_process_state.h" - -#include <map> -#include <set> -#include <vector> - -#include "base/basictypes.h" -#include "chrome/browser/sync/syncable/directory_manager.h" -#include "chrome/browser/sync/syncable/syncable.h" - -using std::map; -using std::set; -using std::vector; - -namespace browser_sync { - -SyncProcessState::SyncProcessState(const SyncProcessState& counts) - : connection_manager_(counts.connection_manager_), - account_name_(counts.account_name_), - dirman_(counts.dirman_), - resolver_(counts.resolver_), - model_safe_worker_(counts.model_safe_worker_), - syncer_event_channel_(counts.syncer_event_channel_) { - *this = counts; -} - -SyncProcessState::SyncProcessState(syncable::DirectoryManager* dirman, - std::string account_name, - ServerConnectionManager* connection_manager, - ConflictResolver* const resolver, - SyncerEventChannel* syncer_event_channel, - ModelSafeWorker* model_safe_worker) - : connection_manager_(connection_manager), - account_name_(account_name), - dirman_(dirman), - resolver_(resolver), - model_safe_worker_(model_safe_worker), - syncer_event_channel_(syncer_event_channel), - current_sync_timestamp_(0), - num_server_changes_remaining_(0), - syncing_(false), - invalid_store_(false), - syncer_stuck_(false), - error_commits_(0), - conflicting_commits_(0), - consecutive_problem_get_updates_(0), - consecutive_problem_commits_(0), - consecutive_transient_error_commits_(0), - consecutive_errors_(0), - successful_commits_(0), - dirty_(false), - auth_dirty_(false), - auth_failed_(false) { - syncable::ScopedDirLookup dir(dirman_, account_name_); - - // The directory must be good here. - LOG_IF(ERROR, !dir.good()); - syncing_ = !dir->initial_sync_ended(); - - // If we have never synced then we are invalid until made otherwise. - set_invalid_store((dir->last_sync_timestamp() <= 0)); -} - -SyncProcessState& SyncProcessState::operator=(const SyncProcessState& counts) { - if (this == &counts) { - return *this; - } - CleanupSets(); - silenced_until_ = counts.silenced_until_; - current_sync_timestamp_ = counts.current_sync_timestamp_; - num_server_changes_remaining_ = counts.num_server_changes_remaining_; - error_commits_ = counts.error_commits_; - conflicting_commits_ = counts.conflicting_commits_; - consecutive_problem_get_updates_ = - counts.consecutive_problem_get_updates_; - consecutive_problem_commits_ = - counts.consecutive_problem_commits_; - consecutive_transient_error_commits_ = - counts.consecutive_transient_error_commits_; - consecutive_errors_ = counts.consecutive_errors_; - conflicting_item_ids_ = counts.conflicting_item_ids_; - successful_commits_ = counts.successful_commits_; - syncer_stuck_ = counts.syncer_stuck_; - - // TODO(chron): Is it safe to set these? - // - // Pointers: - // - // connection_manager_ - // account_name_ - // dirman_ - // model_safe_worker_ - // syncer_event_channel_ - // - // Status members: - // syncing_ - // invalid_store_ - // syncer_stuck_ - // got_zero_updates_ - // dirty_ - // auth_dirty_ - // auth_failed_ - - for (set<ConflictSet*>::const_iterator it = - counts.ConflictSetsBegin(); - counts.ConflictSetsEnd() != it; ++it) { - const ConflictSet* old_set = *it; - ConflictSet* const new_set = new ConflictSet(*old_set); - conflict_sets_.insert(new_set); - - for (ConflictSet::const_iterator setit = new_set->begin(); - new_set->end() != setit; ++setit) { - id_to_conflict_set_[*setit] = new_set; - } - } - return *this; -} - -void SyncProcessState::set_silenced_until(const base::TimeTicks& val) { - UpdateDirty(val != silenced_until_); - silenced_until_ = val; -} - -// Status maintenance functions. -void SyncProcessState::set_invalid_store(const bool val) { - UpdateDirty(val != invalid_store_); - invalid_store_ = val; -} - -void SyncProcessState::set_syncer_stuck(const bool val) { - UpdateDirty(val != syncer_stuck_); - syncer_stuck_ = val; -} - -void SyncProcessState::set_syncing(const bool val) { - UpdateDirty(val != syncing_); - syncing_ = val; -} - -// Returns true if got zero updates has been set on the directory. -bool SyncProcessState::IsShareUsable() const { - syncable::ScopedDirLookup dir(dirman(), account_name()); - if (!dir.good()) { - LOG(ERROR) << "Scoped dir lookup failed!"; - return false; - } - return dir->initial_sync_ended(); -} - -void SyncProcessState::set_current_sync_timestamp(const int64 val) { - UpdateDirty(val != current_sync_timestamp_); - current_sync_timestamp_ = val; -} - -void SyncProcessState::set_num_server_changes_remaining(const int64 val) { - UpdateDirty(val != num_server_changes_remaining_); - num_server_changes_remaining_ = val; -} - -void SyncProcessState::set_conflicting_commits(const int val) { - UpdateDirty(val != conflicting_commits_); - conflicting_commits_ = val; -} - -// WEIRD COUNTER functions. -void SyncProcessState::increment_consecutive_problem_get_updates() { - UpdateDirty(true); - ++consecutive_problem_get_updates_; -} - -void SyncProcessState::zero_consecutive_problem_get_updates() { - UpdateDirty(0 != consecutive_problem_get_updates_); - consecutive_problem_get_updates_ = 0; -} - -void SyncProcessState::increment_consecutive_problem_commits() { - UpdateDirty(true); - ++consecutive_problem_commits_; -} - -void SyncProcessState::zero_consecutive_problem_commits() { - UpdateDirty(0 != consecutive_problem_commits_); - consecutive_problem_commits_ = 0; -} - -void SyncProcessState::increment_consecutive_transient_error_commits_by( - int value) { - UpdateDirty(0 != value); - consecutive_transient_error_commits_ += value; -} - -void SyncProcessState::zero_consecutive_transient_error_commits() { - UpdateDirty(0 != consecutive_transient_error_commits_); - consecutive_transient_error_commits_ = 0; -} - -void SyncProcessState::increment_consecutive_errors_by(int value) { - UpdateDirty(0 != value); - consecutive_errors_ += value; -} - -void SyncProcessState::zero_consecutive_errors() { - UpdateDirty(0 != consecutive_errors_); - consecutive_errors_ = 0; -} - -void SyncProcessState::increment_successful_commits() { - UpdateDirty(true); - ++successful_commits_; -} - -void SyncProcessState::zero_successful_commits() { - UpdateDirty(0 != successful_commits_); - successful_commits_ = 0; -} - -void SyncProcessState::MergeSets(const syncable::Id& id1, - const syncable::Id& id2) { - // There are no single item sets, we just leave those entries == 0 - vector<syncable::Id>* set1 = id_to_conflict_set_[id1]; - vector<syncable::Id>* set2 = id_to_conflict_set_[id2]; - vector<syncable::Id>* rv = 0; - if (0 == set1 && 0 == set2) { - // Neither item currently has a set so we build one. - rv = new vector<syncable::Id>(); - rv->push_back(id1); - if (id1 != id2) { - rv->push_back(id2); - } else { - LOG(WARNING) << "[BUG] Attempting to merge two identical conflict ids."; - } - conflict_sets_.insert(rv); - } else if (0 == set1) { - // Add the item to the existing set. - rv = set2; - rv->push_back(id1); - } else if (0 == set2) { - // Add the item to the existing set. - rv = set1; - rv->push_back(id2); - } else if (set1 == set2) { - // It's the same set already. - return; - } else { - // Merge the two sets. - rv = set1; - // Point all the second sets id's back to the first. - vector<syncable::Id>::iterator i; - for (i = set2->begin() ; i != set2->end() ; ++i) { - id_to_conflict_set_[*i] = rv; - } - // Copy the second set to the first. - rv->insert(rv->end(), set2->begin(), set2->end()); - conflict_sets_.erase(set2); - delete set2; - } - id_to_conflict_set_[id1] = id_to_conflict_set_[id2] = rv; -} - -void SyncProcessState::CleanupSets() { - // Clean up all the sets. - set<ConflictSet*>::iterator i; - for (i = conflict_sets_.begin(); i != conflict_sets_.end(); i++) { - delete *i; - } - conflict_sets_.clear(); - id_to_conflict_set_.clear(); -} - -SyncProcessState::~SyncProcessState() { - CleanupSets(); -} - -void SyncProcessState::AuthFailed() { - // Dirty if the last one DIDN'T fail. - UpdateAuthDirty(true != auth_failed_); - auth_failed_ = true; -} - -} // namespace browser_sync diff --git a/chrome/browser/sync/engine/sync_process_state.h b/chrome/browser/sync/engine/sync_process_state.h deleted file mode 100644 index cefd22f..0000000 --- a/chrome/browser/sync/engine/sync_process_state.h +++ /dev/null @@ -1,317 +0,0 @@ -// Copyright (c) 2006-2009 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 sync process consists of a sequence of sync cycles, each of which -// (hopefully) moves the client into closer synchronization with the server. -// While SyncCycleState holds state that is pertinent to a single sync cycle, -// this data structure holds state that must be passed from cycle to cycle. -// -// THIS CLASS PROVIDES NO SYNCHRONIZATION GUARANTEES. - -#ifndef CHROME_BROWSER_SYNC_ENGINE_SYNC_PROCESS_STATE_H_ -#define CHROME_BROWSER_SYNC_ENGINE_SYNC_PROCESS_STATE_H_ - -#include <map> -#include <set> -#include <string> -#include <utility> // for pair<> - -#include "base/atomicops.h" -#include "base/basictypes.h" -#include "base/port.h" -#include "base/time.h" -#include "chrome/browser/sync/engine/net/server_connection_manager.h" -#include "chrome/browser/sync/engine/syncer_types.h" -#include "chrome/browser/sync/syncable/syncable_id.h" -#include "testing/gtest/include/gtest/gtest_prod.h" // For FRIEND_TEST - -namespace browser_sync { - -class ConflictResolver; -class ModelSafeWorker; - -class SyncProcessState { - FRIEND_TEST(SyncerSyncProcessState, MergeSetsTest); - FRIEND_TEST(SyncerTest, CopySyncProcessState); - public: - ~SyncProcessState(); - SyncProcessState( - syncable::DirectoryManager* dirman, - std::string account_name, - ServerConnectionManager* connection_manager, - ConflictResolver* const resolver, - SyncerEventChannel* syncer_event_channel, - ModelSafeWorker* model_safe_worker); - - // Intentionally not 'explicit' b/c it's a copy ctor: - SyncProcessState(const SyncProcessState& counts); - SyncProcessState& operator=(const SyncProcessState& that); - - std::string account_name() const { return account_name_; } - - syncable::DirectoryManager* dirman() const { return dirman_; } - - ServerConnectionManager* connection_manager() const { - return connection_manager_; - } - - ConflictResolver* resolver() const { return resolver_; } - - ModelSafeWorker* model_safe_worker() { return model_safe_worker_; } - - SyncerEventChannel* syncer_event_channel() const { - return syncer_event_channel_; - } - - // Functions that deal with conflict set stuff. - IdToConflictSetMap::const_iterator IdToConflictSetFind( - const syncable::Id& the_id) const { - return id_to_conflict_set_.find(the_id); - } - - IdToConflictSetMap::const_iterator IdToConflictSetBegin() const { - return id_to_conflict_set_.begin(); - } - - IdToConflictSetMap::const_iterator IdToConflictSetEnd() const { - return id_to_conflict_set_.end(); - } - - IdToConflictSetMap::size_type IdToConflictSetSize() const { - return id_to_conflict_set_.size(); - } - - const ConflictSet* IdToConflictSetGet(const syncable::Id& the_id) { - return id_to_conflict_set_[the_id]; - } - - std::set<ConflictSet*>::const_iterator ConflictSetsBegin() const { - return conflict_sets_.begin(); - } - - std::set<ConflictSet*>::const_iterator ConflictSetsEnd() const { - return conflict_sets_.end(); - } - - std::set<ConflictSet*>::size_type ConflictSetsSize() const { - return conflict_sets_.size(); - } - - void MergeSets(const syncable::Id& set1, const syncable::Id& set2); - - void CleanupSets(); - // END conflict set functions - - // Item id set manipulation functions. - bool HasConflictingItems() const { - return !conflicting_item_ids_.empty(); - } - - int ConflictingItemsSize() const { - return conflicting_item_ids_.size(); - } - - void AddConflictingItem(const syncable::Id& the_id) { - std::pair<std::set<syncable::Id>::iterator, bool> ret = - conflicting_item_ids_.insert(the_id); - UpdateDirty(ret.second); - } - - void EraseConflictingItem(std::set<syncable::Id>::iterator it) { - UpdateDirty(true); - conflicting_item_ids_.erase(it); - } - - void EraseConflictingItem(const syncable::Id& the_id) { - int items_erased = conflicting_item_ids_.erase(the_id); - UpdateDirty(0 != items_erased); - } - - std::set<syncable::Id>::iterator ConflictingItemsBegin() { - return conflicting_item_ids_.begin(); - } - - std::set<syncable::Id>::iterator ConflictingItemsEnd() { - return conflicting_item_ids_.end(); - } - - // END item id set manipulation functions - - // Assorted other state info. - // DEPRECATED: USE ConflictingItemsSize. - int conflicting_updates() const { return conflicting_item_ids_.size(); } - - base::TimeTicks silenced_until() const { return silenced_until_; } - void set_silenced_until(const base::TimeTicks& val); - - // Info that is tracked purely for status reporting. - - // During inital sync these two members can be used to measure sync progress. - int64 current_sync_timestamp() const { return current_sync_timestamp_; } - - int64 num_server_changes_remaining() const { return num_server_changes_remaining_; } - - void set_current_sync_timestamp(const int64 val); - - void set_num_server_changes_remaining(const int64 val); - - bool invalid_store() const { return invalid_store_; } - - void set_invalid_store(const bool val); - - bool syncer_stuck() const { return syncer_stuck_; } - - void set_syncer_stuck(const bool val); - - bool syncing() const { return syncing_; } - - void set_syncing(const bool val); - - bool IsShareUsable() const; - - int conflicting_commits() const { return conflicting_commits_; } - - void set_conflicting_commits(const int val); - - // WEIRD COUNTER manipulation functions. - int consecutive_problem_get_updates() const { - return consecutive_problem_get_updates_; - } - - void increment_consecutive_problem_get_updates(); - - void zero_consecutive_problem_get_updates(); - - int consecutive_problem_commits() const { - return consecutive_problem_commits_; - } - - void increment_consecutive_problem_commits(); - - void zero_consecutive_problem_commits(); - - int consecutive_transient_error_commits() const { - return consecutive_transient_error_commits_; - } - - void increment_consecutive_transient_error_commits_by(int value); - - void zero_consecutive_transient_error_commits(); - - int consecutive_errors() const { return consecutive_errors_; } - - void increment_consecutive_errors_by(int value); - - void zero_consecutive_errors(); - - int successful_commits() const { return successful_commits_; } - - void increment_successful_commits(); - - void zero_successful_commits(); - // end WEIRD COUNTER manipulation functions. - - // Methods for tracking authentication state. - void AuthFailed(); - - // Returns true if this object has been modified since last SetClean() call. - bool IsDirty() const { return dirty_; } - - // Call to tell this status object that its new state has been seen. - void SetClean() { dirty_ = false; } - - // Returns true if auth status has been modified since last SetClean() call. - bool IsAuthDirty() const { return auth_dirty_; } - - // Call to tell this status object that its auth state has been seen. - void SetAuthClean() { auth_dirty_ = false; } - - private: - // For testing. - SyncProcessState() - : connection_manager_(NULL), - account_name_(PSTR("")), - dirman_(NULL), - resolver_(NULL), - model_safe_worker_(NULL), - syncer_event_channel_(NULL), - current_sync_timestamp_(0), - num_server_changes_remaining_(0), - syncing_(false), - invalid_store_(false), - syncer_stuck_(false), - conflicting_commits_(0), - consecutive_problem_get_updates_(0), - consecutive_problem_commits_(0), - consecutive_transient_error_commits_(0), - consecutive_errors_(0), - successful_commits_(0), - dirty_(false), - auth_dirty_(false), - auth_failed_(false) {} - - ServerConnectionManager* connection_manager_; - const std::string account_name_; - syncable::DirectoryManager* const dirman_; - ConflictResolver* const resolver_; - ModelSafeWorker* const model_safe_worker_; - - // For sending notifications from sync commands out to observers of the - // Syncer. - SyncerEventChannel* syncer_event_channel_; - - // TODO(sync): move away from sets if it makes more sense. - std::set<syncable::Id> conflicting_item_ids_; - std::map<syncable::Id, ConflictSet*> id_to_conflict_set_; - std::set<ConflictSet*> conflict_sets_; - - // When we're over bandwidth quota, we don't update until past this time. - base::TimeTicks silenced_until_; - - // Status information, as opposed to state info that may also be exposed for - // status reporting purposes. - int64 current_sync_timestamp_; // During inital sync these two members - int64 num_server_changes_remaining_; // Can be used to measure sync progress. - - // There remains sync state updating in: - // CommitUnsyncedEntries - bool syncing_; - - // True when we get such an INVALID_STORE error from the server. - bool invalid_store_; - // True iff we're stuck. User should contact support. - bool syncer_stuck_; - // counts of various commit return values. - int error_commits_; - int conflicting_commits_; - - // WEIRD COUNTERS - // Two variables that track the # on consecutive problem requests. - // consecutive_problem_get_updates_ resets when we get any updates (not on - // pings) and increments whenever the request fails. - int consecutive_problem_get_updates_; - // consecutive_problem_commits_ resets whenever we commit any number of items - // and increments whenever all commits fail for any reason. - int consecutive_problem_commits_; - // number of commits hitting transient errors since the last successful - // commit. - int consecutive_transient_error_commits_; - // Incremented when get_updates fails, commit fails, and when hitting - // transient errors. When any of these succeed, this counter is reset. - // TODO(chron): Reduce number of weird counters we use. - int consecutive_errors_; - int successful_commits_; - - bool dirty_; - bool auth_dirty_; - bool auth_failed_; - - void UpdateDirty(bool new_info) { dirty_ |= new_info; } - - void UpdateAuthDirty(bool new_info) { auth_dirty_ |= new_info; } -}; - -} // namespace browser_sync - -#endif // CHROME_BROWSER_SYNC_ENGINE_SYNC_PROCESS_STATE_H_ diff --git a/chrome/browser/sync/engine/syncapi.cc b/chrome/browser/sync/engine/syncapi.cc index ea2be76..1cfd8d2 100755 --- a/chrome/browser/sync/engine/syncapi.cc +++ b/chrome/browser/sync/engine/syncapi.cc @@ -26,7 +26,6 @@ #include "chrome/browser/sync/engine/all_status.h" #include "chrome/browser/sync/engine/auth_watcher.h" #include "chrome/browser/sync/engine/change_reorder_buffer.h" -#include "chrome/browser/sync/engine/client_command_channel.h" #include "chrome/browser/sync/engine/model_safe_worker.h" #include "chrome/browser/sync/engine/net/gaia_authenticator.h" #include "chrome/browser/sync/engine/net/server_connection_manager.h" @@ -36,6 +35,7 @@ #include "chrome/browser/sync/notifier/listener/talk_mediator.h" #include "chrome/browser/sync/notifier/listener/talk_mediator_impl.h" #include "chrome/browser/sync/protocol/service_constants.h" +#include "chrome/browser/sync/sessions/sync_session_context.h" #include "chrome/browser/sync/syncable/directory_manager.h" #include "chrome/browser/sync/syncable/syncable.h" #include "chrome/browser/sync/util/character_set_converters.h" @@ -53,15 +53,13 @@ using browser_sync::AllStatus; using browser_sync::AllStatusEvent; using browser_sync::AuthWatcher; using browser_sync::AuthWatcherEvent; -using browser_sync::ClientCommandChannel; using browser_sync::Syncer; using browser_sync::SyncerEvent; -using browser_sync::SyncerStatus; using browser_sync::SyncerThread; -using browser_sync::SyncerThreadFactory; using browser_sync::UserSettings; using browser_sync::TalkMediator; using browser_sync::TalkMediatorImpl; +using browser_sync::sessions::SyncSessionContext; using std::list; using std::hex; using std::string; @@ -648,7 +646,6 @@ class SyncManager::SyncInternal { public: explicit SyncInternal(SyncManager* sync_manager) : observer_(NULL), - command_channel_(0), auth_problem_(AuthError::NONE), sync_manager_(sync_manager), address_watch_thread_("SyncEngine_AddressWatcher"), @@ -819,9 +816,6 @@ class SyncManager::SyncInternal { // WARNING: This can be NULL! Observer* observer_; - // A sink for client commands from the syncer needed to create a SyncerThread. - ClientCommandChannel command_channel_; - // The ServerConnectionManager used to abstract communication between the // client (the Syncer) and the sync server. scoped_ptr<SyncAPIServerConnectionManager> connection_manager_; @@ -997,16 +991,15 @@ bool SyncManager::SyncInternal::Init( authwatcher_hookup_.reset(NewEventListenerHookup(auth_watcher_->channel(), this, &SyncInternal::HandleAuthWatcherEvent)); - // Tell the SyncerThread to use the ModelSafeWorker for bookmark model work. + // Build a SyncSessionContext and store the worker in it. // We set up both sides of the "bridge" here, with the ModelSafeWorkerBridge // on the Syncer side, and |model_safe_worker| on the API client side. ModelSafeWorkerBridge* worker = new ModelSafeWorkerBridge(model_safe_worker); + SyncSessionContext* context = new SyncSessionContext( + connection_manager_.get(), dir_manager(), worker); - syncer_thread_ = SyncerThreadFactory::Create(&command_channel_, - dir_manager(), - connection_manager(), - &allstatus_, - worker); + // The SyncerThread takes ownership of |context|. + syncer_thread_ = new SyncerThread(context, &allstatus_); syncer_thread()->WatchTalkMediator(talk_mediator()); allstatus()->WatchSyncerThread(syncer_thread()); @@ -1315,7 +1308,7 @@ void SyncManager::SyncInternal::HandleSyncerEvent(const SyncerEvent& event) { // (because we attach HandleSyncerEvent only once we receive notification // of successful authentication [locally or otherwise]), but B) the initial // sync had not completed at that time. - if (SyncerStatus(event.last_session).IsShareUsable()) + if (event.snapshot->is_share_usable) MarkAndNotifyInitializationComplete(); return; } @@ -1331,13 +1324,13 @@ void SyncManager::SyncInternal::HandleSyncerEvent(const SyncerEvent& event) { // Notifications are sent at the end of every sync cycle, regardless of // whether we should sync again. if (event.what_happened == SyncerEvent::SYNC_CYCLE_ENDED) { - if (!event.last_session->HasMoreToSync()) { + if (!event.snapshot->has_more_to_sync) { observer_->OnSyncCycleCompleted(); } - // TODO(chron): Consider changing this back to track HasMoreToSync - // Only notify peers if a successful commit has occurred. - if (event.last_session && event.last_session->HadSuccessfulCommits()) { + // TODO(chron): Consider changing this back to track has_more_to_sync + // only notify peers if a successful commit has occurred. + if (event.snapshot->syncer_status.num_successful_commits > 0) { // We use a member variable here because talk may not have connected yet. // The notification must be stored until it can be sent. notification_pending_ = true; @@ -1357,9 +1350,8 @@ void SyncManager::SyncInternal::HandleSyncerEvent(const SyncerEvent& event) { } } else { LOG(INFO) << "Didn't send XMPP notification!" - << " event.last_session: " << event.last_session - << " event.last_session->items_committed(): " - << event.last_session->items_committed() + << " event.snapshot.did_commit_items: " + << event.snapshot->did_commit_items << " talk_mediator(): " << talk_mediator(); } } @@ -1410,8 +1402,9 @@ void SyncManager::SyncInternal::HandleAuthWatcherEvent( { // Start watching the syncer channel directly here. DCHECK(syncer_thread() != NULL); - syncer_event_.reset(NewEventListenerHookup(syncer_thread()->channel(), - this, &SyncInternal::HandleSyncerEvent)); + syncer_event_.reset( + NewEventListenerHookup(syncer_thread()->relay_channel(), this, + &SyncInternal::HandleSyncerEvent)); } return; // Authentication failures translate to GoogleServiceAuthError events. diff --git a/chrome/browser/sync/engine/syncer.cc b/chrome/browser/sync/engine/syncer.cc index c97e709..2f082e3 100755 --- a/chrome/browser/sync/engine/syncer.cc +++ b/chrome/browser/sync/engine/syncer.cc @@ -6,6 +6,7 @@ #include "base/format_macros.h" #include "base/message_loop.h" +#include "base/time.h" #include "chrome/browser/chrome_thread.h" #include "chrome/browser/sync/engine/apply_updates_command.h" #include "chrome/browser/sync/engine/build_and_process_conflict_sets_command.h" @@ -28,6 +29,7 @@ #include "chrome/browser/sync/syncable/syncable.h" #include "chrome/browser/sync/util/closure.h" +using base::TimeDelta; using sync_pb::ClientCommand; using syncable::Blob; using syncable::IS_UNAPPLIED_UPDATE; @@ -48,60 +50,41 @@ using syncable::WriteTransaction; namespace browser_sync { -Syncer::Syncer( - syncable::DirectoryManager* dirman, - const PathString& account_name, - ServerConnectionManager* connection_manager, - ModelSafeWorker* model_safe_worker) - : account_name_(account_name), - early_exit_requested_(false), +using sessions::StatusController; +using sessions::SyncSession; +using sessions::ConflictProgress; + +Syncer::Syncer(sessions::SyncSessionContext* context) + : early_exit_requested_(false), max_commit_batch_size_(kDefaultMaxCommitBatchSize), - connection_manager_(connection_manager), - dirman_(dirman), - command_channel_(NULL), - model_safe_worker_(model_safe_worker), + syncer_event_channel_(new SyncerEventChannel(SyncerEvent( + SyncerEvent::SHUTDOWN_USE_WITH_CARE))), + resolver_scoper_(context, &resolver_), + event_channel_scoper_(context, syncer_event_channel_.get()), + context_(context), updates_source_(sync_pb::GetUpdatesCallerInfo::UNKNOWN), - notifications_enabled_(false), pre_conflict_resolution_closure_(NULL) { - SyncerEvent shutdown = { SyncerEvent::SHUTDOWN_USE_WITH_CARE }; - syncer_event_channel_.reset(new SyncerEventChannel(shutdown)); shutdown_channel_.reset(new ShutdownChannel(this)); - extensions_monitor_ = new ExtensionsActivityMonitor(); - - ScopedDirLookup dir(dirman_, account_name_); + ScopedDirLookup dir(context->directory_manager(), context->account_name()); // The directory must be good here. CHECK(dir.good()); } -Syncer::~Syncer() { - if (!ChromeThread::DeleteSoon(ChromeThread::UI, FROM_HERE, - extensions_monitor_)) { - // In unittests, there may be no UI thread, so the above will fail. - delete extensions_monitor_; - } - extensions_monitor_ = NULL; -} - void Syncer::RequestNudge(int milliseconds) { - SyncerEvent event; - event.what_happened = SyncerEvent::REQUEST_SYNC_NUDGE; + SyncerEvent event(SyncerEvent::REQUEST_SYNC_NUDGE); event.nudge_delay_milliseconds = milliseconds; - channel()->NotifyListeners(event); + syncer_event_channel_->NotifyListeners(event); } -bool Syncer::SyncShare() { - SyncProcessState state(dirman_, account_name_, connection_manager_, - &resolver_, syncer_event_channel_.get(), - model_safe_worker()); - return SyncShare(&state); +bool Syncer::SyncShare(sessions::SyncSession::Delegate* delegate) { + sessions::SyncSession session(context_, delegate); + return SyncShare(&session); } -bool Syncer::SyncShare(SyncProcessState* process_state) { - SyncCycleState cycle_state; - SyncerSession session(&cycle_state, process_state); - session.set_source(TestAndSetUpdatesSource()); - session.set_notifications_enabled(notifications_enabled()); +bool Syncer::SyncShare(sessions::SyncSession* session) { + session->status_controller()->ResetTransientState(); + session->set_source(TestAndSetUpdatesSource()); // This isn't perfect, as we can end up bundling extensions activity // intended for the next session into the current one. We could do a // test-and-reset as with the source, but note that also falls short if @@ -111,34 +94,24 @@ bool Syncer::SyncShare(SyncProcessState* process_state) { // the records set here on the original attempt. This should provide us // with the right data "most of the time", and we're only using this for // analysis purposes, so Law of Large Numbers FTW. - extensions_monitor_->GetAndClearRecords( - session.mutable_extensions_activity()); - SyncShare(&session, SYNCER_BEGIN, SYNCER_END); - return session.HasMoreToSync(); + context_->extensions_monitor()->GetAndClearRecords( + session->mutable_extensions_activity()); + SyncShare(session, SYNCER_BEGIN, SYNCER_END); + return session->HasMoreToSync(); } -bool Syncer::SyncShare(SyncerStep first_step, SyncerStep last_step) { - SyncCycleState cycle_state; - SyncProcessState state(dirman_, account_name_, connection_manager_, - &resolver_, syncer_event_channel_.get(), - model_safe_worker()); - SyncerSession session(&cycle_state, &state); +bool Syncer::SyncShare(SyncerStep first_step, SyncerStep last_step, + sessions::SyncSession::Delegate* delegate) { + sessions::SyncSession session(context_, delegate); SyncShare(&session, first_step, last_step); return session.HasMoreToSync(); } -void Syncer::SyncShare(SyncerSession* session) { - SyncShare(session, SYNCER_BEGIN, SYNCER_END); -} - -void Syncer::SyncShare(SyncerSession* session, +void Syncer::SyncShare(sessions::SyncSession* session, const SyncerStep first_step, const SyncerStep last_step) { SyncerStep current_step = first_step; - // Reset silenced_until_, it is the callers responsibility to honor throttles. - silenced_until_ = session->silenced_until(); - SyncerStep next_step = current_step; while (!ExitRequested()) { switch (current_step) { @@ -172,7 +145,7 @@ void Syncer::SyncShare(SyncerSession* session, process_updates.Execute(session); // We should download all of the updates before attempting to process // them. - if (session->CountUpdates() == 0) { + if (session->status_controller()->CountUpdates() == 0) { next_step = APPLY_UPDATES; } else { next_step = DOWNLOAD_UPDATES; @@ -189,23 +162,23 @@ void Syncer::SyncShare(SyncerSession* session, // These two steps are combined since they are executed within the same // write transaction. case BUILD_COMMIT_REQUEST: { - SyncerStatus status(session); - status.set_syncing(true); + session->status_controller()->set_syncing(true); LOG(INFO) << "Processing Commit Request"; - ScopedDirLookup dir(session->dirman(), session->account_name()); + ScopedDirLookup dir(context_->directory_manager(), + context_->account_name()); if (!dir.good()) { LOG(ERROR) << "Scoped dir lookup failed!"; return; } WriteTransaction trans(dir, SYNCER, __FILE__, __LINE__); - SyncerSession::ScopedSetWriteTransaction set_trans(session, &trans); + sessions::ScopedSetSessionWriteTransaction set_trans(session, &trans); LOG(INFO) << "Getting the Commit IDs"; GetCommitIdsCommand get_commit_ids_command(max_commit_batch_size_); get_commit_ids_command.Execute(session); - if (!session->commit_ids().empty()) { + if (!session->status_controller()->commit_ids().empty()) { LOG(INFO) << "Building a commit message"; BuildCommitCommand build_commit_command; build_commit_command.Execute(session); @@ -226,8 +199,7 @@ void Syncer::SyncShare(SyncerSession* session, } case PROCESS_COMMIT_RESPONSE: { LOG(INFO) << "Processing the commit response"; - ProcessCommitResponseCommand process_response_command( - extensions_monitor_); + ProcessCommitResponseCommand process_response_command; process_response_command.Execute(session); next_step = BUILD_AND_PROCESS_CONFLICT_SETS; break; @@ -236,7 +208,7 @@ void Syncer::SyncShare(SyncerSession* session, LOG(INFO) << "Building and Processing Conflict Sets"; BuildAndProcessConflictSetsCommand build_process_conflict_sets; build_process_conflict_sets.Execute(session); - if (session->conflict_sets_built()) + if (session->status_controller()->conflict_sets_built()) next_step = SYNCER_END; else next_step = RESOLVE_CONFLICTS; @@ -253,22 +225,24 @@ void Syncer::SyncShare(SyncerSession* session, ResolveConflictsCommand resolve_conflicts_command; resolve_conflicts_command.Execute(session); - if (session->HasConflictingUpdates()) + StatusController* status = session->status_controller(); + if (status->update_progress().HasConflictingUpdates()) next_step = APPLY_UPDATES_TO_RESOLVE_CONFLICTS; else next_step = SYNCER_END; break; } case APPLY_UPDATES_TO_RESOLVE_CONFLICTS: { + StatusController* status = session->status_controller(); + const ConflictProgress* progress = status->conflict_progress(); LOG(INFO) << "Applying updates to resolve conflicts"; ApplyUpdatesCommand apply_updates; - int num_conflicting_updates = session->conflicting_update_count(); + int num_conflicting_updates = progress->ConflictingItemsSize(); apply_updates.Execute(session); - int post_facto_conflicting_updates = - session->conflicting_update_count(); - session->set_conflicts_resolved(session->conflicts_resolved() || + int post_facto_conflicting_updates = progress->ConflictingItemsSize(); + status->set_conflicts_resolved(status->conflicts_resolved() || num_conflicting_updates > post_facto_conflicting_updates); - if (session->conflicts_resolved()) + if (status->conflicts_resolved()) next_step = RESOLVE_CONFLICTS; else next_step = SYNCER_END; @@ -289,21 +263,27 @@ void Syncer::SyncShare(SyncerSession* session, current_step = next_step; } post_while: - // Copy any lingering useful state out of the session. - silenced_until_ = session->silenced_until(); return; } -void Syncer::ProcessClientCommand(SyncerSession* session) { - if (!session->update_response().has_client_command()) +void Syncer::ProcessClientCommand(sessions::SyncSession* session) { + const ClientToServerResponse& response = + session->status_controller()->updates_response(); + if (!response.has_client_command()) return; - const ClientCommand command = session->update_response().client_command(); - if (command_channel_) - command_channel_->NotifyListeners(&command); + const ClientCommand& command = response.client_command(); // The server limits the number of items a client can commit in one batch. if (command.has_max_commit_batch_size()) max_commit_batch_size_ = command.max_commit_batch_size(); + if (command.has_set_sync_long_poll_interval()) { + session->delegate()->OnReceivedLongPollIntervalUpdate( + TimeDelta::FromSeconds(command.set_sync_long_poll_interval())); + } + if (command.has_set_sync_poll_interval()) { + session->delegate()->OnReceivedShortPollIntervalUpdate( + TimeDelta::FromSeconds(command.set_sync_poll_interval())); + } } void CopyServerFields(syncable::Entry* src, syncable::MutableEntry* dest) { diff --git a/chrome/browser/sync/engine/syncer.h b/chrome/browser/sync/engine/syncer.h index c25ddca..dffee03 100755 --- a/chrome/browser/sync/engine/syncer.h +++ b/chrome/browser/sync/engine/syncer.h @@ -11,11 +11,10 @@ #include "base/basictypes.h" #include "base/scoped_ptr.h" -#include "base/time.h" -#include "chrome/browser/sync/engine/client_command_channel.h" #include "chrome/browser/sync/engine/conflict_resolver.h" #include "chrome/browser/sync/engine/syncer_types.h" #include "chrome/browser/sync/engine/syncproto.h" +#include "chrome/browser/sync/sessions/sync_session.h" #include "chrome/browser/sync/syncable/directory_event.h" #include "chrome/browser/sync/util/closure.h" #include "chrome/browser/sync/util/event_sys-inl.h" @@ -37,7 +36,6 @@ namespace browser_sync { class ModelSafeWorker; class ServerConnectionManager; class SyncProcessState; -class SyncerSession; class URLFactory; struct HttpResponse; @@ -75,10 +73,8 @@ class Syncer { // The constructor may be called from a thread that is not the Syncer's // dedicated thread, to allow some flexibility in the setup. - Syncer(syncable::DirectoryManager* dirman, const PathString& account_name, - ServerConnectionManager* connection_manager, - ModelSafeWorker* model_safe_worker); - ~Syncer(); + explicit Syncer(sessions::SyncSessionContext* context); + ~Syncer() {} // Called by other threads to tell the syncer to stop what it's doing // and return early from SyncShare, if possible. @@ -96,33 +92,21 @@ class Syncer { // for the sync cycle. It is treated as an input/output parameter. // When |first_step| and |last_step| are provided, this means to perform // a partial sync cycle, stopping after |last_step| is performed. - bool SyncShare(); - bool SyncShare(SyncProcessState* sync_process_state); - bool SyncShare(SyncerStep first_step, SyncerStep last_step); + bool SyncShare(sessions::SyncSession::Delegate* delegate); + bool SyncShare(SyncerStep first_step, SyncerStep last_step, + sessions::SyncSession::Delegate* delegate); // Limit the batch size of commit operations to a specified number of items. void set_max_commit_batch_size(int x) { max_commit_batch_size_ = x; } - ConflictResolver* conflict_resolver() { return &resolver_; } - - PathString account_name() { return account_name_; } - - SyncerEventChannel* channel() const { return syncer_event_channel_.get(); } - ShutdownChannel* shutdown_channel() const { return shutdown_channel_.get(); } - ModelSafeWorker* model_safe_worker() { return model_safe_worker_; } - // Syncer will take ownership of this channel and it will be destroyed along // with the Syncer instance. void set_shutdown_channel(ShutdownChannel* channel) { shutdown_channel_.reset(channel); } - void set_command_channel(ClientCommandChannel* channel) { - command_channel_ = channel; - } - // Volatile reader for the source member of the syncer session object. The // value is set to the SYNC_CYCLE_CONTINUATION value to signal that it has // been read. @@ -138,63 +122,43 @@ class Syncer { updates_source_ = source; } - bool notifications_enabled() const { - return notifications_enabled_; - } - - void set_notifications_enabled(bool state) { - notifications_enabled_ = state; - } - - base::TimeTicks silenced_until() const { return silenced_until_; } - bool is_silenced() const { return !silenced_until_.is_null(); } private: void RequestNudge(int milliseconds); // Implements the PROCESS_CLIENT_COMMAND syncer step. - void ProcessClientCommand(SyncerSession *session); + void ProcessClientCommand(sessions::SyncSession *session); + + // Resets transient state and runs from SYNCER_BEGIN to SYNCER_END. + bool SyncShare(sessions::SyncSession* session); - void SyncShare(SyncerSession* session); - void SyncShare(SyncerSession* session, + // This is the bottom-most SyncShare variant, and does not cause transient + // state to be reset in session. + void SyncShare(sessions::SyncSession* session, SyncerStep first_step, SyncerStep last_step); - PathString account_name_; bool early_exit_requested_; int32 max_commit_batch_size_; - ServerConnectionManager* connection_manager_; - ConflictResolver resolver_; - syncable::DirectoryManager* const dirman_; - - // When we're over bandwidth quota, we don't update until past this time. - base::TimeTicks silenced_until_; - scoped_ptr<SyncerEventChannel> syncer_event_channel_; - scoped_ptr<ShutdownChannel> shutdown_channel_; - ClientCommandChannel* command_channel_; - - // A worker capable of processing work closures on a thread that is - // guaranteed to be safe for model modifications. This is created and owned - // by the SyncerThread that created us. - ModelSafeWorker* model_safe_worker_; + sessions::ScopedSessionContextConflictResolver resolver_scoper_; + sessions::ScopedSessionContextSyncerEventChannel event_channel_scoper_; + sessions::SyncSessionContext* context_; - // We use this to stuff extensions activity into CommitMessages so the server - // can correlate commit traffic with extension-related bookmark mutations. - ExtensionsActivityMonitor* extensions_monitor_; + scoped_ptr<ShutdownChannel> shutdown_channel_; // The source of the last nudge. sync_pb::GetUpdatesCallerInfo::GET_UPDATES_SOURCE updates_source_; - // True only if the notification channel is authorized and open. - bool notifications_enabled_; - // A callback hook used in unittests to simulate changes between conflict set // building and conflict resolution. Closure* pre_conflict_resolution_closure_; + friend class SyncerTest; + FRIEND_TEST(SyncerTest, NameClashWithResolver); + FRIEND_TEST(SyncerTest, IllegalAndLegalUpdates); FRIEND_TEST(SusanDeletingTest, NewServerItemInAFolderHierarchyWeHaveDeleted3); FRIEND_TEST(SyncerTest, TestCommitListOrderingAndNewParent); @@ -203,6 +167,11 @@ class Syncer { FRIEND_TEST(SyncerTest, TestCommitListOrderingWithNesting); FRIEND_TEST(SyncerTest, TestCommitListOrderingWithNewItems); FRIEND_TEST(SyncerTest, TestGetUnsyncedAndSimpleCommit); + FRIEND_TEST(SyncerTest, UnappliedUpdateDuringCommit); + FRIEND_TEST(SyncerTest, DeletingEntryInFolder); + FRIEND_TEST(SyncerTest, LongChangelistCreatesFakeOrphanedEntries); + FRIEND_TEST(SyncerTest, QuicklyMergeDualCreatedHierarchy); + FRIEND_TEST(SyncerTest, LongChangelistWithApplicationConflict); DISALLOW_COPY_AND_ASSIGN(Syncer); }; diff --git a/chrome/browser/sync/engine/syncer_command.cc b/chrome/browser/sync/engine/syncer_command.cc index 61df463..951c138 100644 --- a/chrome/browser/sync/engine/syncer_command.cc +++ b/chrome/browser/sync/engine/syncer_command.cc @@ -5,49 +5,49 @@ #include "chrome/browser/sync/engine/syncer_command.h" #include "chrome/browser/sync/engine/net/server_connection_manager.h" -#include "chrome/browser/sync/engine/syncer_session.h" -#include "chrome/browser/sync/engine/syncer_status.h" +#include "chrome/browser/sync/sessions/sync_session.h" #include "chrome/browser/sync/syncable/directory_manager.h" #include "chrome/browser/sync/util/event_sys-inl.h" #include "chrome/browser/sync/util/sync_types.h" namespace browser_sync { +using sessions::SyncSession; SyncerCommand::SyncerCommand() {} SyncerCommand::~SyncerCommand() {} -void SyncerCommand::Execute(SyncerSession* session) { +void SyncerCommand::Execute(SyncSession* session) { ExecuteImpl(session); SendNotifications(session); } -void SyncerCommand::SendNotifications(SyncerSession* session) { - syncable::ScopedDirLookup dir(session->dirman(), session->account_name()); +void SyncerCommand::SendNotifications(SyncSession* session) { + syncable::ScopedDirLookup dir(session->context()->directory_manager(), + session->context()->account_name()); if (!dir.good()) { LOG(ERROR) << "Scoped dir lookup failed!"; return; } - SyncerStatus status(session); - - if (status.IsDirty()) { - SyncerEvent event = { SyncerEvent::STATUS_CHANGED}; - event.last_session = session; - session->syncer_event_channel()->NotifyListeners(event); - if (status.over_quota()) { - SyncerEvent quota_event = {SyncerEvent::OVER_QUOTA}; - quota_event.last_session = session; - session->syncer_event_channel()->NotifyListeners(quota_event); + if (session->status_controller()->TestAndClearIsDirty()) { + SyncerEvent event(SyncerEvent::STATUS_CHANGED); + const sessions::SyncSessionSnapshot& snapshot(session->TakeSnapshot()); + event.snapshot = &snapshot; + DCHECK(session->context()->syncer_event_channel()); + session->context()->syncer_event_channel()->NotifyListeners(event); + if (session->status_controller()->syncer_status().over_quota) { + SyncerEvent quota_event(SyncerEvent::OVER_QUOTA); + quota_event.snapshot = &snapshot; + session->context()->syncer_event_channel()->NotifyListeners(quota_event); } - status.SetClean(); } - if (status.IsAuthDirty()) { + if (session->auth_failure_occurred()) { ServerConnectionEvent event; event.what_happened = ServerConnectionEvent::STATUS_CHANGED; event.server_reachable = true; event.connection_code = HttpResponse::SYNC_AUTH_ERROR; - session->connection_manager()->channel()->NotifyListeners(event); - status.SetAuthClean(); + session->context()->connection_manager()->channel()->NotifyListeners(event); + session->clear_auth_failure_occurred(); } } diff --git a/chrome/browser/sync/engine/syncer_command.h b/chrome/browser/sync/engine/syncer_command.h index 718c4e3..5c38788 100644 --- a/chrome/browser/sync/engine/syncer_command.h +++ b/chrome/browser/sync/engine/syncer_command.h @@ -9,7 +9,9 @@ namespace browser_sync { -class SyncerSession; +namespace sessions { +class SyncSession; +} // Implementation of a simple command pattern intended to be driven by the // Syncer. SyncerCommand is abstract and all subclasses must implement @@ -18,7 +20,7 @@ class SyncerSession; // // Example Usage: // -// SyncerSession session = ...; +// SyncSession session = ...; // SyncerCommand *cmd = SomeCommandFactory.createCommand(...); // cmd->Execute(session); // delete cmd; @@ -29,12 +31,12 @@ class SyncerCommand { virtual ~SyncerCommand(); // Execute dispatches to a derived class's ExecuteImpl. - void Execute(SyncerSession* session); + void Execute(sessions::SyncSession* session); // ExecuteImpl is where derived classes actually do work. - virtual void ExecuteImpl(SyncerSession* session) = 0; + virtual void ExecuteImpl(sessions::SyncSession* session) = 0; private: - void SendNotifications(SyncerSession* session); + void SendNotifications(sessions::SyncSession* session); DISALLOW_COPY_AND_ASSIGN(SyncerCommand); }; diff --git a/chrome/browser/sync/engine/syncer_end_command.cc b/chrome/browser/sync/engine/syncer_end_command.cc index 3ae002d..48bdc13 100644 --- a/chrome/browser/sync/engine/syncer_end_command.cc +++ b/chrome/browser/sync/engine/syncer_end_command.cc @@ -4,10 +4,8 @@ #include "chrome/browser/sync/engine/syncer_end_command.h" -#include "chrome/browser/sync/engine/conflict_resolution_view.h" -#include "chrome/browser/sync/engine/syncer_session.h" -#include "chrome/browser/sync/engine/syncer_status.h" #include "chrome/browser/sync/engine/syncer_types.h" +#include "chrome/browser/sync/sessions/sync_session.h" #include "chrome/browser/sync/syncable/directory_manager.h" #include "chrome/browser/sync/util/event_sys-inl.h" @@ -16,15 +14,16 @@ namespace browser_sync { SyncerEndCommand::SyncerEndCommand() {} SyncerEndCommand::~SyncerEndCommand() {} -void SyncerEndCommand::ExecuteImpl(SyncerSession* session) { - SyncerStatus status(session); - status.set_syncing(false); +void SyncerEndCommand::ExecuteImpl(sessions::SyncSession* session) { + sessions::StatusController* status(session->status_controller()); + status->set_syncing(false); if (!session->HasMoreToSync()) { // This might be the first time we've fully completed a sync cycle. - DCHECK(session->got_zero_updates()); + DCHECK(status->got_zero_updates()); - syncable::ScopedDirLookup dir(session->dirman(), session->account_name()); + syncable::ScopedDirLookup dir(session->context()->directory_manager(), + session->context()->account_name()); if (!dir.good()) { LOG(ERROR) << "Scoped dir lookup failed!"; return; @@ -34,9 +33,10 @@ void SyncerEndCommand::ExecuteImpl(SyncerSession* session) { dir->set_initial_sync_ended(true); } - SyncerEvent event = { SyncerEvent::SYNC_CYCLE_ENDED }; - event.last_session = session; - session->syncer_event_channel()->NotifyListeners(event); + SyncerEvent event(SyncerEvent::SYNC_CYCLE_ENDED); + sessions::SyncSessionSnapshot snapshot(session->TakeSnapshot()); + event.snapshot = &snapshot; + session->context()->syncer_event_channel()->NotifyListeners(event); } } // namespace browser_sync diff --git a/chrome/browser/sync/engine/syncer_end_command.h b/chrome/browser/sync/engine/syncer_end_command.h index 253907a..10cbd76 100644 --- a/chrome/browser/sync/engine/syncer_end_command.h +++ b/chrome/browser/sync/engine/syncer_end_command.h @@ -10,8 +10,6 @@ namespace browser_sync { -class SyncerSession; - // A syncer command for wrapping up a sync cycle. // // Preconditions - syncing is complete. @@ -23,7 +21,8 @@ class SyncerEndCommand : public SyncerCommand { SyncerEndCommand(); virtual ~SyncerEndCommand(); - virtual void ExecuteImpl(SyncerSession* session); + // SyncerCommand implementation. + virtual void ExecuteImpl(sessions::SyncSession* session); private: DISALLOW_COPY_AND_ASSIGN(SyncerEndCommand); }; diff --git a/chrome/browser/sync/engine/syncer_proto_util.cc b/chrome/browser/sync/engine/syncer_proto_util.cc index 75b6689..7f94896 100755 --- a/chrome/browser/sync/engine/syncer_proto_util.cc +++ b/chrome/browser/sync/engine/syncer_proto_util.cc @@ -8,6 +8,7 @@ #include "chrome/browser/sync/engine/syncer.h" #include "chrome/browser/sync/engine/syncer_util.h" #include "chrome/browser/sync/protocol/service_constants.h" +#include "chrome/browser/sync/sessions/sync_session.h" #include "chrome/browser/sync/syncable/directory_manager.h" #include "chrome/browser/sync/syncable/syncable-inl.h" #include "chrome/browser/sync/syncable/syncable.h" @@ -26,6 +27,7 @@ using syncable::ScopedDirLookup; using syncable::SyncName; namespace browser_sync { +using sessions::SyncSession; namespace { @@ -93,12 +95,13 @@ void LogResponseProfilingData(const ClientToServerResponse& response) { // static bool SyncerProtoUtil::PostClientToServerMessage(ClientToServerMessage* msg, - ClientToServerResponse* response, SyncerSession* session) { + ClientToServerResponse* response, SyncSession* session) { bool rv = false; string tx, rx; CHECK(response); - ScopedDirLookup dir(session->dirman(), session->account_name()); + ScopedDirLookup dir(session->context()->directory_manager(), + session->context()->account_name()); if (!dir.good()) return false; string birthday = dir->store_birthday(); @@ -114,17 +117,17 @@ bool SyncerProtoUtil::PostClientToServerMessage(ClientToServerMessage* msg, tx, &rx, &http_response }; - if (!session->connection_manager()->PostBufferWithCachedAuth(¶ms)) { + ServerConnectionManager* scm = session->context()->connection_manager(); + if (!scm->PostBufferWithCachedAuth(¶ms)) { LOG(WARNING) << "Error posting from syncer:" << http_response; } else { rv = response->ParseFromString(rx); } - SyncerStatus status(session); if (rv) { if (!VerifyResponseBirthday(dir, response)) { // TODO(ncarter): Add a unit test for the case where the syncer becomes // stuck due to a bad birthday. - status.set_syncer_stuck(true); + session->status_controller()->set_syncer_stuck(true); return false; } @@ -142,7 +145,7 @@ bool SyncerProtoUtil::PostClientToServerMessage(ClientToServerMessage* msg, case ClientToServerResponse::ACCESS_DENIED: LOG(INFO) << "Authentication expired, re-requesting"; LOG(INFO) << "Not implemented in syncer yet!!!"; - status.AuthFailed(); + session->set_auth_failure_occurred(); rv = false; break; case ClientToServerResponse::NOT_MY_BIRTHDAY: @@ -151,7 +154,7 @@ bool SyncerProtoUtil::PostClientToServerMessage(ClientToServerMessage* msg, break; case ClientToServerResponse::THROTTLED: LOG(WARNING) << "Client silenced by server."; - session->set_silenced_until(base::TimeTicks::Now() + + session->delegate()->OnSilencedUntil(base::TimeTicks::Now() + base::TimeDelta::FromSeconds(kSyncDelayAfterThrottled)); rv = false; break; diff --git a/chrome/browser/sync/engine/syncer_proto_util.h b/chrome/browser/sync/engine/syncer_proto_util.h index 287cf49..fe20fef 100755 --- a/chrome/browser/sync/engine/syncer_proto_util.h +++ b/chrome/browser/sync/engine/syncer_proto_util.h @@ -7,7 +7,6 @@ #include <string> -#include "chrome/browser/sync/engine/syncer_session.h" #include "chrome/browser/sync/syncable/blob.h" #include "chrome/browser/sync/util/sync_types.h" @@ -23,8 +22,11 @@ class ClientToServerResponse; namespace browser_sync { +namespace sessions { +class SyncSession; +} + class ClientToServerMessage; -class SyncerSession; class SyncEntity; class CommitResponse_EntryResponse; @@ -35,7 +37,8 @@ class SyncerProtoUtil { // session->status()->syncer_stuck_ is set true if the birthday is // incorrect. A false value will always be returned if birthday is bad. static bool PostClientToServerMessage(ClientToServerMessage* msg, - sync_pb::ClientToServerResponse* response, SyncerSession* session); + sync_pb::ClientToServerResponse* response, + sessions::SyncSession* session); // Compares a syncable Entry to SyncEntity, returns true iff the data is // identical. diff --git a/chrome/browser/sync/engine/syncer_session.h b/chrome/browser/sync/engine/syncer_session.h deleted file mode 100644 index 6b24098..0000000 --- a/chrome/browser/sync/engine/syncer_session.h +++ /dev/null @@ -1,354 +0,0 @@ -// Copyright (c) 2009 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. -// -// SyncerSession holds the entire state of a single sync cycle; GetUpdates, -// Commit, and Conflict Resolution. After said cycle, the Session may contain -// items that were unable to be processed because of errors. -// -// THIS CLASS PROVIDES NO SYNCHRONIZATION GUARANTEES. - -#ifndef CHROME_BROWSER_SYNC_ENGINE_SYNCER_SESSION_H_ -#define CHROME_BROWSER_SYNC_ENGINE_SYNCER_SESSION_H_ - -#include <string> -#include <utility> -#include <vector> - -#include "base/time.h" -#include "chrome/browser/sync/engine/net/server_connection_manager.h" -#include "chrome/browser/sync/engine/sync_cycle_state.h" -#include "chrome/browser/sync/engine/sync_process_state.h" -#include "chrome/browser/sync/engine/syncer_status.h" -#include "chrome/browser/sync/engine/syncer_types.h" -#include "chrome/browser/sync/engine/syncproto.h" -#include "chrome/browser/sync/util/event_sys.h" -#include "chrome/browser/sync/util/extensions_activity_monitor.h" -#include "chrome/browser/sync/util/sync_types.h" -#include "testing/gtest/include/gtest/gtest_prod.h" // For FRIEND_TEST - -namespace browser_sync { - -class ConflictResolver; -class ModelSafeWorker; -class ServerConnectionManager; -class SyncerStatus; -struct SyncerEvent; - -class SyncerSession { - friend class ConflictResolutionView; - friend class SyncerStatus; - public: - // A utility to set the session's write transaction member, and later clear - // it when it the utility falls out of scope. - class ScopedSetWriteTransaction { - public: - ScopedSetWriteTransaction(SyncerSession* session, - syncable::WriteTransaction* trans) - : session_(session) { - session_->set_write_transaction(trans); - } - ~ScopedSetWriteTransaction() { - session_->ClearWriteTransaction(); - } - private: - SyncerSession* session_; - DISALLOW_COPY_AND_ASSIGN(ScopedSetWriteTransaction); - }; - - SyncerSession(SyncCycleState* cycle_state, SyncProcessState* process_state) - : sync_process_state_(process_state), - sync_cycle_state_(cycle_state), - source_(sync_pb::GetUpdatesCallerInfo::UNKNOWN), - notifications_enabled_(false) { - DCHECK(NULL != process_state); - DCHECK(NULL != cycle_state); - } - - // Perhaps this should dictate the next step. (ie, don't do apply if you - // didn't get any from download). or put it in the while loop. - void set_update_response(const ClientToServerResponse& update_response) { - sync_cycle_state_->set_update_response(update_response); - } - - const ClientToServerResponse& update_response() const { - return sync_cycle_state_->update_response(); - } - - void set_commit_response(const ClientToServerResponse& commit_response) { - sync_cycle_state_->set_commit_response(commit_response); - } - - const ClientToServerResponse& commit_response() const { - return sync_cycle_state_->commit_response(); - } - - void AddVerifyResult(const VerifyResult& verify_result, - const sync_pb::SyncEntity& entity) { - sync_cycle_state_->AddVerifyResult(verify_result, entity); - } - - bool HasVerifiedUpdates() const { - return sync_cycle_state_->HasVerifiedUpdates(); - } - - void AddAppliedUpdate(const UpdateAttemptResponse& response, - const syncable::Id& id) { - sync_cycle_state_->AddAppliedUpdate(response, id); - } - - bool HasAppliedUpdates() const { - return sync_cycle_state_->HasAppliedUpdates(); - } - - std::string account_name() const { - return sync_process_state_->account_name(); - } - - syncable::DirectoryManager* dirman() const { - return sync_process_state_->dirman(); - } - - ServerConnectionManager* connection_manager() const { - return sync_process_state_->connection_manager(); - } - - ConflictResolver* resolver() const { - return sync_process_state_->resolver(); - } - - SyncerEventChannel* syncer_event_channel() const { - return sync_process_state_->syncer_event_channel(); - } - - int conflicting_update_count() const { - return sync_process_state_->conflicting_updates(); - } - - base::TimeTicks silenced_until() const { - return sync_process_state_->silenced_until(); - } - - void set_silenced_until(base::TimeTicks silenced_until) const { - sync_process_state_->set_silenced_until(silenced_until); - } - - const std::vector<int64>& unsynced_handles() const { - return sync_cycle_state_->unsynced_handles(); - } - - void set_unsynced_handles(const std::vector<int64>& unsynced_handles) { - sync_cycle_state_->set_unsynced_handles(unsynced_handles); - } - - int64 unsynced_count() const { return sync_cycle_state_->unsynced_count(); } - - const std::vector<syncable::Id>& commit_ids() const { - return sync_cycle_state_->commit_ids(); - } - - void set_commit_ids(const std::vector<syncable::Id>& commit_ids) { - sync_cycle_state_->set_commit_ids(commit_ids); - } - - bool commit_ids_empty() const { - return sync_cycle_state_->commit_ids_empty(); - } - - bool HadSuccessfulCommits() const { - return sync_process_state_->successful_commits() > 0; - } - - syncable::WriteTransaction* write_transaction() const { - return sync_cycle_state_->write_transaction(); - } - - bool has_open_write_transaction() const { - return sync_cycle_state_->has_open_write_transaction(); - } - - ClientToServerMessage* commit_message() const { - return sync_cycle_state_->commit_message(); - } - - void set_commit_message(const ClientToServerMessage& message) { - sync_cycle_state_->set_commit_message(message); - } - - bool HasRemainingItemsToCommit() const { - return commit_ids().size() < unsynced_handles().size(); - } - - void AddCommitConflict(const syncable::Id& the_id) { - sync_process_state_->AddConflictingItem(the_id); - } - - void EraseCommitConflict(const syncable::Id& the_id) { - sync_process_state_->EraseConflictingItem(the_id); - } - - // Returns true if at least one update application failed due to a conflict - // during this sync cycle. - bool HasConflictingUpdates() const { - std::vector<AppliedUpdate>::const_iterator it; - for (it = sync_cycle_state_->AppliedUpdatesBegin(); - it < sync_cycle_state_->AppliedUpdatesEnd(); - ++it) { - if (it->first == CONFLICT) { - return true; - } - } - return false; - } - - std::vector<VerifiedUpdate>::iterator VerifiedUpdatesBegin() const { - return sync_cycle_state_->VerifiedUpdatesBegin(); - } - - std::vector<VerifiedUpdate>::iterator VerifiedUpdatesEnd() const { - return sync_cycle_state_->VerifiedUpdatesEnd(); - } - - // Returns the number of updates received from the sync server. - int64 CountUpdates() const { - if (update_response().has_get_updates()) { - return update_response().get_updates().entries().size(); - } else { - return 0; - } - } - - bool got_zero_updates() const { - return CountUpdates() == 0; - } - - void DumpSessionInfo() const { - LOG(INFO) << "Dumping session info"; - if (update_response().has_get_updates()) { - LOG(INFO) << update_response().get_updates().entries().size() - << " updates downloaded by last get_updates"; - } else { - LOG(INFO) << "No update response found"; - } - LOG(INFO) << sync_cycle_state_->VerifiedUpdatesSize() - << " updates verified"; - LOG(INFO) << sync_cycle_state_->AppliedUpdatesSize() << " updates applied"; - LOG(INFO) << commit_ids().size() << " items to commit"; - LOG(INFO) << unsynced_count() << " unsynced items"; - } - - void set_conflict_sets_built(const bool b) { - sync_cycle_state_->set_conflict_sets_built(b); - } - - bool conflict_sets_built() const { - return sync_cycle_state_->conflict_sets_built(); - } - - void set_conflicts_resolved(const bool b) { - sync_cycle_state_->set_conflicts_resolved(b); - } - - bool conflicts_resolved() const { - return sync_cycle_state_->conflicts_resolved(); - } - - ModelSafeWorker* model_safe_worker() const { - return sync_process_state_->model_safe_worker(); - } - - void set_item_committed() { - sync_cycle_state_->set_item_committed(); - } - - bool items_committed() const { - return sync_cycle_state_->items_committed(); - } - - void set_over_quota(const bool b) { - sync_cycle_state_->set_over_quota(b); - } - - // Volitile reader for the source member of the syncer session object. The - // value is set to the SYNC_CYCLE_CONTINUATION value to signal that it has - // been read. - sync_pb::GetUpdatesCallerInfo::GET_UPDATES_SOURCE TestAndSetSource() { - sync_pb::GetUpdatesCallerInfo::GET_UPDATES_SOURCE old_source = - source_; - set_source(sync_pb::GetUpdatesCallerInfo::SYNC_CYCLE_CONTINUATION); - return old_source; - } - - void set_source(sync_pb::GetUpdatesCallerInfo::GET_UPDATES_SOURCE source) { - source_ = source; - } - - const ExtensionsActivityMonitor::Records& extensions_activity() const { - return extensions_activity_; - } - - ExtensionsActivityMonitor::Records* mutable_extensions_activity() { - return &extensions_activity_; - } - - bool notifications_enabled() const { - return notifications_enabled_; - } - - void set_notifications_enabled(const bool state) { - notifications_enabled_ = state; - } - - void set_timestamp_dirty() { - sync_cycle_state_->set_timestamp_dirty(); - } - - bool timestamp_dirty() const { - return sync_cycle_state_->is_timestamp_dirty(); - } - - // TODO(chron): Unit test for this method. - // returns true iff this session contains data that should go through the - // sync engine again. - bool HasMoreToSync() const { - return (HasRemainingItemsToCommit() && - sync_process_state_->successful_commits() > 0) || - conflict_sets_built() || - conflicts_resolved() || - // Or, we have conflicting updates, but we're making progress on - // resolving them... - !got_zero_updates() || - timestamp_dirty(); - } - - private: - // The write transaction must be destructed by the caller of this function. - // Here, we just clear the reference. - void set_write_transaction(syncable::WriteTransaction* write_transaction) { - sync_cycle_state_->set_write_transaction(write_transaction); - } - - // Sets the write transaction to null, but doesn't free the memory. - void ClearWriteTransaction() { - sync_cycle_state_->ClearWriteTransaction(); - } - - SyncProcessState* sync_process_state_; - SyncCycleState* sync_cycle_state_; - - // The source for initiating this syncer session. - sync_pb::GetUpdatesCallerInfo::GET_UPDATES_SOURCE source_; - - // Information about extensions activity since the last successful commit. - ExtensionsActivityMonitor::Records extensions_activity_; - - // True if notifications are enabled when this session was created. - bool notifications_enabled_; - - FRIEND_TEST(SyncerTest, TestCommitListOrderingCounterexample); - DISALLOW_COPY_AND_ASSIGN(SyncerSession); -}; - -} // namespace browser_sync - -#endif // CHROME_BROWSER_SYNC_ENGINE_SYNCER_SESSION_H_ diff --git a/chrome/browser/sync/engine/syncer_status.cc b/chrome/browser/sync/engine/syncer_status.cc deleted file mode 100644 index f356bcd..0000000 --- a/chrome/browser/sync/engine/syncer_status.cc +++ /dev/null @@ -1,15 +0,0 @@ -// Copyright (c) 2009 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 "chrome/browser/sync/engine/syncer_session.h" -#include "chrome/browser/sync/engine/syncer_status.h" - -namespace browser_sync { -SyncerStatus::SyncerStatus(SyncerSession* s) { - sync_process_state_ = s->sync_process_state_; - sync_cycle_state_ = s->sync_cycle_state_; -} -SyncerStatus::~SyncerStatus() {} - -} // namespace browser_sync diff --git a/chrome/browser/sync/engine/syncer_status.h b/chrome/browser/sync/engine/syncer_status.h deleted file mode 100644 index cb8d4fb..0000000 --- a/chrome/browser/sync/engine/syncer_status.h +++ /dev/null @@ -1,213 +0,0 @@ -// Copyright (c) 2009 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. -// -// TODO(sync): We eventually want to fundamentally change how we represent -// status and inform the UI about the ways in which our status has changed. -// Right now, we're just trying to keep the various command classes from having -// to worry about this class. -// -// The UI will request that we fill this struct so it can show the current sync -// state. -// -// THIS CLASS PROVIDES NO SYNCHRONIZATION GUARANTEES. - -#ifndef CHROME_BROWSER_SYNC_ENGINE_SYNCER_STATUS_H_ -#define CHROME_BROWSER_SYNC_ENGINE_SYNCER_STATUS_H_ - -#include "base/atomicops.h" -#include "base/port.h" -#include "chrome/browser/sync/engine/sync_cycle_state.h" -#include "chrome/browser/sync/engine/sync_process_state.h" - -namespace browser_sync { - -class SyncerSession; - -class SyncerStatus { - public: - SyncerStatus(SyncCycleState* cycle_state, SyncProcessState* state) - : sync_cycle_state_(cycle_state), - sync_process_state_(state) {} - explicit SyncerStatus(SyncerSession* s); - ~SyncerStatus(); - - bool invalid_store() const { - return sync_process_state_->invalid_store(); - } - - bool syncer_stuck() const { - return sync_process_state_->syncer_stuck(); - } - - void set_syncer_stuck(const bool val) { - sync_process_state_->set_syncer_stuck(val); - } - - bool syncing() const { - return sync_process_state_->syncing(); - } - - void set_syncing(const bool val) { - sync_process_state_->set_syncing(val); - } - - bool IsShareUsable() const { - return sync_process_state_->IsShareUsable(); - } - - // During initial sync these two members can be used to measure sync - // progress. - int64 current_sync_timestamp() const { - return sync_process_state_->current_sync_timestamp(); - } - - void set_current_sync_timestamp(const int64 val) { - sync_process_state_->set_current_sync_timestamp(val); - } - - int64 num_server_changes_remaining() const { - return sync_process_state_->num_server_changes_remaining(); - } - - void set_num_server_changes_remaining(const int64 val) { - sync_process_state_->set_num_server_changes_remaining(val); - } - - int64 unsynced_count() const { - return sync_cycle_state_->unsynced_count(); - } - - int conflicting_updates() const { - return sync_process_state_->conflicting_updates(); - } - - int conflicting_commits() const { - return sync_process_state_->conflicting_commits(); - } - - void set_conflicting_commits(const int val) { - sync_process_state_->set_conflicting_commits(val); - } - - // WEIRD COUNTER manipulation functions. - int consecutive_problem_get_updates() const { - return sync_process_state_->consecutive_problem_get_updates(); - } - - void increment_consecutive_problem_get_updates() { - sync_process_state_->increment_consecutive_problem_get_updates(); - } - - void zero_consecutive_problem_get_updates() { - sync_process_state_->zero_consecutive_problem_get_updates(); - } - - int consecutive_problem_commits() const { - return sync_process_state_->consecutive_problem_commits(); - } - - void increment_consecutive_problem_commits() { - sync_process_state_->increment_consecutive_problem_commits(); - } - - void zero_consecutive_problem_commits() { - sync_process_state_->zero_consecutive_problem_commits(); - } - - int consecutive_transient_error_commits() const { - return sync_process_state_->consecutive_transient_error_commits(); - } - - void increment_consecutive_transient_error_commits_by(int value) { - sync_process_state_->increment_consecutive_transient_error_commits_by( - value); - } - - void zero_consecutive_transient_error_commits() { - sync_process_state_->zero_consecutive_transient_error_commits(); - } - - int consecutive_errors() const { - return sync_process_state_->consecutive_errors(); - } - - void increment_consecutive_errors() { - increment_consecutive_errors_by(1); - } - - void increment_consecutive_errors_by(int value) { - sync_process_state_->increment_consecutive_errors_by(value); - } - - void zero_consecutive_errors() { - sync_process_state_->zero_consecutive_errors(); - } - - int successful_commits() const { - return sync_process_state_->successful_commits(); - } - - void increment_successful_commits() { - sync_process_state_->increment_successful_commits(); - } - - void zero_successful_commits() { - sync_process_state_->zero_successful_commits(); - } - // End WEIRD COUNTER manipulation functions. - - bool over_quota() const { return sync_cycle_state_->over_quota(); } - - void AuthFailed() { sync_process_state_->AuthFailed(); } - - // Returns true if this object has been modified since last SetClean() call. - bool IsDirty() const { - return sync_cycle_state_->IsDirty() || sync_process_state_->IsDirty(); - } - - // Returns true if auth status has been modified since last SetClean() call. - bool IsAuthDirty() const { return sync_process_state_->IsAuthDirty(); } - - // Call to tell this status object that its new state has been seen. - void SetClean() { - sync_process_state_->SetClean(); - sync_cycle_state_->SetClean(); - } - - // Call to tell this status object that its auth state has been seen. - void SetAuthClean() { sync_process_state_->SetAuthClean(); } - - void DumpStatusInfo() const { - LOG(INFO) << "Dumping status info: " << (IsDirty() ? "DIRTY" : "CLEAN"); - - LOG(INFO) << "invalid store = " << invalid_store(); - LOG(INFO) << "syncer_stuck = " << syncer_stuck(); - LOG(INFO) << "syncing = " << syncing(); - LOG(INFO) << "over_quota = " << over_quota(); - - LOG(INFO) << "current_sync_timestamp = " << current_sync_timestamp(); - LOG(INFO) << "num_server_changes_remaining = " - << num_server_changes_remaining(); - LOG(INFO) << "unsynced_count = " << unsynced_count(); - LOG(INFO) << "conflicting_updates = " << conflicting_updates(); - LOG(INFO) << "conflicting_commits = " << conflicting_commits(); - - LOG(INFO) << "consecutive_problem_get_updates = " - << consecutive_problem_get_updates(); - LOG(INFO) << "consecutive_problem_commits = " - << consecutive_problem_commits(); - LOG(INFO) << "consecutive_transient_error_commits = " - << consecutive_transient_error_commits(); - LOG(INFO) << "consecutive_errors = " << consecutive_errors(); - LOG(INFO) << "successful_commits = " << successful_commits(); - } - - private: - SyncCycleState* sync_cycle_state_; - SyncProcessState* sync_process_state_; -}; - -} // namespace browser_sync - -#endif // CHROME_BROWSER_SYNC_ENGINE_SYNCER_STATUS_H_ diff --git a/chrome/browser/sync/engine/syncer_thread.cc b/chrome/browser/sync/engine/syncer_thread.cc index 8d63cd7..d8271a2 100644 --- a/chrome/browser/sync/engine/syncer_thread.cc +++ b/chrome/browser/sync/engine/syncer_thread.cc @@ -20,7 +20,6 @@ #include "chrome/browser/sync/engine/model_safe_worker.h" #include "chrome/browser/sync/engine/net/server_connection_manager.h" #include "chrome/browser/sync/engine/syncer.h" -#include "chrome/browser/sync/engine/syncer_thread_timed_stop.h" #include "chrome/browser/sync/notifier/listener/talk_mediator.h" #include "chrome/browser/sync/notifier/listener/talk_mediator_impl.h" #include "chrome/browser/sync/syncable/directory_manager.h" @@ -113,23 +112,6 @@ const int SyncerThread::kDefaultShortPollIntervalSeconds = 60; const int SyncerThread::kDefaultLongPollIntervalSeconds = 3600; const int SyncerThread::kDefaultMaxPollIntervalMs = 30 * 60 * 1000; -SyncerThread* SyncerThreadFactory::Create( - ClientCommandChannel* command_channel, - syncable::DirectoryManager* mgr, - ServerConnectionManager* connection_manager, AllStatus* all_status, - ModelSafeWorker* model_safe_worker) { - const CommandLine* cmd = CommandLine::ForCurrentProcess(); - if (cmd->HasSwitch(switches::kSyncerThreadTimedStop)) { - return new SyncerThreadTimedStop(command_channel, mgr, connection_manager, - all_status, model_safe_worker); - } else { - // The default SyncerThread implementation, which does not time-out when - // Stop is called. - return new SyncerThread(command_channel, mgr, connection_manager, - all_status, model_safe_worker); - } -} - void SyncerThread::NudgeSyncer(int milliseconds_from_now, NudgeSource source) { AutoLock lock(lock_); if (vault_.syncer_ == NULL) { @@ -139,77 +121,42 @@ void SyncerThread::NudgeSyncer(int milliseconds_from_now, NudgeSource source) { NudgeSyncImpl(milliseconds_from_now, source); } -SyncerThread::SyncerThread() +SyncerThread::SyncerThread(sessions::SyncSessionContext* context, + AllStatus* all_status) : thread_main_started_(false, false), thread_("SyncEngine_SyncerThread"), vault_field_changed_(&lock_), p2p_authenticated_(false), p2p_subscribed_(false), - client_command_hookup_(NULL), - conn_mgr_hookup_(NULL), - allstatus_(NULL), - dirman_(NULL), - scm_(NULL), - syncer_short_poll_interval_seconds_(kDefaultShortPollIntervalSeconds), - syncer_long_poll_interval_seconds_(kDefaultLongPollIntervalSeconds), - syncer_polling_interval_(kDefaultShortPollIntervalSeconds), - syncer_max_interval_(kDefaultMaxPollIntervalMs), - talk_mediator_hookup_(NULL), - command_channel_(NULL), - directory_manager_hookup_(NULL), - syncer_events_(NULL), - model_safe_worker_(NULL), - disable_idle_detection_(false) { -} - -SyncerThread::SyncerThread( - ClientCommandChannel* command_channel, - syncable::DirectoryManager* mgr, - ServerConnectionManager* connection_manager, - AllStatus* all_status, - ModelSafeWorker* model_safe_worker) - : thread_main_started_(false, false), - thread_("SyncEngine_SyncerThread"), - vault_field_changed_(&lock_), - p2p_authenticated_(false), - p2p_subscribed_(false), - client_command_hookup_(NULL), conn_mgr_hookup_(NULL), allstatus_(all_status), - dirman_(mgr), - scm_(connection_manager), syncer_short_poll_interval_seconds_(kDefaultShortPollIntervalSeconds), syncer_long_poll_interval_seconds_(kDefaultLongPollIntervalSeconds), syncer_polling_interval_(kDefaultShortPollIntervalSeconds), syncer_max_interval_(kDefaultMaxPollIntervalMs), talk_mediator_hookup_(NULL), - command_channel_(command_channel), directory_manager_hookup_(NULL), syncer_events_(NULL), - model_safe_worker_(model_safe_worker), + session_context_(context), disable_idle_detection_(false) { + DCHECK(context); + syncer_event_relay_channel_.reset(new SyncerEventChannel(SyncerEvent( + SyncerEvent::SHUTDOWN_USE_WITH_CARE))); - SyncerEvent shutdown = { SyncerEvent::SHUTDOWN_USE_WITH_CARE }; - syncer_event_channel_.reset(new SyncerEventChannel(shutdown)); - - if (dirman_) { + if (context->directory_manager()) { directory_manager_hookup_.reset(NewEventListenerHookup( - dirman_->channel(), this, &SyncerThread::HandleDirectoryManagerEvent)); + context->directory_manager()->channel(), this, + &SyncerThread::HandleDirectoryManagerEvent)); } - if (scm_) { - WatchConnectionManager(scm_); - } + if (context->connection_manager()) + WatchConnectionManager(context->connection_manager()); - if (command_channel_) { - WatchClientCommands(command_channel_); - } } SyncerThread::~SyncerThread() { - client_command_hookup_.reset(); conn_mgr_hookup_.reset(); - syncer_event_channel_.reset(); + syncer_event_relay_channel_.reset(); directory_manager_hookup_.reset(); syncer_events_.reset(); delete vault_.syncer_; @@ -280,25 +227,24 @@ bool SyncerThread::Stop(int max_wait) { return true; } -void SyncerThread::WatchClientCommands(ClientCommandChannel* channel) { - AutoLock lock(lock_); - client_command_hookup_.reset(NewEventListenerHookup(channel, this, - &SyncerThread::HandleClientCommand)); +void SyncerThread::OnReceivedLongPollIntervalUpdate( + const base::TimeDelta& new_interval) { + syncer_long_poll_interval_seconds_ = static_cast<int>( + new_interval.InSeconds()); } -void SyncerThread::HandleClientCommand(ClientCommandChannel::EventType event) { - if (!event) { - return; - } +void SyncerThread::OnReceivedShortPollIntervalUpdate( + const base::TimeDelta& new_interval) { + syncer_short_poll_interval_seconds_ = static_cast<int>( + new_interval.InSeconds()); +} - // Mutex not really necessary for these. - if (event->has_set_sync_poll_interval()) { - syncer_short_poll_interval_seconds_ = event->set_sync_poll_interval(); - } +void SyncerThread::OnSilencedUntil(const base::TimeTicks& silenced_until) { + silenced_until_ = silenced_until; +} - if (event->has_set_sync_long_poll_interval()) { - syncer_long_poll_interval_seconds_ = event->set_sync_long_poll_interval(); - } +bool SyncerThread::IsSyncingCurrentlySilenced() { + return (silenced_until_ - TimeTicks::Now()) >= TimeDelta::FromSeconds(0); } void SyncerThread::ThreadMainLoop() { @@ -398,11 +344,10 @@ SyncerThread::WaitInterval SyncerThread::CalculatePollingWaitTime( WaitInterval return_interval; // Server initiated throttling trumps everything. - if (vault_.syncer_ && vault_.syncer_->is_silenced()) { + if (!silenced_until_.is_null()) { // We don't need to reset other state, it can continue where it left off. return_interval.mode = WaitInterval::THROTTLED; - return_interval.poll_delta = vault_.syncer_->silenced_until() - - TimeTicks::Now(); + return_interval.poll_delta = silenced_until_ - TimeTicks::Now(); return return_interval; } @@ -480,8 +425,14 @@ void SyncerThread::ThreadMain() { void SyncerThread::SyncMain(Syncer* syncer) { CHECK(syncer); + + // Since we are initiating a new session for which we are the delegate, we + // are not currently silenced so reset this state for the next session which + // may need to use it. + silenced_until_ = base::TimeTicks(); + AutoUnlock unlock(lock_); - while (syncer->SyncShare() && !syncer->is_silenced()) { + while (syncer->SyncShare(this) && silenced_until_.is_null()) { LOG(INFO) << "Looping in sync share"; } LOG(INFO) << "Done looping in sync share"; @@ -541,7 +492,7 @@ void SyncerThread::SetUpdatesSource(bool nudged, NudgeSource nudge_source, void SyncerThread::HandleSyncerEvent(const SyncerEvent& event) { AutoLock lock(lock_); - channel()->NotifyListeners(event); + relay_channel()->NotifyListeners(event); if (SyncerEvent::REQUEST_SYNC_NUDGE != event.what_happened) { return; } @@ -557,12 +508,12 @@ void SyncerThread::HandleDirectoryManagerEvent( // The underlying database structure is ready, and we should create // the syncer. CHECK(vault_.syncer_ == NULL); - vault_.syncer_ = - new Syncer(dirman_, event.dirname, scm_, model_safe_worker_.get()); + session_context_->set_account_name(event.dirname); + vault_.syncer_ = new Syncer(session_context_.get()); - vault_.syncer_->set_command_channel(command_channel_); syncer_events_.reset(NewEventListenerHookup( - vault_.syncer_->channel(), this, &SyncerThread::HandleSyncerEvent)); + session_context_->syncer_event_channel(), this, + &SyncerThread::HandleSyncerEvent)); vault_field_changed_.Broadcast(); } } @@ -599,8 +550,8 @@ void SyncerThread::HandleServerConnectionEvent( } } -SyncerEventChannel* SyncerThread::channel() { - return syncer_event_channel_.get(); +SyncerEventChannel* SyncerThread::relay_channel() { + return syncer_event_relay_channel_.get(); } // Inputs and return value in milliseconds. @@ -682,10 +633,8 @@ void SyncerThread::HandleTalkMediatorEvent(const TalkMediatorEvent& event) { break; } - if (NULL != vault_.syncer_) { - vault_.syncer_->set_notifications_enabled( - p2p_authenticated_ && p2p_subscribed_); - } + session_context_->set_notifications_enabled(p2p_authenticated_ && + p2p_subscribed_); } } // namespace browser_sync diff --git a/chrome/browser/sync/engine/syncer_thread.h b/chrome/browser/sync/engine/syncer_thread.h index 84abfdd..f9e3dea 100644 --- a/chrome/browser/sync/engine/syncer_thread.h +++ b/chrome/browser/sync/engine/syncer_thread.h @@ -21,7 +21,7 @@ #include "base/time.h" #include "base/waitable_event.h" #include "chrome/browser/sync/engine/all_status.h" -#include "chrome/browser/sync/engine/client_command_channel.h" +#include "chrome/browser/sync/sessions/sync_session.h" #include "chrome/browser/sync/util/event_sys-inl.h" #include "testing/gtest/include/gtest/gtest_prod.h" // For FRIEND_TEST @@ -44,29 +44,8 @@ struct SyncerEvent; struct SyncerShutdownEvent; struct TalkMediatorEvent; -class SyncerThreadFactory { - public: - // Creates a SyncerThread based on the default (or user-overridden) - // implementation. The thread does not start running until you call Start(), - // which will cause it to check-and-wait for certain conditions to be met - // (such as valid connection with Server established, syncable::Directory has - // been opened) before performing an intial sync with a server. It uses - // |connection_manager| to detect valid connections, and |mgr| to detect the - // opening of a Directory, which will cause it to create a Syncer object for - // said Directory, and assign |model_safe_worker| to it. |connection_manager| - // and |mgr| should outlive the SyncerThread. You must stop the thread by - // calling Stop before destroying the object. Stopping will first tear down - // the Syncer object, allowing it to finish work in progress, before joining - // the Stop-calling thread with the internal thread. - static SyncerThread* Create(ClientCommandChannel* command_channel, - syncable::DirectoryManager* mgr, - ServerConnectionManager* connection_manager, AllStatus* all_status, - ModelSafeWorker* model_safe_worker); - private: - DISALLOW_IMPLICIT_CONSTRUCTORS(SyncerThreadFactory); -}; - -class SyncerThread : public base::RefCountedThreadSafe<SyncerThread> { +class SyncerThread : public base::RefCountedThreadSafe<SyncerThread>, + public sessions::SyncSession::Delegate { FRIEND_TEST(SyncerThreadTest, CalculateSyncWaitTime); FRIEND_TEST(SyncerThreadTest, CalculatePollingWaitTime); FRIEND_TEST(SyncerThreadWithSyncerTest, Polling); @@ -117,6 +96,7 @@ class SyncerThread : public base::RefCountedThreadSafe<SyncerThread> { // longest possible poll interval. static const int kDefaultMaxPollIntervalMs; + SyncerThread(sessions::SyncSessionContext* context, AllStatus* all_status); virtual ~SyncerThread(); virtual void WatchConnectionManager(ServerConnectionManager* conn_mgr); @@ -136,16 +116,9 @@ class SyncerThread : public base::RefCountedThreadSafe<SyncerThread> { // Registers this thread to watch talk mediator events. virtual void WatchTalkMediator(TalkMediator* talk_mediator); - virtual void WatchClientCommands(ClientCommandChannel* channel); - - virtual SyncerEventChannel* channel(); + virtual SyncerEventChannel* relay_channel(); protected: - SyncerThread(); // Necessary for temporary pthreads-based PIMPL impl. - SyncerThread(ClientCommandChannel* command_channel, - syncable::DirectoryManager* mgr, - ServerConnectionManager* connection_manager, AllStatus* all_status, - ModelSafeWorker* model_safe_worker); virtual void ThreadMain(); void ThreadMainLoop(); @@ -227,7 +200,14 @@ class SyncerThread : public base::RefCountedThreadSafe<SyncerThread> { void HandleDirectoryManagerEvent( const syncable::DirectoryManagerEvent& event); void HandleSyncerEvent(const SyncerEvent& event); - void HandleClientCommand(ClientCommandChannel::EventType event); + + // SyncSession::Delegate implementation. + virtual void OnSilencedUntil(const base::TimeTicks& silenced_until); + virtual bool IsSyncingCurrentlySilenced(); + virtual void OnReceivedShortPollIntervalUpdate( + const base::TimeDelta& new_interval); + virtual void OnReceivedLongPollIntervalUpdate( + const base::TimeDelta& new_interval); void HandleServerConnectionEvent(const ServerConnectionEvent& event); @@ -274,13 +254,9 @@ class SyncerThread : public base::RefCountedThreadSafe<SyncerThread> { bool p2p_authenticated_; bool p2p_subscribed_; - scoped_ptr<EventListenerHookup> client_command_hookup_; scoped_ptr<EventListenerHookup> conn_mgr_hookup_; const AllStatus* allstatus_; - syncable::DirectoryManager* dirman_; - ServerConnectionManager* scm_; - // Modifiable versions of kDefaultLongPollIntervalSeconds which can be // updated by the server. int syncer_short_poll_interval_seconds_; @@ -295,22 +271,29 @@ class SyncerThread : public base::RefCountedThreadSafe<SyncerThread> { // also takes previous failures into account. int syncer_max_interval_; - scoped_ptr<SyncerEventChannel> syncer_event_channel_; - // This causes syncer to start syncing ASAP. If the rate of requests is too // high the request will be silently dropped. mutex_ should be held when // this is called. void NudgeSyncImpl(int milliseconds_from_now, NudgeSource source); scoped_ptr<EventListenerHookup> talk_mediator_hookup_; - ClientCommandChannel* const command_channel_; scoped_ptr<EventListenerHookup> directory_manager_hookup_; scoped_ptr<EventListenerHookup> syncer_events_; - // Handles any tasks that will result in model changes (modifications of - // syncable::Entries). Pass this to the syncer created and managed by |this|. - // Only non-null in syncapi case. - scoped_ptr<ModelSafeWorker> model_safe_worker_; + scoped_ptr<sessions::SyncSessionContext> session_context_; + + // Events from the Syncer's syncer_event_channel are first processed by the + // SyncerThread and then get relayed onto this channel for consumers. + // TODO(timsteele): Wow did this confused me. I had removed the channel from + // here thinking there was only one, and then realized this relay was + // happening. Is this strict event handling order needed?! + scoped_ptr<SyncerEventChannel> syncer_event_relay_channel_; + + // Set whenever the server instructs us to stop sending it requests until + // a specified time, and reset for each call to SyncShare. (Note that the + // WaitInterval::THROTTLED contract is such that we don't call SyncShare at + // all until the "silenced until" embargo expires.) + base::TimeTicks silenced_until_; // Useful for unit tests bool disable_idle_detection_; diff --git a/chrome/browser/sync/engine/syncer_thread_timed_stop.cc b/chrome/browser/sync/engine/syncer_thread_timed_stop.cc deleted file mode 100644 index 12f553b..0000000 --- a/chrome/browser/sync/engine/syncer_thread_timed_stop.cc +++ /dev/null @@ -1,121 +0,0 @@ -// Copyright (c) 2009 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 "chrome/browser/sync/engine/syncer_thread_timed_stop.h" - -#include "build/build_config.h" - -#if defined(OS_MACOSX) -#include <CoreFoundation/CFNumber.h> -#include <IOKit/IOTypes.h> -#include <IOKit/IOKitLib.h> -#endif - -#include <algorithm> -#include <map> -#include <queue> - -#include "base/auto_reset.h" -#include "chrome/browser/sync/engine/auth_watcher.h" -#include "chrome/browser/sync/engine/model_safe_worker.h" -#include "chrome/browser/sync/engine/net/server_connection_manager.h" -#include "chrome/browser/sync/engine/syncer.h" -#include "chrome/browser/sync/notifier/listener/talk_mediator.h" -#include "chrome/browser/sync/notifier/listener/talk_mediator_impl.h" -#include "chrome/browser/sync/syncable/directory_manager.h" - -using std::priority_queue; -using std::min; -using base::Time; -using base::TimeDelta; -using base::TimeTicks; - -namespace browser_sync { - -SyncerThreadTimedStop::SyncerThreadTimedStop( - ClientCommandChannel* command_channel, - syncable::DirectoryManager* mgr, - ServerConnectionManager* connection_manager, - AllStatus* all_status, - ModelSafeWorker* model_safe_worker) - : SyncerThread(command_channel, mgr, connection_manager, all_status, - model_safe_worker), - in_thread_main_loop_(false) { -} - -// Stop processing. A max wait of at least 2*server RTT time is recommended. -// Returns true if we stopped, false otherwise. -bool SyncerThreadTimedStop::Stop(int max_wait) { - AutoLock lock(lock_); - // If the thread has been started, then we either already have or are about to - // enter ThreadMainLoop so we have to proceed with shutdown and wait for it to - // finish. If the thread has not been started --and we now own the lock-- - // then we can early out because the caller has not called Start(). - if (!thread_.IsRunning()) - return true; - - LOG(INFO) << "SyncerThread::Stop - setting ThreadMain exit condition to " - << "true (vault_.stop_syncer_thread_)"; - // Exit the ThreadMainLoop once the syncer finishes (we tell it to exit - // below). - vault_.stop_syncer_thread_ = true; - if (NULL != vault_.syncer_) { - // Try to early exit the syncer. - vault_.syncer_->RequestEarlyExit(); - } - - // stop_syncer_thread_ is now true and the Syncer has been told to exit. - // We want to wake up all waiters so they can re-examine state. We signal, - // causing all waiters to try to re-acquire the lock, and then we atomically - // release the lock and wait. Our wait can be spuriously signaled, so we - // recalculate the remaining sleep time each time through and re- - // check the condition before exiting the loop. - vault_field_changed_.Broadcast(); - TimeTicks start = TimeTicks::Now(); - TimeTicks end = start + TimeDelta::FromMilliseconds(max_wait); - bool timed_out = false; - // Eventually the combination of RequestEarlyExit and setting - // stop_syncer_thread_ to true above will cause in_thread_main_loop_ to become - // false. - while (in_thread_main_loop_) { - TimeDelta sleep_time = end - TimeTicks::Now(); - if (sleep_time < TimeDelta::FromSeconds(0)) { - timed_out = true; - break; - } - LOG(INFO) << "Waiting in stop for " << sleep_time.InSeconds() << "s."; - vault_field_changed_.TimedWait(sleep_time); - } - - if (timed_out) { - LOG(ERROR) << "SyncerThread::Stop timed out or error. Problems likely."; - return false; - } - - // Stop() should not block on anything at this point, given above madness. - DLOG(INFO) << "Calling SyncerThread::thread_.Stop() at " - << Time::Now().ToInternalValue(); - thread_.Stop(); - DLOG(INFO) << "SyncerThread::thread_.Stop() finished at " - << Time::Now().ToInternalValue(); - return true; -} - -void SyncerThreadTimedStop::ThreadMain() { - AutoLock lock(lock_); - // Signal Start() to let it know we've made it safely are now running on the - // message loop, and unblock it's caller. - thread_main_started_.Signal(); - - // The only thing that could be waiting on this value is Stop, and we don't - // release the lock until we're far enough along to Stop safely. - { - AutoReset auto_reset_in_thread_main_loop(&in_thread_main_loop_, true); - vault_field_changed_.Broadcast(); - ThreadMainLoop(); - } - vault_field_changed_.Broadcast(); - LOG(INFO) << "Syncer thread ThreadMain is done."; -} - -} // namespace browser_sync diff --git a/chrome/browser/sync/engine/syncer_thread_timed_stop.h b/chrome/browser/sync/engine/syncer_thread_timed_stop.h deleted file mode 100644 index e9b260a..0000000 --- a/chrome/browser/sync/engine/syncer_thread_timed_stop.h +++ /dev/null @@ -1,53 +0,0 @@ -// Copyright (c) 2009 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. -// -// A class to run the syncer on a thread. This guy is the closest chrome-based -// (as opposed to pthreads based) SyncerThread to the old pthread implementation -// in semantics, as it supports a timeout on Stop() -- It is just an override of -// two methods from SyncerThread: ThreadMain and Stop -- to provide this. -#ifndef CHROME_BROWSER_SYNC_ENGINE_SYNCER_THREAD_TIMED_STOP_H_ -#define CHROME_BROWSER_SYNC_ENGINE_SYNCER_THREAD_TIMED_STOP_H_ - -#include <list> -#include <map> -#include <queue> -#include <vector> - -#include "chrome/browser/sync/engine/syncer_thread.h" - -namespace browser_sync { - -class SyncerThreadTimedStop : public SyncerThread { - FRIEND_TEST(SyncerThreadTest, CalculateSyncWaitTime); - FRIEND_TEST(SyncerThreadTest, CalculatePollingWaitTime); - FRIEND_TEST(SyncerThreadWithSyncerTest, Polling); - FRIEND_TEST(SyncerThreadWithSyncerTest, Nudge); - friend class SyncerThreadWithSyncerTest; - friend class SyncerThreadFactory; - public: - virtual ~SyncerThreadTimedStop() {} - - // Stop processing. This version comes with a supported max_wait. - // A max wait of at least 2*server RTT time is recommended. - // Returns true if we stopped, false otherwise. - virtual bool Stop(int max_wait); - - private: - SyncerThreadTimedStop(ClientCommandChannel* command_channel, - syncable::DirectoryManager* mgr, - ServerConnectionManager* connection_manager, AllStatus* all_status, - ModelSafeWorker* model_safe_worker); - virtual void ThreadMain(); - - // We use this to track when our synthesized thread loop is active, so we can - // timed-wait for it to become false. For this and only this (temporary) - // implementation, we protect this variable using our parent lock_. - bool in_thread_main_loop_; - - DISALLOW_COPY_AND_ASSIGN(SyncerThreadTimedStop); -}; - -} // namespace browser_sync - -#endif // CHROME_BROWSER_SYNC_ENGINE_SYNCER_THREAD_TIMED_STOP_H_ diff --git a/chrome/browser/sync/engine/syncer_thread_unittest.cc b/chrome/browser/sync/engine/syncer_thread_unittest.cc index 0701384..319d3df 100644 --- a/chrome/browser/sync/engine/syncer_thread_unittest.cc +++ b/chrome/browser/sync/engine/syncer_thread_unittest.cc @@ -9,9 +9,10 @@ #include "base/command_line.h" #include "base/scoped_ptr.h" #include "base/time.h" +#include "base/waitable_event.h" #include "chrome/browser/sync/engine/model_safe_worker.h" #include "chrome/browser/sync/engine/syncer_thread.h" -#include "chrome/browser/sync/engine/syncer_thread_timed_stop.h" +#include "chrome/browser/sync/sessions/sync_session_context.h" #include "chrome/test/sync/engine/mock_server_connection.h" #include "chrome/test/sync/engine/test_directory_setter_upper.h" #include "testing/gtest/include/gtest/gtest.h" @@ -20,22 +21,25 @@ using base::TimeTicks; using base::TimeDelta; namespace browser_sync { +using sessions::SyncSessionContext; typedef testing::Test SyncerThreadTest; typedef SyncerThread::WaitInterval WaitInterval; class SyncerThreadWithSyncerTest : public testing::Test { public: - SyncerThreadWithSyncerTest() {} + SyncerThreadWithSyncerTest() : sync_cycle_ended_event_(false, false) {} virtual void SetUp() { metadb_.SetUp(); connection_.reset(new MockConnectionManager(metadb_.manager(), metadb_.name())); allstatus_.reset(new AllStatus()); - - syncer_thread_ = SyncerThreadFactory::Create(NULL, metadb_.manager(), - connection_.get(), allstatus_.get(), new ModelSafeWorker()); - + SyncSessionContext* context = new SyncSessionContext(connection_.get(), + metadb_.manager(), new ModelSafeWorker()); + syncer_thread_ = new SyncerThread(context, allstatus_.get()); + syncer_event_hookup_.reset( + NewEventListenerHookup(syncer_thread_->relay_channel(), this, + &SyncerThreadWithSyncerTest::HandleSyncerEvent)); allstatus_->WatchSyncerThread(syncer_thread_); syncer_thread_->SetConnected(true); } @@ -49,11 +53,27 @@ class SyncerThreadWithSyncerTest : public testing::Test { ManuallyOpenedTestDirectorySetterUpper* metadb() { return &metadb_; } MockConnectionManager* connection() { return connection_.get(); } SyncerThread* syncer_thread() { return syncer_thread_; } + + // Waits an indefinite amount of sync cycles for the syncer thread to become + // throttled. Only call this if a throttle is supposed to occur! + void WaitForThrottle() { + while (!syncer_thread()->IsSyncingCurrentlySilenced()) + sync_cycle_ended_event_.Wait(); + } + private: + + void HandleSyncerEvent(const SyncerEvent& event) { + if (event.what_happened == SyncerEvent::SYNC_CYCLE_ENDED) + sync_cycle_ended_event_.Signal(); + } + ManuallyOpenedTestDirectorySetterUpper metadb_; scoped_ptr<MockConnectionManager> connection_; scoped_ptr<AllStatus> allstatus_; scoped_refptr<SyncerThread> syncer_thread_; + scoped_ptr<EventListenerHookup> syncer_event_hookup_; + base::WaitableEvent sync_cycle_ended_event_; DISALLOW_COPY_AND_ASSIGN(SyncerThreadWithSyncerTest); }; @@ -92,13 +112,13 @@ class SyncShareIntercept : public MockConnectionManager::ThrottleRequestVisitor, }; TEST_F(SyncerThreadTest, Construction) { - scoped_refptr<SyncerThread> syncer_thread( - SyncerThreadFactory::Create(NULL, NULL, NULL, NULL, NULL)); + SyncSessionContext* context = new SyncSessionContext(NULL, NULL, NULL); + scoped_refptr<SyncerThread> syncer_thread(new SyncerThread(context, NULL)); } TEST_F(SyncerThreadTest, StartStop) { - scoped_refptr<SyncerThread> syncer_thread( - SyncerThreadFactory::Create(NULL, NULL, NULL, NULL, NULL)); + SyncSessionContext* context = new SyncSessionContext(NULL, NULL, NULL); + scoped_refptr<SyncerThread> syncer_thread(new SyncerThread(context, NULL)); EXPECT_TRUE(syncer_thread->Start()); EXPECT_TRUE(syncer_thread->Stop(2000)); @@ -109,8 +129,8 @@ TEST_F(SyncerThreadTest, StartStop) { } TEST_F(SyncerThreadTest, CalculateSyncWaitTime) { - scoped_refptr<SyncerThread> syncer_thread( - SyncerThreadFactory::Create(NULL, NULL, NULL, NULL, NULL)); + SyncSessionContext* context = new SyncSessionContext(NULL, NULL, NULL); + scoped_refptr<SyncerThread> syncer_thread(new SyncerThread(context, NULL)); syncer_thread->DisableIdleDetection(); // Syncer_polling_interval_ is less than max poll interval. @@ -169,8 +189,8 @@ TEST_F(SyncerThreadTest, CalculateSyncWaitTime) { TEST_F(SyncerThreadTest, CalculatePollingWaitTime) { // Set up the environment. int user_idle_milliseconds_param = 0; - scoped_refptr<SyncerThread> syncer_thread( - SyncerThreadFactory::Create(NULL, NULL, NULL, NULL, NULL)); + SyncSessionContext* context = new SyncSessionContext(NULL, NULL, NULL); + scoped_refptr<SyncerThread> syncer_thread(new SyncerThread(context, NULL)); syncer_thread->DisableIdleDetection(); // Hold the lock to appease asserts in code. AutoLock lock(syncer_thread->lock_); @@ -596,9 +616,11 @@ TEST_F(SyncerThreadWithSyncerTest, Throttling) { syncer_thread()->NudgeSyncer(0, SyncerThread::kUnknown); syncer_thread()->NudgeSyncer(0, SyncerThread::kUnknown); - // Stick around for several poll intervals for good measure. Any sync is - // a failure. - interceptor.WaitForSyncShare(1, poll_interval * 10); + // Wait until the syncer thread reports that it is throttled. Any further + // sync share interceptions will result in failure. If things are broken, + // we may never halt. + WaitForThrottle(); + EXPECT_TRUE(syncer_thread()->IsSyncingCurrentlySilenced()); EXPECT_TRUE(syncer_thread()->Stop(2000)); } diff --git a/chrome/browser/sync/engine/syncer_types.h b/chrome/browser/sync/engine/syncer_types.h index 0cb1e56..79c53f9 100755 --- a/chrome/browser/sync/engine/syncer_types.h +++ b/chrome/browser/sync/engine/syncer_types.h @@ -19,9 +19,9 @@ class Id; // in a single place without having dependencies between other files. namespace browser_sync { -class SyncProcessState; -class SyncCycleState; -class SyncerSession; +namespace sessions { +struct SyncSessionSnapshot; +} class Syncer; enum UpdateAttemptResponse { @@ -93,6 +93,11 @@ struct SyncerEvent { SYNC_CYCLE_ENDED, }; + explicit SyncerEvent(EventCause cause) : what_happened(cause), + snapshot(NULL), + successful_commit_count(0), + nudge_delay_milliseconds(0) {} + static bool IsChannelShutdownEvent(const SyncerEvent& e) { return SHUTDOWN_USE_WITH_CARE == e.what_happened; } @@ -105,7 +110,7 @@ struct SyncerEvent { EventCause what_happened; // The last session used for syncing. - SyncerSession* last_session; + const sessions::SyncSessionSnapshot* snapshot; int successful_commit_count; diff --git a/chrome/browser/sync/engine/syncer_unittest.cc b/chrome/browser/sync/engine/syncer_unittest.cc index 31840cd..a0babad 100755 --- a/chrome/browser/sync/engine/syncer_unittest.cc +++ b/chrome/browser/sync/engine/syncer_unittest.cc @@ -13,8 +13,6 @@ #include "base/scoped_ptr.h" #include "build/build_config.h" -#include "chrome/browser/sync/engine/client_command_channel.h" -#include "chrome/browser/sync/engine/conflict_resolution_view.h" #include "chrome/browser/sync/engine/conflict_resolver.h" #include "chrome/browser/sync/engine/get_commit_ids_command.h" #include "chrome/browser/sync/engine/model_safe_worker.h" @@ -23,7 +21,6 @@ #include "chrome/browser/sync/engine/syncer.h" #include "chrome/browser/sync/engine/syncer_util.h" #include "chrome/browser/sync/engine/syncer_proto_util.h" -#include "chrome/browser/sync/engine/syncer_session.h" #include "chrome/browser/sync/protocol/sync.pb.h" #include "chrome/browser/sync/syncable/directory_manager.h" #include "chrome/browser/sync/syncable/syncable.h" @@ -35,6 +32,8 @@ #include "chrome/test/sync/engine/test_syncable_utils.h" #include "testing/gtest/include/gtest/gtest.h" +using base::TimeDelta; + using std::map; using std::multimap; using std::set; @@ -84,19 +83,36 @@ using syncable::SERVER_VERSION; using syncable::SINGLETON_TAG; using syncable::UNITTEST; +using sessions::ConflictProgress; +using sessions::ScopedSetSessionWriteTransaction; +using sessions::StatusController; +using sessions::SyncSessionContext; +using sessions::SyncSession; + namespace { const char* kTestData = "Hello World!"; const int kTestDataLen = 12; const int64 kTestLogRequestTimestamp = 123456; } // namespace -class SyncerTest : public testing::Test { +class SyncerTest : public testing::Test, public SyncSession::Delegate { protected: - SyncerTest() : client_command_channel_(0) { - } + SyncerTest() : syncer_(NULL) {} - void HandleClientCommand(const sync_pb::ClientCommand* event) { - last_client_command_ = *event; + // SyncSession::Delegate implementation. + virtual void OnSilencedUntil(const base::TimeTicks& silenced_until) { + FAIL() << "Should not get silenced."; + } + virtual bool IsSyncingCurrentlySilenced() { + return false; + } + virtual void OnReceivedLongPollIntervalUpdate( + const base::TimeDelta& new_interval) { + last_long_poll_interval_received_ = new_interval; + } + virtual void OnReceivedShortPollIntervalUpdate( + const base::TimeDelta& new_interval) { + last_short_poll_interval_received_ = new_interval; } void HandleSyncerEvent(SyncerEvent event) { @@ -121,16 +137,11 @@ class SyncerTest : public testing::Test { } void LoopSyncShare(Syncer* syncer) { - SyncProcessState state(syncdb_.manager(), syncdb_.name(), - mock_server_.get(), - syncer->conflict_resolver(), - syncer->channel(), - syncer->model_safe_worker()); bool should_loop = false; int loop_iterations = 0; do { ASSERT_LT(++loop_iterations, 100) << "infinite loop detected. please fix"; - should_loop = syncer->SyncShare(&state); + should_loop = syncer->SyncShare(this); } while (should_loop); } @@ -139,26 +150,20 @@ class SyncerTest : public testing::Test { mock_server_.reset( new MockConnectionManager(syncdb_.manager(), syncdb_.name())); - model_safe_worker_.reset(new ModelSafeWorker()); - // Safe to pass NULL as Authwatcher for now since the code path that - // uses it is not unittested yet. - syncer_ = new Syncer(syncdb_.manager(), syncdb_.name(), - mock_server_.get(), - model_safe_worker_.get()); - CHECK(syncer_->channel()); - - hookup_.reset(NewEventListenerHookup(syncer_->channel(), this, - &SyncerTest::HandleSyncerEvent)); - - command_channel_hookup_.reset(NewEventListenerHookup( - &client_command_channel_, this, &SyncerTest::HandleClientCommand)); - syncer_->set_command_channel(&client_command_channel_); - state_.reset(new SyncProcessState(syncdb_.manager(), syncdb_.name(), - mock_server_.get(), - syncer_->conflict_resolver(), - syncer_->channel(), - syncer_->model_safe_worker())); + context_.reset(new SyncSessionContext(mock_server_.get(), syncdb_.manager(), + new ModelSafeWorker())); + context_->set_account_name(syncdb_.name()); + ASSERT_FALSE(context_->syncer_event_channel()); + ASSERT_FALSE(context_->resolver()); + syncer_ = new Syncer(context_.get()); + // The Syncer installs some components on the context. + ASSERT_TRUE(context_->syncer_event_channel()); + ASSERT_TRUE(context_->resolver()); + + hookup_.reset(NewEventListenerHookup(context_->syncer_event_channel(), this, + &SyncerTest::HandleSyncerEvent)); + session_.reset(new SyncSession(context_.get(), this)); ScopedDirLookup dir(syncdb_.manager(), syncdb_.name()); CHECK(dir.good()); @@ -175,8 +180,8 @@ class SyncerTest : public testing::Test { virtual void TearDown() { mock_server_.reset(); hookup_.reset(); - command_channel_hookup_.reset(); delete syncer_; + syncer_ = NULL; syncdb_.TearDown(); } void WriteTestDataToEntry(WriteTransaction* trans, MutableEntry* entry) { @@ -197,21 +202,16 @@ class SyncerTest : public testing::Test { EXPECT_FALSE(attr.is_deleted()); EXPECT_TRUE(test_value == attr.value()); } - bool SyncerStuck(SyncProcessState* state) { - SyncerStatus status(NULL, state); - return status.syncer_stuck(); - } - void SyncRepeatedlyToTriggerConflictResolution(SyncProcessState* state) { - // We should trigger after less than 6 syncs, but we want to avoid brittle - // tests. + void SyncRepeatedlyToTriggerConflictResolution(SyncSession* session) { + // We should trigger after less than 6 syncs, but extra does no harm. for (int i = 0 ; i < 6 ; ++i) - syncer_->SyncShare(state); + syncer_->SyncShare(session); } - void SyncRepeatedlyToTriggerStuckSignal(SyncProcessState* state) { + void SyncRepeatedlyToTriggerStuckSignal(SyncSession* session) { // We should trigger after less than 10 syncs, but we want to avoid brittle // tests. for (int i = 0 ; i < 12 ; ++i) - syncer_->SyncShare(state); + syncer_->SyncShare(session); } // Enumeration of alterations to entries for commit ordering tests. @@ -302,14 +302,15 @@ class SyncerTest : public testing::Test { const vector<syncable::Id>& expected_id_order) { // The expected order is "x", "b", "c", "e", truncated appropriately. for (size_t limit = expected_id_order.size() + 2; limit > 0; --limit) { - SyncCycleState cycle_state; - SyncerSession session(&cycle_state, state_.get()); + StatusController* status = session_->status_controller(); + status->ResetTransientState(); WriteTransaction wtrans(dir, UNITTEST, __FILE__, __LINE__); - SyncerSession::ScopedSetWriteTransaction set_trans(&session, &wtrans); - session.set_unsynced_handles(unsynced_handle_view); + ScopedSetSessionWriteTransaction set_trans(session_.get(), &wtrans); + status->set_unsynced_handles(unsynced_handle_view); GetCommitIdsCommand command(limit); - command.BuildCommitIds(&session); + command.BuildCommitIds(session_->status_controller()->unsynced_handles(), + session_->write_transaction()); vector<syncable::Id> output = command.ordered_commit_set_.GetCommitIds(); size_t truncated_size = std::min(limit, expected_id_order.size()); ASSERT_TRUE(truncated_size == output.size()); @@ -354,14 +355,14 @@ class SyncerTest : public testing::Test { TestDirectorySetterUpper syncdb_; scoped_ptr<MockConnectionManager> mock_server_; scoped_ptr<EventListenerHookup> hookup_; - scoped_ptr<EventListenerHookup> command_channel_hookup_; - ClientCommandChannel client_command_channel_; Syncer* syncer_; - scoped_ptr<SyncProcessState> state_; - scoped_ptr<ModelSafeWorker> model_safe_worker_; + + scoped_ptr<SyncSession> session_; + scoped_ptr<SyncSessionContext> context_; std::set<SyncerEvent> syncer_events_; - sync_pb::ClientCommand last_client_command_; + base::TimeDelta last_short_poll_interval_received_; + base::TimeDelta last_long_poll_interval_received_; DISALLOW_COPY_AND_ASSIGN(SyncerTest); }; @@ -423,17 +424,17 @@ TEST_F(SyncerTest, GetCommitIdsCommandTruncates) { // TODO(chron): More corner case unit tests around validation. TEST_F(SyncerTest, TestCommitMetahandleIterator) { - SyncCycleState cycle_state; - SyncerSession session(&cycle_state, state_.get()); ScopedDirLookup dir(syncdb_.manager(), syncdb_.name()); ASSERT_TRUE(dir.good()); + StatusController* status = session_->status_controller(); + const vector<int64>& unsynced(status->unsynced_handles()); { WriteTransaction wtrans(dir, UNITTEST, __FILE__, __LINE__); - SyncerSession::ScopedSetWriteTransaction set_trans(&session, &wtrans); + ScopedSetSessionWriteTransaction set_trans(session_.get(), &wtrans); GetCommitIdsCommand::OrderedCommitSet commit_set; - GetCommitIdsCommand::CommitMetahandleIterator iterator(&session, + GetCommitIdsCommand::CommitMetahandleIterator iterator(unsynced, &wtrans, &commit_set); EXPECT_FALSE(iterator.Valid()); EXPECT_FALSE(iterator.Increment()); @@ -447,12 +448,12 @@ TEST_F(SyncerTest, TestCommitMetahandleIterator) { CreateUnsyncedDirectory(PSTR("test2"), "testid2")); session_metahandles.push_back( CreateUnsyncedDirectory(PSTR("test3"), "testid3")); - session.set_unsynced_handles(session_metahandles); + status->set_unsynced_handles(session_metahandles); WriteTransaction wtrans(dir, UNITTEST, __FILE__, __LINE__); - SyncerSession::ScopedSetWriteTransaction set_trans(&session, &wtrans); + ScopedSetSessionWriteTransaction set_trans(session_.get(), &wtrans); GetCommitIdsCommand::OrderedCommitSet commit_set; - GetCommitIdsCommand::CommitMetahandleIterator iterator(&session, + GetCommitIdsCommand::CommitMetahandleIterator iterator(unsynced, &wtrans, &commit_set); EXPECT_TRUE(iterator.Valid()); @@ -491,11 +492,9 @@ TEST_F(SyncerTest, TestGetUnsyncedAndSimpleCommit) { WriteTestDataToEntry(&wtrans, &child); } - SyncCycleState cycle_state; - SyncerSession session(&cycle_state, state_.get()); - - syncer_->SyncShare(&session); - EXPECT_TRUE(2 == session.unsynced_count()); + StatusController* status = session_->status_controller(); + syncer_->SyncShare(session_.get()); + EXPECT_TRUE(2 == status->unsynced_handles().size()); ASSERT_TRUE(2 == mock_server_->committed_ids().size()); // If this test starts failing, be aware other sort orders could be valid. EXPECT_TRUE(parent_id_ == mock_server_->committed_ids()[0]); @@ -699,10 +698,8 @@ TEST_F(SyncerTest, TestCommitListOrderingWithNesting) { } } - SyncCycleState cycle_state; - SyncerSession session(&cycle_state, state_.get()); - syncer_->SyncShare(&session); - EXPECT_TRUE(6 == session.unsynced_count()); + syncer_->SyncShare(session_.get()); + EXPECT_TRUE(6 == session_->status_controller()->unsynced_handles().size()); ASSERT_TRUE(6 == mock_server_->committed_ids().size()); // This test will NOT unroll deletes because SERVER_PARENT_ID is not set. // It will treat these like moves. @@ -765,10 +762,8 @@ TEST_F(SyncerTest, TestCommitListOrderingWithNewItems) { child.Put(syncable::BASE_VERSION, 1); } - SyncCycleState cycle_state; - SyncerSession session(&cycle_state, state_.get()); - syncer_->SyncShare(&session); - EXPECT_TRUE(6 == session.unsynced_count()); + syncer_->SyncShare(session_.get()); + EXPECT_TRUE(6 == session_->status_controller()->unsynced_handles().size()); ASSERT_TRUE(6 == mock_server_->committed_ids().size()); // If this test starts failing, be aware other sort orders could be valid. EXPECT_TRUE(parent_id_ == mock_server_->committed_ids()[0]); @@ -805,10 +800,8 @@ TEST_F(SyncerTest, TestCommitListOrderingCounterexample) { child2.Put(syncable::BASE_VERSION, 1); } - SyncCycleState cycle_state; - SyncerSession session(&cycle_state, state_.get()); - syncer_->SyncShare(&session); - EXPECT_TRUE(3 == session.unsynced_count()); + syncer_->SyncShare(session_.get()); + EXPECT_TRUE(3 == session_->status_controller()->unsynced_handles().size()); ASSERT_TRUE(3 == mock_server_->committed_ids().size()); // If this test starts failing, be aware other sort orders could be valid. EXPECT_TRUE(parent_id_ == mock_server_->committed_ids()[0]); @@ -852,11 +845,8 @@ TEST_F(SyncerTest, TestCommitListOrderingAndNewParent) { child.Put(syncable::BASE_VERSION, 1); } - SyncCycleState cycle_state; - SyncerSession session(&cycle_state, state_.get()); - - syncer_->SyncShare(&session); - EXPECT_TRUE(3 == session.unsynced_count()); + syncer_->SyncShare(session_.get()); + EXPECT_TRUE(3 == session_->status_controller()->unsynced_handles().size()); ASSERT_TRUE(3 == mock_server_->committed_ids().size()); // If this test starts failing, be aware other sort orders could be valid. EXPECT_TRUE(parent_id_ == mock_server_->committed_ids()[0]); @@ -927,11 +917,8 @@ TEST_F(SyncerTest, TestCommitListOrderingAndNewParentAndChild) { meta_handle_b = child.Get(syncable::META_HANDLE); } - SyncCycleState cycle_state; - SyncerSession session(&cycle_state, state_.get()); - - syncer_->SyncShare(&session); - EXPECT_TRUE(3 == session.unsynced_count()); + syncer_->SyncShare(session_.get()); + EXPECT_TRUE(3 == session_->status_controller()->unsynced_handles().size()); ASSERT_TRUE(3 == mock_server_->committed_ids().size()); // If this test starts failing, be aware other sort orders could be valid. EXPECT_TRUE(parent_id_ == mock_server_->committed_ids()[0]); @@ -969,11 +956,11 @@ TEST_F(SyncerTest, UpdateWithZeroLengthName) { mock_server_->AddUpdateDirectory(1, 0, "", 1, 10); // And one legal one that we're going to delete. mock_server_->AddUpdateDirectory(2, 0, "FOO", 1, 10); - syncer_->SyncShare(); + syncer_->SyncShare(this); // Delete the legal one. The new update has a null name. mock_server_->AddUpdateDirectory(2, 0, "", 2, 20); mock_server_->SetLastUpdateDeleted(); - syncer_->SyncShare(); + syncer_->SyncShare(this); } TEST_F(SyncerTest, DontGetStuckWithTwoSameNames) { @@ -982,10 +969,10 @@ TEST_F(SyncerTest, DontGetStuckWithTwoSameNames) { ScopedDirLookup dir(syncdb_.manager(), syncdb_.name()); ASSERT_TRUE(dir.good()); mock_server_->AddUpdateDirectory(1, 0, "foo:", 1, 10); - syncer_->SyncShare(); + syncer_->SyncShare(this); mock_server_->AddUpdateDirectory(2, 0, "foo:", 1, 20); - SyncRepeatedlyToTriggerStuckSignal(state_.get()); - EXPECT_FALSE(SyncerStuck(state_.get())); + SyncRepeatedlyToTriggerStuckSignal(session_.get()); + EXPECT_FALSE(session_->status_controller()->syncer_status().syncer_stuck); syncer_events_.clear(); } @@ -1010,7 +997,7 @@ TEST_F(SyncerTest, ExtendedAttributeWithNullCharacter) { mock_server_->AddUpdateBookmark(2, 0, "fred", 2, 10); mock_server_->AddUpdateBookmark(3, 0, "sue", 15, 10); - syncer_->SyncShare(); + syncer_->SyncShare(this); ReadTransaction trans(dir, __FILE__, __LINE__); Entry entry1(&trans, syncable::GET_BY_ID, ids_.FromNumber(1)); ASSERT_TRUE(entry1.good()); @@ -1044,8 +1031,7 @@ TEST_F(SyncerTest, TestBasicUpdate) { int64 timestamp = 10; mock_server_->AddUpdateDirectory(id, parent_id, name, version, timestamp); - syncer_->SyncShare(state_.get()); - SyncerStatus status(NULL, state_.get()); + syncer_->SyncShare(this); { WriteTransaction trans(dir, UNITTEST, __FILE__, __LINE__); Entry entry(&trans, GET_BY_ID, @@ -1078,12 +1064,11 @@ TEST_F(SyncerTest, IllegalAndLegalUpdates) { // until an item with the server ID "-80" arrives. mock_server_->AddUpdateDirectory(3, -80, "bad_parent", 10, 10); - syncer_->SyncShare(state_.get()); + syncer_->SyncShare(session_.get()); + StatusController* status = session_->status_controller(); - ConflictResolutionView conflict_view(state_.get()); - SyncerStatus status(NULL, state_.get()); // Id 3 should be in conflict now. - EXPECT_TRUE(1 == conflict_view.conflicting_updates()); + EXPECT_TRUE(1 == status->conflict_progress()->ConflictingItemsSize()); // These entries will be used in the second set of updates. mock_server_->AddUpdateDirectory(4, 0, "newer_version", 20, 10); @@ -1093,9 +1078,10 @@ TEST_F(SyncerTest, IllegalAndLegalUpdates) { mock_server_->AddUpdateDirectory(100, 9, "bad_parent_child2", 10, 10); mock_server_->AddUpdateDirectory(10, 0, "dir_to_bookmark", 10, 10); - syncer_->SyncShare(state_.get()); + syncer_->SyncShare(session_.get()); // The three items with an unresolved parent should be unapplied (3, 9, 100). - EXPECT_TRUE(3 == conflict_view.conflicting_updates()); + // The name clash should also still be in conflict. + EXPECT_TRUE(3 == status->conflict_progress()->ConflictingItemsSize()); { WriteTransaction trans(dir, UNITTEST, __FILE__, __LINE__); // Even though it has the same name, it should work. @@ -1128,11 +1114,11 @@ TEST_F(SyncerTest, IllegalAndLegalUpdates) { // Flip the is_dir bit: should fail verify & be dropped. mock_server_->AddUpdateBookmark(10, 0, "dir_to_bookmark", 20, 20); - syncer_->SyncShare(state_.get()); + syncer_->SyncShare(session_.get()); // Version number older than last known: should fail verify & be dropped. mock_server_->AddUpdateDirectory(4, 0, "old_version", 10, 10); - syncer_->SyncShare(state_.get()); + syncer_->SyncShare(session_.get()); { ReadTransaction trans(dir, __FILE__, __LINE__); @@ -1183,7 +1169,7 @@ TEST_F(SyncerTest, IllegalAndLegalUpdates) { } EXPECT_TRUE(0 == syncer_events_.size()); - EXPECT_TRUE(4 == conflict_view.conflicting_updates()); + EXPECT_TRUE(4 == status->conflict_progress()->ConflictingItemsSize()); } TEST_F(SyncerTest, CommitTimeRename) { @@ -1210,7 +1196,7 @@ TEST_F(SyncerTest, CommitTimeRename) { // Mix in a directory creation too for later. mock_server_->AddUpdateDirectory(2, 0, "dir_in_root", 10, 10); mock_server_->SetCommitTimeRename("renamed_"); - syncer_->SyncShare(); + syncer_->SyncShare(this); // Verify it was correctly renamed. { @@ -1252,7 +1238,7 @@ TEST_F(SyncerTest, CommitTimeRenameI18N) { } mock_server_->SetCommitTimeRename(i18nString); - syncer_->SyncShare(); + syncer_->SyncShare(this); // Verify it was correctly renamed. { @@ -1334,7 +1320,7 @@ TEST_F(SyncerTest, CommitReuniteUpdateAdjustsChildren) { mock_server_->set_conflict_all_commits(true); // Alright! Apply that update! - syncer_->SyncShare(); + syncer_->SyncShare(this); { // The folder's ID should have been updated. ReadTransaction trans(dir, __FILE__, __LINE__); @@ -1403,7 +1389,7 @@ TEST_F(SyncerTest, CommitReuniteUpdate) { mock_server_->set_conflict_all_commits(true); // Alright! Apply that update! - syncer_->SyncShare(); + syncer_->SyncShare(this); { ReadTransaction trans(dir, __FILE__, __LINE__); Entry entry(&trans, GET_BY_HANDLE, entry_metahandle); @@ -1469,7 +1455,7 @@ TEST_F(SyncerTest, CommitReuniteUpdateDoesNotChokeOnDeletedLocalEntry) { } // Just don't CHECK fail in sync, have the update split. - syncer_->SyncShare(); + syncer_->SyncShare(this); { ReadTransaction trans(dir, __FILE__, __LINE__); Id new_entry_id = GetOnlyEntryWithName( @@ -1491,7 +1477,7 @@ TEST_F(SyncerTest, ConflictMatchingEntryHandlesUnsanitizedNames) { mock_server_->AddUpdateDirectory(1, 0, "A/A", 10, 10); mock_server_->AddUpdateDirectory(2, 0, "B/B", 10, 10); mock_server_->set_conflict_all_commits(true); - syncer_->SyncShare(); + syncer_->SyncShare(this); { WriteTransaction wtrans(dir, UNITTEST, __FILE__, __LINE__); @@ -1533,7 +1519,7 @@ TEST_F(SyncerTest, ConflictMatchingEntryHandlesNormalNames) { mock_server_->AddUpdateDirectory(1, 0, "A", 10, 10); mock_server_->AddUpdateDirectory(2, 0, "B", 10, 10); mock_server_->set_conflict_all_commits(true); - syncer_->SyncShare(); + syncer_->SyncShare(this); { WriteTransaction wtrans(dir, UNITTEST, __FILE__, __LINE__); @@ -1592,7 +1578,7 @@ class EntryCreatedInNewFolderTest : public SyncerTest { ScopedDirLookup dir(syncdb_.manager(), syncdb_.name()); CHECK(dir.good()); - WriteTransaction trans(dir, UNITTEST, __FILE__, __LINE__); + WriteTransaction trans(dir, UNITTEST, __FILE__, __LINE__); MutableEntry bob(&trans, syncable::GET_BY_ID, GetOnlyEntryWithName(&trans, @@ -1600,12 +1586,12 @@ class EntryCreatedInNewFolderTest : public SyncerTest { PSTR("bob"))); CHECK(bob.good()); - MutableEntry entry2(&trans, syncable::CREATE, bob.Get(syncable::ID), - PSTR("bob")); - CHECK(entry2.good()); - entry2.Put(syncable::IS_DIR, true); - entry2.Put(syncable::IS_UNSYNCED, true); - } + MutableEntry entry2(&trans, syncable::CREATE, bob.Get(syncable::ID), + PSTR("bob")); + CHECK(entry2.good()); + entry2.Put(syncable::IS_DIR, true); + entry2.Put(syncable::IS_UNSYNCED, true); +} }; TEST_F(EntryCreatedInNewFolderTest, EntryCreatedInNewFolderMidSync) { @@ -1623,7 +1609,7 @@ TEST_F(EntryCreatedInNewFolderTest, EntryCreatedInNewFolderMidSync) { mock_server_->SetMidCommitCallback( NewCallback<EntryCreatedInNewFolderTest>(this, &EntryCreatedInNewFolderTest::CreateFolderInBob)); - syncer_->SyncShare(BUILD_COMMIT_REQUEST, SYNCER_END); + syncer_->SyncShare(BUILD_COMMIT_REQUEST, SYNCER_END, this); EXPECT_TRUE(1 == mock_server_->committed_ids().size()); { ReadTransaction trans(dir, __FILE__, __LINE__); @@ -1636,14 +1622,14 @@ TEST_F(EntryCreatedInNewFolderTest, EntryCreatedInNewFolderMidSync) { Entry child(&trans, syncable::GET_BY_ID, child_id); ASSERT_TRUE(child.good()); EXPECT_EQ(parent_entry.Get(ID), child.Get(PARENT_ID)); - } +} } TEST_F(SyncerTest, NegativeIDInUpdate) { ScopedDirLookup dir(syncdb_.manager(), syncdb_.name()); CHECK(dir.good()); mock_server_->AddUpdateBookmark(-10, 0, "bad", 40, 40); - syncer_->SyncShare(); + syncer_->SyncShare(this); // The negative id would make us CHECK! } @@ -1662,7 +1648,7 @@ TEST_F(SyncerTest, UnappliedUpdateOnCreatedItemItemDoesNotCrash) { WriteTestDataToEntry(&trans, &fred_match); } // Commit it. - syncer_->SyncShare(); + syncer_->SyncShare(this); EXPECT_TRUE(1 == mock_server_->committed_ids().size()); mock_server_->set_conflict_all_commits(true); syncable::Id fred_match_id; @@ -1678,7 +1664,7 @@ TEST_F(SyncerTest, UnappliedUpdateOnCreatedItemItemDoesNotCrash) { } // Run the syncer. for (int i = 0 ; i < 30 ; ++i) { - syncer_->SyncShare(); + syncer_->SyncShare(this); } } @@ -1687,6 +1673,7 @@ TEST_F(SyncerTest, UnappliedUpdateOnCreatedItemItemDoesNotCrash) { * the client and the server, the conflict resolver should just drop one of * them and accept the other. */ + TEST_F(SyncerTest, DoublyChangedWithResolver) { ScopedDirLookup dir(syncdb_.manager(), syncdb_.name()); CHECK(dir.good()); @@ -1739,7 +1726,7 @@ TEST_F(SyncerTest, CommitsUpdateDoesntAlterEntry) { entry.Put(syncable::MTIME, test_time); entry_metahandle = entry.Get(META_HANDLE); } - syncer_->SyncShare(); + syncer_->SyncShare(this); syncable::Id id; int64 version; int64 server_position_in_parent; @@ -1754,7 +1741,7 @@ TEST_F(SyncerTest, CommitsUpdateDoesntAlterEntry) { } mock_server_->AddUpdateDirectory(id, root_id_, "Pete", version, 10); mock_server_->SetLastUpdatePosition(server_position_in_parent); - syncer_->SyncShare(); + syncer_->SyncShare(this); { ReadTransaction trans(dir, __FILE__, __LINE__); Entry entry(&trans, syncable::GET_BY_ID, id); @@ -1789,9 +1776,9 @@ TEST_F(SyncerTest, ParentAndChildBothMatch) { mock_server_->AddUpdateDirectory(parent_id, root_id_, "Folder", 10, 10); mock_server_->AddUpdateBookmark(child_id, parent_id, "test.htm", 10, 10); mock_server_->set_conflict_all_commits(true); - syncer_->SyncShare(); - syncer_->SyncShare(); - syncer_->SyncShare(); + syncer_->SyncShare(this); + syncer_->SyncShare(this); + syncer_->SyncShare(this); { ReadTransaction trans(dir, __FILE__, __LINE__); Directory::ChildHandles children; @@ -1818,7 +1805,7 @@ TEST_F(SyncerTest, CommittingNewDeleted) { entry.Put(IS_UNSYNCED, true); entry.Put(IS_DEL, true); } - syncer_->SyncShare(); + syncer_->SyncShare(this); EXPECT_TRUE(0 == mock_server_->committed_ids().size()); } @@ -1846,10 +1833,11 @@ TEST_F(SyncerTest, UnappliedUpdateDuringCommit) { entry.Put(IS_UNAPPLIED_UPDATE, true); entry.Put(IS_DEL, false); } - syncer_->SyncShare(state_.get()); - syncer_->SyncShare(state_.get()); - SyncerStatus status(NULL, state_.get()); - EXPECT_TRUE(0 == status.conflicting_updates()); + syncer_->SyncShare(session_.get()); + syncer_->SyncShare(session_.get()); + const ConflictProgress* progress = + session_->status_controller()->conflict_progress(); + EXPECT_TRUE(0 == progress->ConflictingItemsSize()); syncer_events_.clear(); } @@ -1877,7 +1865,7 @@ TEST_F(SyncerTest, DeletingEntryInFolder) { entry.Put(IS_UNSYNCED, true); existing_metahandle = entry.Get(META_HANDLE); } - syncer_->SyncShare(state_.get()); + syncer_->SyncShare(session_.get()); { WriteTransaction trans(dir, UNITTEST, __FILE__, __LINE__); MutableEntry newfolder(&trans, CREATE, trans.root_id(), PSTR("new")); @@ -1894,9 +1882,9 @@ TEST_F(SyncerTest, DeletingEntryInFolder) { newfolder.Put(IS_DEL, true); existing.Put(IS_DEL, true); } - syncer_->SyncShare(state_.get()); - SyncerStatus status(NULL, state_.get()); - EXPECT_TRUE(0 == status.conflicting_commits()); + syncer_->SyncShare(session_.get()); + StatusController* status(session_->status_controller()); + EXPECT_TRUE(0 == status->error_counters().num_conflicting_commits); } TEST_F(SyncerTest, DeletingEntryWithLocalEdits) { @@ -1905,7 +1893,7 @@ TEST_F(SyncerTest, DeletingEntryWithLocalEdits) { int64 newfolder_metahandle; mock_server_->AddUpdateDirectory(1, 0, "bob", 1, 10); - syncer_->SyncShare(); + syncer_->SyncShare(this); { WriteTransaction trans(dir, UNITTEST, __FILE__, __LINE__); MutableEntry newfolder(&trans, CREATE, ids_.FromNumber(1), PSTR("local")); @@ -1915,7 +1903,7 @@ TEST_F(SyncerTest, DeletingEntryWithLocalEdits) { } mock_server_->AddUpdateDirectory(1, 0, "bob", 2, 20); mock_server_->SetLastUpdateDeleted(); - syncer_->SyncShare(SYNCER_BEGIN, APPLY_UPDATES); + syncer_->SyncShare(SYNCER_BEGIN, APPLY_UPDATES, this); { ReadTransaction trans(dir, __FILE__, __LINE__); Entry entry(&trans, syncable::GET_BY_HANDLE, newfolder_metahandle); @@ -1928,10 +1916,10 @@ TEST_F(SyncerTest, FolderSwapUpdate) { CHECK(dir.good()); mock_server_->AddUpdateDirectory(7801, 0, "bob", 1, 10); mock_server_->AddUpdateDirectory(1024, 0, "fred", 1, 10); - syncer_->SyncShare(); + syncer_->SyncShare(this); mock_server_->AddUpdateDirectory(1024, 0, "bob", 2, 20); mock_server_->AddUpdateDirectory(7801, 0, "fred", 2, 20); - syncer_->SyncShare(); + syncer_->SyncShare(this); { ReadTransaction trans(dir, __FILE__, __LINE__); Entry id1(&trans, GET_BY_ID, ids_.FromNumber(7801)); @@ -1952,7 +1940,7 @@ TEST_F(SyncerTest, NameCollidingFolderSwapWorksFine) { mock_server_->AddUpdateDirectory(7801, 0, "bob", 1, 10); mock_server_->AddUpdateDirectory(1024, 0, "fred", 1, 10); mock_server_->AddUpdateDirectory(4096, 0, "alice", 1, 10); - syncer_->SyncShare(); + syncer_->SyncShare(this); { ReadTransaction trans(dir, __FILE__, __LINE__); Entry id1(&trans, GET_BY_ID, ids_.FromNumber(7801)); @@ -1971,7 +1959,7 @@ TEST_F(SyncerTest, NameCollidingFolderSwapWorksFine) { mock_server_->AddUpdateDirectory(1024, 0, "bob", 2, 20); mock_server_->AddUpdateDirectory(7801, 0, "fred", 2, 20); mock_server_->AddUpdateDirectory(4096, 0, "bob", 2, 20); - syncer_->SyncShare(); + syncer_->SyncShare(this); { ReadTransaction trans(dir, __FILE__, __LINE__); Entry id1(&trans, GET_BY_ID, ids_.FromNumber(7801)); @@ -2006,7 +1994,7 @@ TEST_F(SyncerTest, CommitManyItemsInOneGo) { } } uint32 num_loops = 0; - while (syncer_->SyncShare()) { + while (syncer_->SyncShare(this)) { num_loops++; ASSERT_LT(num_loops, max_batches * 2); } @@ -2026,15 +2014,14 @@ TEST_F(SyncerTest, HugeConflict) { // Generate a huge deep tree which should all fail to apply at first. { WriteTransaction trans(dir, UNITTEST, __FILE__, __LINE__); - for (int i = 0; i < item_count; i++) { + for (int i = 0; i < item_count ; i++) { syncable::Id next_id = ids_.NewServerId(); tree_ids.push_back(next_id); mock_server_->AddUpdateDirectory(next_id, last_id, "BOB", 2, 20); last_id = next_id; } } - - syncer_->SyncShare(); + syncer_->SyncShare(this); // Check they're in the expected conflict state. { @@ -2051,7 +2038,7 @@ TEST_F(SyncerTest, HugeConflict) { // Add the missing parent directory. mock_server_->AddUpdateDirectory(parent_id, TestIdFactory::root(), "BOB", 2, 20); - syncer_->SyncShare(); + syncer_->SyncShare(this); // Now they should all be OK. { @@ -2069,7 +2056,7 @@ TEST_F(SyncerTest, DontCrashOnCaseChange) { ScopedDirLookup dir(syncdb_.manager(), syncdb_.name()); CHECK(dir.good()); mock_server_->AddUpdateDirectory(1, 0, "bob", 1, 10); - syncer_->SyncShare(); + syncer_->SyncShare(this); { WriteTransaction trans(dir, UNITTEST, __FILE__, __LINE__); MutableEntry e(&trans, GET_BY_ID, ids_.FromNumber(1)); @@ -2078,7 +2065,7 @@ TEST_F(SyncerTest, DontCrashOnCaseChange) { } mock_server_->set_conflict_all_commits(true); mock_server_->AddUpdateDirectory(1, 0, "BOB", 2, 20); - syncer_->SyncShare(); // USED TO CAUSE AN ASSERT + syncer_->SyncShare(this); // USED TO CAUSE AN ASSERT syncer_events_.clear(); } @@ -2086,10 +2073,10 @@ TEST_F(SyncerTest, UnsyncedItemAndUpdate) { ScopedDirLookup dir(syncdb_.manager(), syncdb_.name()); CHECK(dir.good()); mock_server_->AddUpdateDirectory(1, 0, "bob", 1, 10); - syncer_->SyncShare(); + syncer_->SyncShare(this); mock_server_->set_conflict_all_commits(true); mock_server_->AddUpdateDirectory(2, 0, "bob", 2, 20); - syncer_->SyncShare(); // USED TO CAUSE AN ASSERT + syncer_->SyncShare(this); // USED TO CAUSE AN ASSERT syncer_events_.clear(); } @@ -2097,7 +2084,7 @@ TEST_F(SyncerTest, NewEntryAndAlteredServerEntrySharePath) { ScopedDirLookup dir(syncdb_.manager(), syncdb_.name()); CHECK(dir.good()); mock_server_->AddUpdateBookmark(1, 0, "Foo.htm", 10, 10); - syncer_->SyncShare(); + syncer_->SyncShare(this); int64 local_folder_handle; syncable::Id local_folder_id; { @@ -2113,7 +2100,7 @@ TEST_F(SyncerTest, NewEntryAndAlteredServerEntrySharePath) { } mock_server_->AddUpdateBookmark(1, 0, "Bar.htm", 20, 20); mock_server_->set_conflict_all_commits(true); - syncer_->SyncShare(); + syncer_->SyncShare(this); syncer_events_.clear(); } @@ -2124,7 +2111,7 @@ TEST_F(SyncerTest, SiblingDirectoriesBecomeCircular) { CHECK(dir.good()); mock_server_->AddUpdateDirectory(1, 0, "A", 10, 10); mock_server_->AddUpdateDirectory(2, 0, "B", 10, 10); - syncer_->SyncShare(); + syncer_->SyncShare(this); { WriteTransaction wtrans(dir, UNITTEST, __FILE__, __LINE__); MutableEntry A(&wtrans, GET_BY_ID, ids_.FromNumber(1)); @@ -2135,7 +2122,7 @@ TEST_F(SyncerTest, SiblingDirectoriesBecomeCircular) { } mock_server_->AddUpdateDirectory(2, 1, "A", 20, 20); mock_server_->set_conflict_all_commits(true); - syncer_->SyncShare(); + syncer_->SyncShare(this); syncer_events_.clear(); { WriteTransaction wtrans(dir, UNITTEST, __FILE__, __LINE__); @@ -2156,7 +2143,7 @@ TEST_F(SyncerTest, ConflictSetClassificationError) { mock_server_->AddUpdateDirectory(1, 0, "A", 10, 10); mock_server_->AddUpdateDirectory(2, 0, "B", 10, 10); mock_server_->set_conflict_all_commits(true); - syncer_->SyncShare(); + syncer_->SyncShare(this); { WriteTransaction wtrans(dir, UNITTEST, __FILE__, __LINE__); MutableEntry A(&wtrans, GET_BY_ID, ids_.FromNumber(1)); @@ -2169,7 +2156,7 @@ TEST_F(SyncerTest, ConflictSetClassificationError) { B.Put(IS_UNAPPLIED_UPDATE, true); B.Put(SERVER_NON_UNIQUE_NAME, PSTR("A")); } - syncer_->SyncShare(); + syncer_->SyncShare(this); syncer_events_.clear(); } @@ -2180,7 +2167,7 @@ TEST_F(SyncerTest, SwapEntryNames) { mock_server_->AddUpdateDirectory(1, 0, "A", 10, 10); mock_server_->AddUpdateDirectory(2, 0, "B", 10, 10); mock_server_->set_conflict_all_commits(true); - syncer_->SyncShare(); + syncer_->SyncShare(this); { WriteTransaction wtrans(dir, UNITTEST, __FILE__, __LINE__); MutableEntry A(&wtrans, GET_BY_ID, ids_.FromNumber(1)); @@ -2193,7 +2180,7 @@ TEST_F(SyncerTest, SwapEntryNames) { ASSERT_TRUE(B.Put(NON_UNIQUE_NAME, PSTR("A"))); ASSERT_TRUE(A.Put(NON_UNIQUE_NAME, PSTR("B"))); } - syncer_->SyncShare(); + syncer_->SyncShare(this); syncer_events_.clear(); } @@ -2203,7 +2190,7 @@ TEST_F(SyncerTest, DualDeletionWithNewItemNameClash) { mock_server_->AddUpdateDirectory(1, 0, "A", 10, 10); mock_server_->AddUpdateBookmark(2, 0, "B", 10, 10); mock_server_->set_conflict_all_commits(true); - syncer_->SyncShare(); + syncer_->SyncShare(this); { WriteTransaction trans(dir, UNITTEST, __FILE__, __LINE__); MutableEntry B(&trans, GET_BY_ID, ids_.FromNumber(2)); @@ -2213,7 +2200,7 @@ TEST_F(SyncerTest, DualDeletionWithNewItemNameClash) { } mock_server_->AddUpdateBookmark(2, 0, "A", 11, 11); mock_server_->SetLastUpdateDeleted(); - syncer_->SyncShare(); + syncer_->SyncShare(this); { ReadTransaction trans(dir, __FILE__, __LINE__); Entry B(&trans, GET_BY_ID, ids_.FromNumber(2)); @@ -2229,7 +2216,7 @@ TEST_F(SyncerTest, FixDirectoryLoopConflict) { CHECK(dir.good()); mock_server_->AddUpdateDirectory(1, 0, "bob", 1, 10); mock_server_->AddUpdateDirectory(2, 0, "fred", 1, 10); - syncer_->SyncShare(); + syncer_->SyncShare(this); { WriteTransaction trans(dir, UNITTEST, __FILE__, __LINE__); MutableEntry bob(&trans, GET_BY_ID, ids_.FromNumber(1)); @@ -2239,8 +2226,8 @@ TEST_F(SyncerTest, FixDirectoryLoopConflict) { } mock_server_->AddUpdateDirectory(2, 1, "fred", 2, 20); mock_server_->set_conflict_all_commits(true); - syncer_->SyncShare(); - syncer_->SyncShare(); + syncer_->SyncShare(this); + syncer_->SyncShare(this); { ReadTransaction trans(dir, __FILE__, __LINE__); Entry bob(&trans, GET_BY_ID, ids_.FromNumber(1)); @@ -2262,7 +2249,7 @@ TEST_F(SyncerTest, ResolveWeWroteTheyDeleted) { int64 bob_metahandle; mock_server_->AddUpdateBookmark(1, 0, "bob", 1, 10); - syncer_->SyncShare(); + syncer_->SyncShare(this); { WriteTransaction trans(dir, UNITTEST, __FILE__, __LINE__); MutableEntry bob(&trans, GET_BY_ID, ids_.FromNumber(1)); @@ -2273,8 +2260,8 @@ TEST_F(SyncerTest, ResolveWeWroteTheyDeleted) { mock_server_->AddUpdateBookmark(1, 0, "bob", 2, 10); mock_server_->SetLastUpdateDeleted(); mock_server_->set_conflict_all_commits(true); - syncer_->SyncShare(); - syncer_->SyncShare(); + syncer_->SyncShare(this); + syncer_->SyncShare(this); { ReadTransaction trans(dir, __FILE__, __LINE__); Entry bob(&trans, GET_BY_HANDLE, bob_metahandle); @@ -2298,7 +2285,7 @@ TEST_F(SyncerTest, ServerDeletingFolderWeHaveMovedSomethingInto) { "bob", 1, 10); mock_server_->AddUpdateDirectory(fred_id, TestIdFactory::root(), "fred", 1, 10); - syncer_->SyncShare(); + syncer_->SyncShare(this); { WriteTransaction trans(dir, UNITTEST, __FILE__, __LINE__); MutableEntry bob(&trans, GET_BY_ID, bob_id); @@ -2310,8 +2297,8 @@ TEST_F(SyncerTest, ServerDeletingFolderWeHaveMovedSomethingInto) { "fred", 2, 20); mock_server_->SetLastUpdateDeleted(); mock_server_->set_conflict_all_commits(true); - syncer_->SyncShare(); - syncer_->SyncShare(); + syncer_->SyncShare(this); + syncer_->SyncShare(this); { ReadTransaction trans(dir, __FILE__, __LINE__); @@ -2346,7 +2333,7 @@ TEST_F(SyncerTest, DISABLED_ServerDeletingFolderWeHaveAnOpenEntryIn) { CHECK(dir.good()); mock_server_->AddUpdateBookmark(1, 0, "bob", 1, 10); mock_server_->AddUpdateDirectory(2, 0, "fred", 1, 10); - syncer_->SyncShare(state_.get()); + syncer_->SyncShare(this); { WriteTransaction trans(dir, UNITTEST, __FILE__, __LINE__); MutableEntry bob(&trans, GET_BY_ID, ids_.FromNumber(1)); @@ -2354,7 +2341,7 @@ TEST_F(SyncerTest, DISABLED_ServerDeletingFolderWeHaveAnOpenEntryIn) { bob.Put(IS_UNSYNCED, true); WriteTestDataToEntry(&trans, &bob); } - syncer_->SyncShare(state_.get()); + syncer_->SyncShare(this); { WriteTransaction trans(dir, UNITTEST, __FILE__, __LINE__); MutableEntry bob(&trans, GET_BY_ID, ids_.FromNumber(1)); @@ -2368,14 +2355,14 @@ TEST_F(SyncerTest, DISABLED_ServerDeletingFolderWeHaveAnOpenEntryIn) { mock_server_->set_conflict_all_commits(true); syncer_events_.clear(); // These SyncShares would cause a CHECK because we'd think we were stuck. - syncer_->SyncShare(state_.get()); - syncer_->SyncShare(state_.get()); - syncer_->SyncShare(state_.get()); - syncer_->SyncShare(state_.get()); - syncer_->SyncShare(state_.get()); - syncer_->SyncShare(state_.get()); - syncer_->SyncShare(state_.get()); - syncer_->SyncShare(state_.get()); + syncer_->SyncShare(this); + syncer_->SyncShare(this); + syncer_->SyncShare(this); + syncer_->SyncShare(this); + syncer_->SyncShare(this); + syncer_->SyncShare(this); + syncer_->SyncShare(this); + syncer_->SyncShare(this); EXPECT_TRUE(0 == syncer_events_.size()); { ReadTransaction trans(dir, __FILE__, __LINE__); @@ -2405,7 +2392,7 @@ TEST_F(SyncerTest, WeMovedSomethingIntoAFolderServerHasDeleted) { "bob", 1, 10); mock_server_->AddUpdateDirectory(fred_id, TestIdFactory::root(), "fred", 1, 10); - syncer_->SyncShare(); + syncer_->SyncShare(this); { WriteTransaction trans(dir, UNITTEST, __FILE__, __LINE__); Entry fred(&trans, GET_BY_ID, fred_id); @@ -2420,8 +2407,8 @@ TEST_F(SyncerTest, WeMovedSomethingIntoAFolderServerHasDeleted) { "fred", 2, 20); mock_server_->SetLastUpdateDeleted(); mock_server_->set_conflict_all_commits(true); - syncer_->SyncShare(); - syncer_->SyncShare(); + syncer_->SyncShare(this); + syncer_->SyncShare(this); { ReadTransaction trans(dir, __FILE__, __LINE__); Entry bob(&trans, GET_BY_ID, bob_id); @@ -2470,24 +2457,24 @@ class FolderMoveDeleteRenameTest : public SyncerTest { CHECK(dir.good()); if (--move_bob_count_ > 0) { - return false; + return false; } if (move_bob_count_ == 0) { - WriteTransaction trans(dir, UNITTEST, __FILE__, __LINE__); + WriteTransaction trans(dir, UNITTEST, __FILE__, __LINE__); Entry alice(&trans, GET_BY_ID, TestIdFactory::FromNumber(fred_id_number)); - CHECK(alice.good()); - CHECK(!alice.Get(IS_DEL)); + CHECK(alice.good()); + CHECK(!alice.Get(IS_DEL)); MutableEntry bob(&trans, GET_BY_ID, TestIdFactory::FromNumber(bob_id_number)); - CHECK(bob.good()); - bob.Put(IS_UNSYNCED, true); - bob.Put(PARENT_ID, alice.Get(ID)); - return true; - } - return false; + CHECK(bob.good()); + bob.Put(IS_UNSYNCED, true); + bob.Put(PARENT_ID, alice.Get(ID)); + return true; } + return false; +} }; TEST_F(FolderMoveDeleteRenameTest, @@ -2504,7 +2491,7 @@ TEST_F(FolderMoveDeleteRenameTest, "bob", 1, 10); mock_server_->AddUpdateDirectory(fred_id, TestIdFactory::root(), "fred", 1, 10); - syncer_->SyncShare(); + syncer_->SyncShare(this); { WriteTransaction trans(dir, UNITTEST, __FILE__, __LINE__); MutableEntry fred(&trans, GET_BY_ID, fred_id); @@ -2523,9 +2510,9 @@ TEST_F(FolderMoveDeleteRenameTest, mock_server_->SetMidCommitCallback( NewCallback<FolderMoveDeleteRenameTest>(this, &FolderMoveDeleteRenameTest::MoveBobIntoID2Runner)); - syncer_->SyncShare(); - syncer_->SyncShare(); - syncer_->SyncShare(); + syncer_->SyncShare(this); + syncer_->SyncShare(this); + syncer_->SyncShare(this); { ReadTransaction trans(dir, __FILE__, __LINE__); Entry bob(&trans, GET_BY_ID, bob_id); @@ -2563,7 +2550,7 @@ TEST_F(SyncerTest, "bob", 1, 10); mock_server_->AddUpdateDirectory(fred_id, TestIdFactory::root(), "fred", 1, 10); - syncer_->SyncShare(); + syncer_->SyncShare(this); syncable::Id new_item_id; { WriteTransaction trans(dir, UNITTEST, __FILE__, __LINE__); @@ -2579,8 +2566,8 @@ TEST_F(SyncerTest, "fred", 2, 20); mock_server_->SetLastUpdateDeleted(); mock_server_->set_conflict_all_commits(true); - syncer_->SyncShare(); - syncer_->SyncShare(); + syncer_->SyncShare(this); + syncer_->SyncShare(this); { ReadTransaction trans(dir, __FILE__, __LINE__); @@ -2648,7 +2635,7 @@ TEST_F(SyncerTest, ServerMovedAFolderIntoAFolderWeHaveDeletedAndMovedIntoIt) { CHECK(dir.good()); mock_server_->AddUpdateDirectory(1, 0, "bob", 1, 10); mock_server_->AddUpdateDirectory(2, 0, "fred", 1, 10); - syncer_->SyncShare(); + syncer_->SyncShare(this); { WriteTransaction trans(dir, UNITTEST, __FILE__, __LINE__); MutableEntry bob(&trans, GET_BY_ID, ids_.FromNumber(1)); @@ -2659,8 +2646,8 @@ TEST_F(SyncerTest, ServerMovedAFolderIntoAFolderWeHaveDeletedAndMovedIntoIt) { } mock_server_->AddUpdateDirectory(2, 1, "fred", 2, 20); mock_server_->set_conflict_all_commits(true); - syncer_->SyncShare(); - syncer_->SyncShare(); + syncer_->SyncShare(this); + syncer_->SyncShare(this); { ReadTransaction trans(dir, __FILE__, __LINE__); Entry bob(&trans, GET_BY_ID, ids_.FromNumber(1)); @@ -2816,15 +2803,15 @@ class SusanDeletingTest : public SyncerTest { const syncable::Id susan_id = TestIdFactory::FromNumber(susan_int_id_); ASSERT_GT(countdown_till_delete_, 0); if (0 != --countdown_till_delete_) - return; - WriteTransaction trans(dir, UNITTEST, __FILE__, __LINE__); + return; + WriteTransaction trans(dir, UNITTEST, __FILE__, __LINE__); MutableEntry susan(&trans, GET_BY_ID, susan_id); - Directory::ChildHandles children; - dir->GetChildHandles(&trans, susan.Get(ID), &children); - ASSERT_TRUE(0 == children.size()); - susan.Put(IS_DEL, true); - susan.Put(IS_UNSYNCED, true); - } + Directory::ChildHandles children; + dir->GetChildHandles(&trans, susan.Get(ID), &children); + ASSERT_TRUE(0 == children.size()); + susan.Put(IS_DEL, true); + susan.Put(IS_UNSYNCED, true); +} protected: int countdown_till_delete_; @@ -2865,8 +2852,8 @@ TEST_F(SusanDeletingTest, syncer_->pre_conflict_resolution_closure_ = NewCallback<SusanDeletingTest>(this, &SusanDeletingTest::DeleteSusanInRoot); - syncer_->SyncShare(); - syncer_->SyncShare(); + syncer_->SyncShare(this); + syncer_->SyncShare(this); { ReadTransaction trans(dir, __FILE__, __LINE__); Entry bob(&trans, GET_BY_ID, bob_id); @@ -2929,7 +2916,7 @@ TEST_F(SyncerTest, WeMovedSomethingIntoAFolderHierarchyServerHasDeleted) { mock_server_->AddUpdateDirectory(fred_id, TestIdFactory::root(), "fred", 1, 10); mock_server_->AddUpdateDirectory(alice_id, fred_id, "alice", 1, 10); - syncer_->SyncShare(); + syncer_->SyncShare(this); { WriteTransaction trans(dir, UNITTEST, __FILE__, __LINE__); MutableEntry bob(&trans, GET_BY_ID, bob_id); @@ -2944,8 +2931,8 @@ TEST_F(SyncerTest, WeMovedSomethingIntoAFolderHierarchyServerHasDeleted) { "alice", 2, 20); mock_server_->SetLastUpdateDeleted(); mock_server_->set_conflict_all_commits(true); - syncer_->SyncShare(); - syncer_->SyncShare(); + syncer_->SyncShare(this); + syncer_->SyncShare(this); { // Bob is the entry at the bottom of the tree. // The tree should be regenerated and old IDs removed. @@ -3001,7 +2988,7 @@ TEST_F(SyncerTest, WeMovedSomethingIntoAFolderHierarchyServerHasDeleted2) { "susan", 1, 10); mock_server_->AddUpdateDirectory(fred_id, susan_id, "fred", 1, 10); mock_server_->AddUpdateDirectory(alice_id, fred_id, "alice", 1, 10); - syncer_->SyncShare(); + syncer_->SyncShare(this); { WriteTransaction trans(dir, UNITTEST, __FILE__, __LINE__); MutableEntry bob(&trans, GET_BY_ID, bob_id); @@ -3016,8 +3003,8 @@ TEST_F(SyncerTest, WeMovedSomethingIntoAFolderHierarchyServerHasDeleted2) { "alice", 2, 20); mock_server_->SetLastUpdateDeleted(); mock_server_->set_conflict_all_commits(true); - syncer_->SyncShare(); - syncer_->SyncShare(); + syncer_->SyncShare(this); + syncer_->SyncShare(this); { // Root // |- Susan @@ -3084,9 +3071,9 @@ TEST_F(SyncerTest, DuplicateIDReturn) { } mock_server_->set_next_new_id(10000); EXPECT_TRUE(1 == dir->unsynced_entity_count()); - syncer_->SyncShare(); // we get back a bad id in here (should never happen). + syncer_->SyncShare(this); // we get back a bad id in here (should never happen). EXPECT_TRUE(1 == dir->unsynced_entity_count()); - syncer_->SyncShare(); // another bad id in here. + syncer_->SyncShare(this); // another bad id in here. EXPECT_TRUE(0 == dir->unsynced_entity_count()); syncer_events_.clear(); } @@ -3095,7 +3082,7 @@ TEST_F(SyncerTest, DeletedEntryWithBadParentInLoopCalculation) { ScopedDirLookup dir(syncdb_.manager(), syncdb_.name()); ASSERT_TRUE(dir.good()); mock_server_->AddUpdateDirectory(1, 0, "bob", 1, 10); - syncer_->SyncShare(); + syncer_->SyncShare(this); { WriteTransaction trans(dir, UNITTEST, __FILE__, __LINE__); MutableEntry bob(&trans, GET_BY_ID, ids_.FromNumber(1)); @@ -3106,8 +3093,8 @@ TEST_F(SyncerTest, DeletedEntryWithBadParentInLoopCalculation) { bob.Put(IS_UNSYNCED, true); } mock_server_->AddUpdateDirectory(2, 1, "fred", 1, 10); - syncer_->SyncShare(); - syncer_->SyncShare(); + syncer_->SyncShare(this); + syncer_->SyncShare(this); } TEST_F(SyncerTest, ConflictResolverMergeOverwritesLocalEntry) { @@ -3140,11 +3127,8 @@ TEST_F(SyncerTest, ConflictResolverMergeOverwritesLocalEntry) { conflict_set.push_back(ids_.FromNumber(3)); } { - SyncCycleState cycle_state; - SyncerSession session(&cycle_state, state_.get()); WriteTransaction trans(dir, UNITTEST, __FILE__, __LINE__); - syncer_->conflict_resolver()->ProcessConflictSet(&trans, &conflict_set, 50, - &session); + context_->resolver()->ProcessConflictSet(&trans, &conflict_set, 50); } } @@ -3168,7 +3152,7 @@ TEST_F(SyncerTest, ConflictResolverMergesLocalDeleteAndServerUpdate) { // We don't care about actually committing, just the resolution. mock_server_->set_conflict_all_commits(true); - syncer_->SyncShare(); + syncer_->SyncShare(this); { ReadTransaction trans(dir, __FILE__, __LINE__); @@ -3206,7 +3190,7 @@ TEST_F(SyncerTest, UpdateFlipsTheFolderBit) { mock_server_->set_conflict_all_commits(true); // The syncer should not attempt to apply the invalid update. - syncer_->SyncShare(); + syncer_->SyncShare(this); { ReadTransaction trans(dir, __FILE__, __LINE__); @@ -3225,7 +3209,7 @@ TEST(SyncerSyncProcessState, MergeSetsTest) { for (int i = 1; i < 7; i++) { id[i] = id_factory.NewServerId(); } - SyncProcessState c; + ConflictProgress c; c.MergeSets(id[1], id[2]); c.MergeSets(id[2], id[3]); c.MergeSets(id[4], id[5]); @@ -3243,7 +3227,7 @@ TEST(SyncerSyncProcessState, MergeSetsTest) { } // Check dupes don't cause double sets. - SyncProcessState identical_set; + ConflictProgress identical_set; identical_set.MergeSets(id[1], id[1]); EXPECT_TRUE(identical_set.IdToConflictSetSize() == 1); EXPECT_TRUE(identical_set.IdToConflictSetGet(id[1])->size() == 1); @@ -3257,14 +3241,14 @@ TEST_F(SyncerTest, MergingExistingItems) { CHECK(dir.good()); mock_server_->set_conflict_all_commits(true); mock_server_->AddUpdateBookmark(1, 0, "base", 10, 10); - syncer_->SyncShare(); + syncer_->SyncShare(this); { WriteTransaction trans(dir, UNITTEST, __FILE__, __LINE__); MutableEntry entry(&trans, CREATE, trans.root_id(), PSTR("Copy of base")); WriteTestDataToEntry(&trans, &entry); } mock_server_->AddUpdateBookmark(1, 0, "Copy of base", 50, 50); - SyncRepeatedlyToTriggerConflictResolution(state_.get()); + SyncRepeatedlyToTriggerConflictResolution(session_.get()); } // In this test a long changelog contains a child at the start of the changelog @@ -3282,14 +3266,14 @@ TEST_F(SyncerTest, LongChangelistWithApplicationConflict) { mock_server_->AddUpdateDirectory(stuck_entry_id, folder_id, "stuck", 1, 1); mock_server_->SetChangesRemaining(DEPTH - 1); - syncer_->SyncShare(state_.get()); + syncer_->SyncShare(session_.get()); // Very long changelist. We should never be stuck. for (int i = 0; i < DEPTH; i++) { mock_server_->SetNewTimestamp(i); mock_server_->SetChangesRemaining(DEPTH - i); - syncer_->SyncShare(state_.get()); - EXPECT_FALSE(SyncerStuck(state_.get())); + syncer_->SyncShare(session_.get()); + EXPECT_FALSE(session_->status_controller()->syncer_status().syncer_stuck); // Ensure our folder hasn't somehow applied. ReadTransaction trans(dir, __FILE__, __LINE__); @@ -3324,7 +3308,7 @@ TEST_F(SyncerTest, DontMergeTwoExistingItems) { mock_server_->set_conflict_all_commits(true); mock_server_->AddUpdateBookmark(1, 0, "base", 10, 10); mock_server_->AddUpdateBookmark(2, 0, "base2", 10, 10); - syncer_->SyncShare(); + syncer_->SyncShare(this); { WriteTransaction trans(dir, UNITTEST, __FILE__, __LINE__); MutableEntry entry(&trans, GET_BY_ID, ids_.FromNumber(2)); @@ -3333,7 +3317,7 @@ TEST_F(SyncerTest, DontMergeTwoExistingItems) { entry.Put(IS_UNSYNCED, true); } mock_server_->AddUpdateBookmark(1, 0, "Copy of base", 50, 50); - SyncRepeatedlyToTriggerConflictResolution(state_.get()); + SyncRepeatedlyToTriggerConflictResolution(session_.get()); { ReadTransaction trans(dir, __FILE__, __LINE__); Entry entry1(&trans, GET_BY_ID, ids_.FromNumber(1)); @@ -3354,10 +3338,10 @@ TEST_F(SyncerTest, TestUndeleteUpdate) { mock_server_->set_conflict_all_commits(true); mock_server_->AddUpdateDirectory(1, 0, "foo", 1, 1); mock_server_->AddUpdateDirectory(2, 1, "bar", 1, 2); - syncer_->SyncShare(); + syncer_->SyncShare(this); mock_server_->AddUpdateDirectory(2, 1, "bar", 2, 3); mock_server_->SetLastUpdateDeleted(); - syncer_->SyncShare(); + syncer_->SyncShare(this); { ReadTransaction trans(dir, __FILE__, __LINE__); Entry entry(&trans, GET_BY_ID, ids_.FromNumber(2)); @@ -3366,11 +3350,11 @@ TEST_F(SyncerTest, TestUndeleteUpdate) { } mock_server_->AddUpdateDirectory(1, 0, "foo", 2, 4); mock_server_->SetLastUpdateDeleted(); - syncer_->SyncShare(); + syncer_->SyncShare(this); // This used to be rejected as it's an undeletion. Now, it results in moving // the delete path aside. mock_server_->AddUpdateDirectory(2, 1, "bar", 3, 5); - syncer_->SyncShare(); + syncer_->SyncShare(this); { ReadTransaction trans(dir, __FILE__, __LINE__); Entry entry(&trans, GET_BY_ID, ids_.FromNumber(2)); @@ -3386,7 +3370,7 @@ TEST_F(SyncerTest, TestMoveSanitizedNamedFolder) { EXPECT_TRUE(dir.good()); mock_server_->AddUpdateDirectory(1, 0, "foo", 1, 1); mock_server_->AddUpdateDirectory(2, 0, ":::", 1, 2); - syncer_->SyncShare(); + syncer_->SyncShare(this); { WriteTransaction trans(dir, UNITTEST, __FILE__, __LINE__); MutableEntry entry(&trans, GET_BY_ID, ids_.FromNumber(2)); @@ -3394,10 +3378,10 @@ TEST_F(SyncerTest, TestMoveSanitizedNamedFolder) { EXPECT_TRUE(entry.Put(PARENT_ID, ids_.FromNumber(1))); EXPECT_TRUE(entry.Put(IS_UNSYNCED, true)); } - syncer_->SyncShare(); + syncer_->SyncShare(this); // We use the same sync ts as before so our times match up. mock_server_->AddUpdateDirectory(2, 1, ":::", 2, 2); - syncer_->SyncShare(); + syncer_->SyncShare(this); } TEST(SortedCollectionsIntersect, SortedCollectionsIntersectTest) { @@ -3425,7 +3409,7 @@ TEST_F(SyncerTest, UpdateWhereParentIsNotAFolder) { mock_server_->AddUpdateBookmark(1, 0, "B", 10, 10); mock_server_->AddUpdateDirectory(2, 1, "BookmarkParent", 10, 10); // Used to cause a CHECK - syncer_->SyncShare(); + syncer_->SyncShare(this); { ReadTransaction rtrans(dir, __FILE__, __LINE__); Entry good_entry(&rtrans, syncable::GET_BY_ID, ids_.FromNumber(1)); @@ -3450,7 +3434,7 @@ TEST_F(SyncerTest, DirectoryUpdateTest) { "in_root_name", 2, 2); mock_server_->AddUpdateDirectory(in_in_root_id, in_root_id, "in_in_root_name", 3, 3); - syncer_->SyncShare(); + syncer_->SyncShare(this); { ReadTransaction trans(dir, __FILE__, __LINE__); Entry in_root(&trans, GET_BY_ID, in_root_id); @@ -3489,7 +3473,7 @@ TEST_F(SyncerTest, DirectoryCommitTest) { bar_metahandle = child.Get(META_HANDLE); in_dir_id = parent.Get(syncable::ID); } - syncer_->SyncShare(); + syncer_->SyncShare(this); { ReadTransaction trans(dir, __FILE__, __LINE__); Entry fail_by_old_id_entry(&trans, GET_BY_ID, in_root_id); @@ -3516,7 +3500,7 @@ TEST_F(SyncerTest, ConflictSetSizeReducedToOne) { mock_server_->AddUpdateBookmark(in_root_id, TestIdFactory::root(), "in_root", 1, 1); - syncer_->SyncShare(); + syncer_->SyncShare(this); { WriteTransaction trans(dir, UNITTEST, __FILE__, __LINE__); MutableEntry oentry(&trans, GET_BY_ID, in_root_id); @@ -3529,7 +3513,7 @@ TEST_F(SyncerTest, ConflictSetSizeReducedToOne) { } mock_server_->set_conflict_all_commits(true); // This SyncShare call used to result in a CHECK failure. - syncer_->SyncShare(); + syncer_->SyncShare(this); syncer_events_.clear(); } @@ -3542,23 +3526,23 @@ TEST_F(SyncerTest, TestClientCommand) { command->set_set_sync_poll_interval(8); command->set_set_sync_long_poll_interval(800); mock_server_->AddUpdateDirectory(1, 0, "in_root", 1, 1); - syncer_->SyncShare(); + syncer_->SyncShare(this); - EXPECT_TRUE(last_client_command_.has_set_sync_poll_interval()); - EXPECT_TRUE(last_client_command_.has_set_sync_long_poll_interval()); - EXPECT_TRUE(8 == last_client_command_.set_sync_poll_interval()); - EXPECT_TRUE(800 == last_client_command_.set_sync_long_poll_interval()); + EXPECT_TRUE(TimeDelta::FromSeconds(8) == + last_short_poll_interval_received_); + EXPECT_TRUE(TimeDelta::FromSeconds(800) == + last_long_poll_interval_received_); command = mock_server_->GetNextClientCommand(); command->set_set_sync_poll_interval(180); command->set_set_sync_long_poll_interval(190); mock_server_->AddUpdateDirectory(1, 0, "in_root", 1, 1); - syncer_->SyncShare(); + syncer_->SyncShare(this); - EXPECT_TRUE(last_client_command_.has_set_sync_poll_interval()); - EXPECT_TRUE(last_client_command_.has_set_sync_long_poll_interval()); - EXPECT_TRUE(180 == last_client_command_.set_sync_poll_interval()); - EXPECT_TRUE(190 == last_client_command_.set_sync_long_poll_interval()); + EXPECT_TRUE(TimeDelta::FromSeconds(180) == + last_short_poll_interval_received_); + EXPECT_TRUE(TimeDelta::FromSeconds(190) == + last_long_poll_interval_received_); } TEST_F(SyncerTest, EnsureWeSendUpOldParent) { @@ -3572,7 +3556,7 @@ TEST_F(SyncerTest, EnsureWeSendUpOldParent) { "folder_one", 1, 1); mock_server_->AddUpdateDirectory(folder_two_id, TestIdFactory::root(), "folder_two", 1, 1); - syncer_->SyncShare(); + syncer_->SyncShare(this); { // A moved entry should send an old parent. WriteTransaction trans(dir, UNITTEST, __FILE__, __LINE__); @@ -3584,7 +3568,7 @@ TEST_F(SyncerTest, EnsureWeSendUpOldParent) { MutableEntry create(&trans, CREATE, trans.root_id(), PSTR("new_folder")); create.Put(IS_UNSYNCED, true); } - syncer_->SyncShare(); + syncer_->SyncShare(this); const sync_pb::CommitMessage& commit = mock_server_->last_sent_commit(); ASSERT_TRUE(2 == commit.entries_size()); EXPECT_TRUE(commit.entries(0).parent_id_string() == "2"); @@ -3623,7 +3607,7 @@ TEST_F(SyncerTest, TestSimpleUndelete) { mock_server_->set_conflict_all_commits(true); // Let there be an entry from the server. mock_server_->AddUpdateBookmark(id, root, "foo", 1, 10); - syncer_->SyncShare(); + syncer_->SyncShare(this); // Check it out and delete it. { WriteTransaction wtrans(dir, UNITTEST, __FILE__, __LINE__); @@ -3635,7 +3619,7 @@ TEST_F(SyncerTest, TestSimpleUndelete) { // Delete it locally. entry.Put(IS_DEL, true); } - syncer_->SyncShare(); + syncer_->SyncShare(this); // Confirm we see IS_DEL and not SERVER_IS_DEL. { ReadTransaction trans(dir, __FILE__, __LINE__); @@ -3646,11 +3630,11 @@ TEST_F(SyncerTest, TestSimpleUndelete) { EXPECT_TRUE(entry.Get(IS_DEL)); EXPECT_FALSE(entry.Get(SERVER_IS_DEL)); } - syncer_->SyncShare(); + syncer_->SyncShare(this); // Update from server confirming deletion. mock_server_->AddUpdateBookmark(id, root, "foo", 2, 11); mock_server_->SetLastUpdateDeleted(); - syncer_->SyncShare(); + syncer_->SyncShare(this); // IS_DEL AND SERVER_IS_DEL now both true. { ReadTransaction trans(dir, __FILE__, __LINE__); @@ -3663,7 +3647,7 @@ TEST_F(SyncerTest, TestSimpleUndelete) { } // Undelete from server. mock_server_->AddUpdateBookmark(id, root, "foo", 2, 12); - syncer_->SyncShare(); + syncer_->SyncShare(this); // IS_DEL and SERVER_IS_DEL now both false. { ReadTransaction trans(dir, __FILE__, __LINE__); @@ -3683,7 +3667,7 @@ TEST_F(SyncerTest, TestUndeleteWithMissingDeleteUpdate) { // Let there be a entry, from the server. mock_server_->set_conflict_all_commits(true); mock_server_->AddUpdateBookmark(id, root, "foo", 1, 10); - syncer_->SyncShare(); + syncer_->SyncShare(this); // Check it out and delete it. { WriteTransaction wtrans(dir, UNITTEST, __FILE__, __LINE__); @@ -3695,7 +3679,7 @@ TEST_F(SyncerTest, TestUndeleteWithMissingDeleteUpdate) { // Delete it locally. entry.Put(IS_DEL, true); } - syncer_->SyncShare(); + syncer_->SyncShare(this); // Confirm we see IS_DEL and not SERVER_IS_DEL. { ReadTransaction trans(dir, __FILE__, __LINE__); @@ -3706,11 +3690,11 @@ TEST_F(SyncerTest, TestUndeleteWithMissingDeleteUpdate) { EXPECT_TRUE(entry.Get(IS_DEL)); EXPECT_FALSE(entry.Get(SERVER_IS_DEL)); } - syncer_->SyncShare(); + syncer_->SyncShare(this); // Say we do not get an update from server confirming deletion. Undelete // from server mock_server_->AddUpdateBookmark(id, root, "foo", 2, 12); - syncer_->SyncShare(); + syncer_->SyncShare(this); // IS_DEL and SERVER_IS_DEL now both false. { ReadTransaction trans(dir, __FILE__, __LINE__); @@ -3732,30 +3716,9 @@ TEST_F(SyncerTest, TestUndeleteIgnoreCorrectlyUnappliedUpdate) { mock_server_->set_conflict_all_commits(true); mock_server_->AddUpdateBookmark(id1, root, "foo", 1, 10); mock_server_->AddUpdateBookmark(id2, root, "foo", 1, 10); - syncer_->SyncShare(); + syncer_->SyncShare(this); mock_server_->AddUpdateBookmark(id2, root, "foo2", 1, 10); - syncer_->SyncShare(); // Now just don't explode. -} - -TEST_F(SyncerTest, CopySyncProcessState) { - scoped_ptr<SyncProcessState> b; - { - SyncProcessState a; - a.MergeSets(ids_.FromNumber(1), ids_.FromNumber(2)); - a.MergeSets(ids_.FromNumber(2), ids_.FromNumber(3)); - a.MergeSets(ids_.FromNumber(4), ids_.FromNumber(5)); - EXPECT_TRUE(a.ConflictSetsSize() == 2); - { - SyncProcessState b = a; - b = b; - EXPECT_TRUE(b.ConflictSetsSize() == 2); - } - EXPECT_TRUE(a.ConflictSetsSize() == 2); - a.MergeSets(ids_.FromNumber(3), ids_.FromNumber(4)); - EXPECT_TRUE(a.ConflictSetsSize() == 1); - b.reset(new SyncProcessState(a)); - } - EXPECT_TRUE(b->ConflictSetsSize() == 1); + syncer_->SyncShare(this); // Now just don't explode. } TEST_F(SyncerTest, SingletonTagUpdates) { @@ -3784,7 +3747,7 @@ TEST_F(SyncerTest, SingletonTagUpdates) { mock_server_->SetLastUpdateSingletonTag("alpha"); mock_server_->AddUpdateDirectory(2, 0, "update2", 2, 20); mock_server_->SetLastUpdateSingletonTag("bob"); - syncer_->SyncShare(); + syncer_->SyncShare(this); { ReadTransaction trans(dir, __FILE__, __LINE__); @@ -3868,7 +3831,7 @@ TEST_F(SyncerPositionUpdateTest, InOrderPositive) { AddRootItemWithPosition(201); AddRootItemWithPosition(400); - syncer_->SyncShare(); + syncer_->SyncShare(this); ExpectLocalItemsInServerOrder(); } @@ -3880,7 +3843,7 @@ TEST_F(SyncerPositionUpdateTest, InOrderNegative) { AddRootItemWithPosition(-150); AddRootItemWithPosition(100); - syncer_->SyncShare(); + syncer_->SyncShare(this); ExpectLocalItemsInServerOrder(); } @@ -3895,7 +3858,7 @@ TEST_F(SyncerPositionUpdateTest, ReverseOrder) { AddRootItemWithPosition(-200); AddRootItemWithPosition(-400); - syncer_->SyncShare(); + syncer_->SyncShare(this); ExpectLocalItemsInServerOrder(); } @@ -3907,7 +3870,7 @@ TEST_F(SyncerPositionUpdateTest, RandomOrderInBatches) { AddRootItemWithPosition(-400); AddRootItemWithPosition(100); - syncer_->SyncShare(); + syncer_->SyncShare(this); ExpectLocalItemsInServerOrder(); AddRootItemWithPosition(-150); @@ -3915,12 +3878,12 @@ TEST_F(SyncerPositionUpdateTest, RandomOrderInBatches) { AddRootItemWithPosition(200); AddRootItemWithPosition(-201); - syncer_->SyncShare(); + syncer_->SyncShare(this); ExpectLocalItemsInServerOrder(); AddRootItemWithPosition(-144); - syncer_->SyncShare(); + syncer_->SyncShare(this); ExpectLocalItemsInServerOrder(); } @@ -3982,7 +3945,7 @@ TEST_F(SyncerPositionTiebreakingTest, LowMidHigh) { Add(low_id_); Add(mid_id_); Add(high_id_); - syncer_->SyncShare(); + syncer_->SyncShare(this); ExpectLocalOrderIsByServerId(); } @@ -3990,7 +3953,7 @@ TEST_F(SyncerPositionTiebreakingTest, LowHighMid) { Add(low_id_); Add(high_id_); Add(mid_id_); - syncer_->SyncShare(); + syncer_->SyncShare(this); ExpectLocalOrderIsByServerId(); } @@ -3998,7 +3961,7 @@ TEST_F(SyncerPositionTiebreakingTest, HighMidLow) { Add(high_id_); Add(mid_id_); Add(low_id_); - syncer_->SyncShare(); + syncer_->SyncShare(this); ExpectLocalOrderIsByServerId(); } @@ -4006,7 +3969,7 @@ TEST_F(SyncerPositionTiebreakingTest, HighLowMid) { Add(high_id_); Add(low_id_); Add(mid_id_); - syncer_->SyncShare(); + syncer_->SyncShare(this); ExpectLocalOrderIsByServerId(); } @@ -4014,7 +3977,7 @@ TEST_F(SyncerPositionTiebreakingTest, MidHighLow) { Add(mid_id_); Add(high_id_); Add(low_id_); - syncer_->SyncShare(); + syncer_->SyncShare(this); ExpectLocalOrderIsByServerId(); } @@ -4022,7 +3985,7 @@ TEST_F(SyncerPositionTiebreakingTest, MidLowHigh) { Add(mid_id_); Add(low_id_); Add(high_id_); - syncer_->SyncShare(); + syncer_->SyncShare(this); ExpectLocalOrderIsByServerId(); } diff --git a/chrome/browser/sync/engine/update_applicator.cc b/chrome/browser/sync/engine/update_applicator.cc index d0ff392..1484d5f 100755 --- a/chrome/browser/sync/engine/update_applicator.cc +++ b/chrome/browser/sync/engine/update_applicator.cc @@ -8,6 +8,7 @@ #include "base/logging.h" #include "chrome/browser/sync/engine/syncer_util.h" +#include "chrome/browser/sync/sessions/session_state.h" #include "chrome/browser/sync/syncable/syncable.h" #include "chrome/browser/sync/syncable/syncable_id.h" @@ -73,18 +74,20 @@ bool UpdateApplicator::AllUpdatesApplied() const { return conflicting_ids_.empty() && begin_ == end_; } -void UpdateApplicator::SaveProgressIntoSessionState(SyncerSession* session) { +void UpdateApplicator::SaveProgressIntoSessionState( + sessions::ConflictProgress* conflict_progress, + sessions::UpdateProgress* update_progress) { 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->AddCommitConflict(*i); - session->AddAppliedUpdate(CONFLICT, *i); + conflict_progress->AddConflictingItemById(*i); + update_progress->AddAppliedUpdate(CONFLICT, *i); } for (i = successful_ids_.begin(); i != successful_ids_.end(); ++i) { - session->EraseCommitConflict(*i); - session->AddAppliedUpdate(SUCCESS, *i); + conflict_progress->EraseConflictingItemById(*i); + update_progress->AddAppliedUpdate(SUCCESS, *i); } } diff --git a/chrome/browser/sync/engine/update_applicator.h b/chrome/browser/sync/engine/update_applicator.h index dc27e17..6f4e02e 100755 --- a/chrome/browser/sync/engine/update_applicator.h +++ b/chrome/browser/sync/engine/update_applicator.h @@ -20,8 +20,12 @@ namespace browser_sync { +namespace sessions { +class ConflictProgress; +class UpdateProgress; +} + class ConflictResolver; -class SyncerSession; class UpdateApplicator { public: @@ -38,10 +42,12 @@ class UpdateApplicator { bool AllUpdatesApplied() const; // This class does not automatically save its progress into the - // SyncerSession -- to get that to happen, call this method after update + // SyncSession -- to get that to happen, call this method after update // application is finished (i.e., when AttemptOneAllocation stops returning // true). - void SaveProgressIntoSessionState(SyncerSession* session); + void SaveProgressIntoSessionState( + sessions::ConflictProgress* conflict_progress, + sessions::UpdateProgress* update_progress); private: // Used to resolve conflicts when trying to apply updates. diff --git a/chrome/browser/sync/engine/verify_updates_command.cc b/chrome/browser/sync/engine/verify_updates_command.cc index 216eff5..8a1a508 100644 --- a/chrome/browser/sync/engine/verify_updates_command.cc +++ b/chrome/browser/sync/engine/verify_updates_command.cc @@ -12,7 +12,6 @@ #include "chrome/browser/sync/engine/syncproto.h" #include "chrome/browser/sync/syncable/directory_manager.h" #include "chrome/browser/sync/syncable/syncable.h" -#include "chrome/browser/sync/util/sync_types.h" namespace browser_sync { @@ -26,15 +25,17 @@ using syncable::SYNCER; VerifyUpdatesCommand::VerifyUpdatesCommand() {} VerifyUpdatesCommand::~VerifyUpdatesCommand() {} -void VerifyUpdatesCommand::ExecuteImpl(SyncerSession* session) { +void VerifyUpdatesCommand::ExecuteImpl(sessions::SyncSession* session) { LOG(INFO) << "Beginning Update Verification"; - ScopedDirLookup dir(session->dirman(), session->account_name()); + ScopedDirLookup dir(session->context()->directory_manager(), + session->context()->account_name()); if (!dir.good()) { LOG(ERROR) << "Scoped dir lookup failed!"; return; } WriteTransaction trans(dir, SYNCER, __FILE__, __LINE__); - GetUpdatesResponse updates = session->update_response().get_updates(); + sessions::StatusController* status = session->status_controller(); + const GetUpdatesResponse& updates = status->updates_response().get_updates(); int update_count = updates.entries().size(); LOG(INFO) << update_count << " entries to verify"; @@ -47,7 +48,7 @@ void VerifyUpdatesCommand::ExecuteImpl(SyncerSession* session) { SyncerUtil::AttemptReuniteLostCommitResponses(&trans, entry, trans.directory()->cache_guid()); VerifyResult result = VerifyUpdate(&trans, entry); - session->AddVerifyResult(result, entry); + status->mutable_update_progress()->AddVerifyResult(result, entry); } } diff --git a/chrome/browser/sync/engine/verify_updates_command.h b/chrome/browser/sync/engine/verify_updates_command.h index 87c6ff7..f4ccf2b 100644 --- a/chrome/browser/sync/engine/verify_updates_command.h +++ b/chrome/browser/sync/engine/verify_updates_command.h @@ -8,9 +8,8 @@ #include "base/basictypes.h" #include "chrome/browser/sync/engine/syncer_command.h" -#include "chrome/browser/sync/engine/syncer_session.h" #include "chrome/browser/sync/engine/syncproto.h" -#include "chrome/browser/sync/util/sync_types.h" +#include "chrome/browser/sync/engine/syncer_types.h" namespace syncable { class WriteTransaction; @@ -19,12 +18,14 @@ class WriteTransaction; namespace browser_sync { // Verifies the response from a GetUpdates request. All invalid updates will be -// noted in the SyncerSession after this command is executed. +// noted in the SyncSession after this command is executed. class VerifyUpdatesCommand : public SyncerCommand { public: VerifyUpdatesCommand(); virtual ~VerifyUpdatesCommand(); - virtual void ExecuteImpl(SyncerSession* session); + + // SyncerCommand implementation. + virtual void ExecuteImpl(sessions::SyncSession* session); VerifyResult VerifyUpdate(syncable::WriteTransaction* trans, const SyncEntity& entry); |