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

base/win/object_watcher.cc

Index: base/win/object_watcher.cc
diff --git a/base/win/object_watcher.cc b/base/win/object_watcher.cc
index 4df54a45d6bf90b5286da96e93dcf674653c2d68..aba74359db5b16aa13fa438920f90f7b81454a04 100644
--- a/base/win/object_watcher.cc
+++ b/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.
@@ -51,7 +46,6 @@ bool ObjectWatcher::StopWatching() {
   object_ = NULL;

[grt (UTC plus 2), 2016/07/14 07:19:11]

nit: nullptr throughout

[fdoray, 2016/07/18 16:05:18]

Done.

   wait_object_ = NULL;

[grt (UTC plus 2), 2016/07/14 07:19:11]

take absolutely everything back to its initial state here to be sure that
dangling refs aren't held (the task runner is the only culprit, but it's good to
be thorough). for style points, retain the ordering in the class decl so that
it's easy to audit them. may as well make a helper function to do this that can
be called from StartWatchingInternal's failure path:
  callback_.Reset();
  object_ = nullptr;
  wait_object_ = nullptr;
  task_runner_.reset();
  run_once_ = true;
  weak_factory_.InvalidateWeakPtrs();

[fdoray, 2016/07/18 16:05:18]

Done.

 
-  origin_loop_->RemoveDestructionObserver(this);
   return true;
 }
 
@@ -70,7 +64,7 @@ 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();
 }
@@ -83,9 +77,9 @@ bool ObjectWatcher::StartWatchingInternal(HANDLE object, Delegate* delegate,
     return false;
   }
 
-  origin_loop_ = MessageLoop::current();
-  if (!origin_loop_)
+  if (!ThreadTaskRunnerHandle::IsSet())

[grt (UTC plus 2), 2016/07/14 07:19:11]

NOTREACHED for this, too? isn't it a programming error as well? does it make
sense for this to be a DHCECK(ThreadTaskRunnerHandle::IsSet()); at the top of
the function and no "return false;", since gracefully handling programmer error
is counter to coding style?

[fdoray, 2016/07/18 16:05:18]

Done.

     return false;
+  task_runner_ = ThreadTaskRunnerHandle::Get();
 
   run_once_ = execute_only_once;
 
@@ -109,9 +103,6 @@ bool ObjectWatcher::StartWatchingInternal(HANDLE object, Delegate* delegate,
     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,11 +116,5 @@ 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();
-}
-
 }  // namespace win
 }  // namespace base