[Sync] Signal UIModelWorker to abort on sync shutdown.

components/sync/engine/ui_model_worker.cc

Index: components/sync/engine/ui_model_worker.cc
diff --git a/components/sync/engine/ui_model_worker.cc b/components/sync/engine/ui_model_worker.cc
index 69541553cab7404553758b1229de2b234dd9f83c..77fe9c7799a65db7437df9cc45edf62a0611467c 100644
--- a/components/sync/engine/ui_model_worker.cc
+++ b/components/sync/engine/ui_model_worker.cc
@@ -9,55 +9,88 @@
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/callback.h"
-#include "base/synchronization/waitable_event.h"
 #include "components/sync/base/scoped_event_signal.h"
 
 namespace syncer {
 
 namespace {
 
-void CallDoWorkAndSignalEvent(const WorkCallback& work,
-                              syncer::ScopedEventSignal scoped_event_signal,
-                              SyncerError* error_info) {
-  *error_info = work.Run();
-  // The event in |scoped_event_signal| is signaled at the end of this scope.
+class ScopedEventSignalWithWorker {
+ public:
+  ScopedEventSignalWithWorker(scoped_refptr<UIModelWorker> ui_model_worker,
+                              base::WaitableEvent* event)
+      : ui_model_worker_(std::move(ui_model_worker)),
+        scoped_event_signal_(event) {}
+
+  ScopedEventSignalWithWorker(ScopedEventSignalWithWorker&&) = default;
+  ScopedEventSignalWithWorker& operator=(ScopedEventSignalWithWorker&&) =
+      default;
+
+  bool IsStopped() const { return ui_model_worker_->IsStopped(); }
+
+ private:
+  // This reference prevents the event in |scoped_event_signal_| from being
+  // signaled after being deleted.
+  scoped_refptr<UIModelWorker> ui_model_worker_;
+
+  ScopedEventSignal scoped_event_signal_;
+
+  DISALLOW_COPY_AND_ASSIGN(ScopedEventSignalWithWorker);
+};
+
+void CallDoWorkAndSignalEvent(
+    const WorkCallback& work,
+    ScopedEventSignalWithWorker scoped_event_signal_with_worker,
+    SyncerError* error_info) {
+  if (!scoped_event_signal_with_worker.IsStopped())
+    *error_info = work.Run();
+  // The event in |scoped_event_signal_with_worker| is signaled at the end of
+  // this scope.
 }
 
 }  // namespace
 
 UIModelWorker::UIModelWorker(
     scoped_refptr<base::SingleThreadTaskRunner> ui_thread)
-    : ui_thread_(std::move(ui_thread)) {}
+    : ui_thread_(std::move(ui_thread)),
+      work_done_or_abandoned_(base::WaitableEvent::ResetPolicy::MANUAL,

[maxbogue, 2016/11/21 17:17:46]

I suspect this could be AUTOMATIC and you could omit the Reset() now, but I'm
not positive.

[fdoray, 2016/11/21 19:06:11]

If it was AUTOMATIC, we would still need a reset when PostTask() fails. Since we
don't wait for the event when PostTask() fails, it is not automatically reset.

I prefer having a Reset() that runs in all cases than a Reset() just for this
special case.

+                              base::WaitableEvent::InitialState::NOT_SIGNALED),
+      stop_requested_(base::WaitableEvent::ResetPolicy::MANUAL,
+                      base::WaitableEvent::InitialState::NOT_SIGNALED) {
+  sequence_checker_.DetachFromSequence();
+}
 
 SyncerError UIModelWorker::DoWorkAndWaitUntilDoneImpl(
     const WorkCallback& work) {
-  SyncerError error_info;
-  if (ui_thread_->BelongsToCurrentThread()) {
-    DLOG(WARNING) << "DoWorkAndWaitUntilDone called from "
-                  << "ui_loop_. Probably a nested invocation?";
-    return work.Run();
-  }
+  DCHECK(sequence_checker_.CalledOnValidSequence());
+  DCHECK(!ui_thread_->BelongsToCurrentThread());
 
-  // Signaled when the task is deleted, i.e. after it runs or when it is
-  // abandoned.
-  base::WaitableEvent work_done_or_abandoned(
-      base::WaitableEvent::ResetPolicy::AUTOMATIC,
-      base::WaitableEvent::InitialState::NOT_SIGNALED);
-
-  if (!ui_thread_->PostTask(FROM_HERE,
-                            base::Bind(&CallDoWorkAndSignalEvent, work,
-                                       base::Passed(syncer::ScopedEventSignal(
-                                           &work_done_or_abandoned)),
-                                       &error_info))) {
+  SyncerError error_info;
+  work_done_or_abandoned_.Reset();
+
+  if (!ui_thread_->PostTask(
+          FROM_HERE,
+          base::Bind(&CallDoWorkAndSignalEvent, work,
+                     base::Passed(syncer::ScopedEventSignalWithWorker(
+                         this, &work_done_or_abandoned_)),
+                     &error_info))) {
     DLOG(WARNING) << "Could not post work to UI loop.";
     error_info = CANNOT_DO_WORK;
     return error_info;
   }
-  work_done_or_abandoned.Wait();
+
+  base::WaitableEvent* events[] = {&work_done_or_abandoned_, &stop_requested_};
+  base::WaitableEvent::WaitMany(events, arraysize(events));
 
   return error_info;
 }
 
+void UIModelWorker::RequestStop() {
+  DCHECK(ui_thread_->BelongsToCurrentThread());
+  ModelSafeWorker::RequestStop();
+  stop_requested_.Signal();
+}
+
 ModelSafeGroup UIModelWorker::GetModelSafeGroup() {
   return GROUP_UI;
 }