diff options
Diffstat (limited to 'sync/sessions')
-rw-r--r-- | sync/sessions/session_state_unittest.cc | 3 | ||||
-rw-r--r-- | sync/sessions/sync_session.cc | 58 | ||||
-rw-r--r-- | sync/sessions/sync_session.h | 39 | ||||
-rw-r--r-- | sync/sessions/sync_session_unittest.cc | 4 | ||||
-rw-r--r-- | sync/sessions/test_util.cc | 8 |
5 files changed, 36 insertions, 76 deletions
diff --git a/sync/sessions/session_state_unittest.cc b/sync/sessions/session_state_unittest.cc index 9d20437..92d4b6f 100644 --- a/sync/sessions/session_state_unittest.cc +++ b/sync/sessions/session_state_unittest.cc @@ -90,8 +90,7 @@ TEST_F(SessionStateTest, SyncSessionSnapshotToValue) { source, false, 0, - base::Time::Now(), - false); + base::Time::Now()); scoped_ptr<DictionaryValue> value(snapshot.ToValue()); EXPECT_EQ(20u, value->size()); ExpectDictIntegerValue(model_neutral.num_successful_commits, diff --git a/sync/sessions/sync_session.cc b/sync/sessions/sync_session.cc index 5a2b572..9be78d8 100644 --- a/sync/sessions/sync_session.cc +++ b/sync/sessions/sync_session.cc @@ -77,8 +77,7 @@ SyncSession::SyncSession(SyncSessionContext* context, Delegate* delegate, delegate_(delegate), workers_(workers), routing_info_(routing_info), - enabled_groups_(ComputeEnabledGroups(routing_info_, workers_)), - finished_(false) { + enabled_groups_(ComputeEnabledGroups(routing_info_, workers_)) { status_controller_.reset(new StatusController(routing_info_)); std::sort(workers_.begin(), workers_.end()); } @@ -115,30 +114,31 @@ void SyncSession::Coalesce(const SyncSession& session) { enabled_groups_ = ComputeEnabledGroups(routing_info_, workers_); } -void SyncSession::RebaseRoutingInfoWithLatest(const SyncSession& session) { +void SyncSession::RebaseRoutingInfoWithLatest( + const ModelSafeRoutingInfo& routing_info, + const std::vector<ModelSafeWorker*>& workers) { ModelSafeRoutingInfo temp_routing_info; - // Take the intersecion and also set the routing info(it->second) from the + // Take the intersection and also set the routing info(it->second) from the // passed in session. for (ModelSafeRoutingInfo::const_iterator it = - session.routing_info_.begin(); it != session.routing_info_.end(); + routing_info.begin(); it != routing_info.end(); ++it) { if (routing_info_.find(it->first) != routing_info_.end()) { temp_routing_info[it->first] = it->second; } } - - // Now swap it. routing_info_.swap(temp_routing_info); - // Now update the payload map. - PurgeStaleStates(&source_.types, session.routing_info_); + PurgeStaleStates(&source_.types, routing_info); // Now update the workers. std::vector<ModelSafeWorker*> temp; + std::vector<ModelSafeWorker*> sorted_workers = workers; + std::sort(sorted_workers.begin(), sorted_workers.end()); std::set_intersection(workers_.begin(), workers_.end(), - session.workers_.begin(), session.workers_.end(), - std::back_inserter(temp)); + sorted_workers.begin(), sorted_workers.end(), + std::back_inserter(temp)); workers_.swap(temp); // Now update enabled groups. @@ -146,7 +146,6 @@ void SyncSession::RebaseRoutingInfoWithLatest(const SyncSession& session) { } void SyncSession::PrepareForAnotherSyncCycle() { - finished_ = false; source_.updates_source = sync_pb::GetUpdatesCallerInfo::SYNC_CYCLE_CONTINUATION; status_controller_.reset(new StatusController(routing_info_)); @@ -183,8 +182,7 @@ SyncSessionSnapshot SyncSession::TakeSnapshot() const { source_, context_->notifications_enabled(), dir->GetEntriesCount(), - status_controller_->sync_start_time(), - !Succeeded()); + status_controller_->sync_start_time()); } void SyncSession::SendEventNotification(SyncEngineEvent::EventCause cause) { @@ -219,35 +217,11 @@ std::set<ModelSafeGroup> SyncSession::GetEnabledGroupsWithConflicts() const { return enabled_groups_with_conflicts; } -namespace { - -// Returns false iff one of the command results had an error. -bool HadErrors(const ModelNeutralState& state) { - const bool get_key_error = SyncerErrorIsError(state.last_get_key_result); - const bool download_updates_error = - SyncerErrorIsError(state.last_download_updates_result); - const bool commit_error = SyncerErrorIsError(state.commit_result); - return get_key_error || download_updates_error || commit_error; -} -} // namespace - -bool SyncSession::Succeeded() const { - return finished_ && !HadErrors(status_controller_->model_neutral_state()); -} - -bool SyncSession::SuccessfullyReachedServer() const { +bool SyncSession::DidReachServer() const { const ModelNeutralState& state = status_controller_->model_neutral_state(); - bool reached_server = state.last_get_key_result == SYNCER_OK || - state.last_download_updates_result == SYNCER_OK; - // It's possible that we reached the server on one attempt, then had an error - // on the next (or didn't perform some of the server-communicating commands). - // We want to verify that, for all commands attempted, we successfully spoke - // with the server. Therefore, we verify no errors and at least one SYNCER_OK. - return reached_server && !HadErrors(state); -} - -void SyncSession::SetFinished() { - finished_ = true; + return state.last_get_key_result >= FIRST_SERVER_RETURN_VALUE || + state.last_download_updates_result >= FIRST_SERVER_RETURN_VALUE || + state.commit_result >= FIRST_SERVER_RETURN_VALUE; } } // namespace sessions diff --git a/sync/sessions/sync_session.h b/sync/sessions/sync_session.h index 93d7f41..4026e88 100644 --- a/sync/sessions/sync_session.h +++ b/sync/sessions/sync_session.h @@ -112,25 +112,11 @@ class SyncSession { // engine again. bool HasMoreToSync() const; - // Returns true if we completely ran the session without errors. - // - // There are many errors that could prevent a sync cycle from succeeding. - // These include invalid local state, inability to contact the server, - // inability to authenticate with the server, and server errors. What they - // have in common is that the we either need to take some action and then - // retry the sync cycle or, in the case of transient errors, retry after some - // backoff timer has expired. Most importantly, the SyncScheduler should not - // assume that the original action that triggered the sync cycle (ie. a nudge - // or a notification) has been properly serviced. - // - // This function also returns false if SyncShare has not been called on this - // session yet, or if ResetTransientState() has been called on this session - // since the last call to SyncShare. - bool Succeeded() const; - - // Returns true if we reached the server successfully and the server did not - // return any error codes. Returns false if no connection was attempted. - bool SuccessfullyReachedServer() const; + // Returns true if we reached the server. Note that "reaching the server" + // here means that from an HTTP perspective, we succeeded (HTTP 200). The + // server **MAY** have returned a sync protocol error. + // See SERVER_RETURN_* in the SyncerError enum for values. + bool DidReachServer() const; // Collects all state pertaining to how and why |s| originated and unions it // with corresponding state in |this|, leaving |s| unchanged. Allows |this| @@ -139,11 +125,13 @@ class SyncSession { // sessions. void Coalesce(const SyncSession& session); - // Compares the routing_info_, workers and payload map with the passed in - // session. Purges types from the above 3 which are not in session. Useful + // Compares the routing_info_, workers and payload map with those passed in. + // Purges types from the above 3 which are not present in latest. Useful // to update the sync session when the user has disabled some types from // syncing. - void RebaseRoutingInfoWithLatest(const SyncSession& session); + void RebaseRoutingInfoWithLatest( + const ModelSafeRoutingInfo& routing_info, + const std::vector<ModelSafeWorker*>& workers); // Should be called any time |this| is being re-used in a new call to // SyncShare (e.g., HasMoreToSync returned true). @@ -177,9 +165,6 @@ class SyncSession { // Returns the set of enabled groups that have conflicts. std::set<ModelSafeGroup> GetEnabledGroupsWithConflicts() const; - // Mark the session has having finished all the sync steps it needed. - void SetFinished(); - private: // Extend the encapsulation boundary to utilities for internal member // assignments. This way, the scope of these actions is explicit, they can't @@ -217,10 +202,6 @@ class SyncSession { // |routing_info_|. std::set<ModelSafeGroup> enabled_groups_; - // Whether this session has reached its last step or not. Gets reset on each - // new cycle (via PrepareForAnotherSyncCycle). - bool finished_; - DISALLOW_COPY_AND_ASSIGN(SyncSession); }; diff --git a/sync/sessions/sync_session_unittest.cc b/sync/sessions/sync_session_unittest.cc index 285f33a..e23f6f6 100644 --- a/sync/sessions/sync_session_unittest.cc +++ b/sync/sessions/sync_session_unittest.cc @@ -347,7 +347,7 @@ TEST_F(SyncSessionTest, RebaseRoutingInfoWithLatestRemoveOneType) { EXPECT_EQ(expected_enabled_groups_one, one.GetEnabledGroups()); EXPECT_EQ(expected_enabled_groups_two, two.GetEnabledGroups()); - two.RebaseRoutingInfoWithLatest(one); + two.RebaseRoutingInfoWithLatest(one.routing_info(), one.workers()); EXPECT_EQ(expected_enabled_groups_one, one.GetEnabledGroups()); EXPECT_EQ(expected_enabled_groups_one, two.GetEnabledGroups()); @@ -417,7 +417,7 @@ TEST_F(SyncSessionTest, RebaseRoutingInfoWithLatestWithSameType) { EXPECT_EQ(expected_enabled_groups, first.GetEnabledGroups()); EXPECT_EQ(expected_enabled_groups, second.GetEnabledGroups()); - second.RebaseRoutingInfoWithLatest(first); + second.RebaseRoutingInfoWithLatest(first.routing_info(), first.workers()); EXPECT_EQ(expected_enabled_groups, first.GetEnabledGroups()); EXPECT_EQ(expected_enabled_groups, second.GetEnabledGroups()); diff --git a/sync/sessions/test_util.cc b/sync/sessions/test_util.cc index 4214c3d..cef5121 100644 --- a/sync/sessions/test_util.cc +++ b/sync/sessions/test_util.cc @@ -31,6 +31,9 @@ void SimulateDownloadUpdatesFailed(sessions::SyncSession* session, void SimulateCommitFailed(sessions::SyncSession* session, SyncerStep begin, SyncerStep end) { + session->mutable_status_controller()->set_last_get_key_result(SYNCER_OK); + session->mutable_status_controller()->set_last_download_updates_result( + SYNCER_OK); session->mutable_status_controller()->set_commit_result( SERVER_RETURN_TRANSIENT_ERROR); } @@ -47,7 +50,6 @@ void SimulateSuccess(sessions::SyncSession* session, ADD_FAILURE() << "Shouldn't have more to sync"; } ASSERT_EQ(0U, session->status_controller().num_server_changes_remaining()); - session->SetFinished(); switch(end) { case SYNCER_END: session->mutable_status_controller()->set_commit_result(SYNCER_OK); @@ -66,16 +68,20 @@ void SimulateSuccess(sessions::SyncSession* session, void SimulateThrottledImpl(sessions::SyncSession* session, const base::TimeDelta& delta) { + session->mutable_status_controller()->set_last_download_updates_result( + SERVER_RETURN_THROTTLED); session->delegate()->OnSilencedUntil(base::TimeTicks::Now() + delta); } void SimulatePollIntervalUpdateImpl(sessions::SyncSession* session, const base::TimeDelta& new_poll) { + SimulateSuccess(session, SYNCER_BEGIN, SYNCER_END); session->delegate()->OnReceivedLongPollIntervalUpdate(new_poll); } void SimulateSessionsCommitDelayUpdateImpl(sessions::SyncSession* session, const base::TimeDelta& new_delay) { + SimulateSuccess(session, SYNCER_BEGIN, SYNCER_END); session->delegate()->OnReceivedSessionsCommitDelay(new_delay); } |