Fix memory leak introduced by 120942

BUG=
TEST=
TBR=zea@chromium.org

Review URL: https://chromiumcodereview.appspot.com/9360013

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

2 files changed, 21 insertions, 9 deletions

diff --git a/chrome/browser/sync/glue/shared_change_processor.cc b/chrome/browser/sync/glue/shared_change_processor.cc
index c6afd9e..c28ab2b 100644
--- a/chrome/browser/sync/glue/shared_change_processor.cc
+++ b/chrome/browser/sync/glue/shared_change_processor.cc
@@ -29,7 +29,9 @@ SharedChangeProcessor::~SharedChangeProcessor() {
   // deleted on |backend_loop_|.
   if (BrowserThread::CurrentlyOn(BrowserThread::UI)) {
     if (backend_loop_.get()) {
-      backend_loop_->DeleteSoon(FROM_HERE, generic_change_processor_);
+      if (!backend_loop_->DeleteSoon(FROM_HERE, generic_change_processor_)) {
+        NOTREACHED();
+      }
     } else {
       DCHECK(!generic_change_processor_);
     }
diff --git a/chrome/browser/sync/glue/shared_change_processor_unittest.cc b/chrome/browser/sync/glue/shared_change_processor_unittest.cc
index ce62624..f747052 100644
--- a/chrome/browser/sync/glue/shared_change_processor_unittest.cc
+++ b/chrome/browser/sync/glue/shared_change_processor_unittest.cc
@@ -53,8 +53,14 @@ class SyncSharedChangeProcessorTest : public testing::Test {
         FROM_HERE,
         base::Bind(&SyncSharedChangeProcessorTest::TearDownDBSyncableService,
                    base::Unretained(this))));
-    db_thread_.Stop();
+    // This must happen before the DB thread is stopped since
+    // |shared_change_processor_| may post tasks to delete its members
+    // on the correct thread.
+    //
+    // TODO(akalin): Write deterministic tests for the destruction of
+    // |shared_change_processor_| on the UI and DB threads.
     shared_change_processor_ = NULL;
+    db_thread_.Stop();
   }
 
   // Connect |shared_change_processor_| on the DB thread.
@@ -63,7 +69,8 @@ class SyncSharedChangeProcessorTest : public testing::Test {
         BrowserThread::DB,
         FROM_HERE,
         base::Bind(&SyncSharedChangeProcessorTest::ConnectOnDBThread,
-                   base::Unretained(this))));
+                   base::Unretained(this),
+                   shared_change_processor_)));
   }
 
  private:
@@ -82,14 +89,17 @@ class SyncSharedChangeProcessorTest : public testing::Test {
     db_syncable_service_ = NULL;
   }
 
-  // Used by Connect().
-  void ConnectOnDBThread() {
+  // Used by Connect().  The SharedChangeProcessor is passed in
+  // because we modify |shared_change_processor_| on the main thread
+  // (in TearDown()).
+  void ConnectOnDBThread(
+      const scoped_refptr<SharedChangeProcessor>& shared_change_processor) {
     DCHECK(BrowserThread::CurrentlyOn(BrowserThread::DB));
     EXPECT_TRUE(
-        shared_change_processor_->Connect(&sync_factory_,
-                                          &sync_service_,
-                                          &sync_service_,
-                                          db_syncable_service_->AsWeakPtr()));
+        shared_change_processor->Connect(&sync_factory_,
+                                         &sync_service_,
+                                         &sync_service_,
+                                         db_syncable_service_->AsWeakPtr()));
   }
 
   MessageLoopForUI ui_loop_;