Remove unnecessary thread-affinity in /components/browser_sync

This switches a bunch of places in /components/browser_sync from
ThreadTaskRunnerHandle/SingleThreadTaskRunner to
SequencedTaskRunnerHandle/SequencedTaskRunner. A few references to
SingleThreadTaskRunner remain, where it makes sense (e.g. UIModelWorker)
or where lower layers require it (e.g. WebDataServiceBase).

While we're here, this also changes some Callbacks in
SigninConfirmationHelper to OnceCallbacks.

Bug: 846238
Change-Id: I951d95ec442b6b6df4ba3be08674a9eefd4da02c
Reviewed-on: https://chromium-review.googlesource.com/1131189Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Commit-Queue: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#574306}

components/browser_sync/abstract_profile_sync_service_test.cc

@@ -11,7 +11,7 @@
 #include "base/files/file_path.h"
 #include "base/location.h"
 #include "base/run_loop.h"
-#include "base/threading/thread_task_runner_handle.h"
+#include "base/threading/sequenced_task_runner_handle.h"
 #include "components/browser_sync/test_http_bridge_factory.h"
 #include "components/browser_sync/test_profile_sync_service.h"
 #include "components/sync/driver/glue/sync_backend_host_core.h"
@@ -105,7 +105,7 @@ void SyncEngineForProfileSyncTest::ConfigureDataTypes(ConfigureParams params) {
   // send back the list of newly configured types instead and hope it doesn't
   // break anything.
   // Posted to avoid re-entrancy issues.
-  base::ThreadTaskRunnerHandle::Get()->PostTask(
+  base::SequencedTaskRunnerHandle::Get()->PostTask(
       FROM_HERE,
       base::Bind(
           &SyncEngineForProfileSyncTest::FinishConfigureDataTypesOnFrontendLoop,

components/browser_sync/profile_sync_test_util.cc

@@ -8,7 +8,9 @@
 #include <utility>
 
 #include "base/bind.h"
+#include "base/sequenced_task_runner.h"
 #include "base/single_thread_task_runner.h"
+#include "base/threading/sequenced_task_runner_handle.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "components/bookmarks/browser/bookmark_model.h"
 #include "components/history/core/browser/history_model_worker.h"
@@ -53,8 +55,8 @@ class BundleSyncClient : public syncer::FakeSyncClient {
                        get_sync_service_callback,
                    const base::Callback<bookmarks::BookmarkModel*(void)>&
                        get_bookmark_model_callback,
-                   scoped_refptr<base::SingleThreadTaskRunner> db_thread,
-                   scoped_refptr<base::SingleThreadTaskRunner> file_thread,
+                   scoped_refptr<base::SequencedTaskRunner> db_thread,
+                   scoped_refptr<base::SequencedTaskRunner> file_thread,
                    history::HistoryService* history_service);
 
   ~BundleSyncClient() override;
@@ -81,8 +83,8 @@ class BundleSyncClient : public syncer::FakeSyncClient {
   const base::Callback<bookmarks::BookmarkModel*(void)>
       get_bookmark_model_callback_;
   // These task runners, if not null, are used in CreateModelWorkerForGroup.
-  const scoped_refptr<base::SingleThreadTaskRunner> db_thread_;
-  const scoped_refptr<base::SingleThreadTaskRunner> file_thread_;
+  const scoped_refptr<base::SequencedTaskRunner> db_thread_;
+  const scoped_refptr<base::SequencedTaskRunner> file_thread_;
   history::HistoryService* history_service_;
 };
 
@@ -96,8 +98,8 @@ BundleSyncClient::BundleSyncClient(
     const base::Callback<syncer::SyncService*(void)>& get_sync_service_callback,
     const base::Callback<bookmarks::BookmarkModel*(void)>&
         get_bookmark_model_callback,
-    scoped_refptr<base::SingleThreadTaskRunner> db_thread,
-    scoped_refptr<base::SingleThreadTaskRunner> file_thread,
+    scoped_refptr<base::SequencedTaskRunner> db_thread,
+    scoped_refptr<base::SequencedTaskRunner> file_thread,
     history::HistoryService* history_service)
     : syncer::FakeSyncClient(factory),
       pref_service_(pref_service),
@@ -232,12 +234,13 @@ ProfileSyncServiceBundle::SyncClientBuilder::Build() {
       get_syncable_service_callback_, get_sync_service_callback_,
       get_bookmark_model_callback_,
       activate_model_creation_ ? bundle_->db_thread() : nullptr,
-      activate_model_creation_ ? base::ThreadTaskRunnerHandle::Get() : nullptr,
+      activate_model_creation_ ? base::SequencedTaskRunnerHandle::Get()
+                               : nullptr,
       history_service_);
 }
 
 ProfileSyncServiceBundle::ProfileSyncServiceBundle()
-    : db_thread_(base::ThreadTaskRunnerHandle::Get()),
+    : db_thread_(base::SequencedTaskRunnerHandle::Get()),
       signin_client_(&pref_service_),
 #if defined(OS_CHROMEOS)
       signin_manager_(&signin_client_, &account_tracker_),

components/browser_sync/profile_sync_test_util.h

@@ -158,15 +158,15 @@ class ProfileSyncServiceBundle {
     return &fake_invalidation_service_;
   }
 
-  base::SingleThreadTaskRunner* db_thread() { return db_thread_.get(); }
+  base::SequencedTaskRunner* db_thread() { return db_thread_.get(); }
 
   void set_db_thread(
-      const scoped_refptr<base::SingleThreadTaskRunner>& db_thread) {
+      const scoped_refptr<base::SequencedTaskRunner>& db_thread) {
     db_thread_ = db_thread;
   }
 
  private:
-  scoped_refptr<base::SingleThreadTaskRunner> db_thread_;
+  scoped_refptr<base::SequencedTaskRunner> db_thread_;
   sync_preferences::TestingPrefServiceSyncable pref_service_;
   TestSigninClient signin_client_;
   AccountTrackerService account_tracker_;

components/browser_sync/signin_confirmation_helper.cc

@@ -5,10 +5,13 @@
 #include "components/browser_sync/signin_confirmation_helper.h"
 
 #include <memory>
+#include <utility>
 
-#include "base/single_thread_task_runner.h"
+#include "base/bind.h"
+#include "base/callback.h"
+#include "base/sequenced_task_runner.h"
 #include "base/strings/string16.h"
-#include "base/threading/thread_task_runner_handle.h"
+#include "base/threading/sequenced_task_runner_handle.h"
 #include "components/history/core/browser/history_backend.h"
 #include "components/history/core/browser/history_db_task.h"
 #include "components/history/core/browser/history_service.h"
@@ -21,8 +24,9 @@ namespace {
 // Determines whether there are any typed URLs in a history backend.
 class HasTypedURLsTask : public history::HistoryDBTask {
  public:
-  explicit HasTypedURLsTask(const base::Callback<void(bool)>& cb)
-      : has_typed_urls_(false), cb_(cb) {}
+  explicit HasTypedURLsTask(base::OnceCallback<void(bool)> cb)
+      : has_typed_urls_(false), cb_(std::move(cb)) {}
+  ~HasTypedURLsTask() override {}
 
   bool RunOnDBThread(history::HistoryBackend* backend,
                      history::HistoryDatabase* db) override {
@@ -36,27 +40,25 @@ class HasTypedURLsTask : public history::HistoryDBTask {
     return true;
   }
 
-  void DoneRunOnMainThread() override { cb_.Run(has_typed_urls_); }
+  void DoneRunOnMainThread() override { std::move(cb_).Run(has_typed_urls_); }
 
  private:
-  ~HasTypedURLsTask() override {}
-
   bool has_typed_urls_;
-  base::Callback<void(bool)> cb_;
+  base::OnceCallback<void(bool)> cb_;
 };
 
 }  // namespace
 
 SigninConfirmationHelper::SigninConfirmationHelper(
     history::HistoryService* history_service,
-    const base::Callback<void(bool)>& return_result)
-    : origin_thread_(base::ThreadTaskRunnerHandle::Get()),
+    base::OnceCallback<void(bool)> return_result)
+    : origin_sequence_(base::SequencedTaskRunnerHandle::Get()),
       history_service_(history_service),
       pending_requests_(0),
-      return_result_(return_result) {}
+      return_result_(std::move(return_result)) {}
 
 SigninConfirmationHelper::~SigninConfirmationHelper() {
-  DCHECK(origin_thread_->BelongsToCurrentThread());
+  DCHECK(origin_sequence_->RunsTasksInCurrentSequence());
 }
 
 void SigninConfirmationHelper::OnHistoryQueryResults(
@@ -95,23 +97,23 @@ void SigninConfirmationHelper::CheckHasTypedURLs() {
   }
   history_service_->ScheduleDBTask(
       FROM_HERE,
-      std::unique_ptr<history::HistoryDBTask>(new HasTypedURLsTask(base::Bind(
-          &SigninConfirmationHelper::ReturnResult, base::Unretained(this)))),
+      std::make_unique<HasTypedURLsTask>(base::BindOnce(
+          &SigninConfirmationHelper::ReturnResult, base::Unretained(this))),
       &task_tracker_);
 }
 
 void SigninConfirmationHelper::PostResult(bool result) {
-  origin_thread_->PostTask(FROM_HERE,
-                           base::Bind(&SigninConfirmationHelper::ReturnResult,
-                                      base::Unretained(this), result));
+  origin_sequence_->PostTask(
+      FROM_HERE, base::BindOnce(&SigninConfirmationHelper::ReturnResult,
+                                base::Unretained(this), result));
 }
 
 void SigninConfirmationHelper::ReturnResult(bool result) {
-  DCHECK(origin_thread_->BelongsToCurrentThread());
+  DCHECK(origin_sequence_->RunsTasksInCurrentSequence());
   // Pass |true| into the callback as soon as one of the tasks passes a
   // result of |true|, otherwise pass the last returned result.
   if (--pending_requests_ == 0 || result) {
-    return_result_.Run(result);
+    std::move(return_result_).Run(result);
 
     // This leaks at shutdown if the HistoryService is destroyed, but
     // the process is going to die anyway.

components/browser_sync/signin_confirmation_helper.h

@@ -12,7 +12,7 @@
 #include "base/task/cancelable_task_tracker.h"
 
 namespace base {
-class SingleThreadTaskRunner;
+class SequencedTaskRunner;
 }
 
 namespace history {
@@ -28,7 +28,7 @@ namespace browser_sync {
 class SigninConfirmationHelper {
  public:
   SigninConfirmationHelper(history::HistoryService* history_service,
-                           const base::Callback<void(bool)>& return_result);
+                           base::OnceCallback<void(bool)> return_result);
 
   // This helper checks if there are history entries in the history service.
   void CheckHasHistory(int max_entries);
@@ -44,15 +44,15 @@ class SigninConfirmationHelper {
   void OnHistoryQueryResults(size_t max_entries,
                              history::QueryResults* results);
 
-  // Posts the given result to the origin thread.
+  // Posts the given result to the origin sequence.
   void PostResult(bool result);
 
   // Calls |return_result_| if |result| == true or if it's the result of the
   // last pending check.
   void ReturnResult(bool result);
 
-  // The task runner for the thread this object was constructed on.
-  const scoped_refptr<base::SingleThreadTaskRunner> origin_thread_;
+  // The task runner for the sequence this object was constructed on.
+  const scoped_refptr<base::SequencedTaskRunner> origin_sequence_;
 
   // Pointer to the history service.
   history::HistoryService* history_service_;
@@ -64,7 +64,7 @@ class SigninConfirmationHelper {
   int pending_requests_;
 
   // Callback to pass the result back to the caller.
-  const base::Callback<void(bool)> return_result_;
+  base::OnceCallback<void(bool)> return_result_;
 
   DISALLOW_COPY_AND_ASSIGN(SigninConfirmationHelper);
 };