Avoid SEGFAULT when scheduler is stopped as part of sync cycle.

Sync cycle might receive response from server that causes scheduler to
stop and draws pending_configure_params_ invalid. Check started_
before dereferencing pending_configure_params_ to avoid SEGFAULT.

BUG=320738

Review URL: https://codereview.chromium.org/62263003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@236138 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 38 insertions, 1 deletions

diff --git a/sync/engine/sync_scheduler_impl.cc b/sync/engine/sync_scheduler_impl.cc
index 66460d3..85e1b67 100644
--- a/sync/engine/sync_scheduler_impl.cc
+++ b/sync/engine/sync_scheduler_impl.cc
@@ -528,7 +528,9 @@ void SyncSchedulerImpl::DoConfigurationSyncSessionJob(JobPriority priority) {
     NotifyRetryTime(base::Time());
   } else {
     HandleFailure(session->status_controller().model_neutral_state());
-    if (!pending_configure_params_->retry_task.is_null()) {
+    // Sync cycle might receive response from server that causes scheduler to
+    // stop and draws pending_configure_params_ invalid.
+    if (started_ && !pending_configure_params_->retry_task.is_null()) {
       pending_configure_params_->retry_task.Run();
       pending_configure_params_->retry_task.Reset();
     }
diff --git a/sync/engine/sync_scheduler_unittest.cc b/sync/engine/sync_scheduler_unittest.cc
index 56ac356..1a60f86 100644
--- a/sync/engine/sync_scheduler_unittest.cc
+++ b/sync/engine/sync_scheduler_unittest.cc
@@ -256,6 +256,10 @@ ACTION_P2(RecordSyncShareMultiple, times, quit_after) {
   return true;
 }
 
+ACTION_P(StopScheduler, scheduler) {
+  scheduler->Stop();
+}
+
 ACTION(AddFailureAndQuitLoopNow) {
   ADD_FAILURE();
   QuitLoopNow();
@@ -364,6 +368,37 @@ TEST_F(SyncSchedulerTest, ConfigWithBackingOff) {
   ASSERT_EQ(1, ready_counter.times_called());
 }
 
+// Simuilate SyncSchedulerImpl::Stop being called in the middle of Configure.
+// This can happen if server returns NOT_MY_BIRTHDAY.
+TEST_F(SyncSchedulerTest, ConfigWithStop) {
+  UseMockDelayProvider();
+  EXPECT_CALL(*delay(), GetDelay(_))
+      .WillRepeatedly(Return(TimeDelta::FromMilliseconds(1)));
+  SyncShareTimes times;
+  const ModelTypeSet model_types(BOOKMARKS);
+
+  StartSyncScheduler(SyncScheduler::CONFIGURATION_MODE);
+
+  // Make ConfigureSyncShare call scheduler->Stop(). It is not supposed to call
+  // retry_task or dereference configuration params.
+  EXPECT_CALL(*syncer(), ConfigureSyncShare(_,_,_))
+      .WillOnce(DoAll(Invoke(sessions::test_util::SimulateConfigureFailed),
+                      StopScheduler(scheduler()),
+                      RecordSyncShare(&times)));
+
+  CallbackCounter ready_counter;
+  CallbackCounter retry_counter;
+  ConfigurationParams params(
+      GetUpdatesCallerInfo::RECONFIGURATION,
+      model_types,
+      TypesToRoutingInfo(model_types),
+      base::Bind(&CallbackCounter::Callback, base::Unretained(&ready_counter)),
+      base::Bind(&CallbackCounter::Callback, base::Unretained(&retry_counter)));
+  scheduler()->ScheduleConfiguration(params);
+  ASSERT_EQ(0, ready_counter.times_called());
+  ASSERT_EQ(0, retry_counter.times_called());
+}
+
 // Issue a nudge when the config has failed. Make sure both the config and
 // nudge are executed.
 TEST_F(SyncSchedulerTest, NudgeWithConfigWithBackingOff) {