Revert of Remove MessageLoop::current() from base::win::ObjectWatcher....

Revert of Remove MessageLoop::current() from base::win::ObjectWatcher. (patchset #5 id:80001 of https://codereview.chromium.org/2125763003/ )

Reason for revert:
Breaks Win10 builder. crbug.com/632184

Original issue's description:
> Remove MessageLoop::current() from base::win::ObjectWatcher.
>
> 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=616447
>
> Committed: https://crrev.com/76db1513714741d8d6e6dd6e872d50f20b090fce
> Cr-Commit-Position: refs/heads/master@{#407874}

TBR=grt@chromium.org,ben@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=616447

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

base/files/file_path_watcher_win.cc

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

base/win/object_watcher.cc

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

base/win/object_watcher.h

@@ -10,9 +10,8 @@
 #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/single_thread_task_runner.h"
+#include "base/message_loop/message_loop.h"
 
 namespace base {
 namespace win {
@@ -47,22 +46,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: 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 {
+// 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 {
  public:
   class BASE_EXPORT Delegate {
    public:
     virtual ~Delegate() {}
-    // Called from the thread that started the watch when a signaled object is
-    // detected. To continue watching the object, StartWatching must be called
-    // again.
+    // Called from the MessageLoop when a signaled object is detected.  To
+    // continue watching the object, StartWatching must be called again.
     virtual void OnObjectSignaled(HANDLE object) = 0;
   };
 
   ObjectWatcher();
-  ~ObjectWatcher();
+  ~ObjectWatcher() override;
 
   // When the object is signaled, the given delegate is notified on the thread
   // where StartWatchingOnce is called. The ObjectWatcher is not responsible for
@@ -100,23 +99,15 @@ class BASE_EXPORT ObjectWatcher {
 
   void Signal(Delegate* delegate);
 
-  void Reset();
+  // MessageLoop::DestructionObserver implementation:
+  void WillDestroyCurrentMessageLoop() override;
 
-  // A callback pre-bound to Signal() that is posted to the caller's task runner
-  // when the wait completes.
+  // Internal state.
   Closure callback_;
-
-  // 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;
-
+  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_;
   WeakPtrFactory<ObjectWatcher> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(ObjectWatcher);

chrome/common/service_process_util_win.cc

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

ui/gfx/font_fallback_win.cc

@@ -14,7 +14,6 @@
 #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"