Adds Mojo IDL.

third_party/WebKit/Source/core/mojo/MojoWatcher.cpp

+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "core/mojo/MojoWatcher.h"
+
+#include "bindings/core/v8/MojoWatchCallback.h"
+#include "bindings/core/v8/ScriptState.h"
+#include "core/dom/ExecutionContext.h"
+#include "core/dom/ExecutionContextTask.h"
+#include "core/dom/TaskRunnerHelper.h"
+#include "core/mojo/MojoWatchOptions.h"
+#include "platform/WebTaskRunner.h"
+
+namespace blink {
+
+MojoWatcher* MojoWatcher::create(mojo::Handle handle,

[jbroman, 2017/01/11 23:00:00]

What if !handle.is_valid() to begin with? (i.e. we were passed an invalid
handle).

It looks like this can happen if I close a handle, and then call watch on it.
(Aside: Ordinarily using something in an invalid state would throw an exception;
I haven't thought through the merits of exposing MojoResult vs throwing
exceptions for this case. It sounds like not all non-OK results are
exception-worthy, so maybe sticking with MojoResult is fine.)

Consider this case:

var handle; // a MojoHandle of some kind
handle.close();
handle.watch({ readable: true }, function(result) {
  console.log(result);
});

As written now, whether or not we call the callback with MOJO_INVALID_ARGUMENT
depends on whether we run garbage-collection between when watch is called and
when the callback was scheduled below. Since the watcher has no pending
activity, the context won't keep it alive. Since there's no reference from
script, that won't keep it alive. And since you're using wrapWeakPersistent,
that also won't keep it alive.

If we don't handle this case another way, I think the postTask should use
wrapPersistent(this), to ensure the callback is consistently delivered. (In the
case where the callback is called from Mojo rather than from here, I'm convinced
the difference isn't observable.) An alternative might be to invoke the callback
synchronously, but I assume the choice to be async is intentional.

[alokp, 2017/01/12 01:20:26]

Great catch.

I have TODOs about using exception in a couple places including here (see line
34). The other ones are in MojoCreateMessagePipeResult.idl and
MojoReadMessageResult.idl. Do you recommend removing those TODOs and sticking
with MojoResult for now? If so, using wrapPersistent here would be simplest -
Done.

+                                 const MojoWatchOptions& options,
+                                 MojoWatchCallback* callback,
+                                 ExecutionContext* context) {
+  MojoWatcher* watcher = new MojoWatcher(handle, callback, context);
+
+  MojoHandleSignals signals = MOJO_HANDLE_SIGNAL_NONE;
+  if (options.readable())
+    signals |= MOJO_HANDLE_SIGNAL_READABLE;
+  if (options.writable())
+    signals |= MOJO_HANDLE_SIGNAL_WRITABLE;
+  if (options.peerClosed())
+    signals |= MOJO_HANDLE_SIGNAL_PEER_CLOSED;
+
+  MojoResult result =
+      MojoWatch(handle.value(), signals, &MojoWatcher::onHandleReady,
+                reinterpret_cast<uintptr_t>(watcher));
+  // TODO(alokp): Consider raising an exception.
+  // Current clients expect to recieve the initial error returned by MojoWatch
+  // via watch callback.
+  if (result != MOJO_RESULT_OK) {
+    watcher->m_taskRunner->postTask(
+        BLINK_FROM_HERE, WTF::bind(&MojoWatcher::runReadyCallback,
+                                   wrapWeakPersistent(watcher), result));
+  }
+  return watcher;
+}
+
+MojoWatcher::MojoWatcher(mojo::Handle handle,
+                         MojoWatchCallback* callback,
+                         ExecutionContext* context)
+    : ContextLifecycleObserver(context),
+      m_handle(handle),
+      m_callback(this, callback),
+      m_taskRunner(TaskRunnerHelper::get(TaskType::MiscPlatformAPI, context)) {}
+
+MojoWatcher::~MojoWatcher() {
+  DCHECK(!m_handle.is_valid());
+}
+
+MojoResult MojoWatcher::cancel() {
+  MojoResult result = MOJO_RESULT_OK;
+  if (m_handle.is_valid()) {

[jbroman, 2017/01/11 23:00:00]

nit: I mildly prefer early return, but I don't feel strongly.

if (!m_handle.is_valid())
  return MOJO_RESULT_OK;

MojoResult result = MojoCancelWatch(...);
m_handle = mojo::Handle();
return result;

[alokp, 2017/01/12 01:20:26]

Done.

+    result =
+        MojoCancelWatch(m_handle.value(), reinterpret_cast<uintptr_t>(this));
+    m_handle = mojo::Handle();
+  }
+  return result;
+}
+
+DEFINE_TRACE(MojoWatcher) {
+  ContextLifecycleObserver::trace(visitor);
+}

[jbroman, 2017/01/11 23:00:01]

m_callback must be traced here as well

[alokp, 2017/01/12 01:20:26]

Done.

+
+DEFINE_TRACE_WRAPPERS(MojoWatcher) {
+  visitor->traceWrappers(m_callback);
+}
+
+bool MojoWatcher::hasPendingActivity() const {
+  return m_handle.is_valid() && getExecutionContext();

[jbroman, 2017/01/11 23:00:01]

nit: the getExecutionContext() check is implied and happens elsewhere

https://cs.chromium.org/chromium/src/third_party/WebKit/Source/bindings/core/...

[alokp, 2017/01/12 01:20:26]

Done.

+}
+
+void MojoWatcher::contextDestroyed() {
+  cancel();
+}
+
+void MojoWatcher::onHandleReady(uintptr_t context,
+                                MojoResult result,
+                                MojoHandleSignalsState,
+                                MojoWatchNotificationFlags) {
+  // It is safe to assume the MojoWatcher still exists because this
+  // callback will never be run after MojoWatcher destructor,
+  // which cancels the watch.
+  MojoWatcher* watcher = reinterpret_cast<MojoWatcher*>(context);
+  watcher->m_taskRunner->postTask(
+      BLINK_FROM_HERE,
+      crossThreadBind(&MojoWatcher::runReadyCallback,
+                      wrapCrossThreadWeakPersistent(watcher), result));
+}
+
+void MojoWatcher::runReadyCallback(MojoResult result) {
+  if (!m_handle.is_valid())
+    return;
+
+  if (result != MOJO_RESULT_OK)
+    cancel();

[jbroman, 2017/01/11 23:00:01]

Is this the right thing to do here? mojo/public/c/system/functions.h lists
several cases where it would be illegal to cancel a watch, like if MojoWatch
returns MOJO_RESULT_CANCELLED (or its callback receives that value).

[alokp, 2017/01/12 01:20:26]

Fixed.

+
+  m_callback->call(this, result);
+}
+
+}  // namespace blink