sync: make scheduling logic and job ownership more obvious.

sync/engine/sync_scheduler_unittest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/compiler_specific.h"
 #include "base/memory/weak_ptr.h"
 #include "base/message_loop.h"
 #include "base/test/test_timeouts.h"
@@ skipping 24 matching lines @@
 using testing::WithArg;
 
 namespace syncer {
 using sessions::SyncSession;
 using sessions::SyncSessionContext;
 using sessions::SyncSessionSnapshot;
 using sync_pb::GetUpdatesCallerInfo;
 
 class MockSyncer : public Syncer {
  public:
-  MOCK_METHOD3(SyncShare, void(sessions::SyncSession*, SyncerStep,
+  MOCK_METHOD3(SyncShare, bool(sessions::SyncSession*, SyncerStep,
                                SyncerStep));
 };
 
 // Used when tests want to record syncing activity to examine later.
 struct SyncShareRecords {
   std::vector<TimeTicks> times;
   std::vector<SyncSessionSnapshot> snapshots;
 };
 
 void QuitLoopNow() {
@@ skipping 38 matching lines @@
    public:
     MockDelayProvider() : BackoffDelayProvider(
         TimeDelta::FromSeconds(kInitialBackoffRetrySeconds),
         TimeDelta::FromSeconds(kInitialBackoffShortRetrySeconds)) {
     }
 
     MOCK_METHOD1(GetDelay, TimeDelta(const TimeDelta&));
   };
 
   virtual void SetUp() {
+    message_loop_.reset(new MessageLoop());
     dir_maker_.SetUp();
     syncer_ = new MockSyncer();
     delay_ = NULL;
 
     routing_info_[BOOKMARKS] = GROUP_UI;
     routing_info_[AUTOFILL] = GROUP_DB;
     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)));
 
     std::vector<ModelSafeWorker*> workers;
     for (std::vector<scoped_refptr<FakeModelWorker> >::iterator it =
          workers_.begin(); it != workers_.end(); ++it) {
       workers.push_back(it->get());
     }
 
@@ skipping 93 matching lines @@
   }
 
   SyncSessionContext* context() { return context_.get(); }
 
  private:
   syncable::Directory* directory() {
     return dir_maker_.directory();
   }
 
   base::WeakPtrFactory<SyncSchedulerTest> weak_ptr_factory_;
-  MessageLoop message_loop_;
+  scoped_ptr<MessageLoop> message_loop_;

[akalin, 2012/10/26 06:52:29]

why'd you have to do this?

[tim (not reviewing), 2012/10/26 22:52:27]

