Do not observe MessageLoop destruction from ModelSafeWorker.

components/sync/driver/glue/sync_backend_registrar_unittest.cc

 // Copyright 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 "components/sync/driver/glue/sync_backend_registrar.h"
 
+#include <memory>
+
 #include "base/location.h"
 #include "base/memory/ptr_util.h"
 #include "base/run_loop.h"
 #include "base/single_thread_task_runner.h"
 #include "components/sync/driver/change_processor_mock.h"
 #include "components/sync/driver/fake_sync_client.h"
 #include "components/sync/driver/glue/browser_thread_model_worker.h"
 #include "components/sync/driver/sync_api_component_factory_mock.h"
 #include "components/sync/engine/passive_model_worker.h"
 #include "components/sync/syncable/test_user_share.h"
@@ skipping 18 matching lines @@
  public:
   RegistrarSyncClient(
       const scoped_refptr<base::SingleThreadTaskRunner>& ui_task_runner,
       const scoped_refptr<base::SingleThreadTaskRunner>& db_task_runner,
       const scoped_refptr<base::SingleThreadTaskRunner>& file_task_runner)
       : ui_task_runner_(ui_task_runner),
         db_task_runner_(db_task_runner),
         file_task_runner_(file_task_runner) {}
 
   scoped_refptr<ModelSafeWorker> CreateModelWorkerForGroup(
-      ModelSafeGroup group,
-      WorkerLoopDestructionObserver* observer) override {
+      ModelSafeGroup group) override {
     switch (group) {
       case GROUP_UI:
-        return new BrowserThreadModelWorker(ui_task_runner_, group, observer);
+        return new BrowserThreadModelWorker(ui_task_runner_, group);
       case GROUP_DB:
-        return new BrowserThreadModelWorker(db_task_runner_, group, observer);
+        return new BrowserThreadModelWorker(db_task_runner_, group);
       case GROUP_FILE:
-        return new BrowserThreadModelWorker(file_task_runner_, group, observer);
+        return new BrowserThreadModelWorker(file_task_runner_, group);
       case GROUP_PASSIVE:
-        return new PassiveModelWorker(observer);
+        return new PassiveModelWorker();
       default:
         return nullptr;
     }
   }
 
  private:
   const scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner_;
   const scoped_refptr<base::SingleThreadTaskRunner> db_task_runner_;
   const scoped_refptr<base::SingleThreadTaskRunner> file_task_runner_;
 };
 
 // Flaky: https://crbug.com/498238
 class SyncBackendRegistrarTest : public testing::Test {
  public:
   void TestNonUIDataTypeActivationAsync(ChangeProcessor* processor,
                                         base::WaitableEvent* done) {
     registrar_->ActivateDataType(AUTOFILL, GROUP_DB, processor,
                                  test_user_share_.user_share());
     ExpectRoutingInfo(registrar_.get(), {{AUTOFILL, GROUP_DB}});
     ExpectHasProcessorsForTypes(*registrar_, ModelTypeSet(AUTOFILL));
     TriggerChanges(registrar_.get(), AUTOFILL);
     done->Signal();
   }
 
  protected:
   SyncBackendRegistrarTest()
-      : db_thread_("DBThreadForTest"),
-        file_thread_("FileThreadForTest"),
-        sync_thread_(nullptr) {}
+      : db_thread_("DBThreadForTest"), file_thread_("FileThreadForTest") {}
 
   ~SyncBackendRegistrarTest() override {}
 
   void SetUp() override {
     db_thread_.StartAndWaitForTesting();
     file_thread_.StartAndWaitForTesting();
     test_user_share_.SetUp();
     sync_client_ = base::MakeUnique<RegistrarSyncClient>(
         ui_task_runner(), db_task_runner(), file_task_runner());
     registrar_ = base::MakeUnique<SyncBackendRegistrar>(
         "test", sync_client_.get(), std::unique_ptr<base::Thread>(),
         ui_task_runner(), db_task_runner(), file_task_runner());
-    sync_thread_ = registrar_->sync_thread();
   }
 
   void TearDown() override {
     registrar_->RequestWorkerStopOnUIThread();
     test_user_share_.TearDown();
-    sync_thread_->task_runner()->PostTask(
-        FROM_HERE, base::Bind(&SyncBackendRegistrar::Shutdown,
-                              base::Unretained(registrar_.release())));
-    sync_thread_->WaitUntilThreadStarted();
+    {
+      std::unique_ptr<base::Thread> released_sync_thread =
+          registrar_->ReleaseSyncThread();
+      released_sync_thread->task_runner()->DeleteSoon(FROM_HERE,
+                                                      registrar_.release());
+      // |released_sync_thread| is joined at the end of this scope.
+    }
     base::RunLoop().RunUntilIdle();
   }
 
   void ExpectRoutingInfo(SyncBackendRegistrar* registrar,
                          const ModelSafeRoutingInfo& expected_routing_info) {
     ModelSafeRoutingInfo actual_routing_info;
     registrar->GetModelSafeRoutingInfo(&actual_routing_info);
     EXPECT_EQ(expected_routing_info, actual_routing_info);
   }
 
@@ skipping 24 matching lines @@
     return db_thread_.task_runner();
   }
 
   base::MessageLoop message_loop_;
   base::Thread db_thread_;
   base::Thread file_thread_;
 
   TestUserShare test_user_share_;
   std::unique_ptr<RegistrarSyncClient> sync_client_;
   std::unique_ptr<SyncBackendRegistrar> registrar_;
