Remove MessageLoop::current() from base::win::ObjectWatcher (reland).

---
This is a reland of https://codereview.chromium.org/2125763003 which
was reverted because it broke the Win10 builder. It should not break
anything now that TestSimpleTaskRunner is thread-safe
(https://codereview.chromium.org/2296923003)
---

Why? The fact that there's a MessageLoop on the thread is an
unnecessary implementation detail. When browser threads are migrated to
base/task_scheduler, tasks will no longer have access to a MessageLoop
but they will be able to get the current ThreadTaskRunnerHandle.

Before this CL, ObjectWatcher implemented
WillDestroyCurrentMessageLoop() to stop the watch when the
MessageLoop responsible for running the callback was destroyed.
This prevented the watch callback from being sent to a destroyed
MessageLoop.

Now that ObjectWatcher keeps a reference to a TaskRunner, this is
no longer required. The TaskRunner can't be deleted while there are
references to it. If the underlying MessageLoop has been deleted
when the watch callback is posted, the callback will simply not run.

Note that the watch will still be stopped when the ObjectWatcher is
deleted.

TBR=ben@chromium.org
BUG=650318

Review-Url: https://codereview.chromium.org/2365423003
Cr-Commit-Position: refs/heads/master@{#421179}

base/files/file_path_watcher_win.cc

@@ -11,6 +11,7 @@
 #include "base/logging.h"
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
+#include "base/message_loop/message_loop.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "base/time/time.h"
 #include "base/win/object_watcher.h"

base/process/kill_win.cc

@@ -8,6 +8,7 @@
 #include <io.h>
 #include <stdint.h>
 
+#include <algorithm>
 #include <utility>
 
 #include "base/bind.h"

base/win/object_watcher.cc

@@ -6,19 +6,14 @@
 
 #include "base/bind.h"
 #include "base/logging.h"
+#include "base/threading/thread_task_runner_handle.h"
 
 namespace base {
 namespace win {
 
 //-----------------------------------------------------------------------------
 
-ObjectWatcher::ObjectWatcher()
-    : object_(NULL),
-      wait_object_(NULL),
-      origin_loop_(NULL),
-      run_once_(true),
-      weak_factory_(this) {
-}
+ObjectWatcher::ObjectWatcher() : weak_factory_(this) {}
 
 ObjectWatcher::~ObjectWatcher() {
   StopWatching();
@@ -38,7 +33,7 @@ bool ObjectWatcher::StopWatching() {
     return false;
 
   // Make sure ObjectWatcher is used in a single-threaded fashion.
-  DCHECK_EQ(origin_loop_, MessageLoop::current());
+  DCHECK(task_runner_->BelongsToCurrentThread());
 
   // Blocking call to cancel the wait. Any callbacks already in progress will
   // finish before we return from this call.
@@ -47,16 +42,12 @@ bool ObjectWatcher::StopWatching() {
     return false;
   }
 
-  weak_factory_.InvalidateWeakPtrs();
-  object_ = NULL;
-  wait_object_ = NULL;
-
-  origin_loop_->RemoveDestructionObserver(this);
+  Reset();
   return true;
 }
 
 bool ObjectWatcher::IsWatching() const {
-  return object_ != NULL;
+  return object_ != nullptr;
 }
 
 HANDLE ObjectWatcher::GetWatchedObject() const {
@@ -70,22 +61,18 @@ void CALLBACK ObjectWatcher::DoneWaiting(void* param, BOOLEAN timed_out) {
   // The destructor blocks on any callbacks that are in flight, so we know that
   // that is always a pointer to a valid ObjectWater.
   ObjectWatcher* that = static_cast<ObjectWatcher*>(param);
-  that->origin_loop_->task_runner()->PostTask(FROM_HERE, that->callback_);
+  that->task_runner_->PostTask(FROM_HERE, that->callback_);
   if (that->run_once_)
     that->callback_.Reset();
 }
 
 bool ObjectWatcher::StartWatchingInternal(HANDLE object, Delegate* delegate,
                                           bool execute_only_once) {
-  CHECK(delegate);
-  if (wait_object_) {
-    NOTREACHED() << "Already watching an object";
-    return false;
-  }
+  DCHECK(delegate);
+  DCHECK(!wait_object_) << "Already watching an object";
+  DCHECK(ThreadTaskRunnerHandle::IsSet());
 
-  origin_loop_ = MessageLoop::current();
-  if (!origin_loop_)
-    return false;
+  task_runner_ = ThreadTaskRunnerHandle::Get();
 
   run_once_ = execute_only_once;
 
@@ -97,21 +84,17 @@ bool ObjectWatcher::StartWatchingInternal(HANDLE object, Delegate* delegate,
 
   // DoneWaiting can be synchronously called from RegisterWaitForSingleObject,
   // so set up all state now.
-  callback_ = base::Bind(&ObjectWatcher::Signal, weak_factory_.GetWeakPtr(),
-                         delegate);
+  callback_ =
+      Bind(&ObjectWatcher::Signal, weak_factory_.GetWeakPtr(), delegate);
   object_ = object;
 
   if (!RegisterWaitForSingleObject(&wait_object_, object, DoneWaiting,
                                    this, INFINITE, wait_flags)) {
     DPLOG(FATAL) << "RegisterWaitForSingleObject failed";
-    object_ = NULL;
-    wait_object_ = NULL;
+    Reset();
     return false;
   }
 
-  // We need to know if the current message loop is going away so we can
-  // prevent the wait thread from trying to access a dead message loop.
-  origin_loop_->AddDestructionObserver(this);
   return true;
 }
 
@@ -125,10 +108,13 @@ void ObjectWatcher::Signal(Delegate* delegate) {
   delegate->OnObjectSignaled(object);
 }
 
-void ObjectWatcher::WillDestroyCurrentMessageLoop() {
-  // Need to shutdown the watch so that we don't try to access the MessageLoop
-  // after this point.
-  StopWatching();
+void ObjectWatcher::Reset() {
+  callback_.Reset();
+  object_ = nullptr;
+  wait_object_ = nullptr;
+  task_runner_ = nullptr;
+  run_once_ = true;
+  weak_factory_.InvalidateWeakPtrs();
 }
 
 }  // namespace win

base/win/object_watcher.h

@@ -10,8 +10,9 @@
 #include "base/base_export.h"
 #include "base/callback.h"
 #include "base/macros.h"
+#include "base/memory/ref_counted.h"
 #include "base/memory/weak_ptr.h"
-#include "base/message_loop/message_loop.h"
+#include "base/single_thread_task_runner.h"
 
 namespace base {
 namespace win {
@@ -46,22 +47,22 @@ namespace win {
 // If the object is already signaled before being watched, OnObjectSignaled is
 // still called after (but not necessarily immediately after) watch is started.
 //
-// NOTE: Use of this class requires that there be a current message loop;
-// otherwise, when the object is signaled, there would be no loop to post the
-// callback task to.  This means that you cannot use ObjectWatcher in test code
-// that doesn't create a message loop (unless you add such a loop).
-class BASE_EXPORT ObjectWatcher : public MessageLoop::DestructionObserver {
+// NOTE: Except for the constructor, all public methods of this class must be
+// called on the same thread. A ThreadTaskRunnerHandle must be set on that
+// thread.
+class BASE_EXPORT ObjectWatcher {
  public:
   class BASE_EXPORT Delegate {
    public:
     virtual ~Delegate() {}
-    // Called from the MessageLoop when a signaled object is detected.  To
-    // continue watching the object, StartWatching must be called again.
+    // Called from the thread that started the watch when a signaled object is
+    // detected. To continue watching the object, StartWatching must be called
+    // again.
     virtual void OnObjectSignaled(HANDLE object) = 0;
   };
 
   ObjectWatcher();
-  ~ObjectWatcher() override;
+  ~ObjectWatcher();
 
   // When the object is signaled, the given delegate is notified on the thread
   // where StartWatchingOnce is called. The ObjectWatcher is not responsible for
@@ -99,15 +100,23 @@ class BASE_EXPORT ObjectWatcher : public MessageLoop::DestructionObserver {
 
   void Signal(Delegate* delegate);
 
-  // MessageLoop::DestructionObserver implementation:
-  void WillDestroyCurrentMessageLoop() override;
+  void Reset();
 
-  // Internal state.
+  // A callback pre-bound to Signal() that is posted to the caller's task runner
+  // when the wait completes.
   Closure callback_;
-  HANDLE object_;             // The object being watched
-  HANDLE wait_object_;        // Returned by RegisterWaitForSingleObject
-  MessageLoop* origin_loop_;  // Used to get back to the origin thread
-  bool run_once_;
+
+  // The object being watched.
+  HANDLE object_ = nullptr;
+
+  // The wait handle returned by RegisterWaitForSingleObject.
+  HANDLE wait_object_ = nullptr;
+
+  // The task runner of the thread on which the watch was started.
+  scoped_refptr<SingleThreadTaskRunner> task_runner_;
+
+  bool run_once_ = true;
+
   WeakPtrFactory<ObjectWatcher> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(ObjectWatcher);

chrome/common/service_process_util_win.cc

@@ -4,6 +4,7 @@
 
 #include "chrome/common/service_process_util.h"
 
+#include <algorithm>
 #include <memory>
 
 #include "base/callback.h"

ui/gfx/font_fallback_win.cc

@@ -14,6 +14,7 @@
 #include "base/i18n/rtl.h"
 #include "base/macros.h"
 #include "base/memory/singleton.h"
+#include "base/message_loop/message_loop.h"
 #include "base/profiler/scoped_tracker.h"
 #include "base/strings/string_split.h"
 #include "base/strings/string_util.h"