Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(256)

Unified Diff: sync/engine/sync_scheduler_unittest.cc

Issue 10542044: [Sync] Remove unnecessary posting of methods in sync scheduler. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Rebase and fix dcheck Created 8 years, 6 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: sync/engine/sync_scheduler_unittest.cc
diff --git a/sync/engine/sync_scheduler_unittest.cc b/sync/engine/sync_scheduler_unittest.cc
index c5de28ef7e78595148c055e97b816112d36725ce..43da04e79d4e840879d1791cb6bfd745f5c085b2 100644
--- a/sync/engine/sync_scheduler_unittest.cc
+++ b/sync/engine/sync_scheduler_unittest.cc
@@ -150,10 +150,7 @@ class SyncSchedulerTest : public testing::Test {
}
void StartSyncScheduler(SyncScheduler::Mode mode) {
- scheduler()->Start(
- mode,
- base::Bind(&SyncSchedulerTest::DoQuitLoopNow,
- weak_ptr_factory_.GetWeakPtr()));
+ scheduler()->Start(mode, base::Closure());
}
// This stops the scheduler synchronously.
@@ -166,7 +163,6 @@ class SyncSchedulerTest : public testing::Test {
bool RunAndGetBackoff() {
ModelTypeSet nudge_types;
StartSyncScheduler(SyncScheduler::NORMAL_MODE);
- RunLoop();
scheduler()->ScheduleNudge(
zero(), NUDGE_SOURCE_LOCAL, nudge_types, FROM_HERE);
@@ -215,20 +211,6 @@ class SyncSchedulerTest : public testing::Test {
FakeExtensionsActivityMonitor extensions_activity_monitor_;
};
-class BackoffTriggersSyncSchedulerTest : public SyncSchedulerTest {
- void SetUp() {
- SyncSchedulerTest::SetUp();
- UseMockDelayProvider();
- EXPECT_CALL(*delay(), GetDelay(_))
- .WillRepeatedly(Return(TimeDelta::FromMilliseconds(1)));
- }
-
- void TearDown() {
- StopSyncScheduler();
- SyncSchedulerTest::TearDown();
- }
-};
-
void RecordSyncShareImpl(SyncSession* s, SyncShareRecords* record) {
record->times.push_back(TimeTicks::Now());
record->snapshots.push_back(s->TakeSnapshot());
@@ -236,13 +218,15 @@ void RecordSyncShareImpl(SyncSession* s, SyncShareRecords* record) {
ACTION_P(RecordSyncShare, record) {
RecordSyncShareImpl(arg0, record);
- QuitLoopNow();
+ if (MessageLoop::current()->is_running())
+ QuitLoopNow();
}
ACTION_P2(RecordSyncShareMultiple, record, quit_after) {
RecordSyncShareImpl(arg0, record);
EXPECT_LE(record->times.size(), quit_after);
- if (record->times.size() >= quit_after) {
+ if (record->times.size() >= quit_after &&
+ MessageLoop::current()->is_running()) {
QuitLoopNow();
}
}
@@ -267,7 +251,6 @@ TEST_F(SyncSchedulerTest, Nudge) {
.RetiresOnSaturation();
StartSyncScheduler(SyncScheduler::NORMAL_MODE);
- RunLoop();
scheduler()->ScheduleNudge(
zero(), NUDGE_SOURCE_LOCAL, model_types, FROM_HERE);
@@ -310,11 +293,9 @@ TEST_F(SyncSchedulerTest, Config) {
WithArg<0>(RecordSyncShare(&records))));
StartSyncScheduler(SyncScheduler::CONFIGURATION_MODE);
- RunLoop();
- scheduler()->ScheduleConfig(
+ scheduler()->Configure(
model_types, GetUpdatesCallerInfo::RECONFIGURATION);
- RunLoop();
ASSERT_EQ(1U, records.snapshots.size());
EXPECT_TRUE(CompareModelTypeSetToModelTypePayloadMap(model_types,
@@ -338,12 +319,10 @@ TEST_F(SyncSchedulerTest, ConfigWithBackingOff) {
WithArg<0>(RecordSyncShare(&records))));
StartSyncScheduler(SyncScheduler::CONFIGURATION_MODE);
- RunLoop();
ASSERT_EQ(0U, records.snapshots.size());
- scheduler()->ScheduleConfig(
+ scheduler()->Configure(
model_types, GetUpdatesCallerInfo::RECONFIGURATION);
- RunLoop();
ASSERT_EQ(1U, records.snapshots.size());
RunLoop();
@@ -375,16 +354,16 @@ TEST_F(SyncSchedulerTest, MultipleConfigWithBackingOff) {
WithArg<0>(RecordSyncShare(&records))));
StartSyncScheduler(SyncScheduler::CONFIGURATION_MODE);
- RunLoop();
ASSERT_EQ(0U, records.snapshots.size());
- scheduler()->ScheduleConfig(
+ scheduler()->Configure(
model_types1, GetUpdatesCallerInfo::RECONFIGURATION);
- RunLoop();
ASSERT_EQ(1U, records.snapshots.size());
- scheduler()->ScheduleConfig(
+ scheduler()->Configure(
model_types2, GetUpdatesCallerInfo::RECONFIGURATION);
+
+ // A canary job gets posted when we go into exponential backoff.
RunLoop();
ASSERT_EQ(2U, records.snapshots.size());
@@ -417,12 +396,10 @@ TEST_F(SyncSchedulerTest, NudgeWithConfigWithBackingOff) {
WithArg<0>(RecordSyncShare(&records))));
StartSyncScheduler(SyncScheduler::CONFIGURATION_MODE);
- RunLoop();
ASSERT_EQ(0U, records.snapshots.size());
- scheduler()->ScheduleConfig(
+ scheduler()->Configure(
model_types, GetUpdatesCallerInfo::RECONFIGURATION);
- RunLoop();
ASSERT_EQ(1U, records.snapshots.size());
scheduler()->ScheduleNudge(
@@ -435,7 +412,6 @@ TEST_F(SyncSchedulerTest, NudgeWithConfigWithBackingOff) {
// Now change the mode so nudge can execute.
ASSERT_EQ(3U, records.snapshots.size());
StartSyncScheduler(SyncScheduler::NORMAL_MODE);
- RunLoop();
ASSERT_EQ(4U, records.snapshots.size());
@@ -454,7 +430,6 @@ TEST_F(SyncSchedulerTest, NudgeWithConfigWithBackingOff) {
// Test that nudges are coalesced.
TEST_F(SyncSchedulerTest, NudgeCoalescing) {
StartSyncScheduler(SyncScheduler::NORMAL_MODE);
- RunLoop();
SyncShareRecords r;
EXPECT_CALL(*syncer(), SyncShare(_,_,_))
@@ -499,7 +474,6 @@ TEST_F(SyncSchedulerTest, NudgeCoalescing) {
// Test that nudges are coalesced.
TEST_F(SyncSchedulerTest, NudgeCoalescingWithDifferentTimings) {
StartSyncScheduler(SyncScheduler::NORMAL_MODE);
- RunLoop();
SyncShareRecords r;
EXPECT_CALL(*syncer(), SyncShare(_,_,_))
@@ -535,7 +509,6 @@ TEST_F(SyncSchedulerTest, NudgeCoalescingWithDifferentTimings) {
// Test nudge scheduling.
TEST_F(SyncSchedulerTest, NudgeWithPayloads) {
StartSyncScheduler(SyncScheduler::NORMAL_MODE);
- RunLoop();
SyncShareRecords records;
syncable::ModelTypePayloadMap model_types_with_payloads;
@@ -576,7 +549,6 @@ TEST_F(SyncSchedulerTest, NudgeWithPayloads) {
// Test that nudges are coalesced.
TEST_F(SyncSchedulerTest, NudgeWithPayloadsCoalescing) {
StartSyncScheduler(SyncScheduler::NORMAL_MODE);
- RunLoop();
SyncShareRecords r;
EXPECT_CALL(*syncer(), SyncShare(_,_,_))
@@ -631,7 +603,6 @@ TEST_F(SyncSchedulerTest, Polling) {
TimeTicks optimal_start = TimeTicks::Now() + poll_interval;
StartSyncScheduler(SyncScheduler::NORMAL_MODE);
- RunLoop();
// Run again to wait for polling.
RunLoop();
@@ -653,9 +624,8 @@ TEST_F(SyncSchedulerTest, PollNotificationsDisabled) {
TimeTicks optimal_start = TimeTicks::Now() + poll_interval;
StartSyncScheduler(SyncScheduler::NORMAL_MODE);
- RunLoop();
- // Run again to wait for polling.
+ // Runn again to wait for polling.
RunLoop();
StopSyncScheduler();
@@ -678,9 +648,8 @@ TEST_F(SyncSchedulerTest, PollIntervalUpdate) {
TimeTicks optimal_start = TimeTicks::Now() + poll1 + poll2;
StartSyncScheduler(SyncScheduler::NORMAL_MODE);
- RunLoop();
- // Run again to wait for polling.
+ // Runn again to wait for polling.
RunLoop();
StopSyncScheduler();
@@ -705,7 +674,6 @@ TEST_F(SyncSchedulerTest, SessionsCommitDelay) {
EXPECT_EQ(delay1, scheduler()->sessions_commit_delay());
StartSyncScheduler(SyncScheduler::NORMAL_MODE);
- RunLoop();
EXPECT_EQ(delay1, scheduler()->sessions_commit_delay());
const ModelTypeSet model_types(syncable::BOOKMARKS);
@@ -724,7 +692,6 @@ TEST_F(SyncSchedulerTest, HasMoreToSync) {
.WillOnce(DoAll(Invoke(sessions::test_util::SimulateSuccess),
QuitLoopNowAction()));
StartSyncScheduler(SyncScheduler::NORMAL_MODE);
- RunLoop();
scheduler()->ScheduleNudge(
zero(), NUDGE_SOURCE_LOCAL, ModelTypeSet(), FROM_HERE);
@@ -740,7 +707,6 @@ TEST_F(SyncSchedulerTest, HasMoreToSyncThenFails) {
.WillOnce(DoAll(Invoke(sessions::test_util::SimulateCommitFailed),
QuitLoopNowAction()));
StartSyncScheduler(SyncScheduler::NORMAL_MODE);
- RunLoop();
scheduler()->ScheduleNudge(
zero(), NUDGE_SOURCE_LOCAL, ModelTypeSet(), FROM_HERE);
@@ -760,18 +726,15 @@ TEST_F(SyncSchedulerTest, ThrottlingDoesThrottle) {
.WillRepeatedly(AddFailureAndQuitLoopNow());
StartSyncScheduler(SyncScheduler::NORMAL_MODE);
- RunLoop();
scheduler()->ScheduleNudge(
zero(), NUDGE_SOURCE_LOCAL, types, FROM_HERE);
PumpLoop();
StartSyncScheduler(SyncScheduler::CONFIGURATION_MODE);
- RunLoop();
- scheduler()->ScheduleConfig(
+ scheduler()->Configure(
types, GetUpdatesCallerInfo::RECONFIGURATION);
- PumpLoop();
}
TEST_F(SyncSchedulerTest, ThrottlingExpires) {
@@ -790,7 +753,6 @@ TEST_F(SyncSchedulerTest, ThrottlingExpires) {
TimeTicks optimal_start = TimeTicks::Now() + poll + throttle1;
StartSyncScheduler(SyncScheduler::NORMAL_MODE);
- RunLoop();
// Run again to wait for polling.
RunLoop();
@@ -805,11 +767,10 @@ TEST_F(SyncSchedulerTest, ConfigurationMode) {
SyncShareRecords records;
scheduler()->OnReceivedLongPollIntervalUpdate(poll);
EXPECT_CALL(*syncer(), SyncShare(_,_,_))
- .WillOnce((Invoke(sessions::test_util::SimulateSuccess),
- WithArg<0>(RecordSyncShare(&records))));
+ .WillOnce(DoAll(Invoke(sessions::test_util::SimulateSuccess),
+ WithArg<0>(RecordSyncShare(&records))));
StartSyncScheduler(SyncScheduler::CONFIGURATION_MODE);
- RunLoop();
const ModelTypeSet nudge_types(syncable::AUTOFILL);
scheduler()->ScheduleNudge(
@@ -819,15 +780,28 @@ TEST_F(SyncSchedulerTest, ConfigurationMode) {
const ModelTypeSet config_types(syncable::BOOKMARKS);
- scheduler()->ScheduleConfig(
+ scheduler()->Configure(
config_types, GetUpdatesCallerInfo::RECONFIGURATION);
- RunLoop();
ASSERT_EQ(1U, records.snapshots.size());
EXPECT_TRUE(CompareModelTypeSetToModelTypePayloadMap(config_types,
records.snapshots[0].source().types));
}
+class BackoffTriggersSyncSchedulerTest : public SyncSchedulerTest {
+ void SetUp() {
+ SyncSchedulerTest::SetUp();
+ UseMockDelayProvider();
+ EXPECT_CALL(*delay(), GetDelay(_))
+ .WillRepeatedly(Return(TimeDelta::FromMilliseconds(1)));
+ }
+
+ void TearDown() {
+ StopSyncScheduler();
+ SyncSchedulerTest::TearDown();
+ }
+};
+
// Have the sycner fail during commit. Expect that the scheduler enters
// backoff.
TEST_F(BackoffTriggersSyncSchedulerTest, FailCommitOnce) {
@@ -883,7 +857,6 @@ TEST_F(SyncSchedulerTest, BackoffDropsJobs) {
WillRepeatedly(Return(TimeDelta::FromDays(1)));
StartSyncScheduler(SyncScheduler::NORMAL_MODE);
- RunLoop();
// This nudge should fail and put us into backoff. Thanks to our mock
// GetDelay() setup above, this will be a long backoff.
@@ -917,15 +890,12 @@ TEST_F(SyncSchedulerTest, BackoffDropsJobs) {
EXPECT_CALL(*delay(), GetDelay(_)).Times(0);
StartSyncScheduler(SyncScheduler::CONFIGURATION_MODE);
- RunLoop();
- scheduler()->ScheduleCleanupDisabledTypes();
- scheduler()->ScheduleConfig(
+ scheduler()->CleanupDisabledTypes();
+ scheduler()->Configure(
types, GetUpdatesCallerInfo::RECONFIGURATION);
- PumpLoop();
StartSyncScheduler(SyncScheduler::NORMAL_MODE);
- RunLoop();
scheduler()->ScheduleNudge(
zero(), NUDGE_SOURCE_LOCAL, types, FROM_HERE);
@@ -961,7 +931,6 @@ TEST_F(SyncSchedulerTest, BackoffElevation) {
EXPECT_CALL(*delay(), GetDelay(Eq(fifth))).WillOnce(Return(sixth));
StartSyncScheduler(SyncScheduler::NORMAL_MODE);
- RunLoop();
// Run again with a nudge.
scheduler()->ScheduleNudge(
@@ -994,7 +963,6 @@ TEST_F(SyncSchedulerTest, BackoffRelief) {
// Optimal start for the post-backoff poll party.
TimeTicks optimal_start = TimeTicks::Now();
StartSyncScheduler(SyncScheduler::NORMAL_MODE);
- RunLoop();
// Run again to wait for polling.
scheduler()->ScheduleNudge(zero(), NUDGE_SOURCE_LOCAL,
@@ -1041,9 +1009,8 @@ TEST_F(SyncSchedulerTest, TransientPollFailure) {
RecordSyncShare(&r)));
StartSyncScheduler(SyncScheduler::NORMAL_MODE);
- RunLoop();
- // Run the unsucessful poll. The failed poll should not trigger backoff.
+ // Run the unsucessful poll. The failed poll should not trigger backoff.
RunLoop();
EXPECT_FALSE(scheduler()->IsBackingOff());
@@ -1075,7 +1042,6 @@ TEST_F(SyncSchedulerTest, SyncerSteps) {
EXPECT_CALL(*syncer(), SyncShare(_, SYNCER_BEGIN, SYNCER_END))
.WillOnce(Invoke(sessions::test_util::SimulateSuccess));
StartSyncScheduler(SyncScheduler::NORMAL_MODE);
- RunLoop();
scheduler()->ScheduleNudge(
zero(), NUDGE_SOURCE_LOCAL, ModelTypeSet(), FROM_HERE);
@@ -1090,11 +1056,8 @@ TEST_F(SyncSchedulerTest, SyncerSteps) {
EXPECT_CALL(*syncer(), SyncShare(_, CLEAR_PRIVATE_DATA, CLEAR_PRIVATE_DATA))
.WillOnce(Invoke(sessions::test_util::SimulateSuccess));
StartSyncScheduler(SyncScheduler::NORMAL_MODE);
- RunLoop();
- scheduler()->ScheduleClearUserData();
- PumpLoop();
- PumpLoop();
+ scheduler()->ClearUserData();
StopSyncScheduler();
Mock::VerifyAndClearExpectations(syncer());
@@ -1103,12 +1066,9 @@ TEST_F(SyncSchedulerTest, SyncerSteps) {
EXPECT_CALL(*syncer(), SyncShare(_, DOWNLOAD_UPDATES, APPLY_UPDATES))
.WillOnce(Invoke(sessions::test_util::SimulateSuccess));
StartSyncScheduler(SyncScheduler::CONFIGURATION_MODE);
- RunLoop();
- scheduler()->ScheduleConfig(
+ scheduler()->Configure(
ModelTypeSet(), GetUpdatesCallerInfo::RECONFIGURATION);
- PumpLoop();
- PumpLoop();
StopSyncScheduler();
Mock::VerifyAndClearExpectations(syncer());
@@ -1118,12 +1078,8 @@ TEST_F(SyncSchedulerTest, SyncerSteps) {
SyncShare(_, CLEANUP_DISABLED_TYPES, CLEANUP_DISABLED_TYPES))
.WillOnce(Invoke(sessions::test_util::SimulateSuccess));
StartSyncScheduler(SyncScheduler::NORMAL_MODE);
- RunLoop();
- scheduler()->ScheduleCleanupDisabledTypes();
- // Only need to pump once, as ScheduleCleanupDisabledTypes()
- // schedules the job directly.
- PumpLoop();
+ scheduler()->CleanupDisabledTypes();
StopSyncScheduler();
Mock::VerifyAndClearExpectations(syncer());
@@ -1137,7 +1093,6 @@ TEST_F(SyncSchedulerTest, SyncerSteps) {
scheduler()->OnReceivedLongPollIntervalUpdate(poll);
StartSyncScheduler(SyncScheduler::NORMAL_MODE);
- RunLoop();
// Run again to wait for polling.
RunLoop();
@@ -1160,7 +1115,6 @@ TEST_F(SyncSchedulerTest, StartWhenNotConnected) {
.WillOnce(Invoke(sessions::test_util::SimulateConnectionFailure))
.WillOnce(QuitLoopNowAction());
StartSyncScheduler(SyncScheduler::NORMAL_MODE);
- MessageLoop::current()->RunAllPending();
scheduler()->ScheduleNudge(
zero(), NUDGE_SOURCE_LOCAL, ModelTypeSet(), FROM_HERE);
@@ -1181,7 +1135,6 @@ TEST_F(SyncSchedulerTest, SetsPreviousRoutingInfo) {
EXPECT_CALL(*syncer(), SyncShare(_,_,_)).Times(1);
StartSyncScheduler(SyncScheduler::NORMAL_MODE);
- RunLoop();
scheduler()->ScheduleNudge(
zero(), NUDGE_SOURCE_LOCAL, ModelTypeSet(), FROM_HERE);

Powered by Google App Engine
This is Rietveld 408576698