Reland 148792

Relanding after rebasing.

[Sync] Remove CleanupDisabledTypes command and move purge logic into SyncManager."

Original codereview at http://codereview.chromium.org/10541079/

BUG=131433, 90868
TBR=tim@chromium.org


Review URL: https://chromiumcodereview.appspot.com/10829086

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

8 files changed, 6 insertions, 308 deletions

diff --git a/sync/engine/cleanup_disabled_types_command.cc b/sync/engine/cleanup_disabled_types_command.cc
deleted file mode 100644
index d7dc7a6..0000000
--- a/sync/engine/cleanup_disabled_types_command.cc
+++ /dev/null
@@ -1,67 +0,0 @@
-// 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 "sync/engine/cleanup_disabled_types_command.h"
-
-#include <algorithm>
-
-#include "sync/internal_api/public/base/model_type.h"
-#include "sync/sessions/sync_session.h"
-#include "sync/sessions/sync_session_context.h"
-#include "sync/syncable/directory.h"
-
-namespace syncer {
-
-CleanupDisabledTypesCommand::CleanupDisabledTypesCommand() {}
-CleanupDisabledTypesCommand::~CleanupDisabledTypesCommand() {}
-
-SyncerError CleanupDisabledTypesCommand::ExecuteImpl(
-    sessions::SyncSession* session) {
-  // Because a full directory purge is slow, we avoid purging
-  // undesired types unless we have reason to believe they were
-  // previously enabled.  Because purging could theoretically fail on
-  // the first sync session (when there's no previous routing info) we
-  // pay the full directory scan price once and do a "deep clean" of
-  // types that may potentially need cleanup so that we converge to
-  // the correct state.
-  //
-  //                          in_previous  |   !in_previous
-  //                                       |
-  //   initial_sync_ended     should clean |  may have attempted cleanup
-  //  !initial_sync_ended     should clean |  may have never been enabled, or
-  //                                       |  could have been disabled before
-  //                                       |  initial sync ended and cleanup
-  //                                       |  may not have happened yet
-  //                                       |  (failure, browser restart
-  //                                       |  before another sync session,..)
-
-  const ModelTypeSet enabled_types =
-      GetRoutingInfoTypes(session->routing_info());
-
-  const ModelTypeSet previous_enabled_types =
-      GetRoutingInfoTypes(
-          session->context()->previous_session_routing_info());
-
-  ModelTypeSet to_cleanup = Difference(ModelTypeSet::All(), enabled_types);
-
-  // If |previous_enabled_types| is non-empty (i.e., not the first
-  // sync session), set |to_cleanup| to its intersection with
-  // |previous_enabled_types|.
-  if (!previous_enabled_types.Empty()) {
-    to_cleanup.RetainAll(previous_enabled_types);
-  }
-
-  DVLOG(1) << "enabled_types = " << ModelTypeSetToString(enabled_types)
-           << ", previous_enabled_types = "
-           << ModelTypeSetToString(previous_enabled_types)
-           << ", to_cleanup = " << ModelTypeSetToString(to_cleanup);
-
-  if (to_cleanup.Empty())
-    return SYNCER_OK;
-
-  session->context()->directory()->PurgeEntriesWithTypeIn(to_cleanup);
-  return SYNCER_OK;
-}
-
-}  // namespace syncer
diff --git a/sync/engine/cleanup_disabled_types_command.h b/sync/engine/cleanup_disabled_types_command.h
deleted file mode 100644
index 2090f1d..0000000
--- a/sync/engine/cleanup_disabled_types_command.h
+++ /dev/null
@@ -1,44 +0,0 @@
-// 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.
-
-#ifndef SYNC_ENGINE_CLEANUP_DISABLED_TYPES_COMMAND_H_
-#define SYNC_ENGINE_CLEANUP_DISABLED_TYPES_COMMAND_H_
-
-#include "base/compiler_specific.h"
-#include "sync/engine/syncer_command.h"
-
-namespace syncer {
-
-// A syncer command that purges (from memory and disk) entries belonging to
-// a ModelType or ServerModelType that the user has not elected to sync.
-//
-// This is done as part of a session to 1) ensure it does not block the UI,
-// and 2) avoid complicated races that could arise between a) deleting
-// things b) a sync session trying to use these things c) and the potential
-// re-enabling of the data type by the user before some scheduled deletion
-// took place.  Here, we are safe to perform I/O synchronously and we know it
-// is a safe time to delete as we are in the only active session.
-//
-// The removal from memory is done synchronously, while the disk purge is left
-// to an asynchronous SaveChanges operation.  However, all the updates for
-// meta data fields (such as initial_sync_ended) as well as the actual entry
-// deletions will be committed in a single sqlite transaction.  Thus it is
-// possible that disabled types re-appear (in the sync db) after a reboot,
-// but things will remain in a consistent state.  This kind of error case is
-// cared for in this command by retrying; see ExecuteImpl.
-class CleanupDisabledTypesCommand : public SyncerCommand {
- public:
-  CleanupDisabledTypesCommand();
-  virtual ~CleanupDisabledTypesCommand();
-
-  // SyncerCommand implementation.
-  virtual SyncerError ExecuteImpl(sessions::SyncSession* session) OVERRIDE;
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(CleanupDisabledTypesCommand);
-};
-
-}  // namespace syncer
-
-#endif  // SYNC_ENGINE_CLEANUP_DISABLED_TYPES_COMMAND_H_
diff --git a/sync/engine/cleanup_disabled_types_command_unittest.cc b/sync/engine/cleanup_disabled_types_command_unittest.cc
deleted file mode 100644
index 286ca8d..0000000
--- a/sync/engine/cleanup_disabled_types_command_unittest.cc
+++ /dev/null
@@ -1,75 +0,0 @@
-// 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 <vector>
-
-#include "sync/engine/cleanup_disabled_types_command.h"
-
-#include "sync/internal_api/public/base/model_type_test_util.h"
-#include "sync/sessions/sync_session.h"
-#include "sync/test/engine/syncer_command_test.h"
-#include "testing/gmock/include/gmock/gmock.h"
-#include "testing/gtest/include/gtest/gtest.h"
-
-namespace syncer {
-
-namespace {
-
-using testing::_;
-
-class CleanupDisabledTypesCommandTest : public MockDirectorySyncerCommandTest {
- public:
-  CleanupDisabledTypesCommandTest() {}
-
-  virtual void SetUp() {
-    mutable_routing_info()->clear();
-    (*mutable_routing_info())[BOOKMARKS] = GROUP_PASSIVE;
-    MockDirectorySyncerCommandTest::SetUp();
-  }
-};
-
-// TODO(tim): Add syncer test to verify previous routing info is set.
-TEST_F(CleanupDisabledTypesCommandTest, NoPreviousRoutingInfo) {
-  CleanupDisabledTypesCommand command;
-  ModelTypeSet expected = ModelTypeSet::All();
-  expected.Remove(BOOKMARKS);
-  EXPECT_CALL(*mock_directory(),
-              PurgeEntriesWithTypeIn(HasModelTypes(expected)));
-  command.ExecuteImpl(session());
-}
-
-TEST_F(CleanupDisabledTypesCommandTest, NoPurge) {
-  CleanupDisabledTypesCommand command;
-  EXPECT_CALL(*mock_directory(), PurgeEntriesWithTypeIn(_)).Times(0);
-
-  ModelSafeRoutingInfo prev(routing_info());
-  session()->context()->set_previous_session_routing_info(prev);
-  (*mutable_routing_info())[AUTOFILL] = GROUP_PASSIVE;
-  command.ExecuteImpl(session());
-
-  prev = routing_info();
-  command.ExecuteImpl(session());
-}
-
-TEST_F(CleanupDisabledTypesCommandTest, TypeDisabled) {
-  CleanupDisabledTypesCommand command;
-
-  (*mutable_routing_info())[AUTOFILL] = GROUP_PASSIVE;
-  (*mutable_routing_info())[THEMES] = GROUP_PASSIVE;
-  (*mutable_routing_info())[EXTENSIONS] = GROUP_PASSIVE;
-
-  ModelSafeRoutingInfo prev(routing_info());
-  prev[PASSWORDS] = GROUP_PASSIVE;
-  prev[PREFERENCES] = GROUP_PASSIVE;
-  session()->context()->set_previous_session_routing_info(prev);
-
-  const ModelTypeSet expected(PASSWORDS, PREFERENCES);
-  EXPECT_CALL(*mock_directory(),
-              PurgeEntriesWithTypeIn(HasModelTypes(expected)));
-  command.ExecuteImpl(session());
-}
-
-}  // namespace
-
-}  // namespace syncer
diff --git a/sync/engine/sync_scheduler_impl.cc b/sync/engine/sync_scheduler_impl.cc
index e02de2c..72f8243 100644
--- a/sync/engine/sync_scheduler_impl.cc
+++ b/sync/engine/sync_scheduler_impl.cc
@@ -132,7 +132,6 @@ const char* SyncSchedulerImpl::SyncSessionJob::GetPurposeString(
     ENUM_CASE(POLL);
     ENUM_CASE(NUDGE);
     ENUM_CASE(CONFIGURATION);
-    ENUM_CASE(CLEANUP_DISABLED_TYPES);
   }
   NOTREACHED();
   return "";
