Add extra CHECKs and DLOGs, and missing MX_PORT_OPT_V2.

This reverts https://chromium-review.googlesource.com/562660.

Our current Fuchsia SDK has a kernel version which supports both the
old ports API, and the new "V2" API, so we must pass the MX_PORT_OPT_V2
option, to create the right version of port. We will un-do this change,
and possibly migrate to using the ulib/port APIs, in future.

This CL also fixes StopWatchingFileDescriptor() to cope with being
called more than once, to match the API/interface expectations, adds
some additional Debug logging for API failures, and adds CHECKs on
calls which we cannot cope with seeing fail.

Bug: 740305

Change-Id: I2579039fbd747e528da3c59cba9f2e1d07bf7326
Reviewed-on: https://chromium-review.googlesource.com/564739
Commit-Queue: Nico Weber <thakis@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#485787}

base/message_loop/message_pump_fuchsia.cc

@@ -16,7 +16,8 @@ MessagePumpFuchsia::FileDescriptorWatcher::FileDescriptorWatcher(
 }
 
 MessagePumpFuchsia::FileDescriptorWatcher::~FileDescriptorWatcher() {
-  StopWatchingFileDescriptor();
+  if (!StopWatchingFileDescriptor())
+    NOTREACHED();
   if (io_)
     __mxio_release(io_);
   if (was_destroyed_) {
@@ -26,13 +27,21 @@ MessagePumpFuchsia::FileDescriptorWatcher::~FileDescriptorWatcher() {
 }
 
 bool MessagePumpFuchsia::FileDescriptorWatcher::StopWatchingFileDescriptor() {
+  if (handle_ == MX_HANDLE_INVALID)
+    return true;
   uint64_t this_as_key =
       static_cast<uint64_t>(reinterpret_cast<uintptr_t>(this));
-  return mx_port_cancel(port_, handle_, this_as_key) == MX_OK;
+  int result = mx_port_cancel(port_, handle_, this_as_key);
+  DLOG_IF(ERROR, result != MX_OK)
+      << "mx_port_cancel(handle=" << handle_ << ") failed: result=" << result;
+  handle_ = MX_HANDLE_INVALID;
+  return result == MX_OK;
 }
 
 MessagePumpFuchsia::MessagePumpFuchsia() : keep_running_(true) {
-  CHECK(mx_port_create(0, &port_) == MX_OK);
+  // TODO(wez): Remove MX_PORT_OPT_V2 once the SDK is rolled, or migrate
+  // this implementation use ulib/port helpers.
+  CHECK(mx_port_create(MX_PORT_OPT_V2, &port_) == MX_OK);
 }
 
 MessagePumpFuchsia::~MessagePumpFuchsia() {
@@ -74,8 +83,10 @@ bool MessagePumpFuchsia::WatchFileDescriptor(int fd,
   controller->desired_events_ = events;
 
   controller->io_ = __mxio_fd_to_io(fd);
-  if (!controller->io_)
+  if (!controller->io_) {
+    DLOG(ERROR) << "Failed to get IO for FD";
     return false;
+  }
 
   controller->fd_ = fd;
   controller->persistent_ = persistent;
@@ -86,15 +97,18 @@ bool MessagePumpFuchsia::WatchFileDescriptor(int fd,
 bool MessagePumpFuchsia::FileDescriptorWatcher::WaitBegin() {
   uint32_t signals = 0u;
   __mxio_wait_begin(io_, desired_events_, &handle_, &signals);
-  if (handle_ == MX_HANDLE_INVALID)
+  if (handle_ == MX_HANDLE_INVALID) {
+    DLOG(ERROR) << "mxio_wait_begin failed";
     return false;
+  }
 
   uint64_t this_as_key =
       static_cast<uint64_t>(reinterpret_cast<uintptr_t>(this));
   mx_status_t status = mx_object_wait_async(handle_, port_, this_as_key,
                                             signals, MX_WAIT_ASYNC_ONCE);
   if (status != MX_OK) {
-    DLOG(ERROR) << "mx_object_wait_async failed: " << status;
+    DLOG(ERROR) << "mx_object_wait_async failed: " << status
+                << " (port=" << port_ << ")";
     return false;
   }
   return true;
@@ -164,21 +178,14 @@ void MessagePumpFuchsia::Run(Delegate* delegate) {
       // pointer is now invalid.
       bool controller_was_destroyed = false;
       controller->was_destroyed_ = &controller_was_destroyed;
-      if ((events & (MXIO_EVT_READABLE | MXIO_EVT_WRITABLE)) ==
-          (MXIO_EVT_READABLE | MXIO_EVT_WRITABLE)) {
-        controller->watcher_->OnFileCanWriteWithoutBlocking(controller->fd_);
-        if (!controller_was_destroyed)
-          controller->watcher_->OnFileCanReadWithoutBlocking(controller->fd_);
-      } else if (events & MXIO_EVT_WRITABLE) {
+      if (events & MXIO_EVT_WRITABLE)
         controller->watcher_->OnFileCanWriteWithoutBlocking(controller->fd_);
-      } else if (events & MXIO_EVT_READABLE) {
+      if (!controller_was_destroyed && (events & MXIO_EVT_READABLE))
         controller->watcher_->OnFileCanReadWithoutBlocking(controller->fd_);
-      }
       if (!controller_was_destroyed) {
         controller->was_destroyed_ = nullptr;
-      }
-      if (!controller_was_destroyed && controller->persistent_) {
-        controller->WaitBegin();
+        if (controller->persistent_)
+          controller->WaitBegin();
       }
     } else {
       // Wakeup caused by ScheduleWork().
@@ -199,9 +206,8 @@ void MessagePumpFuchsia::ScheduleWork() {
   mx_port_packet_t packet = {};
   packet.type = MX_PKT_TYPE_USER;
   mx_status_t status = mx_port_queue(port_, &packet, 0);
-  if (status != MX_OK) {
-    DLOG(ERROR) << "mx_port_queue failed: " << status;
-  }
+  DLOG_IF(ERROR, status != MX_OK)
+      << "mx_port_queue failed: " << status << " (port=" << port_ << ")";
 }
 
 void MessagePumpFuchsia::ScheduleDelayedWork(