[Sync] Persist keystore key across restarts

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 e92d73ecb8b721d93cafa84163f1e69d7f9d05f3..4b28fd5697462fcacc3140a270f757c5fedf7467 100644
--- a/chrome/browser/sync/glue/sync_backend_host.cc
+++ b/chrome/browser/sync/glue/sync_backend_host.cc
@@ -378,6 +378,7 @@ void SyncBackendHost::Initialize(
       &sync_notifier_factory_,
       delete_sync_data_folder,
       sync_prefs_->GetEncryptionBootstrapToken(),
+      sync_prefs_->GetKeystoreEncryptionBootstrapToken(),
       sync_api::SyncManager::NON_TEST,
       unrecoverable_error_handler,
       report_unrecoverable_error_function));
@@ -720,6 +721,7 @@ void SyncBackendHost::RequestConfigureSyncer(
 }
 
 void SyncBackendHost::FinishConfigureDataTypesOnFrontendLoop(
+    const std::string& keystore_bootstrap_token,

[rlarocque, 2012/06/13 23:35:04]

Is it really necessary to send the token back on every call to
FinishConfigureDataTypesOnFrontendLoop()?

If the value is not going to change, could we pass it back in some other way? 
Maybe a 'HandleGetKeyReponseOnFrontendLoop()'?

[Nicolas Zea, 2012/06/15 00:42:07]

Yeah, I guess the bootstrap token doesn't really relate to configuring
datatypes. I've gone ahead and changed this to reuse the PersistEncryptionToken
method and key off the last_get_key_result being successful in
OnSyncCycleCompleted.

[rlarocque, 2012/06/15 19:02:00]

I'm not sure that's much better.  Wouldn't it make more sense to only update it
when it changes, as is the case with the bootstrap token?

[Nicolas Zea, 2012/06/15 20:15:30]

This is effectively the same logic used in the bootstrap token, i.e. any time we
successfully set a passphrase we update the bootstrap token (even if it didn't
change). We don't have any logic comparing the already persisted token and the
new one; the pref service does that for you (it checks to see if the new pref is
different or not). I don't see why we should duplicate that logic.

Note also that we'll only ever perform a GetKey if we didn't have a valid key in
the first place. In other words, if the GetKey succeeded, it means we're writing
a new key anyways (barring those cases where the getkey succeeds but the
download updates fails, but that goes back to the pref service only persisting
new strings).

     const syncable::ModelTypeSet& types_to_configure,
     const syncable::ModelTypeSet& configured_types,
     const base::Callback<void(syncable::ModelTypeSet)>& ready_task) {
@@ -742,6 +744,11 @@ void SyncBackendHost::FinishConfigureDataTypesOnFrontendLoop(
       base::Bind(&SyncBackendHost::Core::DoUpdateEnabledTypes, core_.get(),
                  configured_types));
 
+  if (!keystore_bootstrap_token.empty()) {
+    DVLOG(1) << "Persisting keystore encryption bootstrap token.";
+    sync_prefs_->SetKeystoreEncryptionBootstrapToken(keystore_bootstrap_token);
+  }
+
   if (!ready_task.is_null())
     ready_task.Run(failed_configuration_types);
 }
@@ -760,6 +767,7 @@ SyncBackendHost::DoInitializeOptions::DoInitializeOptions(
     sync_notifier::SyncNotifierFactory* sync_notifier_factory,
     bool delete_sync_data_folder,
     const std::string& restored_key_for_bootstrapping,
+    const std::string& restored_keystore_key_for_bootstrapping,
     sync_api::SyncManager::TestingMode testing_mode,
     UnrecoverableErrorHandler* unrecoverable_error_handler,
     ReportUnrecoverableErrorFunction report_unrecoverable_error_function)
@@ -776,6 +784,8 @@ SyncBackendHost::DoInitializeOptions::DoInitializeOptions(
       sync_notifier_factory(sync_notifier_factory),
       delete_sync_data_folder(delete_sync_data_folder),
       restored_key_for_bootstrapping(restored_key_for_bootstrapping),
+      restored_keystore_key_for_bootstrapping(
+          restored_keystore_key_for_bootstrapping),
       testing_mode(testing_mode),
       unrecoverable_error_handler(unrecoverable_error_handler),
       report_unrecoverable_error_function(
@@ -1010,6 +1020,7 @@ void SyncBackendHost::Core::DoInitialize(const DoInitializeOptions& options) {
           options.chrome_sync_notification_bridge,
           options.sync_notifier_factory->CreateSyncNotifier()),
       options.restored_key_for_bootstrapping,
+      options.restored_keystore_key_for_bootstrapping,
       CommandLine::ForCurrentProcess()->HasSwitch(
               switches::kSyncKeystoreEncryption),
       options.testing_mode,
@@ -1131,8 +1142,13 @@ void SyncBackendHost::Core::DoFinishConfigureDataTypes(
   syncable::ModelTypeSet configured_types =
       sync_manager_->InitialSyncEndedTypes();
   configured_types.RetainAll(types_to_config);
+
+  std::string keystore_token;
+  sync_manager_->GetKeystoreKeyBootstrapToken(&keystore_token);
+
   host_.Call(FROM_HERE,
              &SyncBackendHost::FinishConfigureDataTypesOnFrontendLoop,
+             keystore_token,
              types_to_config,
              configured_types,
              ready_task);