Implements HandleWatcher in terms of MessagePumpMojo

mojo/common/message_pump_mojo.cc

Index: mojo/common/message_pump_mojo.cc
diff --git a/mojo/common/message_pump_mojo.cc b/mojo/common/message_pump_mojo.cc
index 7b5b16302c07dbdb6864adc907717a474dabbd33..16396720a5aca9cd9aebcd9316b4f085fb1a6949 100644
--- a/mojo/common/message_pump_mojo.cc
+++ b/mojo/common/message_pump_mojo.cc
@@ -45,18 +45,22 @@ MessagePumpMojo::~MessagePumpMojo() {
 
 void MessagePumpMojo::AddHandler(MessagePumpMojoHandler* handler,
                                  MojoHandle handle,
-                                 MojoWaitFlags wait_flags) {
+                                 MojoWaitFlags wait_flags,
+                                 base::TimeTicks deadline) {
   DCHECK(handler);
   DCHECK_NE(MOJO_HANDLE_INVALID, handle);
+  Handler old_handler;
+  if (handlers_.count(handle))
+    old_handler = handlers_[handle];
   handlers_[handle].handler = handler;

[darin (slow to review), 2013/11/21 18:14:52]

nit: it'd probably be worth optimizing this code to not perform so many lookups
in the map. one approach would be to just construct a new Handler and then do
handlers_[handle] = new_handler;

[sky, 2013/11/22 19:39:44]

Done.

   handlers_[handle].wait_flags = wait_flags;
-
-  SignalControlPipe();
+  handlers_[handle].deadline = deadline;
+  if (old_handler.handler)
+    old_handler.handler->OnHandleError(handle, MOJO_RESULT_CANCELLED);

[darin (slow to review), 2013/11/21 18:14:52]

I'm a little bit worried about calling OnHandleError on some potentially deeply
nested call stack. Normally, calls to the Handler only happen from the run loop.
How important is it to make this callback?

[sky, 2013/11/22 19:39:44]

As discussed I made this DCHECK that there isn't an existing handler.

 }
 
 void MessagePumpMojo::RemoveHandler(MojoHandle handle) {
   handlers_.erase(handle);
-  SignalControlPipe();
 }
 
 void MessagePumpMojo::Run(Delegate* delegate) {
@@ -115,15 +119,7 @@ void MessagePumpMojo::ScheduleDelayedWork(
 }
 
 void MessagePumpMojo::DoInternalWork(bool block) {
-  MojoDeadline deadline;
-  if (block && !run_state_->delayed_work_time.is_null()) {
-    const base::TimeDelta delta = run_state_->delayed_work_time -
-        base::TimeTicks::Now();
-    deadline = std::max(static_cast<MojoDeadline>(0),
-                        static_cast<MojoDeadline>(delta.InMicroseconds()));
-  } else {
-    deadline = 0;
-  }
+  const MojoDeadline deadline = block ? GetDeadlineForWait() : 0;
   const WaitState wait_state = GetWaitState();
   const MojoResult result = MojoWaitMany(
       &wait_state.handles.front(),
@@ -152,6 +148,19 @@ void MessagePumpMojo::DoInternalWork(bool block) {
         NOTREACHED();
     }
   }
+
+  // Notify and remove any handlers whose time has expired.
+  const base::TimeTicks now(base::TimeTicks::Now());
+  for (HandleToHandler::iterator i = handlers_.begin(); i != handlers_.end();) {
+    if (!i->second.deadline.is_null() && i->second.deadline < now) {
+      const MojoHandle handle = i->first;
+      const Handler handler(i->second);
+      handlers_.erase(i++);
+      handler.handler->OnHandleError(handle, MOJO_RESULT_DEADLINE_EXCEEDED);

[darin (slow to review), 2013/11/21 18:14:52]

OnHandleError can call AddHandler or RemoveHandler which could mess up your
iteration here. You probably want to build a separate list of handlers to
notify, and then notify them all as a separate loop.

[sky, 2013/11/22 19:39:44]

Done.

+    } else {
+      ++i;
+    }
+  }
 }
 
 void MessagePumpMojo::RemoveFirstInvalidHandle(const WaitState& wait_state) {
@@ -161,6 +170,8 @@ void MessagePumpMojo::RemoveFirstInvalidHandle(const WaitState& wait_state) {
         MojoWait(wait_state.handles[i], wait_state.wait_flags[i], 0);
     if (result == MOJO_RESULT_INVALID_ARGUMENT ||
         result == MOJO_RESULT_FAILED_PRECONDITION) {
+      // Remove the handle first, this way if OnHandleError() tries to remove
+      // the handle too our iterator isn't invalidated.

[darin (slow to review), 2013/11/21 18:14:52]

nit: too -> to

[sky, 2013/11/22 19:39:44]

Done.

       DCHECK(handlers_.find(wait_state.handles[i]) != handlers_.end());
       MessagePumpMojoHandler* handler =
           handlers_[wait_state.handles[i]].handler;
@@ -195,5 +206,18 @@ MessagePumpMojo::WaitState MessagePumpMojo::GetWaitState() const {
   return wait_state;
 }
 
+MojoDeadline MessagePumpMojo::GetDeadlineForWait() const {
+  base::TimeTicks min_time = run_state_->delayed_work_time;
+  for (HandleToHandler::const_iterator i = handlers_.begin();
+       i != handlers_.end(); ++i) {
+    if (min_time.is_null() && i->second.deadline < min_time)
+      min_time = i->second.deadline;
+  }
+  return min_time.is_null() ? MOJO_DEADLINE_INDEFINITE :
+      std::max(static_cast<MojoDeadline>(0),
+               static_cast<MojoDeadline>(
+                   (min_time - base::TimeTicks::Now()).InMicroseconds()));
+}
+
 }  // namespace common
 }  // namespace mojo