[Sync] Restrict types to configure to only include registered types

components/sync/engine_impl/syncer.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 "components/sync/engine_impl/syncer.h"
 
 #include <memory>
 
 #include "base/auto_reset.h"
 #include "base/logging.h"
@@ skipping 61 matching lines @@
   cycle->mutable_status_controller()->set_commit_result(commit_result);
 
   return HandleCycleEnd(cycle, nudge_tracker->GetLegacySource());
 }
 
 bool Syncer::ConfigureSyncShare(
     ModelTypeSet request_types,
     sync_pb::GetUpdatesCallerInfo::GetUpdatesSource source,
     SyncCycle* cycle) {
   base::AutoReset<bool> is_syncing(&is_syncing_, true);
   VLOG(1) << "Configuring types " << ModelTypeSetToString(request_types);

[skym, 2017/04/06 22:27:45]

I know this isn't you, but this line is now sometimes incorrect.

[pavely, 2017/04/06 23:48:25]

You are right. Moving RetainAll above this line.

   HandleCycleBegin(cycle);
   ConfigureGetUpdatesDelegate configure_delegate(source);
 
-  // It is possible during shutdown that datatypes get unregistered from
+  // It is possible during configuration that datatypes get unregistered from
   // ModelTypeRegistry before scheduled configure sync cycle gets executed.
-  // When it happens we should adjust set of types to download to only include
+  // This happens either because DataTypeController::LoadModels fail and type
+  // need to be stopped or during shutdown when all datatypes are stopped. When
+  // it happens we should adjust set of types to download to only include
   // registered types.
-  if (cancelation_signal_->IsSignalled())
-    request_types.RetainAll(cycle->context()->GetEnabledTypes());
+  request_types.RetainAll(cycle->context()->GetEnabledTypes());

[skym, 2017/04/06 22:27:45]

What do you think of moving this to
https://cs.chromium.org/chromium/src/components/sync/engine_impl/sync_schedul...

[pavely, 2017/04/06 23:48:25]

I thought about it when adding this check last time. I think these locations are
equivalent, but scheduler should be concerned with "when" cycle happens, pass
parameters through and let syncer decide "what" to do.

 
   GetUpdatesProcessor get_updates_processor(
       cycle->context()->model_type_registry()->update_handler_map(),
       configure_delegate);
   DownloadAndApplyUpdates(&request_types, cycle, &get_updates_processor,
                           kCreateMobileBookmarksFolder);
   return HandleCycleEnd(cycle, source);
 }
 
 bool Syncer::PollSyncShare(ModelTypeSet request_types, SyncCycle* cycle) {
@@ skipping 97 matching lines @@
 }
 
 bool Syncer::PostClearServerData(SyncCycle* cycle) {
   DCHECK(cycle);
   ClearServerData clear_server_data(cycle->context()->account_name());
   const SyncerError post_result = clear_server_data.SendRequest(cycle);
   return post_result == SYNCER_OK;
 }
 
 }  // namespace syncer