Ah, leftover from some debugging.  Undone.

   TestDirectorySetterUpper dir_maker_;
   scoped_ptr<MockConnectionManager> connection_;
   scoped_ptr<SyncSessionContext> context_;
   scoped_ptr<SyncSchedulerImpl> scheduler_;
   MockSyncer* syncer_;
   MockDelayProvider* delay_;
   std::vector<scoped_refptr<FakeModelWorker> > workers_;
   FakeExtensionsActivityMonitor extensions_activity_monitor_;
   scoped_ptr<ThrottledDataTypeTracker> throttled_data_type_tracker_;
   ModelSafeRoutingInfo routing_info_;
 };
 
 void RecordSyncShareImpl(SyncSession* s, SyncShareRecords* record) {
   record->times.push_back(TimeTicks::Now());
   record->snapshots.push_back(s->TakeSnapshot());
 }
 
 ACTION_P(RecordSyncShare, record) {
   RecordSyncShareImpl(arg0, record);
   if (MessageLoop::current()->is_running())
     QuitLoopNow();
+  return true;
 }
 
 ACTION_P2(RecordSyncShareMultiple, record, quit_after) {
   RecordSyncShareImpl(arg0, record);
   EXPECT_LE(record->times.size(), quit_after);
   if (record->times.size() >= quit_after &&
       MessageLoop::current()->is_running()) {
     QuitLoopNow();
   }
+  return true;
 }
 
 ACTION(AddFailureAndQuitLoopNow) {
   ADD_FAILURE();
   QuitLoopNow();
+  return true;
 }
 
 ACTION(QuitLoopNowAction) {
   QuitLoopNow();
+  return true;
 }
 
 // Test nudge scheduling.
 TEST_F(SyncSchedulerTest, Nudge) {
   SyncShareRecords records;
   ModelTypeSet model_types(BOOKMARKS);
 
   EXPECT_CALL(*syncer(), SyncShare(_,_,_))
       .WillOnce(DoAll(Invoke(sessions::test_util::SimulateSuccess),
                       WithArg<0>(RecordSyncShare(&records))))
@@ skipping 363 matching lines @@
   AnalyzePollRun(records, kMinNumSamples, optimal_start, poll_interval);
 }
 
 // Test that polling intervals are updated when needed.
 TEST_F(SyncSchedulerTest, PollIntervalUpdate) {
   SyncShareRecords records;
   TimeDelta poll1(TimeDelta::FromMilliseconds(120));
   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>(
                     RecordSyncShareMultiple(&records, kMinNumSamples))));
 
   TimeTicks optimal_start = TimeTicks::Now() + poll1 + poll2;
   StartSyncScheduler(SyncScheduler::NORMAL_MODE);
 
   // Run again to wait for polling.
   RunLoop();
@@ skipping 27 matching lines @@
       zero(), NUDGE_SOURCE_LOCAL, model_types, FROM_HERE);
   RunLoop();
 
   EXPECT_EQ(delay2, scheduler()->GetSessionsCommitDelay());
   StopSyncScheduler();
 }
 
 // Test that a sync session is run through to completion.
 TEST_F(SyncSchedulerTest, HasMoreToSync) {
   EXPECT_CALL(*syncer(), SyncShare(_,_,_))
-      .WillOnce(Invoke(sessions::test_util::SimulateHasMoreToSync))
+      .WillOnce(DoAll(Invoke(sessions::test_util::SimulateHasMoreToSync),
+                      Return(true)))
       .WillOnce(DoAll(Invoke(sessions::test_util::SimulateSuccess),
                       QuitLoopNowAction()));
   StartSyncScheduler(SyncScheduler::NORMAL_MODE);
 
   scheduler()->ScheduleNudgeAsync(
       zero(), NUDGE_SOURCE_LOCAL, ModelTypeSet(BOOKMARKS), FROM_HERE);
   RunLoop();
   // If more nudges are scheduled, they'll be waited on by TearDown, and would
   // cause our expectation to break.
 }
 
 // Test that continuations can go into backoff.
 TEST_F(SyncSchedulerTest, HasMoreToSyncThenFails) {
   EXPECT_CALL(*syncer(), SyncShare(_,_,_))
-    .WillOnce(Invoke(sessions::test_util::SimulateHasMoreToSync))
+    .WillOnce(DoAll(Invoke(sessions::test_util::SimulateHasMoreToSync),
+                    Return(true)))
     .WillOnce(DoAll(Invoke(sessions::test_util::SimulateCommitFailed),
                     QuitLoopNowAction()));
   StartSyncScheduler(SyncScheduler::NORMAL_MODE);
 
   scheduler()->ScheduleNudgeAsync(
       zero(), NUDGE_SOURCE_LOCAL, ModelTypeSet(BOOKMARKS), FROM_HERE);
 
   // We should detect the failure on the second sync share, and go into backoff.
   EXPECT_TRUE(RunAndGetBackoff());
 }
 
 // Test that no syncing occurs when throttled.
 TEST_F(SyncSchedulerTest, ThrottlingDoesThrottle) {
   const ModelTypeSet types(BOOKMARKS);
   TimeDelta poll(TimeDelta::FromMilliseconds(5));
   TimeDelta throttle(TimeDelta::FromMinutes(10));
   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);
 
   scheduler()->ScheduleNudgeAsync(
       zero(), NUDGE_SOURCE_LOCAL, types, FROM_HERE);
   PumpLoop();
 
   StartSyncScheduler(SyncScheduler::CONFIGURATION_MODE);
 
   CallbackCounter counter;
   ConfigurationParams params(
       GetUpdatesCallerInfo::RECONFIGURATION,
       types,
       TypesToRoutingInfo(types),
       base::Bind(&CallbackCounter::Callback, base::Unretained(&counter)));
   ASSERT_FALSE(scheduler()->ScheduleConfiguration(params));
   ASSERT_EQ(0, counter.times_called());
 }
 
 TEST_F(SyncSchedulerTest, ThrottlingExpires) {
   SyncShareRecords records;
   TimeDelta poll(TimeDelta::FromMilliseconds(15));
   TimeDelta throttle1(TimeDelta::FromMilliseconds(150));
   scheduler()->OnReceivedLongPollIntervalUpdate(poll);
 
   ::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),
            WithArg<0>(RecordSyncShareMultiple(&records, kMinNumSamples))));
 
   TimeTicks optimal_start = TimeTicks::Now() + poll + throttle1;
   StartSyncScheduler(SyncScheduler::NORMAL_MODE);
 
   // Run again to wait for polling.
   RunLoop();
