Extend base::win::ObjectWatcher to watch an event multiple times.

This is split off from https://codereview.chromium.org/1350023003/. During performance testing, it was found that it was 5x faster to watch an auto-reset event once instead of a manual-reset event every time it fired.

BUG=478251

Review URL: https://codereview.chromium.org/1366093006

Cr-Commit-Position: refs/heads/master@{#351240}

4 files changed, 118 insertions, 47 deletions

diff --git a/base/win/object_watcher.cc b/base/win/object_watcher.cc
index 5ebe185..93efd06 100644
--- a/base/win/object_watcher.cc
+++ b/base/win/object_watcher.cc
@@ -16,6 +16,7 @@ ObjectWatcher::ObjectWatcher()
     : object_(NULL),
       wait_object_(NULL),
       origin_loop_(NULL),
+      run_once_(true),
       weak_factory_(this) {
 }
 
@@ -23,36 +24,13 @@ ObjectWatcher::~ObjectWatcher() {
   StopWatching();
 }
 
-bool ObjectWatcher::StartWatching(HANDLE object, Delegate* delegate) {
-  CHECK(delegate);
-  if (wait_object_) {
-    NOTREACHED() << "Already watching an object";
-    return false;
-  }
-
-  // Since our job is to just notice when an object is signaled and report the
-  // result back to this thread, we can just run on a Windows wait thread.
-  DWORD wait_flags = WT_EXECUTEINWAITTHREAD | WT_EXECUTEONLYONCE;
-
-  // DoneWaiting can be synchronously called from RegisterWaitForSingleObject,
-  // so set up all state now.
-  callback_ = base::Bind(&ObjectWatcher::Signal, weak_factory_.GetWeakPtr(),
-                         delegate);
-  object_ = object;
-  origin_loop_ = MessageLoop::current();
-
-  if (!RegisterWaitForSingleObject(&wait_object_, object, DoneWaiting,
-                                   this, INFINITE, wait_flags)) {
-    DPLOG(FATAL) << "RegisterWaitForSingleObject failed";
-    object_ = NULL;
-    wait_object_ = NULL;
-    return false;
-  }
+bool ObjectWatcher::StartWatchingOnce(HANDLE object, Delegate* delegate) {
+  return StartWatchingInternal(object, delegate, true);
+}
 
-  // 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.
-  MessageLoop::current()->AddDestructionObserver(this);
-  return true;
+bool ObjectWatcher::StartWatchingMultipleTimes(HANDLE object,
+                                               Delegate* delegate) {
+  return StartWatchingInternal(object, delegate, false);
 }
 
 bool ObjectWatcher::StopWatching() {
@@ -93,7 +71,44 @@ void CALLBACK ObjectWatcher::DoneWaiting(void* param, BOOLEAN timed_out) {
   // 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->callback_.Reset();
+  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;
+  }
+  run_once_ = execute_only_once;
+
+  // Since our job is to just notice when an object is signaled and report the
+  // result back to this thread, we can just run on a Windows wait thread.
+  DWORD wait_flags = WT_EXECUTEINWAITTHREAD;
+  if (run_once_)
+    wait_flags |= WT_EXECUTEONLYONCE;
+
+  // DoneWaiting can be synchronously called from RegisterWaitForSingleObject,
+  // so set up all state now.
+  callback_ = base::Bind(&ObjectWatcher::Signal, weak_factory_.GetWeakPtr(),
+                         delegate);
+  object_ = object;
+  origin_loop_ = MessageLoop::current();
+
+  if (!RegisterWaitForSingleObject(&wait_object_, object, DoneWaiting,
+                                   this, INFINITE, wait_flags)) {
+    DPLOG(FATAL) << "RegisterWaitForSingleObject failed";
+    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.
+  MessageLoop::current()->AddDestructionObserver(this);
+  return true;
 }
 
 void ObjectWatcher::Signal(Delegate* delegate) {
@@ -101,7 +116,8 @@ void ObjectWatcher::Signal(Delegate* delegate) {
   // StartWatching(). As a result, we save any state we need and clear previous
   // watcher state before signaling the delegate.
   HANDLE object = object_;
-  StopWatching();
+  if (run_once_)
+    StopWatching();
   delegate->OnObjectSignaled(object);
 }
 
diff --git a/base/win/object_watcher.h b/base/win/object_watcher.h
index d68d935..f4d6085 100644
--- a/base/win/object_watcher.h
+++ b/base/win/object_watcher.h
@@ -26,16 +26,16 @@ namespace win {
 //
 // Typical usage:
 //
-//   class MyClass : public base::ObjectWatcher::Delegate {
+//   class MyClass : public base::win::ObjectWatcher::Delegate {
 //    public:
 //     void DoStuffWhenSignaled(HANDLE object) {
-//       watcher_.StartWatching(object, this);
+//       watcher_.StartWatchingOnce(object, this);
 //     }
-//     virtual void OnObjectSignaled(HANDLE object) {
+//     void OnObjectSignaled(HANDLE object) override {
 //       // OK, time to do stuff!
 //     }
 //    private:
-//     base::ObjectWatcher watcher_;
+//     base::win::ObjectWatcher watcher_;
 //   };
 //
 // In the above example, MyClass wants to "do stuff" when object becomes
@@ -59,19 +59,23 @@ class BASE_EXPORT ObjectWatcher : public MessageLoop::DestructionObserver {
   ~ObjectWatcher() override;
 
   // When the object is signaled, the given delegate is notified on the thread
-  // where StartWatching is called.  The ObjectWatcher is not responsible for
+  // where StartWatchingOnce is called. The ObjectWatcher is not responsible for
   // deleting the delegate.
-  //
   // Returns true if the watch was started.  Otherwise, false is returned.
-  //
-  bool StartWatching(HANDLE object, Delegate* delegate);
+  bool StartWatchingOnce(HANDLE object, Delegate* delegate);
+
+  // Notifies the delegate, on the thread where this method is called, each time
+  // the object is set. By definition, the handle must be an auto-reset object.
+  // The caller must ensure that it (or any Windows system code) doesn't reset
+  // the event or else the delegate won't be called.
+  // Returns true if the watch was started.  Otherwise, false is returned.
+  bool StartWatchingMultipleTimes(HANDLE object, Delegate* delegate);
 
   // Stops watching.  Does nothing if the watch has already completed.  If the
   // watch is still active, then it is canceled, and the associated delegate is
   // not notified.
   //
   // Returns true if the watch was canceled.  Otherwise, false is returned.
-  //
   bool StopWatching();
 
   // Returns true if currently watching an object.
@@ -84,6 +88,10 @@ class BASE_EXPORT ObjectWatcher : public MessageLoop::DestructionObserver {
   // Called on a background thread when done waiting.
   static void CALLBACK DoneWaiting(void* param, BOOLEAN timed_out);
 
+  // Helper used by StartWatchingOnce and StartWatchingMultipleTimes.
+  bool StartWatchingInternal(HANDLE object, Delegate* delegate,
+                             bool execute_only_once);
+
   void Signal(Delegate* delegate);
 
   // MessageLoop::DestructionObserver implementation:
@@ -94,7 +102,7 @@ class BASE_EXPORT ObjectWatcher : public MessageLoop::DestructionObserver {
   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);
diff --git a/base/win/object_watcher_unittest.cc b/base/win/object_watcher_unittest.cc
index b30ca41..511ec49 100644
--- a/base/win/object_watcher_unittest.cc
+++ b/base/win/object_watcher_unittest.cc
@@ -42,7 +42,7 @@ void RunTest_BasicSignal(MessageLoop::Type message_loop_type) {
   HANDLE event = CreateEvent(NULL, TRUE, FALSE, NULL);
 
   QuitDelegate delegate;
-  bool ok = watcher.StartWatching(event, &delegate);
+  bool ok = watcher.StartWatchingOnce(event, &delegate);
   EXPECT_TRUE(ok);
   EXPECT_TRUE(watcher.IsWatching());
   EXPECT_EQ(event, watcher.GetWatchedObject());
@@ -64,7 +64,7 @@ void RunTest_BasicCancel(MessageLoop::Type message_loop_type) {
   HANDLE event = CreateEvent(NULL, TRUE, FALSE, NULL);
 
   QuitDelegate delegate;
-  bool ok = watcher.StartWatching(event, &delegate);
+  bool ok = watcher.StartWatchingOnce(event, &delegate);
   EXPECT_TRUE(ok);
 
   watcher.StopWatching();
@@ -83,7 +83,7 @@ void RunTest_CancelAfterSet(MessageLoop::Type message_loop_type) {
   // A manual-reset event that is not yet signaled.
   HANDLE event = CreateEvent(NULL, TRUE, FALSE, NULL);
 
-  bool ok = watcher.StartWatching(event, &delegate);
+  bool ok = watcher.StartWatchingOnce(event, &delegate);
   EXPECT_TRUE(ok);
 
   SetEvent(event);
@@ -110,7 +110,7 @@ void RunTest_SignalBeforeWatch(MessageLoop::Type message_loop_type) {
   HANDLE event = CreateEvent(NULL, TRUE, TRUE, NULL);
 
   QuitDelegate delegate;
-  bool ok = watcher.StartWatching(event, &delegate);
+  bool ok = watcher.StartWatchingOnce(event, &delegate);
   EXPECT_TRUE(ok);
 
   MessageLoop::current()->Run();
@@ -130,12 +130,53 @@ void RunTest_OutlivesMessageLoop(MessageLoop::Type message_loop_type) {
       MessageLoop message_loop(message_loop_type);
 
       QuitDelegate delegate;
-      watcher.StartWatching(event, &delegate);
+      watcher.StartWatchingOnce(event, &delegate);
     }
   }
   CloseHandle(event);
 }
 
+class QuitAfterMultipleDelegate : public ObjectWatcher::Delegate {
+ public:
+  QuitAfterMultipleDelegate(HANDLE event, int iterations)
+      : event_(event), iterations_(iterations) {}
+  void OnObjectSignaled(HANDLE object) override {
+    if (--iterations_) {
+      SetEvent(event_);
+    } else {
+      MessageLoop::current()->QuitWhenIdle();
+    }
+  }
+
+ private:
+  HANDLE event_;
+  int iterations_;
+};
+
+void RunTest_ExecuteMultipleTimes(MessageLoop::Type message_loop_type) {
+  MessageLoop message_loop(message_loop_type);
+
+  ObjectWatcher watcher;
+  EXPECT_FALSE(watcher.IsWatching());
+
+  // An auto-reset event that is not yet signaled.
+  HANDLE event = CreateEvent(NULL, FALSE, FALSE, NULL);
+
+  QuitAfterMultipleDelegate delegate(event, 2);
+  bool ok = watcher.StartWatchingMultipleTimes(event, &delegate);
+  EXPECT_TRUE(ok);
+  EXPECT_TRUE(watcher.IsWatching());
+  EXPECT_EQ(event, watcher.GetWatchedObject());
+
+  SetEvent(event);
+
+  MessageLoop::current()->Run();
+
+  EXPECT_TRUE(watcher.IsWatching());
+  EXPECT_TRUE(watcher.StopWatching());
+  CloseHandle(event);
+}
+
 }  // namespace
 
 //-----------------------------------------------------------------------------
@@ -170,5 +211,11 @@ TEST(ObjectWatcherTest, OutlivesMessageLoop) {
   RunTest_OutlivesMessageLoop(MessageLoop::TYPE_UI);
 }
 
+TEST(ObjectWatcherTest, ExecuteMultipleTimes) {
+  RunTest_ExecuteMultipleTimes(MessageLoop::TYPE_DEFAULT);
+  RunTest_ExecuteMultipleTimes(MessageLoop::TYPE_IO);
+  RunTest_ExecuteMultipleTimes(MessageLoop::TYPE_UI);
+}
+
 }  // namespace win
 }  // namespace base
diff --git a/base/win/registry.cc b/base/win/registry.cc
index 47afcbf..28e0461 100644
--- a/base/win/registry.cc
+++ b/base/win/registry.cc
@@ -82,7 +82,7 @@ bool RegKey::Watcher::StartWatching(HKEY key, const ChangeCallback& callback) {
   }
 
   callback_ = callback;
-  return object_watcher_.StartWatching(watch_event_.Get(), this);
+  return object_watcher_.StartWatchingOnce(watch_event_.Get(), this);
 }
 
 // RegKey ----------------------------------------------------------------------