@@ -354,17 +353,6 @@ bool SyncSchedulerImpl::ScheduleConfiguration(
                        &restricted_workers);
   session_context_->set_routing_info(params.routing_info);
 
-  // We rely on this not failing, so don't need to worry about checking for
-  // success. In addition, this will be removed as part of crbug.com/131433.
-  SyncSessionJob cleanup_job(
-      SyncSessionJob::CLEANUP_DISABLED_TYPES,
-      TimeTicks::Now(),
-      make_linked_ptr(CreateSyncSession(SyncSourceInfo())),
-      false,
-      ConfigurationParams(),
-      FROM_HERE);
-  DoSyncSessionJob(cleanup_job);
-
   if (params.keystore_key_status == ConfigurationParams::KEYSTORE_KEY_NEEDED) {
     // TODO(zea): implement in such a way that we can handle failures and the
     // subsequent retrys the scheduler might perform. See crbug.com/129665.
@@ -411,7 +399,6 @@ SyncSchedulerImpl::DecideWhileInWaitInterval(
     const SyncSessionJob& job) {
   DCHECK_EQ(MessageLoop::current(), sync_loop_);
   DCHECK(wait_interval_.get());
-  DCHECK_NE(job.purpose, SyncSessionJob::CLEANUP_DISABLED_TYPES);
 
   SDVLOG(2) << "DecideWhileInWaitInterval with WaitInterval mode "
             << WaitInterval::GetModeString(wait_interval_->mode)
@@ -444,8 +431,6 @@ SyncSchedulerImpl::DecideWhileInWaitInterval(
 SyncSchedulerImpl::JobProcessDecision SyncSchedulerImpl::DecideOnJob(
     const SyncSessionJob& job) {
   DCHECK_EQ(MessageLoop::current(), sync_loop_);
-  if (job.purpose == SyncSessionJob::CLEANUP_DISABLED_TYPES)
-    return CONTINUE;
 
   // See if our type is throttled.
   ModelTypeSet throttled_types =
@@ -539,8 +524,6 @@ bool SyncSchedulerImpl::ShouldRunJob(const SyncSessionJob& job) {
 
 void SyncSchedulerImpl::SaveJob(const SyncSessionJob& job) {
   DCHECK_EQ(MessageLoop::current(), sync_loop_);
-  // TODO(sync): Should we also check that job.purpose !=
-  // CLEANUP_DISABLED_TYPES?  (See http://crbug.com/90868.)
   if (job.purpose == SyncSessionJob::NUDGE) {
     SDVLOG(2) << "Saving a nudge job";
     InitOrCoalescePendingJob(job);
@@ -697,10 +680,6 @@ void SyncSchedulerImpl::SetSyncerStepsForPurpose(
       *start = SYNCER_BEGIN;
       *end = SYNCER_END;
       return;
-    case SyncSessionJob::CLEANUP_DISABLED_TYPES:
-      *start = CLEANUP_DISABLED_TYPES;
-      *end = CLEANUP_DISABLED_TYPES;
-      return;
     default:
       NOTREACHED();
       *start = SYNCER_END;
@@ -805,27 +784,6 @@ void SyncSchedulerImpl::DoSyncSessionJob(const SyncSessionJob& job) {
   FinishSyncSessionJob(job);
 }
 
-void SyncSchedulerImpl::UpdateCarryoverSessionState(
-    const SyncSessionJob& old_job) {
-  DCHECK_EQ(MessageLoop::current(), sync_loop_);
-  if (old_job.purpose == SyncSessionJob::CONFIGURATION) {
-    // Whatever types were part of a configuration task will have had updates
-    // downloaded.  For that reason, we make sure they get recorded in the
-    // event that they get disabled at a later time.
-    ModelSafeRoutingInfo r(session_context_->previous_session_routing_info());
-    if (!r.empty()) {
-      ModelSafeRoutingInfo temp_r;
-      ModelSafeRoutingInfo old_info(old_job.session->routing_info());
-      std::set_union(r.begin(), r.end(), old_info.begin(), old_info.end(),
-          std::insert_iterator<ModelSafeRoutingInfo>(temp_r, temp_r.begin()));
-      session_context_->set_previous_session_routing_info(temp_r);
-    }
-  } else {
-    session_context_->set_previous_session_routing_info(
-        old_job.session->routing_info());
-  }
-}
-
 void SyncSchedulerImpl::FinishSyncSessionJob(const SyncSessionJob& job) {
   DCHECK_EQ(MessageLoop::current(), sync_loop_);
   // Update timing information for how often datatypes are triggering nudges.
@@ -855,7 +813,6 @@ void SyncSchedulerImpl::FinishSyncSessionJob(const SyncSessionJob& job) {
   ServerConnectionManager* scm = session_context_->connection_manager();
   UpdateServerConnectionManagerStatus(scm->server_status());
 
-  UpdateCarryoverSessionState(job);
   if (IsSyncingCurrentlySilenced()) {
     SDVLOG(2) << "We are currently throttled; not scheduling the next sync.";
     // TODO(sync): Investigate whether we need to check job.purpose
diff --git a/sync/engine/sync_scheduler_impl.h b/sync/engine/sync_scheduler_impl.h
index 179f900..c86588a 100644
--- a/sync/engine/sync_scheduler_impl.h
+++ b/sync/engine/sync_scheduler_impl.h
@@ -99,12 +99,8 @@ class SyncSchedulerImpl : public SyncScheduler {
       // a sync.  The source is inferable from |session.source()|.
       NUDGE,
       // Typically used for fetching updates for a subset of the enabled types
-      // during initial sync or reconfiguration.  We don't run all steps of
-      // the sync cycle for these (e.g. CleanupDisabledTypes is skipped).
+      // during initial sync or reconfiguration.
       CONFIGURATION,
-      // The user disabled some types and we have to clean up the data
-      // for those.
-      CLEANUP_DISABLED_TYPES,
     };
     SyncSessionJob();
     SyncSessionJob(SyncSessionJobPurpose purpose, base::TimeTicks start,
@@ -220,10 +216,6 @@ class SyncSchedulerImpl : public SyncScheduler {
   // reset our state.
   void FinishSyncSessionJob(const SyncSessionJob& job);
 
-  // Record important state that might be needed in future syncs, such as which
-  // data types may require cleanup.
-  void UpdateCarryoverSessionState(const SyncSessionJob& old_job);
-
   // Helper to FinishSyncSessionJob to schedule the next sync operation.
   void ScheduleNextSync(const SyncSessionJob& old_job);
 
diff --git a/sync/engine/sync_scheduler_unittest.cc b/sync/engine/sync_scheduler_unittest.cc
index 6d98372..7581c4dc 100644
--- a/sync/engine/sync_scheduler_unittest.cc
+++ b/sync/engine/sync_scheduler_unittest.cc
@@ -301,9 +301,7 @@ TEST_F(SyncSchedulerTest, Config) {
   SyncShareRecords records;
   const ModelTypeSet model_types(BOOKMARKS);
 
-  EXPECT_CALL(*syncer(),
-              SyncShare(_,_,_))
-      .WillOnce(Invoke(sessions::test_util::SimulateSuccess))
+  EXPECT_CALL(*syncer(), SyncShare(_,_,_))
       .WillOnce(DoAll(Invoke(sessions::test_util::SimulateSuccess),
                       WithArg<0>(RecordSyncShare(&records))));
 
@@ -334,9 +332,7 @@ TEST_F(SyncSchedulerTest, ConfigWithBackingOff) {
   SyncShareRecords records;
   const ModelTypeSet model_types(BOOKMARKS);
 
-  EXPECT_CALL(*syncer(),
-              SyncShare(_,_,_))
-      .WillOnce(Invoke(sessions::test_util::SimulateSuccess))
+  EXPECT_CALL(*syncer(), SyncShare(_,_,_))
       .WillOnce(DoAll(Invoke(sessions::test_util::SimulateCommitFailed),
                       WithArg<0>(RecordSyncShare(&records))))
       .WillOnce(DoAll(Invoke(sessions::test_util::SimulateSuccess),
@@ -710,19 +706,14 @@ TEST_F(SyncSchedulerTest, HasMoreToSyncThenFails) {
   EXPECT_TRUE(RunAndGetBackoff());
 }
 
-// Test that no syncing occurs when throttled (although CleanupDisabledTypes
-// is allowed).
+// 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(_, CLEANUP_DISABLED_TYPES, CLEANUP_DISABLED_TYPES))
-      .WillOnce(Invoke(sessions::test_util::SimulateSuccess));
-  EXPECT_CALL(*syncer(), SyncShare(_,Not(CLEANUP_DISABLED_TYPES),
-                                     Not(CLEANUP_DISABLED_TYPES)))
+  EXPECT_CALL(*syncer(), SyncShare(_,_,_))
       .WillOnce(WithArg<0>(sessions::test_util::SimulateThrottled(throttle)))
       .WillRepeatedly(AddFailureAndQuitLoopNow());
 
@@ -775,7 +766,6 @@ TEST_F(SyncSchedulerTest, ConfigurationMode) {
   SyncShareRecords records;
   scheduler()->OnReceivedLongPollIntervalUpdate(poll);
   EXPECT_CALL(*syncer(), SyncShare(_,_,_))
-      .WillOnce(Invoke(sessions::test_util::SimulateSuccess))
       .WillOnce(DoAll(Invoke(sessions::test_util::SimulateSuccess),
                       WithArg<0>(RecordSyncShare(&records))));
 
@@ -900,10 +890,6 @@ TEST_F(SyncSchedulerTest, BackoffDropsJobs) {
   EXPECT_EQ(GetUpdatesCallerInfo::LOCAL,
             r.snapshots[1].source().updates_source);
 
-  // Cleanup is not affected by backoff, but it should not relieve it either.
-  EXPECT_CALL(*syncer(),
-              SyncShare(_, CLEANUP_DISABLED_TYPES, CLEANUP_DISABLED_TYPES))
-      .WillOnce(Invoke(sessions::test_util::SimulateSuccess));
   EXPECT_CALL(*delay(), GetDelay(_)).Times(0);
 
   StartSyncScheduler(SyncScheduler::CONFIGURATION_MODE);
@@ -1077,10 +1063,7 @@ TEST_F(SyncSchedulerTest, SyncerSteps) {
   StopSyncScheduler();
   Mock::VerifyAndClearExpectations(syncer());
 
-  // Configuration (always includes a cleanup disabled types).
-  EXPECT_CALL(*syncer(),
-              SyncShare(_, CLEANUP_DISABLED_TYPES, CLEANUP_DISABLED_TYPES))
-      .WillOnce(Invoke(sessions::test_util::SimulateSuccess));
+  // Configuration.
   EXPECT_CALL(*syncer(), SyncShare(_, DOWNLOAD_UPDATES, APPLY_UPDATES))
       .WillOnce(Invoke(sessions::test_util::SimulateSuccess));
   StartSyncScheduler(SyncScheduler::CONFIGURATION_MODE);
@@ -1099,25 +1082,6 @@ TEST_F(SyncSchedulerTest, SyncerSteps) {
   StopSyncScheduler();
   Mock::VerifyAndClearExpectations(syncer());
 
-  // Cleanup disabled types. Because no types are being configured, we just
-  // perform the cleanup.
-  EXPECT_CALL(*syncer(),
-              SyncShare(_, CLEANUP_DISABLED_TYPES, CLEANUP_DISABLED_TYPES)).
-      WillOnce(Invoke(sessions::test_util::SimulateSuccess));
-  StartSyncScheduler(SyncScheduler::CONFIGURATION_MODE);
-
-  CallbackCounter counter2;
-  ConfigurationParams params2(
-      GetUpdatesCallerInfo::RECONFIGURATION,
-      ModelTypeSet(),
-      ModelSafeRoutingInfo(),
-      ConfigurationParams::KEYSTORE_KEY_UNNECESSARY,
-      base::Bind(&CallbackCounter::Callback, base::Unretained(&counter2)));
-  ASSERT_TRUE(scheduler()->ScheduleConfiguration(params2));
-  ASSERT_EQ(1, counter2.times_called());
-  StopSyncScheduler();
-  Mock::VerifyAndClearExpectations(syncer());
-
   StartSyncScheduler(SyncScheduler::NORMAL_MODE);
 
   // Poll.
@@ -1163,24 +1127,4 @@ TEST_F(SyncSchedulerTest, StartWhenNotConnected) {
   MessageLoop::current()->RunAllPending();
 }
 
-TEST_F(SyncSchedulerTest, SetsPreviousRoutingInfo) {
-  ModelSafeRoutingInfo info;
-  EXPECT_TRUE(info == context()->previous_session_routing_info());
-  ModelSafeRoutingInfo expected(context()->routing_info());
-  ASSERT_FALSE(expected.empty());
-  EXPECT_CALL(*syncer(), SyncShare(_,_,_)).Times(1);
-
-  StartSyncScheduler(SyncScheduler::NORMAL_MODE);
-
-  scheduler()->ScheduleNudgeAsync(
-      zero(), NUDGE_SOURCE_LOCAL, ModelTypeSet(), FROM_HERE);
-  PumpLoop();
-  // Pump again to run job.
-  PumpLoop();
-
-  StopSyncScheduler();
-
-  EXPECT_TRUE(expected == context()->previous_session_routing_info());
-}
-
 }  // namespace syncer
diff --git a/sync/engine/syncer.cc b/sync/engine/syncer.cc
index e813bc3..66afdbc 100644
--- a/sync/engine/syncer.cc
+++ b/sync/engine/syncer.cc
@@ -12,7 +12,6 @@
 #include "build/build_config.h"
 #include "sync/engine/apply_updates_command.h"
 #include "sync/engine/build_commit_command.h"
-#include "sync/engine/cleanup_disabled_types_command.h"
 #include "sync/engine/commit.h"
 #include "sync/engine/conflict_resolver.h"
 #include "sync/engine/download_updates_command.h"
@@ -53,7 +52,6 @@ const char* SyncerStepToString(const SyncerStep step)
 {
   switch (step) {
     ENUM_CASE(SYNCER_BEGIN);
-    ENUM_CASE(CLEANUP_DISABLED_TYPES);
     ENUM_CASE(DOWNLOAD_UPDATES);
     ENUM_CASE(PROCESS_CLIENT_COMMAND);
     ENUM_CASE(VERIFY_UPDATES);
@@ -106,14 +104,8 @@ void Syncer::SyncShare(sessions::SyncSession* session,
             PruneUnthrottledTypes(base::TimeTicks::Now());
         session->SendEventNotification(SyncEngineEvent::SYNC_CYCLE_BEGIN);
 
-        next_step = CLEANUP_DISABLED_TYPES;
-        break;
-      case CLEANUP_DISABLED_TYPES: {
-        CleanupDisabledTypesCommand cleanup;
-        cleanup.Execute(session);
         next_step = DOWNLOAD_UPDATES;
         break;
-      }
       case DOWNLOAD_UPDATES: {
         // TODO(akalin): We may want to propagate this switch up
         // eventually.
diff --git a/sync/engine/syncer.h b/sync/engine/syncer.h
index 81271ab..b49e978 100644
--- a/sync/engine/syncer.h
+++ b/sync/engine/syncer.h
@@ -26,7 +26,6 @@ class MutableEntry;
 
 enum SyncerStep {
   SYNCER_BEGIN,
-  CLEANUP_DISABLED_TYPES,
   DOWNLOAD_UPDATES,
   PROCESS_CLIENT_COMMAND,
   VERIFY_UPDATES,