[Sync] Do not deadlock when joining sync thread with a pending HistoryModelWorker task.

components/history/core/browser/history_model_worker.h

Index: components/history/core/browser/history_model_worker.h
diff --git a/components/history/core/browser/history_model_worker.h b/components/history/core/browser/history_model_worker.h
index 6421c64d3a198d1080cc5ccb00a04d219dbcea03..dba6714e664247f203c1e141857d5de9baf43036 100644
--- a/components/history/core/browser/history_model_worker.h
+++ b/components/history/core/browser/history_model_worker.h
@@ -11,10 +11,13 @@
 #include "base/memory/ref_counted.h"
 #include "base/memory/weak_ptr.h"
 #include "base/single_thread_task_runner.h"
+#include "base/synchronization/lock.h"
+#include "base/synchronization/waitable_event.h"
 #include "base/task/cancelable_task_tracker.h"
 #include "components/sync/engine/model_safe_worker.h"
 
 namespace history {
+class HistoryDBTask;
 class HistoryService;
 }
 
@@ -29,6 +32,7 @@ class HistoryModelWorker : public syncer::ModelSafeWorker {
       const scoped_refptr<base::SingleThreadTaskRunner>& ui_thread);
 
   // syncer::ModelSafeWorker implementation.
+  void RequestStop() override;
   syncer::ModelSafeGroup GetModelSafeGroup() override;
   bool IsOnModelThread() override;
 
@@ -37,17 +41,38 @@ class HistoryModelWorker : public syncer::ModelSafeWorker {
       const syncer::WorkCallback& work) override;
 
  private:
+  class WorkerTask;

[maxbogue, 2017/03/20 19:55:50]

Is this still necessary?

[fdoray, 2017/03/21 17:21:31]

No. Removed it.

+
   ~HistoryModelWorker() override;
 
+  void ScheduleHistoryDBTaskFromUIThread(
+      std::unique_ptr<history::HistoryDBTask> worker_task,
+      syncer::SyncerError* error);
+
   const base::WeakPtr<history::HistoryService> history_service_;
 
   // A reference to the UI thread's task runner.
   const scoped_refptr<base::SingleThreadTaskRunner> ui_thread_;
 
+  // Signaled when a WorkCallback posted by DoWorkAndWaitUntilDoneImpl()
+  // finishes to run or is abandoned. Reset at the beginning of
+  // DoWorkAndWaitUntilDoneImpl().
+  base::WaitableEvent work_done_or_abandoned_;
+
+  // True if DoWorkAndWaitUntilDoneImpl() is blocked on a UI task.
+  bool ui_task_pending_ = false;
+
+  // Synchronizes calls to ModelSafeWorker::RequestStop()/IsStopped() and
+  // work_done_or_abandoned_.Reset() and all accesses to |ui_task_pending_|.

[maxbogue, 2017/03/20 19:55:50]

I think this is really just for ui_task_pending_... all the others already use
thread-safe things don't they? You may be able to change ui_task_pending_ to an
AtomicFlag and get rid of this.

[fdoray, 2017/03/21 17:21:31]

Added comment about potential deadlock at the top of
HistoryModelWorker::DoWorkAndWaitUntilDoneImpl() in the .cc file.

+  base::Lock lock_;
+
   // Helper object to make sure we don't leave tasks running on the history
   // thread.
   std::unique_ptr<base::CancelableTaskTracker> cancelable_tracker_;
 
+  // Verifies that calls to DoWorkAndWaitUntilDoneImpl() are sequenced.
+  base::SequenceChecker sequence_checker_;
+
   DISALLOW_COPY_AND_ASSIGN(HistoryModelWorker);
 };