Adds some CHECKs to MessagePumpMojo

The thread watcher appears to be kicking on when we're trying to
remove a handle. Not sure why. This converts some DCHECKs to CHECKs
and fixes handling of the deadline. I suspect none of this will help,
but it's worth a shot.

BUG=399769
TEST=none
R=darin@chromium.org

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

Cr-Commit-Position: refs/heads/master@{#288949}
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@288949 0039d316-1c4b-4281-b951-d872f2087c98

base/message_loop/message_loop.cc

@@ -92,6 +92,8 @@ MessageLoop::MessagePumpFactory* message_pump_for_ui_factory_ = NULL;
 // time for every task that is added to the MessageLoop incoming queue.
 bool AlwaysNotifyPump(MessageLoop::Type type) {
 #if defined(OS_ANDROID)
+  // The Android UI message loop needs to get notified each time a task is added
+  // to the incoming queue.
   return type == MessageLoop::TYPE_UI || type == MessageLoop::TYPE_JAVA;
 #else
   return false;
@@ -528,8 +530,6 @@ void MessageLoop::ReloadWorkQueue() {
 }
 
 void MessageLoop::ScheduleWork(bool was_empty) {
-  // The Android UI message loop needs to get notified each time
-  // a task is added to the incoming queue.
   if (was_empty || AlwaysNotifyPump(type_))
     pump_->ScheduleWork();
 }

mojo/common/data_pipe_utils.cc

@@ -11,7 +11,6 @@
 #include "base/files/scoped_file.h"
 #include "base/message_loop/message_loop.h"
 #include "base/task_runner_util.h"
-#include "mojo/common/handle_watcher.h"
 
 namespace mojo {
 namespace common {

mojo/common/message_pump_mojo.cc

@@ -15,6 +15,19 @@
 
 namespace mojo {
 namespace common {
+namespace {
+
+MojoDeadline TimeTicksToMojoDeadline(base::TimeTicks time_ticks,
+                                     base::TimeTicks now) {
+  // The is_null() check matches that of HandleWatcher.
+  if (time_ticks.is_null())
+    return MOJO_DEADLINE_INDEFINITE;
+  const int64_t delta = (time_ticks - now).InMicroseconds();
+  return delta < 0 ? static_cast<MojoDeadline>(0) :
+                     static_cast<MojoDeadline>(delta);
+}
+
+}  // namespace
 
 // State needed for one iteration of WaitMany. The first handle and flags
 // corresponds to that of the control pipe.
@@ -52,10 +65,10 @@ void MessagePumpMojo::AddHandler(MessagePumpMojoHandler* handler,
                                  const Handle& handle,
                                  MojoHandleSignals wait_signals,
                                  base::TimeTicks deadline) {
-  DCHECK(handler);
+  CHECK(handler);
   DCHECK(handle.is_valid());
   // Assume it's an error if someone tries to reregister an existing handle.
-  DCHECK_EQ(0u, handlers_.count(handle));
+  CHECK_EQ(0u, handlers_.count(handle));
   Handler handler_data;
   handler_data.handler = handler;
   handler_data.wait_signals = wait_signals;
@@ -186,7 +199,7 @@ void MessagePumpMojo::DoInternalWork(const RunState& run_state, bool block) {
 
 void MessagePumpMojo::RemoveFirstInvalidHandle(const WaitState& wait_state) {
   // TODO(sky): deal with control pipe going bad.
-  for (size_t i = 1; i < wait_state.handles.size(); ++i) {
+  for (size_t i = 0; i < wait_state.handles.size(); ++i) {
     const MojoResult result =
         Wait(wait_state.handles[i], wait_state.wait_signals[i], 0);
     if (result == MOJO_RESULT_INVALID_ARGUMENT) {
@@ -196,9 +209,11 @@ void MessagePumpMojo::RemoveFirstInvalidHandle(const WaitState& wait_state) {
       CHECK(false);
     } else if (result == MOJO_RESULT_FAILED_PRECONDITION ||
                result == MOJO_RESULT_CANCELLED) {
+      CHECK_NE(i, 0u);  // Indicates the control pipe went bad.
+
       // Remove the handle first, this way if OnHandleError() tries to remove
       // the handle our iterator isn't invalidated.
-      DCHECK(handlers_.find(wait_state.handles[i]) != handlers_.end());
+      CHECK(handlers_.find(wait_state.handles[i]) != handlers_.end());
       MessagePumpMojoHandler* handler =
           handlers_[wait_state.handles[i]].handler;
       handlers_.erase(wait_state.handles[i]);
@@ -209,9 +224,12 @@ void MessagePumpMojo::RemoveFirstInvalidHandle(const WaitState& wait_state) {
 }
 
 void MessagePumpMojo::SignalControlPipe(const RunState& run_state) {
-  // TODO(sky): deal with error?
-  WriteMessageRaw(run_state.write_handle.get(), NULL, 0, NULL, 0,
-                  MOJO_WRITE_MESSAGE_FLAG_NONE);
+  const MojoResult result =
+      WriteMessageRaw(run_state.write_handle.get(), NULL, 0, NULL, 0,
+                      MOJO_WRITE_MESSAGE_FLAG_NONE);
+  // If we can't write we likely won't wake up the thread and there is a strong
+  // chance we'll deadlock.
+  CHECK_EQ(MOJO_RESULT_OK, result);
 }
 
 MessagePumpMojo::WaitState MessagePumpMojo::GetWaitState(
@@ -230,16 +248,16 @@ MessagePumpMojo::WaitState MessagePumpMojo::GetWaitState(
 
 MojoDeadline MessagePumpMojo::GetDeadlineForWait(
     const RunState& run_state) const {
-  base::TimeTicks min_time = run_state.delayed_work_time;
+  const base::TimeTicks now(internal::NowTicks());
+  MojoDeadline deadline = static_cast<MojoDeadline>(0);
+  if (!run_state.delayed_work_time.is_null())
+    deadline = TimeTicksToMojoDeadline(run_state.delayed_work_time, now);
   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;
+    deadline = std::min(
+        TimeTicksToMojoDeadline(i->second.deadline, now), deadline);
   }
-  return min_time.is_null() ? MOJO_DEADLINE_INDEFINITE :
-      std::max(static_cast<MojoDeadline>(0),
-               static_cast<MojoDeadline>(
-                   (min_time - internal::NowTicks()).InMicroseconds()));
+  return deadline;
 }
 
 }  // namespace common

mojo/common/message_pump_mojo.h

@@ -31,6 +31,7 @@ class MOJO_COMMON_EXPORT MessagePumpMojo : public base::MessagePump {
 
   // Registers a MessagePumpMojoHandler for the specified handle. Only one
   // handler can be registered for a specified handle.
+  // NOTE: a value of 0 for |deadline| indicates an indefinite timeout.
   void AddHandler(MessagePumpMojoHandler* handler,
                   const Handle& handle,
                   MojoHandleSignals wait_signals,