sync: make scheduling logic and job ownership more obvious.

sync/engine/sync_session_job.cc

Index: sync/engine/sync_session_job.cc
diff --git a/sync/engine/sync_session_job.cc b/sync/engine/sync_session_job.cc
new file mode 100644
index 0000000000000000000000000000000000000000..8269b6bf6fbc0892a3dc0d86571ea945fda0cb83
--- /dev/null
+++ b/sync/engine/sync_session_job.cc
@@ -0,0 +1,190 @@
+// 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_session_job.h"
+#include "sync/internal_api/public/sessions/model_neutral_state.h"
+
+namespace syncer {
+
+SyncSessionJob::~SyncSessionJob() {}
+
+SyncSessionJob::SyncSessionJob(
+    Purpose purpose,
+    const base::TimeTicks& start,
+    scoped_ptr<sessions::SyncSession> session,
+    const ConfigurationParams& config_params,
+    const tracked_objects::Location& from_location)
+    : purpose_(purpose),
+      scheduled_start_(start),
+      session_(session.Pass()),
+      is_canary_(false),
+      config_params_(config_params),
+      finished_(NOT_FINISHED),
+      from_location_(from_location) {
+}
+
+#define ENUM_CASE(x) case x: return #x; break;
+const char* SyncSessionJob::GetPurposeString(SyncSessionJob::Purpose purpose) {
+  switch (purpose) {
+    ENUM_CASE(UNKNOWN);
+    ENUM_CASE(POLL);
+    ENUM_CASE(NUDGE);
+    ENUM_CASE(CONFIGURATION);
+  }
+  NOTREACHED();
+  return "";
+}
+#undef ENUM_CASE
+
+void SyncSessionJob::Finish(bool early_exit) {
+  finished_ = early_exit ? EARLY_EXIT : FINISHED;
+  if (!Succeeded())
+    return;
+
+  if (!config_params_.ready_task.is_null())
+    config_params_.ready_task.Run();
+}
+
+bool SyncSessionJob::Succeeded() const {
+  // Did we run through all SyncerSteps from start_step() to end_step()
+  // until the SyncSession returned !HasMoreToSync()?
+  // Note: if not, it's possible the scheduler hasn't started with
+  // SyncShare yet, it's possible there is still more to sync in the session,
+  // and it's also possible the job quit part way through due to a premature
+  // exit condition (such as shutdown).
+  if (finished_ != FINISHED)
+    return false;
+
+  DCHECK(!session_->HasMoreToSync());
+
+  // Did we hit any errors along the way?
+  if (sessions::HasSyncerError(
+      session_->status_controller().model_neutral_state())) {
+    return false;
+  }
+
+#ifndef NDEBUG

[rlarocque, 2012/09/24 21:56:47]

I wouldn't bother with the #ifndef.  The compiler should notice that this block
of statements has no effect other than a few DCHECKs and optimize accordingly. 
Even in the unlikely event that it fails to optimize away the whole thing, the
cost would be tiny because these statements are inexpensive and rarely executed.


However, if you leave this #ifndef, you increase the risk that different
warnings or errors will be generated in debug and release modes.  This is
especially dangerous because we're not likely to compile with NDEBUG set most of
the time.

[akalin, 2012/09/25 22:37:24]

better to use DCHECK_IS_ON() anyway

[tim (not reviewing), 2012/10/08 00:20:03]

Agree w/ both points.  Removed.

+  const sessions::ModelNeutralState& state(
+      session_->status_controller().model_neutral_state());
+  switch (purpose_) {
+    case POLL:
+    case NUDGE:
+      DCHECK(state.last_download_updates_result != UNSET &&
+             state.commit_result != UNSET);
+      break;
+    case CONFIGURATION:
+      DCHECK_NE(state.last_download_updates_result, UNSET);
+      break;
+    case UNKNOWN:
+    default:
+      NOTREACHED();
+  }
+#endif  // NDEBUG
+
+  return true;
+}
+
+scoped_ptr<SyncSessionJob> SyncSessionJob::CloneAndAbandon() {
+  DCHECK_EQ(finished_, NOT_FINISHED);
+  // Clone |this|, and abandon it by NULL-ing session_.
+  return scoped_ptr<SyncSessionJob> (new SyncSessionJob(
+      purpose_, scheduled_start_, session_.Pass(),
+      config_params_, from_location_));
+}
+
+scoped_ptr<SyncSessionJob> SyncSessionJob::Clone() const {
+  DCHECK_GT(finished_, NOT_FINISHED);
+  return scoped_ptr<SyncSessionJob>(new SyncSessionJob(
+      purpose_, scheduled_start_, CloneSession().Pass(),
+      config_params_, from_location_));
+}
+
+scoped_ptr<SyncSessionJob> SyncSessionJob::CloneFromLocation(
+    const tracked_objects::Location& from_here) const {
+  DCHECK_GT(finished_, NOT_FINISHED);
+  return scoped_ptr<SyncSessionJob>(new SyncSessionJob(
+      purpose_, scheduled_start_, CloneSession().Pass(),
+      config_params_, from_here));
+}
+
+scoped_ptr<sessions::SyncSession> SyncSessionJob::CloneSession() const {
+  return scoped_ptr<sessions::SyncSession>(
+      new sessions::SyncSession(session_->context(),
+          session_->delegate(), session_->source(), session_->routing_info(),
+          session_->workers()));
+}
+
+bool SyncSessionJob::is_canary() const {
+  return is_canary_;
+}
+
+SyncSessionJob::Purpose SyncSessionJob::purpose() const {
+  return purpose_;
+}
+
+const base::TimeTicks& SyncSessionJob::scheduled_start() const {
+  return scheduled_start_;
+}
+
+void SyncSessionJob::set_scheduled_start(const base::TimeTicks& start) {
+  scheduled_start_ = start;
+};
+
+const sessions::SyncSession* SyncSessionJob::session() const {
+  return session_.get();
+}
+
+sessions::SyncSession* SyncSessionJob::mutable_session() {
+  return session_.get();
+}
+
+const tracked_objects::Location& SyncSessionJob::from_location() const {
+  return from_location_;
+}
+
+ConfigurationParams SyncSessionJob::config_params() const {
+  return config_params_;
+}
+
+void SyncSessionJob::GrantCanaryPrivilege() {
+  DCHECK_EQ(finished_, NOT_FINISHED);
+  DVLOG(2) << "Granting canary priviliege to " << session_.get();
+  is_canary_ = true;
+}
+
+SyncerStep SyncSessionJob::start_step() const {
+  SyncerStep start, end;
+  GetSyncerStepsForPurpose(purpose_, &start, &end);
+  return start;
+}
+
+SyncerStep SyncSessionJob::end_step() const {
+  SyncerStep start, end;
+  GetSyncerStepsForPurpose(purpose_, &start, &end);
+  return end;
+}
+
+// static
+void SyncSessionJob::GetSyncerStepsForPurpose(Purpose purpose,
+                                              SyncerStep* start,
+                                              SyncerStep* end) {
+  switch (purpose) {
+    case SyncSessionJob::CONFIGURATION:
+      *start = DOWNLOAD_UPDATES;
+      *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;
+      return;
+  }
+}
+
+}  // namespace syncer