@@ skipping 77 matching lines @@
   EXPECT_CALL(*syncer(), SyncShare(_,_,_))
       .WillOnce(DoAll(Invoke(sessions::test_util::SimulateCommitFailed),
                       QuitLoopNowAction()));
   EXPECT_TRUE(RunAndGetBackoff());
 }
 
 // Have the syncer fail during download updates and succeed on the first
 // 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());
 }
 
 // Have the syncer fail during commit and succeed on the first retry.  Expect
 // 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());
 }
 
 // Have the syncer fail to download updates and fail again on the retry.
 // 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()));
   EXPECT_TRUE(RunAndGetBackoff());
 }
 
 // Have the syncer fail to get the encryption key yet succeed in downloading
 // 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()));
   EXPECT_TRUE(RunAndGetBackoff());
 }
 
 // Test that no polls or extraneous nudges occur when in backoff.
 TEST_F(SyncSchedulerTest, BackoffDropsJobs) {
   SyncShareRecords r;
   TimeDelta poll(TimeDelta::FromMilliseconds(5));
@@ skipping 161 matching lines @@
 
   // Run the successful poll.
   RunLoop();
   EXPECT_FALSE(scheduler()->IsBackingOff());
 }
 
 // Test that appropriate syncer steps are requested for each job type.
 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(
       zero(), NUDGE_SOURCE_LOCAL, ModelTypeSet(BOOKMARKS), FROM_HERE);
   PumpLoop();
   // Pump again to run job.
   PumpLoop();
 
   StopSyncScheduler();
   Mock::VerifyAndClearExpectations(syncer());
 
   // 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);
   CallbackCounter counter;
   ConfigurationParams params(
       GetUpdatesCallerInfo::RECONFIGURATION,
       model_types,
       TypesToRoutingInfo(model_types),
       base::Bind(&CallbackCounter::Callback, base::Unretained(&counter)));
   ASSERT_TRUE(scheduler()->ScheduleConfiguration(params));
@@ skipping 20 matching lines @@
   StopSyncScheduler();
   Mock::VerifyAndClearExpectations(syncer());
 }
 
 // Test that starting the syncer thread without a valid connection doesn't
 // break things when a connection is detected.
 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(
       zero(), NUDGE_SOURCE_LOCAL, ModelTypeSet(BOOKMARKS), FROM_HERE);
   // Should save the nudge for until after the server is reachable.
   MessageLoop::current()->RunAllPending();
 
   connection()->SetServerReachable();
   connection()->UpdateConnectionStatus();
   scheduler()->OnConnectionStatusChange();
   MessageLoop::current()->RunAllPending();
 }
 
 }  // namespace syncer