sync: make scheduling logic and job ownership more obvious.

sync/engine/sync_scheduler_unittest.cc

Index: sync/engine/sync_scheduler_unittest.cc
diff --git a/sync/engine/sync_scheduler_unittest.cc b/sync/engine/sync_scheduler_unittest.cc
index 31f850a5b5d0a0cecc4455db6142be95766991ab..7746aea533eae49396d2eef9c104ae7eb2b1a729 100644
--- a/sync/engine/sync_scheduler_unittest.cc
+++ b/sync/engine/sync_scheduler_unittest.cc
@@ -42,7 +42,7 @@ using sync_pb::GetUpdatesCallerInfo;
 
 class MockSyncer : public Syncer {
  public:
-  MOCK_METHOD3(SyncShare, void(sessions::SyncSession*, SyncerStep,
+  MOCK_METHOD3(SyncShare, bool(sessions::SyncSession*, SyncerStep,
                                SyncerStep));
 };
 
@@ -110,6 +110,7 @@ class SyncSchedulerTest : public testing::Test {
     routing_info_[THEMES] = GROUP_UI;
     routing_info_[NIGORI] = GROUP_PASSIVE;
 
+    workers_.clear();
     workers_.push_back(make_scoped_refptr(new FakeModelWorker(GROUP_UI)));
     workers_.push_back(make_scoped_refptr(new FakeModelWorker(GROUP_DB)));
     workers_.push_back(make_scoped_refptr(new FakeModelWorker(GROUP_PASSIVE)));
@@ -245,6 +246,7 @@ ACTION_P(RecordSyncShare, record) {
   RecordSyncShareImpl(arg0, record);
   if (MessageLoop::current()->is_running())
     QuitLoopNow();
+  return true;
 }
 
 ACTION_P2(RecordSyncShareMultiple, record, quit_after) {
@@ -254,15 +256,18 @@ ACTION_P2(RecordSyncShareMultiple, record, quit_after) {
       MessageLoop::current()->is_running()) {
     QuitLoopNow();
   }
+  return true;
 }
 
 ACTION(AddFailureAndQuitLoopNow) {
   ADD_FAILURE();
   QuitLoopNow();
+  return true;
 }
 
 ACTION(QuitLoopNowAction) {
   QuitLoopNow();
+  return true;
 }
 
 // Test nudge scheduling.
@@ -646,8 +651,10 @@ TEST_F(SyncSchedulerTest, PollIntervalUpdate) {
   TimeDelta poll2(TimeDelta::FromMilliseconds(30));
   scheduler()->OnReceivedLongPollIntervalUpdate(poll1);
   EXPECT_CALL(*syncer(), SyncShare(_,_,_)).Times(AtLeast(kMinNumSamples))
-      .WillOnce(WithArg<0>(
-          sessions::test_util::SimulatePollIntervalUpdate(poll2)))
+      .WillOnce(DoAll(
+          WithArg<0>(
+              sessions::test_util::SimulatePollIntervalUpdate(poll2)),
+          Return(true)))
       .WillRepeatedly(
           DoAll(Invoke(sessions::test_util::SimulateSuccess),
                 WithArg<0>(
@@ -700,7 +707,9 @@ TEST_F(SyncSchedulerTest, ThrottlingDoesThrottle) {
   scheduler()->OnReceivedLongPollIntervalUpdate(poll);
 
   EXPECT_CALL(*syncer(), SyncShare(_,_,_))
-      .WillOnce(WithArg<0>(sessions::test_util::SimulateThrottled(throttle)))
+      .WillOnce(DoAll(
+          WithArg<0>(sessions::test_util::SimulateThrottled(throttle)),
+          Return(true)))
       .WillRepeatedly(AddFailureAndQuitLoopNow());
 
   StartSyncScheduler(SyncScheduler::NORMAL_MODE);
@@ -729,7 +738,9 @@ TEST_F(SyncSchedulerTest, ThrottlingExpires) {
 
   ::testing::InSequence seq;
   EXPECT_CALL(*syncer(), SyncShare(_,_,_))
-      .WillOnce(WithArg<0>(sessions::test_util::SimulateThrottled(throttle1)))
+      .WillOnce(DoAll(
+          WithArg<0>(sessions::test_util::SimulateThrottled(throttle1)),
+          Return(true)))
       .RetiresOnSaturation();
   EXPECT_CALL(*syncer(), SyncShare(_,_,_))
       .WillRepeatedly(DoAll(Invoke(sessions::test_util::SimulateSuccess),
@@ -827,7 +838,9 @@ TEST_F(BackoffTriggersSyncSchedulerTest, FailCommitOnce) {
 // retry.  Expect that this clears the backoff state.
 TEST_F(BackoffTriggersSyncSchedulerTest, FailDownloadOnceThenSucceed) {
   EXPECT_CALL(*syncer(), SyncShare(_,_,_))
-      .WillOnce(Invoke(sessions::test_util::SimulateDownloadUpdatesFailed))
+      .WillOnce(DoAll(
+          Invoke(sessions::test_util::SimulateDownloadUpdatesFailed),
+          Return(true)))
       .WillOnce(DoAll(Invoke(sessions::test_util::SimulateSuccess),
                       QuitLoopNowAction()));
   EXPECT_FALSE(RunAndGetBackoff());
@@ -837,7 +850,9 @@ TEST_F(BackoffTriggersSyncSchedulerTest, FailDownloadOnceThenSucceed) {
 // that this clears the backoff state.
 TEST_F(BackoffTriggersSyncSchedulerTest, FailCommitOnceThenSucceed) {
   EXPECT_CALL(*syncer(), SyncShare(_,_,_))
-      .WillOnce(Invoke(sessions::test_util::SimulateCommitFailed))
+      .WillOnce(DoAll(
+          Invoke(sessions::test_util::SimulateCommitFailed),
+          Return(true)))
       .WillOnce(DoAll(Invoke(sessions::test_util::SimulateSuccess),
                       QuitLoopNowAction()));
   EXPECT_FALSE(RunAndGetBackoff());
@@ -847,7 +862,9 @@ TEST_F(BackoffTriggersSyncSchedulerTest, FailCommitOnceThenSucceed) {
 // Expect this will leave the scheduler in backoff.
 TEST_F(BackoffTriggersSyncSchedulerTest, FailDownloadTwice) {
   EXPECT_CALL(*syncer(), SyncShare(_,_,_))
-      .WillOnce(Invoke(sessions::test_util::SimulateDownloadUpdatesFailed))
+      .WillOnce(DoAll(
+          Invoke(sessions::test_util::SimulateDownloadUpdatesFailed),
+          Return(true)))
       .WillRepeatedly(DoAll(
               Invoke(sessions::test_util::SimulateDownloadUpdatesFailed),
               QuitLoopNowAction()));
@@ -858,7 +875,9 @@ TEST_F(BackoffTriggersSyncSchedulerTest, FailDownloadTwice) {
 // updates. Expect this will leave the scheduler in backoff.
 TEST_F(BackoffTriggersSyncSchedulerTest, FailGetEncryptionKey) {
   EXPECT_CALL(*syncer(), SyncShare(_,_,_))
-      .WillOnce(Invoke(sessions::test_util::SimulateGetEncryptionKeyFailed))
+      .WillOnce(DoAll(
+          Invoke(sessions::test_util::SimulateGetEncryptionKeyFailed),
+          Return(true)))
       .WillRepeatedly(DoAll(
               Invoke(sessions::test_util::SimulateGetEncryptionKeyFailed),
               QuitLoopNowAction()));
@@ -1040,7 +1059,8 @@ TEST_F(SyncSchedulerTest, TransientPollFailure) {
 TEST_F(SyncSchedulerTest, SyncerSteps) {
   // Nudges.
   EXPECT_CALL(*syncer(), SyncShare(_, SYNCER_BEGIN, SYNCER_END))
-      .WillOnce(Invoke(sessions::test_util::SimulateSuccess));
+      .WillOnce(DoAll(Invoke(sessions::test_util::SimulateSuccess),
+                      Return(true)));
   StartSyncScheduler(SyncScheduler::NORMAL_MODE);
 
   scheduler()->ScheduleNudgeAsync(
@@ -1054,7 +1074,8 @@ TEST_F(SyncSchedulerTest, SyncerSteps) {
 
   // Configuration.
   EXPECT_CALL(*syncer(), SyncShare(_, DOWNLOAD_UPDATES, APPLY_UPDATES))
-      .WillOnce(Invoke(sessions::test_util::SimulateSuccess));
+      .WillOnce(DoAll(Invoke(sessions::test_util::SimulateSuccess),
+                      Return(true)));
   StartSyncScheduler(SyncScheduler::CONFIGURATION_MODE);
 
   ModelTypeSet model_types(BOOKMARKS);
@@ -1095,8 +1116,10 @@ TEST_F(SyncSchedulerTest, StartWhenNotConnected) {
   connection()->SetServerNotReachable();
   connection()->UpdateConnectionStatus();
   EXPECT_CALL(*syncer(), SyncShare(_,_,_))
-    .WillOnce(Invoke(sessions::test_util::SimulateConnectionFailure))
-    .WillOnce(QuitLoopNowAction());
+    .WillOnce(DoAll(Invoke(sessions::test_util::SimulateConnectionFailure),
+                    Return(true)))
+    .WillOnce(DoAll(Invoke(sessions::test_util::SimulateSuccess),
+                    QuitLoopNowAction()));
   StartSyncScheduler(SyncScheduler::NORMAL_MODE);
 
   scheduler()->ScheduleNudgeAsync(