-
-  base::Thread* sync_thread_;
 };
 
 TEST_F(SyncBackendRegistrarTest, ConstructorEmpty) {
   registrar_->SetInitialTypes(ModelTypeSet());
   EXPECT_FALSE(registrar_->IsNigoriEnabled());
   EXPECT_EQ(4u, GetWorkersSize());
   ExpectRoutingInfo(registrar_.get(), ModelSafeRoutingInfo());
   ExpectHasProcessorsForTypes(*registrar_, ModelTypeSet());
 }
 
@@ skipping 134 matching lines @@
   // it should be included in types to be downloaded.
   ModelTypeSet types_to_add(AUTOFILL, BOOKMARKS);
   ModelTypeSet newly_added_types =
       registrar_->ConfigureDataTypes(types_to_add, ModelTypeSet());
   EXPECT_EQ(ModelTypeSet(BOOKMARKS), newly_added_types);
   EXPECT_EQ(types_to_add, registrar_->GetLastConfiguredTypes());
   ExpectRoutingInfo(registrar_.get(), {{AUTOFILL, GROUP_NON_BLOCKING},
                                        {BOOKMARKS, GROUP_NON_BLOCKING}});
 }
 
-class SyncBackendRegistrarShutdownTest : public testing::Test {
- public:
-  void BlockDBThread() {
-    EXPECT_FALSE(db_thread_lock_.Try());
-
-    db_thread_blocked_.Signal();
-    base::AutoLock l(db_thread_lock_);
-  }
-
- protected:
-  friend class TestRegistrar;
-
-  SyncBackendRegistrarShutdownTest()
-      : db_thread_("DBThreadForTest"),
-        file_thread_("FileThreadForTest"),
-        db_thread_blocked_(base::WaitableEvent::ResetPolicy::AUTOMATIC,
-                           base::WaitableEvent::InitialState::NOT_SIGNALED) {
-    quit_closure_ = run_loop_.QuitClosure();
-  }
-
-  ~SyncBackendRegistrarShutdownTest() override {}
-
-  void SetUp() override {
-    db_thread_.StartAndWaitForTesting();
-    file_thread_.StartAndWaitForTesting();
-    sync_client_ = base::MakeUnique<RegistrarSyncClient>(
-        ui_task_runner(), db_task_runner(), file_task_runner());
-  }
-
-  void PostQuitOnUIMessageLoop() {
-    ui_task_runner()->PostTask(FROM_HERE, quit_closure_);
-  }
-
-  const scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner() {
-    return message_loop_.task_runner();
-  }
-
-  const scoped_refptr<base::SingleThreadTaskRunner> db_task_runner() {
-    return db_thread_.task_runner();
-  }
-
-  const scoped_refptr<base::SingleThreadTaskRunner> file_task_runner() {
-    return file_thread_.task_runner();
-  }
-
-  base::MessageLoop message_loop_;
-  base::Thread db_thread_;
-  base::Thread file_thread_;
-
-  std::unique_ptr<RegistrarSyncClient> sync_client_;
-  base::WaitableEvent db_thread_blocked_;
-
-  base::Lock db_thread_lock_;
-  base::RunLoop run_loop_;
-  base::Closure quit_closure_;
-};
-
-// Wrap SyncBackendRegistrar so that we can monitor its lifetime.
-class TestRegistrar : public SyncBackendRegistrar {
- public:
-  explicit TestRegistrar(
-      SyncClient* sync_client,
-      const scoped_refptr<base::SingleThreadTaskRunner>& ui_thread,
-      const scoped_refptr<base::SingleThreadTaskRunner>& db_thread,
-      const scoped_refptr<base::SingleThreadTaskRunner>& file_thread,
-      SyncBackendRegistrarShutdownTest* test)
-      : SyncBackendRegistrar("test",
-                             sync_client,
-                             std::unique_ptr<base::Thread>(),
-                             ui_thread,
-                             db_thread,
-                             file_thread),
-        test_(test) {}
-
-  ~TestRegistrar() override { test_->PostQuitOnUIMessageLoop(); }
-
- private:
-  SyncBackendRegistrarShutdownTest* test_;
-};
-
-TEST_F(SyncBackendRegistrarShutdownTest, BlockingShutdown) {
-  // Take ownership of |db_thread_lock_| so that the DB thread can't acquire it.
-  db_thread_lock_.Acquire();
-
-  // This will block the DB thread by waiting on |db_thread_lock_|.
-  db_task_runner()->PostTask(
-      FROM_HERE, base::Bind(&SyncBackendRegistrarShutdownTest::BlockDBThread,
-                            base::Unretained(this)));
-
-  std::unique_ptr<TestRegistrar> registrar(
-      new TestRegistrar(sync_client_.get(), ui_task_runner(), db_task_runner(),
-                        file_task_runner(), this));
-  base::Thread* sync_thread = registrar->sync_thread();
-
-  // Stop here until the DB thread gets a chance to run and block on the lock.
-  // Please note that since the task above didn't finish, the task to
-  // initialize the worker on the DB thread hasn't had a chance to run yet too.
-  // Which means ModelSafeWorker::SetWorkingLoopToCurrent hasn't been called
-  // for the DB worker.
-  db_thread_blocked_.Wait();
-
-  registrar->SetInitialTypes(ModelTypeSet());
-
-  // Start the shutdown.
-  registrar->RequestWorkerStopOnUIThread();
-
-  sync_thread->task_runner()->PostTask(
-      FROM_HERE, base::Bind(&SyncBackendRegistrar::Shutdown,
-                            base::Unretained(registrar.release())));
-
-  // Make sure the thread starts running.
-  sync_thread->WaitUntilThreadStarted();
-
-  // The test verifies that the sync thread doesn't block because
-  // of the blocked DB thread and can finish the shutdown.
-  base::RunLoop().RunUntilIdle();
-
-  db_thread_lock_.Release();
-
-  // Run the main thread loop until all workers have been removed and the
-  // registrar destroyed.
-  run_loop_.Run();
-}
-
 }  // namespace
 
 }  // namespace syncer