sync: add a CHECK to shed light on bug 165561.

If a SyncSessionJob is destroyed and the scheduler still references it, CHECK-fail.
This the crash described in 165561 is happening on canary, this should help explain
what's going on.

BUG=165561

Review URL: https://codereview.chromium.org/11887013

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@176732 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 35 insertions, 3 deletions

diff --git a/sync/engine/sync_scheduler_impl.cc b/sync/engine/sync_scheduler_impl.cc
index f07825e..c1424fc 100644
--- a/sync/engine/sync_scheduler_impl.cc
+++ b/sync/engine/sync_scheduler_impl.cc
@@ -185,6 +185,11 @@ SyncSchedulerImpl::~SyncSchedulerImpl() {
   StopImpl(base::Closure());
 }
 
+void SyncSchedulerImpl::OnJobDestroyed(SyncSessionJob* job) {
+  // TODO(tim): Bug 165561 investigation.
+  CHECK(!pending_nudge_ || pending_nudge_ != job);
+}
+
 void SyncSchedulerImpl::OnCredentialsUpdated() {
   DCHECK_EQ(MessageLoop::current(), sync_loop_);
 
@@ -347,6 +352,7 @@ bool SyncSchedulerImpl::ScheduleConfiguration(
         session.Pass(),
         params,
         FROM_HERE));
+    job->set_destruction_observer(weak_ptr_factory_.GetWeakPtr());
     bool succeeded = DoSyncSessionJob(job.Pass());
 
     // If we failed, the job would have been saved as the pending configure
@@ -617,7 +623,7 @@ void SyncSchedulerImpl::ScheduleNudgeImpl(
       CreateSyncSession(info).Pass(),
       ConfigurationParams(),
       nudge_location));
-
+  job->set_destruction_observer(weak_ptr_factory_.GetWeakPtr());
   JobProcessDecision decision = DecideOnJob(*job);
   SDVLOG(2) << "Should run "
             << SyncSessionJob::GetPurposeString(job->purpose())
@@ -1084,6 +1090,7 @@ void SyncSchedulerImpl::PollTimerCallback() {
                                                     s.Pass(),
                                                     ConfigurationParams(),
                                                     FROM_HERE));
+  job->set_destruction_observer(weak_ptr_factory_.GetWeakPtr());
   ScheduleSyncSessionJob(job.Pass());
 }
 
diff --git a/sync/engine/sync_scheduler_impl.h b/sync/engine/sync_scheduler_impl.h
index ea399ec..e2822ce 100644
--- a/sync/engine/sync_scheduler_impl.h
+++ b/sync/engine/sync_scheduler_impl.h
@@ -33,7 +33,9 @@ namespace syncer {
 
 class BackoffDelayProvider;
 
-class SYNC_EXPORT_PRIVATE SyncSchedulerImpl : public SyncScheduler {
+class SYNC_EXPORT_PRIVATE SyncSchedulerImpl :
+    public SyncScheduler,
+    public SyncSessionJob::DestructionObserver {
  public:
   // |name| is a display string to identify the syncer thread.  Takes
   // |ownership of |syncer| and |delay_provider|.
@@ -79,6 +81,9 @@ class SYNC_EXPORT_PRIVATE SyncSchedulerImpl : public SyncScheduler {
   virtual void OnSyncProtocolError(
       const sessions::SyncSessionSnapshot& snapshot) OVERRIDE;
 
+  // SyncSessionJob::DestructionObserver implementation.
+  virtual void OnJobDestroyed(SyncSessionJob* job) OVERRIDE;
+
  private:
   enum JobProcessDecision {
     // Indicates we should continue with the current job.
diff --git a/sync/engine/sync_session_job.cc b/sync/engine/sync_session_job.cc
index 3f13d98..72a7741 100644
--- a/sync/engine/sync_session_job.cc
+++ b/sync/engine/sync_session_job.cc
@@ -7,7 +7,10 @@
 
 namespace syncer {
 
-SyncSessionJob::~SyncSessionJob() {}
+SyncSessionJob::~SyncSessionJob() {
+  if (destruction_observer_)
+    destruction_observer_->OnJobDestroyed(this);
+}
 
 SyncSessionJob::SyncSessionJob(
     Purpose purpose,
@@ -24,6 +27,11 @@ SyncSessionJob::SyncSessionJob(
       from_location_(from_location) {
 }
 
+void SyncSessionJob::set_destruction_observer(
+    const base::WeakPtr<DestructionObserver>& destruction_observer) {
+  destruction_observer_ = destruction_observer;
+}
+
 #define ENUM_CASE(x) case x: return #x; break;
 const char* SyncSessionJob::GetPurposeString(SyncSessionJob::Purpose purpose) {
   switch (purpose) {
diff --git a/sync/engine/sync_session_job.h b/sync/engine/sync_session_job.h
index dfb9c90..9fe0361 100644
--- a/sync/engine/sync_session_job.h
+++ b/sync/engine/sync_session_job.h
@@ -6,6 +6,7 @@
 #define SYNC_ENGINE_SYNC_SESSION_JOB_H_
 
 #include "base/memory/scoped_ptr.h"
+#include "base/memory/weak_ptr.h"
 #include "base/time.h"
 #include "base/tracked_objects.h"
 #include "sync/base/sync_export.h"
@@ -31,6 +32,13 @@ class SYNC_EXPORT_PRIVATE SyncSessionJob {
     CONFIGURATION,
   };
 
+  // This class exists to inform an interested party about the destruction of
+  // a SyncSessionJob.
+  class SYNC_EXPORT_PRIVATE DestructionObserver {
+   public:
+    virtual void OnJobDestroyed(SyncSessionJob* job) = 0;
+  };
+
   SyncSessionJob(Purpose purpose,
                  base::TimeTicks start,
                  scoped_ptr<sessions::SyncSession> session,
@@ -86,6 +94,8 @@ class SYNC_EXPORT_PRIVATE SyncSessionJob {
   SyncerStep end_step() const;
   ConfigurationParams config_params() const;
 
+  void set_destruction_observer(
+      const base::WeakPtr<DestructionObserver>& observer);
  private:
   // A SyncSessionJob can be in one of these three states, controlled by the
   // Finish() function, see method comments.
@@ -117,6 +127,8 @@ class SYNC_EXPORT_PRIVATE SyncSessionJob {
   // first location that came in.
   tracked_objects::Location from_location_;
 
+  base::WeakPtr<DestructionObserver> destruction_observer_;
+
   DISALLOW_COPY_AND_ASSIGN(SyncSessionJob);
 };