[Sync] Add support for performing a GetKey on startup.

sync/engine/sync_scheduler.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 "sync/engine/sync_scheduler.h"
 
 #include <algorithm>
 #include <cstring>
 
 #include "base/bind.h"
@@ skipping 335 matching lines @@
   // crbug.com/133030
   syncer::ModelSafeRoutingInfo restricted_routes;
   std::vector<ModelSafeWorker*> restricted_workers;
   BuildModelSafeParams(params.types_to_download,
                        params.routing_info,
                        session_context_->workers(),
                        &restricted_routes,
                        &restricted_workers);
   session_context_->set_routing_info(params.routing_info);
 
-  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.
-    NOTIMPLEMENTED();
-  }
-
-  // Only reconfigure if we have types to download.
-  if (!params.types_to_download.Empty()) {
-    DCHECK(!restricted_routes.empty());
+  // Only perform configure if we have types to download or need to download
+  // the encryption key.
+  if (params.keystore_key_status == ConfigurationParams::KEYSTORE_KEY_NEEDED ||
+      !params.types_to_download.Empty()) {
     linked_ptr<SyncSession> session(new SyncSession(
         session_context_,
         this,
         SyncSourceInfo(params.source,
                        ModelSafeRoutingInfoToPayloadMap(
                            restricted_routes,
                            std::string())),
         restricted_routes,
         restricted_workers));
     SyncSessionJob job(SyncSessionJob::CONFIGURATION,
@@ skipping 285 matching lines @@
     ENUM_CASE(CONTINUE);
     ENUM_CASE(SAVE);
     ENUM_CASE(DROP);
   }
   return "";
 }
 
 // static
 void SyncScheduler::SetSyncerStepsForPurpose(
     SyncSessionJob::SyncSessionJobPurpose purpose,
+    const ConfigurationParams& config_params,
     SyncerStep* start,
     SyncerStep* end) {
   switch (purpose) {
     case SyncSessionJob::CONFIGURATION:
-      *start = DOWNLOAD_UPDATES;
-      *end = APPLY_UPDATES;
+      DCHECK(config_params.keystore_key_status ==
+                 ConfigurationParams::KEYSTORE_KEY_NEEDED ||
+             !config_params.types_to_download.Empty());
+      if (config_params.keystore_key_status ==

[tim (not reviewing), 2012/07/19 21:42:03]

If it's a separate http request, should we have a separate purpose for fetching
the key? Seems like we're hacking around the fact that we don't.  Either way it
doesn't seem like you need to pass in all the params here, the key_status should
suffice.

Did we consider piggybacking on reconfiguration GUs for this, though?  Maybe via
a flag in the GetUpdatesRequest?

[Nicolas Zea, 2012/07/24 22:51:24]

Based on the discussion with Kevin, moved the GetKey logic into the GetUpdates
step, so it's not just a normal configuration.

+              ConfigurationParams::KEYSTORE_KEY_NEEDED)
+        *start = GET_ENCRYPTION_KEY;
+      else
+        *start = DOWNLOAD_UPDATES;
+      if (config_params.types_to_download.Empty())
+        *end = GET_ENCRYPTION_KEY;
+      else
+        *end = APPLY_UPDATES;
       return;
     case SyncSessionJob::NUDGE:
     case SyncSessionJob::POLL:
       *start = SYNCER_BEGIN;
       *end = SYNCER_END;
       return;
     default:
       NOTREACHED();
       *start = SYNCER_END;
       *end = SYNCER_END;
@@ skipping 74 matching lines @@
     // and update any disabled or modified entries in the job.
     scoped_ptr<SyncSession> session(CreateSyncSession(job.session->source()));
 
     job.session->RebaseRoutingInfoWithLatest(*session);
   }
   SDVLOG(2) << "DoSyncSessionJob with "
             << SyncSessionJob::GetPurposeString(job.purpose) << " job";
 
   SyncerStep begin(SYNCER_END);
   SyncerStep end(SYNCER_END);
-  SetSyncerStepsForPurpose(job.purpose, &begin, &end);
+  SetSyncerStepsForPurpose(job.purpose, job.config_params, &begin, &end);
 
   bool has_more_to_sync = true;
   while (ShouldRunJob(job) && has_more_to_sync) {
     SDVLOG(2) << "Calling SyncShare.";
     // Synchronously perform the sync session from this thread.
     syncer_->SyncShare(job.session.get(), begin, end);
     has_more_to_sync = job.session->HasMoreToSync();
     if (has_more_to_sync)
       job.session->PrepareForAnotherSyncCycle();
   }
@@ skipping 371 matching lines @@
 
 #undef SDVLOG_LOC
 
 #undef SDVLOG
 
 #undef SLOG
 
 #undef ENUM_CASE
 
 }  // namespace syncer