summaryrefslogtreecommitdiffstats
path: root/sync/sessions
diff options
context:
space:
mode:
Diffstat (limited to 'sync/sessions')
-rw-r--r--sync/sessions/session_state_unittest.cc3
-rw-r--r--sync/sessions/sync_session.cc58
-rw-r--r--sync/sessions/sync_session.h39
-rw-r--r--sync/sessions/sync_session_unittest.cc4
-rw-r--r--sync/sessions/test_util.cc8
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);
}