diff options
author | rlarocque@chromium.org <rlarocque@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-07-03 03:44:20 +0000 |
---|---|---|
committer | rlarocque@chromium.org <rlarocque@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-07-03 03:44:20 +0000 |
commit | de84cfd7c982067cb1e691d2e983ec95e08b9e1b (patch) | |
tree | 645dd94ad7ac7fa4dd3ef45b967b0ad23cf1eb89 /sync/sessions | |
parent | e4784baf717103943f13649a2fd385404d409441 (diff) | |
download | chromium_src-de84cfd7c982067cb1e691d2e983ec95e08b9e1b.zip chromium_src-de84cfd7c982067cb1e691d2e983ec95e08b9e1b.tar.gz chromium_src-de84cfd7c982067cb1e691d2e983ec95e08b9e1b.tar.bz2 |
sync: Expose sync functionality as functions
This change is a refactor to clean up ugliness introduced in previous
commits and prepare for future features.
The most notable change is the removal of "state machine" logic from
syncer.cc. This allows us to remove the SyncerSteps enum and related
code. The SyncShare function + enum parameters have been replaced with
the functions NormalSyncShare(), ConfigureSyncShare() and
PollSyncShare(). These changes should make it possible to address
crbug.com/109422, and to re-enable commits during poll-triggered sync
cycles (if desrired, see r206475).
The logic for fetching and applying updates has been modified, too.
Since the behaviour of GetUpdates varies depending on the type of cycle
(Configure, GetUpdates, or Poll) the logic to build and execute these
GetUpdate requests has been split up. This enables us to remove the
NudgeTracker from the SyncSession (an ugly hack introduced in
r199136). It should make it easier to implement crbug.com/147685.
In the interest of keeping this change as small and simple as possible
some obvious refactorings have not been intentionally excluded from this
CL. For example, the logic around when to send SYNC_CYCLE_ENDED events
or when to return true or false frome the SyncShare functions remains
very complicated. Untangling that mess would require some non-trivial
changes to the SyncScheduler, so they've been deferred until later.
BUG=147685,109422
Review URL: https://chromiumcodereview.appspot.com/17052007
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@209867 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'sync/sessions')
-rw-r--r-- | sync/sessions/data_type_tracker.cc | 18 | ||||
-rw-r--r-- | sync/sessions/data_type_tracker.h | 4 | ||||
-rw-r--r-- | sync/sessions/nudge_tracker.cc | 11 | ||||
-rw-r--r-- | sync/sessions/nudge_tracker.h | 13 | ||||
-rw-r--r-- | sync/sessions/nudge_tracker_unittest.cc | 1 | ||||
-rw-r--r-- | sync/sessions/sync_session.cc | 16 | ||||
-rw-r--r-- | sync/sessions/sync_session.h | 15 | ||||
-rw-r--r-- | sync/sessions/test_util.cc | 99 | ||||
-rw-r--r-- | sync/sessions/test_util.h | 58 |
9 files changed, 154 insertions, 81 deletions
diff --git a/sync/sessions/data_type_tracker.cc b/sync/sessions/data_type_tracker.cc index 857b86a..3eabbbe 100644 --- a/sync/sessions/data_type_tracker.cc +++ b/sync/sessions/data_type_tracker.cc @@ -4,6 +4,7 @@ #include "sync/sessions/data_type_tracker.h" +#include "base/logging.h" #include "sync/sessions/nudge_tracker.h" namespace syncer { @@ -73,6 +74,23 @@ std::string DataTypeTracker::GetMostRecentInvalidationPayload() const { return pending_payloads_.back(); } +void DataTypeTracker::SetLegacyNotificationHint( + sync_pb::DataTypeProgressMarker* progress) const { + DCHECK(!IsThrottled()) + << "We should not make requests if the type is throttled."; + + if (HasPendingInvalidation()) { + // The old-style source info can contain only one hint per type. We grab + // the most recent, to mimic the old coalescing behaviour. + progress->set_notification_hint(GetMostRecentInvalidationPayload()); + } else if (HasLocalChangePending()) { + // The old-style source info sent up an empty string (as opposed to + // nothing at all) when the type was locally nudged, but had not received + // any invalidations. + progress->set_notification_hint(""); + } +} + void DataTypeTracker::FillGetUpdatesTriggersMessage( sync_pb::GetUpdateTriggers* msg) const { // Fill the list of payloads, if applicable. The payloads must be ordered diff --git a/sync/sessions/data_type_tracker.h b/sync/sessions/data_type_tracker.h index 65d5926..6488f8e 100644 --- a/sync/sessions/data_type_tracker.h +++ b/sync/sessions/data_type_tracker.h @@ -59,6 +59,10 @@ class DataTypeTracker { // Returns the most recent invalidation payload. std::string GetMostRecentInvalidationPayload() const; + // Fills in the legacy invalidaiton payload information fields. + void SetLegacyNotificationHint( + sync_pb::DataTypeProgressMarker* progress) const; + // Fills some type-specific contents of a GetUpdates request protobuf. These // messages provide the server with the information it needs to decide how to // handle a request. diff --git a/sync/sessions/nudge_tracker.cc b/sync/sessions/nudge_tracker.cc index 97f211f..144860b 100644 --- a/sync/sessions/nudge_tracker.cc +++ b/sync/sessions/nudge_tracker.cc @@ -28,8 +28,8 @@ NudgeTracker::NudgeTracker() NudgeTracker::~NudgeTracker() { } -bool NudgeTracker::IsSyncRequired() { - for (TypeTrackerMap::iterator it = type_trackers_.begin(); +bool NudgeTracker::IsSyncRequired() const { + for (TypeTrackerMap::const_iterator it = type_trackers_.begin(); it != type_trackers_.end(); ++it) { if (it->second.IsSyncRequired()) { return true; @@ -195,6 +195,13 @@ SyncSourceInfo NudgeTracker::GetSourceInfo() const { return SyncSourceInfo(updates_source_, invalidation_map); } +void NudgeTracker::SetLegacyNotificationHint( + ModelType type, + sync_pb::DataTypeProgressMarker* progress) const { + DCHECK(type_trackers_.find(type) != type_trackers_.end()); + type_trackers_.find(type)->second.SetLegacyNotificationHint(progress); +} + sync_pb::GetUpdatesCallerInfo::GetUpdatesSource NudgeTracker::updates_source() const { return updates_source_; diff --git a/sync/sessions/nudge_tracker.h b/sync/sessions/nudge_tracker.h index 3db3143..0e8ab51 100644 --- a/sync/sessions/nudge_tracker.h +++ b/sync/sessions/nudge_tracker.h @@ -32,7 +32,7 @@ class SYNC_EXPORT_PRIVATE NudgeTracker { // Returns true if there is a good reason for performing a sync cycle. // This does not take into account whether or not this is a good *time* to // perform a sync cycle; that's the scheduler's job. - bool IsSyncRequired(); + bool IsSyncRequired() const; // Tells this class that all required update fetching and committing has // completed successfully. @@ -88,13 +88,20 @@ class SYNC_EXPORT_PRIVATE NudgeTracker { // See the implementation for important information about the coalesce logic. sync_pb::GetUpdatesCallerInfo::GetUpdatesSource updates_source() const; - // Fills a ProgressMarker for the next GetUpdates request. This is used by - // the DownloadUpdatesCommand to dump lots of useful per-type state + // Fills a GetUpdatesTrigger message for the next GetUpdates request. This is + // used by the DownloadUpdatesCommand to dump lots of useful per-type state // information into the GetUpdate request before sending it off to the server. void FillProtoMessage( ModelType type, sync_pb::GetUpdateTriggers* msg) const; + // Fills a ProgressMarker with single legacy notification hint expected by the + // sync server. Newer servers will rely on the data set by FillProtoMessage() + // instead of this. + void SetLegacyNotificationHint( + ModelType type, + sync_pb::DataTypeProgressMarker* progress) const; + // Adjusts the number of hints that can be stored locally. void SetHintBufferSize(size_t size); diff --git a/sync/sessions/nudge_tracker_unittest.cc b/sync/sessions/nudge_tracker_unittest.cc index 9e6ce99..8439af1d8 100644 --- a/sync/sessions/nudge_tracker_unittest.cc +++ b/sync/sessions/nudge_tracker_unittest.cc @@ -412,6 +412,5 @@ TEST_F(NudgeTrackerTest, OverlappingThrottleIntervals) { EXPECT_TRUE(nudge_tracker.GetThrottledTypes().Empty()); } - } // namespace sessions } // namespace syncer diff --git a/sync/sessions/sync_session.cc b/sync/sessions/sync_session.cc index e526954..eb81733 100644 --- a/sync/sessions/sync_session.cc +++ b/sync/sessions/sync_session.cc @@ -16,29 +16,19 @@ namespace syncer { namespace sessions { // static -SyncSession* SyncSession::BuildForNudge(SyncSessionContext* context, - Delegate* delegate, - const SyncSourceInfo& source, - const NudgeTracker* nudge_tracker) { - return new SyncSession(context, delegate, source, nudge_tracker); -} - -// static SyncSession* SyncSession::Build(SyncSessionContext* context, Delegate* delegate, const SyncSourceInfo& source) { - return new SyncSession(context, delegate, source, NULL); + return new SyncSession(context, delegate, source); } SyncSession::SyncSession( SyncSessionContext* context, Delegate* delegate, - const SyncSourceInfo& source, - const NudgeTracker* nudge_tracker) + const SyncSourceInfo& source) : context_(context), source_(source), - delegate_(delegate), - nudge_tracker_(nudge_tracker) { + delegate_(delegate) { status_controller_.reset(new StatusController()); } diff --git a/sync/sessions/sync_session.h b/sync/sessions/sync_session.h index 32dbdd1..8f4e9ed 100644 --- a/sync/sessions/sync_session.h +++ b/sync/sessions/sync_session.h @@ -101,19 +101,11 @@ class SYNC_EXPORT_PRIVATE SyncSession { virtual ~Delegate() {} }; - // Build a session with a nudge tracker. Used for sync cycles with origin of - // GU_TRIGGER (ie. notification, local change, and/or refresh request) - static SyncSession* BuildForNudge(SyncSessionContext* context, - Delegate* delegate, - const SyncSourceInfo& source, - const NudgeTracker* nudge_tracker); - // Build a session without a nudge tracker. Used for poll or configure type // sync cycles. static SyncSession* Build(SyncSessionContext* context, Delegate* delegate, const SyncSourceInfo& source); - ~SyncSession(); // Builds a thread-safe and read-only copy of the current session state. @@ -134,13 +126,10 @@ class SYNC_EXPORT_PRIVATE SyncSession { const SyncSourceInfo& source() const { return source_; } - const NudgeTracker* nudge_tracker() const { return nudge_tracker_; } - private: SyncSession(SyncSessionContext* context, Delegate* delegate, - const SyncSourceInfo& source, - const NudgeTracker* nudge_tracker); + const SyncSourceInfo& source); // The context for this session, guaranteed to outlive |this|. SyncSessionContext* const context_; @@ -154,8 +143,6 @@ class SYNC_EXPORT_PRIVATE SyncSession { // Our controller for various status and error counters. scoped_ptr<StatusController> status_controller_; - const NudgeTracker* nudge_tracker_; - DISALLOW_COPY_AND_ASSIGN(SyncSession); }; diff --git a/sync/sessions/test_util.cc b/sync/sessions/test_util.cc index 24caa7b..3abfa97 100644 --- a/sync/sessions/test_util.cc +++ b/sync/sessions/test_util.cc @@ -8,23 +8,56 @@ namespace syncer { namespace sessions { namespace test_util { -void SimulateGetEncryptionKeyFailed(sessions::SyncSession* session, - SyncerStep begin, SyncerStep end) { +void SimulateGetEncryptionKeyFailed(ModelTypeSet requsted_types, + sessions::SyncSession* session) { session->mutable_status_controller()->set_last_get_key_result( SERVER_RESPONSE_VALIDATION_FAILED); session->mutable_status_controller()->set_last_download_updates_result( SYNCER_OK); } -void SimulateDownloadUpdatesFailed(sessions::SyncSession* session, - SyncerStep begin, SyncerStep end) { +void SimulateConfigureSuccess(ModelTypeSet requsted_types, + sessions::SyncSession* session) { + ASSERT_EQ(0U, session->status_controller().num_server_changes_remaining()); + session->mutable_status_controller()->set_last_get_key_result(SYNCER_OK); + session->mutable_status_controller()->set_last_download_updates_result( + SYNCER_OK); +} + +void SimulateConfigureFailed(ModelTypeSet requsted_types, + sessions::SyncSession* session) { session->mutable_status_controller()->set_last_get_key_result(SYNCER_OK); session->mutable_status_controller()->set_last_download_updates_result( SERVER_RETURN_TRANSIENT_ERROR); } -void SimulateCommitFailed(sessions::SyncSession* session, - SyncerStep begin, SyncerStep end) { +void SimulateConfigureConnectionFailure(ModelTypeSet requsted_types, + sessions::SyncSession* session) { + session->mutable_status_controller()->set_last_get_key_result(SYNCER_OK); + session->mutable_status_controller()->set_last_download_updates_result( + NETWORK_CONNECTION_UNAVAILABLE); +} + +void SimulateNormalSuccess(ModelTypeSet requested_types, + const sessions::NudgeTracker& nudge_tracker, + sessions::SyncSession* session) { + ASSERT_EQ(0U, session->status_controller().num_server_changes_remaining()); + session->mutable_status_controller()->set_commit_result(SYNCER_OK); + session->mutable_status_controller()->set_last_download_updates_result( + SYNCER_OK); +} + +void SimulateDownloadUpdatesFailed( + ModelTypeSet requested_types, + const sessions::NudgeTracker& nudge_tracker, + sessions::SyncSession* session) { + session->mutable_status_controller()->set_last_download_updates_result( + SERVER_RETURN_TRANSIENT_ERROR); +} + +void SimulateCommitFailed(ModelTypeSet requested_types, + const sessions::NudgeTracker& nudge_tracker, + sessions::SyncSession* session) { session->mutable_status_controller()->set_last_get_key_result(SYNCER_OK); session->mutable_status_controller()->set_last_download_updates_result( SYNCER_OK); @@ -32,37 +65,30 @@ void SimulateCommitFailed(sessions::SyncSession* session, SERVER_RETURN_TRANSIENT_ERROR); } -void SimulateConnectionFailure(sessions::SyncSession* session, - SyncerStep begin, SyncerStep end) { +void SimulateConnectionFailure( + ModelTypeSet requested_types, + const sessions::NudgeTracker& nudge_tracker, + sessions::SyncSession* session) { session->mutable_status_controller()->set_last_download_updates_result( NETWORK_CONNECTION_UNAVAILABLE); } -void SimulateSuccess(sessions::SyncSession* session, - SyncerStep begin, SyncerStep end) { - const sync_pb::GetUpdatesCallerInfo::GetUpdatesSource source = - session->source().updates_source; +void SimulatePollSuccess(ModelTypeSet requested_types, + sessions::SyncSession* session) { ASSERT_EQ(0U, session->status_controller().num_server_changes_remaining()); - switch(end) { - case SYNCER_END: - session->mutable_status_controller()->set_commit_result(SYNCER_OK); - session->mutable_status_controller()->set_last_get_key_result(SYNCER_OK); - session->mutable_status_controller()->set_last_download_updates_result( - SYNCER_OK); - break; - case APPLY_UPDATES: - DCHECK(source == sync_pb::GetUpdatesCallerInfo::RECONFIGURATION - || source == sync_pb::GetUpdatesCallerInfo::PERIODIC); - session->mutable_status_controller()->set_last_get_key_result(SYNCER_OK); - session->mutable_status_controller()->set_last_download_updates_result( - SYNCER_OK); - break; - default: - ADD_FAILURE() << "Not a valid END state."; - } + session->mutable_status_controller()->set_last_download_updates_result( + SYNCER_OK); } -void SimulateThrottledImpl(sessions::SyncSession* session, +void SimulatePollFailed(ModelTypeSet requested_types, + sessions::SyncSession* session) { + ASSERT_EQ(0U, session->status_controller().num_server_changes_remaining()); + session->mutable_status_controller()->set_last_download_updates_result( + SERVER_RETURN_TRANSIENT_ERROR); +} + +void SimulateThrottledImpl( + sessions::SyncSession* session, const base::TimeDelta& delta) { session->mutable_status_controller()->set_last_download_updates_result( SERVER_RETURN_THROTTLED); @@ -78,15 +104,20 @@ void SimulateTypesThrottledImpl( session->delegate()->OnTypesThrottled(types, delta); } -void SimulatePollIntervalUpdateImpl(sessions::SyncSession* session, +void SimulatePollIntervalUpdateImpl( + ModelTypeSet requested_types, + sessions::SyncSession* session, const base::TimeDelta& new_poll) { - SimulateSuccess(session, SYNCER_BEGIN, SYNCER_END); + SimulatePollSuccess(requested_types, session); session->delegate()->OnReceivedLongPollIntervalUpdate(new_poll); } -void SimulateSessionsCommitDelayUpdateImpl(sessions::SyncSession* session, +void SimulateSessionsCommitDelayUpdateImpl( + ModelTypeSet requested_types, + const sessions::NudgeTracker& nudge_tracker, + sessions::SyncSession* session, const base::TimeDelta& new_delay) { - SimulateSuccess(session, SYNCER_BEGIN, SYNCER_END); + SimulateNormalSuccess(requested_types, nudge_tracker, session); session->delegate()->OnReceivedSessionsCommitDelay(new_delay); } diff --git a/sync/sessions/test_util.h b/sync/sessions/test_util.h index 6ab32b1..56c13c7 100644 --- a/sync/sessions/test_util.h +++ b/sync/sessions/test_util.h @@ -15,25 +15,55 @@ namespace syncer { namespace sessions { namespace test_util { -void SimulateGetEncryptionKeyFailed(sessions::SyncSession* session, - SyncerStep begin, SyncerStep end); -void SimulateDownloadUpdatesFailed(sessions::SyncSession* session, - SyncerStep begin, SyncerStep end); -void SimulateCommitFailed(sessions::SyncSession* session, - SyncerStep begin, SyncerStep end); -void SimulateConnectionFailure(sessions::SyncSession* session, - SyncerStep begin, SyncerStep end); -void SimulateSuccess(sessions::SyncSession* session, - SyncerStep begin, SyncerStep end); +// Configure sync cycle successes and failures. +void SimulateGetEncryptionKeyFailed(ModelTypeSet requested_types, + sessions::SyncSession* session); +void SimulateConfigureSuccess(ModelTypeSet requested_types, + sessions::SyncSession* session); +void SimulateConfigureFailed(ModelTypeSet requested_types, + sessions::SyncSession* session); +void SimulateConfigureConnectionFailure(ModelTypeSet requested_types, + sessions::SyncSession* session); + +// Normal mode sync cycle successes and failures. +void SimulateNormalSuccess(ModelTypeSet requested_types, + const sessions::NudgeTracker& nudge_tracker, + sessions::SyncSession* session); +void SimulateDownloadUpdatesFailed(ModelTypeSet requested_types, + const sessions::NudgeTracker& nudge_tracker, + sessions::SyncSession* session); +void SimulateCommitFailed(ModelTypeSet requested_types, + const sessions::NudgeTracker& nudge_tracker, + sessions::SyncSession* session); +void SimulateConnectionFailure(ModelTypeSet requested_types, + const sessions::NudgeTracker& nudge_tracker, + sessions::SyncSession* session); + +// Poll successes and failures. +void SimulatePollSuccess(ModelTypeSet requested_types, + sessions::SyncSession* session); +void SimulatePollFailed(ModelTypeSet requested_types, + sessions::SyncSession* session); + void SimulateThrottledImpl(sessions::SyncSession* session, const base::TimeDelta& delta); + void SimulateTypesThrottledImpl( sessions::SyncSession* session, ModelTypeSet types, const base::TimeDelta& delta); -void SimulatePollIntervalUpdateImpl(sessions::SyncSession* session, + +// Works with poll cycles. +void SimulatePollIntervalUpdateImpl( + ModelTypeSet requested_types, + sessions::SyncSession* session, const base::TimeDelta& new_poll); -void SimulateSessionsCommitDelayUpdateImpl(sessions::SyncSession* session, + +// Works with normal cycles. +void SimulateSessionsCommitDelayUpdateImpl( + ModelTypeSet requested_types, + const sessions::NudgeTracker& nudge_tracker, + sessions::SyncSession* session, const base::TimeDelta& new_delay); ACTION_P(SimulateThrottled, throttle) { @@ -45,11 +75,11 @@ ACTION_P2(SimulateTypesThrottled, types, throttle) { } ACTION_P(SimulatePollIntervalUpdate, poll) { - SimulatePollIntervalUpdateImpl(arg0, poll); + SimulatePollIntervalUpdateImpl(arg0, arg1, poll); } ACTION_P(SimulateSessionsCommitDelayUpdate, poll) { - SimulateSessionsCommitDelayUpdateImpl(arg0, poll); + SimulateSessionsCommitDelayUpdateImpl(arg0, arg1, arg2, poll); } } // namespace test_util |