[Sync] Refactor ModelSafeWorker::DoWorkAndWaitUntilDone() to avoid code duplication.

components/history/core/browser/history_model_worker.h

 // 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.
 
 #ifndef COMPONENTS_HISTORY_CORE_BROWSER_HISTORY_MODEL_WORKER_H_
 #define COMPONENTS_HISTORY_CORE_BROWSER_HISTORY_MODEL_WORKER_H_
 
 #include <memory>
 
 #include "base/macros.h"
@@ skipping 14 matching lines @@
 class HistoryModelWorker : public syncer::ModelSafeWorker {
  public:
   explicit HistoryModelWorker(
       const base::WeakPtr<history::HistoryService>& history_service,
       const scoped_refptr<base::SingleThreadTaskRunner>& ui_thread);
 
   // syncer::ModelSafeWorker implementation.
   syncer::ModelSafeGroup GetModelSafeGroup() override;
   bool IsOnModelThread() override;
 
- protected:
-  syncer::SyncerError DoWorkAndWaitUntilDoneImpl(
-      const syncer::WorkCallback& work) override;
-
  private:
   ~HistoryModelWorker() override;
 
+  void ScheduleWork(base::Closure work) override;

[skym, 2017/04/06 00:34:54]

In the past we've been passing base::Callbacks to functions as const&s. Is there
any point to this? Should we be simply passing them like this? I've never really
understood the difference.

[fdoray, 2017/04/06 18:10:55]

Closure is a simple class that holds a reference to a BindStateBase. Copying a
Closure increments the reference count of this BindStateBase. Moving the Closure
prevents that. 

void ScheduleWork(const base::Closure& work) {
  task_runner->PostTask(FROM_HERE, work); <-- bump ref count
}

vs.

void ScheduleWork(base::Closure work) {
  task_runner->PostTask(FROM_HERE, std::move(work)); <-- ref count unchanged
} 

I'm currently writing documentation about this, so it's normal that you're not
aware of it.

Note: Bumping the ref count costs CPU cycles, but also creates non-determinism
(who will release the last reference?).

+
   const base::WeakPtr<history::HistoryService> history_service_;
 
   // A reference to the UI thread's task runner.
   const scoped_refptr<base::SingleThreadTaskRunner> ui_thread_;
 
   // Helper object to make sure we don't leave tasks running on the history
   // thread.
   std::unique_ptr<base::CancelableTaskTracker> cancelable_tracker_;
 
   DISALLOW_COPY_AND_ASSIGN(HistoryModelWorker);
 };
 
 }  // namespace browser_sync
 
 #endif  // COMPONENTS_HISTORY_CORE_BROWSER_HISTORY_MODEL_WORKER_H_