sync: add InternalComponentsFactory::Switches to simplify passing switches to internal components.

chrome/browser/sync/glue/sync_backend_host.cc

Index: chrome/browser/sync/glue/sync_backend_host.cc
diff --git a/chrome/browser/sync/glue/sync_backend_host.cc b/chrome/browser/sync/glue/sync_backend_host.cc
index ccf8bf65fa0d5eb0b56e3515e465edea028827a2..eb7e18aa8d1f12e4426fdcce50b052b4008cd947 100644
--- a/chrome/browser/sync/glue/sync_backend_host.cc
+++ b/chrome/browser/sync/glue/sync_backend_host.cc
@@ -415,6 +415,21 @@ void SyncBackendHost::Initialize(
   registrar_->GetModelSafeRoutingInfo(&routing_info);
   registrar_->GetWorkers(&workers);
 
+  InternalComponentsFactory::Switches factory_switches = {
+      InternalComponentsFactory::ENCRYPTION_LEGACY,
+      InternalComponentsFactory::BACKOFF_NORMAL
+  };
+
+  CommandLine* cl = CommandLine::ForCurrentProcess();
+  if (cl->HasSwitch(switches::kSyncKeystoreEncryption)) {

[rlarocque, 2012/08/14 19:54:07]

nit: I think this would be more readable if this was an if/else branch.  That
would make it more clear that the InternalComponentsFactory flag depends
entirely on the command line flag.

[tim (not reviewing), 2012/08/14 20:20:51]

The reason I did it this way is because it is most obvious what the default
expected behavior is, and we ensure that we always start from the same default
regardless of how many times the "extraordinary circumstances" below change or
the code gets re-arranged.

Maybe it'd be nicer to pull the CommandLine changes into their own routine, to
make this more clear?  Something like 

Switches factory_switches = {ENCRYPTION_LEGACY, BACKOFF_NORMAL};
OverrideSwitchesIfNecessary(&switches, CommandLine::ForCurrentProcess());

[rlarocque, 2012/08/14 21:13:04]

I'd rather not hide the processing inside a function.  

I don't feel that strongly about my recommendation.  It's good enough as is.  

+    factory_switches.encryption_method =
+        InternalComponentsFactoryImpl::ENCRYPTION_KEYSTORE;
+  }
+  if (cl->HasSwitch(switches::kSyncShortInitialRetryOverride)) {

[rlarocque, 2012/08/14 19:54:07]

same nit here.

+    factory_switches.backoff_override =
+        InternalComponentsFactoryImpl::BACKOFF_SHORT_INITIAL_RETRY_OVERRIDE;
+  }
+
   initialization_state_ = CREATING_SYNC_MANAGER;
   InitCore(DoInitializeOptions(
       sync_thread_.message_loop(),
@@ -433,7 +448,7 @@ void SyncBackendHost::Initialize(
       delete_sync_data_folder,
       sync_prefs_->GetEncryptionBootstrapToken(),
       sync_prefs_->GetKeystoreEncryptionBootstrapToken(),
-      new InternalComponentsFactoryImpl(),
+      new InternalComponentsFactoryImpl(factory_switches),
       unrecoverable_error_handler,
       report_unrecoverable_error_function));
 }
@@ -1091,8 +1106,6 @@ void SyncBackendHost::Core::DoInitialize(const DoInitializeOptions& options) {
           options.sync_notifier_factory->CreateSyncNotifier())),
       options.restored_key_for_bootstrapping,
       options.restored_keystore_key_for_bootstrapping,
-      CommandLine::ForCurrentProcess()->HasSwitch(
-              switches::kSyncKeystoreEncryption),
       scoped_ptr<InternalComponentsFactory>(
           options.internal_components_factory),
       &encryptor_,