Revert of Mojo EDK: Introduce MojoQueryHandleSignalsState API (patchset

chromium#9 id:160001 of https://codereview.chromium.org/2741033003/ )

Reason for revert:
This CL seems to break mojo_system_unittests on multiple bots:

https://build.chromium.org/p/chromium.linux/builders/Linux%20Tests/builds/53232
https://build.chromium.org/p/chromium.win/builders/Win7%20Tests%20%281%29/builds/64801
https://build.chromium.org/p/chromium.mac/builders/Mac10.11%20Tests/builds/9606

Here are samples of the failing tests. Most of them are failed by time out:
MessagePipeTest.DiscardMode
WatcherTest.WatchDataPipeConsumerReadable
WatcherTest.WatchMessagePipeReadable
MessagePipeTest.Basic
DataPipeTest.PeerClosedProducerWaiting

Original issue's description:
> Mojo EDK: Introduce MojoQueryHandleSignalsState API
>
> The only reliable way to inquire about handle signals now
> is to MojoWait (for e.g. 0 deadline). As a precursor to
> removing the wait APIs in favor of watchers, we need to
> retain the ability to efficiently query a handle's signals
> state.
>
> Rather than trying to retrofit the watcher APIs to support
> this use case in similar fashion to the wait APIs, this adds
> an API explicitly for the purpose of querying signals state.
>
> Adds a corresponding method to the C++ mojo::Handle
> and moves the EDK's internal HandleSignalsState helper class
> to mojo/public/cpp/system, adding some convenient accessors.
>
> Also introduces the API to the JS and Java libraries, and
> replaces any 0-deadline waits in those languages with usage of
> this new API.
>
> Because waitMany is not used in these languages (except for
> tests which test waitMany...) it has been removed. wait() is
> unused in Java after this change, so it has also been removed.
>
> Finally, this moves several tests away from calling MojoWait
> directly, instead using a simplified Watcher-based wait
> implementation in MojoTestBase.
>
> BUG=700171
> TBR=jam@chromium.org
>
> Review-Url: https://codereview.chromium.org/2741033003
> Cr-Commit-Position: refs/heads/master@{#457315}
> Committed: https://chromium.googlesource.com/chromium/src/+/853496a78ae997c2d8b80f3cd8fabf9423fb3361

TBR=yzshen@chromium.org,rockot@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=700171

Review-Url: https://codereview.chromium.org/2750273002
Cr-Commit-Position: refs/heads/master@{#457342}

content/child/web_data_consumer_handle_impl.cc

@@ -62,13 +62,20 @@ Result WebDataConsumerHandleImpl::ReaderImpl::read(void* data,
     // Even if there is unread data available, mojo::ReadDataRaw() returns
     // FAILED_PRECONDITION when |size| is 0 and the producer handle was closed.
     // But in this case, WebDataConsumerHandle::Reader::read() must return Ok.
-    // So we query the signals state directly.
-    mojo::HandleSignalsState state = context_->handle()->QuerySignalsState();
-    if (state.readable())
-      return Ok;
-    if (state.never_readable())
-      return Done;
-    return ShouldWait;
+    // So we use mojo::Wait() with 0 deadline to check whether readable or not.
+    MojoResult wait_result = mojo::Wait(
+        context_->handle().get(), MOJO_HANDLE_SIGNAL_READABLE, 0, nullptr);
+    switch (wait_result) {
+      case MOJO_RESULT_OK:
+        return Ok;
+      case MOJO_RESULT_FAILED_PRECONDITION:
+        return Done;
+      case MOJO_RESULT_DEADLINE_EXCEEDED:
+        return ShouldWait;
+      default:
+        NOTREACHED();
+        return UnexpectedError;
+    }
   }
 
   uint32_t size_to_pass = size;

mojo/android/javatests/src/org/chromium/mojo/HandleMock.java

@@ -5,7 +5,7 @@
 package org.chromium.mojo;
 
 import org.chromium.mojo.system.Core;
-import org.chromium.mojo.system.Core.HandleSignalsState;
+import org.chromium.mojo.system.Core.WaitResult;
 import org.chromium.mojo.system.DataPipe;
 import org.chromium.mojo.system.DataPipe.ConsumerHandle;
 import org.chromium.mojo.system.DataPipe.ProducerHandle;
@@ -35,11 +35,14 @@ public void close() {
     }
 
     /**
-     * @see Handle#querySignalsState()
+     * @see Handle#wait(Core.HandleSignals, long)
      */
     @Override
-    public HandleSignalsState querySignalsState() {
-        return null;
+    public WaitResult wait(Core.HandleSignals signals, long deadline) {
+        // Do nothing.
+        WaitResult result = new WaitResult();
+        result.setMojoResult(MojoResult.OK);
+        return result;
     }
 
     /**

mojo/android/javatests/src/org/chromium/mojo/bindings/RouterTest.java

@@ -12,6 +12,7 @@
 import org.chromium.mojo.bindings.BindingsTestUtils.RecordingMessageReceiverWithResponder;
 import org.chromium.mojo.system.Core;
 import org.chromium.mojo.system.Core.HandleSignals;
+import org.chromium.mojo.system.Core.WaitResult;
 import org.chromium.mojo.system.Handle;
 import org.chromium.mojo.system.MessagePipeHandle;
 import org.chromium.mojo.system.MojoResult;
@@ -226,6 +227,8 @@ public void testDroppingReceiverWithoutUsingIt() {
 
         // Confirm that the pipe was closed on the Router side.
         HandleSignals closedFlag = HandleSignals.none().setPeerClosed(true);
-        assertEquals(closedFlag, mHandle.querySignalsState().getSatisfiedSignals());
+        WaitResult result = mHandle.wait(closedFlag, 0);
+        assertEquals(MojoResult.OK, result.getMojoResult());
+        assertEquals(closedFlag, result.getHandleSignalsState().getSatisfiedSignals());
     }
 }

mojo/android/javatests/src/org/chromium/mojo/system/impl/CoreImplTest.java

@@ -9,6 +9,9 @@
 import org.chromium.mojo.MojoTestCase;
 import org.chromium.mojo.system.Core;
 import org.chromium.mojo.system.Core.HandleSignals;
+import org.chromium.mojo.system.Core.HandleSignalsState;
+import org.chromium.mojo.system.Core.WaitManyResult;
+import org.chromium.mojo.system.Core.WaitResult;
 import org.chromium.mojo.system.DataPipe;
 import org.chromium.mojo.system.Handle;
 import org.chromium.mojo.system.InvalidHandle;
@@ -27,6 +30,7 @@
 import java.util.Random;
 import java.util.concurrent.Executors;
 import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.TimeUnit;
 
 /**
  * Testing the core API.
@@ -72,6 +76,22 @@ private void addHandlePairToClose(Pair<? extends Handle, ? extends Handle> handl
         mHandlesToClose.add(handles.second);
     }
 
+    /**
+     * Runnable that will close the given handle.
+     */
+    private static class CloseHandle implements Runnable {
+        private Handle mHandle;
+
+        CloseHandle(Handle handle) {
+            mHandle = handle;
+        }
+
+        @Override
+        public void run() {
+            mHandle.close();
+        }
+    }
+
     private static void checkSendingMessage(MessagePipeHandle in, MessagePipeHandle out) {
         Random random = new Random();
 
@@ -163,6 +183,46 @@ private static void checkSharing(SharedBufferHandle in, SharedBufferHandle out)
         out.unmap(buffer2);
     }
 
+    /**
+     * Testing {@link Core#waitMany(List, long)}.
+     */
+    @SmallTest
+    public void testWaitMany() {
+        Core core = CoreImpl.getInstance();
+        Pair<MessagePipeHandle, MessagePipeHandle> handles = core.createMessagePipe(null);
+        addHandlePairToClose(handles);
+
+        // Test waiting on handles of a newly created message pipe - each should be writable, but
+        // not readable.
+        List<Pair<Handle, Core.HandleSignals>> handlesToWaitOn =
+                new ArrayList<Pair<Handle, Core.HandleSignals>>();
+        handlesToWaitOn.add(
+                new Pair<Handle, Core.HandleSignals>(handles.second, Core.HandleSignals.READABLE));
+        handlesToWaitOn.add(
+                new Pair<Handle, Core.HandleSignals>(handles.first, Core.HandleSignals.WRITABLE));
+        WaitManyResult result = core.waitMany(handlesToWaitOn, 0);
+        assertEquals(MojoResult.OK, result.getMojoResult());
+        assertEquals(1, result.getHandleIndex());
+        for (HandleSignalsState state : result.getSignalStates()) {
+            assertEquals(HandleSignals.WRITABLE, state.getSatisfiedSignals());
+            assertEquals(ALL_SIGNALS, state.getSatisfiableSignals());
+        }
+
+        // Same test, but swap the handles around.
+        handlesToWaitOn.clear();
+        handlesToWaitOn.add(
+                new Pair<Handle, Core.HandleSignals>(handles.first, Core.HandleSignals.WRITABLE));
+        handlesToWaitOn.add(
+                new Pair<Handle, Core.HandleSignals>(handles.second, Core.HandleSignals.READABLE));
+        result = core.waitMany(handlesToWaitOn, 0);
+        assertEquals(MojoResult.OK, result.getMojoResult());
+        assertEquals(0, result.getHandleIndex());
+        for (HandleSignalsState state : result.getSignalStates()) {
+            assertEquals(HandleSignals.WRITABLE, state.getSatisfiedSignals());
+            assertEquals(ALL_SIGNALS, state.getSatisfiableSignals());
+        }
+    }
+
     /**
      * Testing that Core can be retrieved from a handle.
      */
@@ -214,14 +274,53 @@ public void testMessagePipeEmpty() {
         Core core = CoreImpl.getInstance();
         Pair<MessagePipeHandle, MessagePipeHandle> handles = core.createMessagePipe(null);
         addHandlePairToClose(handles);
+        // Test waiting on handles of a newly created message pipe.
+        WaitResult waitResult = handles.first.wait(
+                Core.HandleSignals.none().setReadable(true).setWritable(true), 0);
+        assertEquals(MojoResult.OK, waitResult.getMojoResult());
+        assertEquals(
+                HandleSignals.WRITABLE, waitResult.getHandleSignalsState().getSatisfiedSignals());
+        assertEquals(ALL_SIGNALS, waitResult.getHandleSignalsState().getSatisfiableSignals());
+
+        waitResult = handles.first.wait(Core.HandleSignals.WRITABLE, 0);
+        assertEquals(MojoResult.OK, waitResult.getMojoResult());
+        assertEquals(
+                HandleSignals.WRITABLE, waitResult.getHandleSignalsState().getSatisfiedSignals());
+        assertEquals(ALL_SIGNALS, waitResult.getHandleSignalsState().getSatisfiableSignals());
+
+        waitResult = handles.first.wait(Core.HandleSignals.READABLE, 0);
+        assertEquals(MojoResult.DEADLINE_EXCEEDED, waitResult.getMojoResult());
+        assertEquals(
+                HandleSignals.WRITABLE, waitResult.getHandleSignalsState().getSatisfiedSignals());
+        assertEquals(ALL_SIGNALS, waitResult.getHandleSignalsState().getSatisfiableSignals());
 
         // Testing read on an empty pipe.
         ResultAnd<MessagePipeHandle.ReadMessageResult> readResult =
                 handles.first.readMessage(null, 0, MessagePipeHandle.ReadFlags.NONE);
         assertEquals(MojoResult.SHOULD_WAIT, readResult.getMojoResult());
 
-        handles.first.close();
-        handles.second.close();
+        // Closing a pipe while waiting.
+        WORKER.schedule(new CloseHandle(handles.first), 10, TimeUnit.MILLISECONDS);
+        waitResult = handles.first.wait(Core.HandleSignals.READABLE, 1000000L);
+        assertEquals(MojoResult.CANCELLED, waitResult.getMojoResult());
+        assertEquals(
+                HandleSignals.none(), waitResult.getHandleSignalsState().getSatisfiedSignals());
+        assertEquals(
+                HandleSignals.none(), waitResult.getHandleSignalsState().getSatisfiableSignals());
+
+        handles = core.createMessagePipe(null);
+        addHandlePairToClose(handles);
+
+        // Closing the other pipe while waiting.
+        WORKER.schedule(new CloseHandle(handles.first), 10, TimeUnit.MILLISECONDS);
+        waitResult = handles.second.wait(Core.HandleSignals.READABLE, 1000000L);
+        assertEquals(MojoResult.FAILED_PRECONDITION, waitResult.getMojoResult());
+
+        // Waiting on a closed pipe.
+        waitResult = handles.second.wait(Core.HandleSignals.READABLE, 0);
+        assertEquals(MojoResult.FAILED_PRECONDITION, waitResult.getMojoResult());
+        waitResult = handles.second.wait(Core.HandleSignals.WRITABLE, 0);
+        assertEquals(MojoResult.FAILED_PRECONDITION, waitResult.getMojoResult());
     }
 
     /**
@@ -441,6 +540,29 @@ public void testInvalidHandle() {
         Core core = CoreImpl.getInstance();
         Handle handle = InvalidHandle.INSTANCE;
 
+        // Checking wait.
+        boolean exception = false;
+        try {
+            core.wait(handle, Core.HandleSignals.WRITABLE, 0);
+        } catch (MojoException e) {
+            assertEquals(MojoResult.INVALID_ARGUMENT, e.getMojoResult());
+            exception = true;
+        }
+        assertTrue(exception);
+
+        // Checking waitMany.
+        exception = false;
+        try {
+            List<Pair<Handle, Core.HandleSignals>> handles =
+                    new ArrayList<Pair<Handle, Core.HandleSignals>>();
+            handles.add(Pair.create(handle, Core.HandleSignals.WRITABLE));
+            core.waitMany(handles, 0);
+        } catch (MojoException e) {
+            assertEquals(MojoResult.INVALID_ARGUMENT, e.getMojoResult());
+            exception = true;
+        }
+        assertTrue(exception);
+
         // Checking sending an invalid handle.
         // Until the behavior is changed on the C++ side, handle gracefully 2 different use case:
         // - Receive a INVALID_ARGUMENT exception

mojo/android/system/core_impl.cc

@@ -26,6 +26,41 @@ static jlong GetTimeTicksNow(JNIEnv* env,
   return MojoGetTimeTicksNow();
 }
 
+static jint WaitMany(JNIEnv* env,
+                     const JavaParamRef<jobject>& jcaller,
+                     const JavaParamRef<jobject>& buffer,
+                     jlong deadline) {
+  // |buffer| contains, in this order
+  // input: The array of N handles (MojoHandle, 4 bytes each)
+  // input: The array of N signals (MojoHandleSignals, 4 bytes each)
+  // space for output: The array of N handle states (MojoHandleSignalsState, 8
+  //                   bytes each)
+  // space for output: The result index (uint32_t, 4 bytes)
+  uint8_t* buffer_start =
+      static_cast<uint8_t*>(env->GetDirectBufferAddress(buffer));
+  DCHECK(buffer_start);
+  DCHECK_EQ(reinterpret_cast<uintptr_t>(buffer_start) % 8, 0u);
+  // Each handle of the input array contributes 4 (MojoHandle) + 4
+  // (MojoHandleSignals) + 8 (MojoHandleSignalsState) = 16 bytes to the size of
+  // the buffer.
+  const size_t size_per_handle = 16;
+  const size_t buffer_size = env->GetDirectBufferCapacity(buffer);
+  DCHECK_EQ((buffer_size - 4) % size_per_handle, 0u);
+
+  const size_t nb_handles = (buffer_size - 4) / size_per_handle;
+  const MojoHandle* handle_start =
+      reinterpret_cast<const MojoHandle*>(buffer_start);
+  const MojoHandleSignals* signals_start =
+      reinterpret_cast<const MojoHandleSignals*>(buffer_start + 4 * nb_handles);
+  MojoHandleSignalsState* states_start =
+      reinterpret_cast<MojoHandleSignalsState*>(buffer_start + 8 * nb_handles);
+  uint32_t* result_index =
+      reinterpret_cast<uint32_t*>(buffer_start + 16 * nb_handles);
+  *result_index = static_cast<uint32_t>(-1);
+  return MojoWaitMany(handle_start, signals_start, nb_handles, deadline,
+                      result_index, states_start);
+}
+
 static ScopedJavaLocalRef<jobject> CreateMessagePipe(
     JNIEnv* env,
     const JavaParamRef<jobject>& jcaller,
@@ -92,16 +127,21 @@ static jint Close(JNIEnv* env,
   return MojoClose(mojo_handle);
 }
 
-static jint QueryHandleSignalsState(JNIEnv* env,
-                                    const JavaParamRef<jobject>& jcaller,
-                                    jint mojo_handle,
-                                    const JavaParamRef<jobject>& buffer) {
-  MojoHandleSignalsState* signals_state =
-      static_cast<MojoHandleSignalsState*>(env->GetDirectBufferAddress(buffer));
-  DCHECK(signals_state);
-  DCHECK_EQ(sizeof(MojoHandleSignalsState),
+static jint Wait(JNIEnv* env,
+                 const JavaParamRef<jobject>& jcaller,
+                 const JavaParamRef<jobject>& buffer,
+                 jint mojo_handle,
+                 jint signals,
+                 jlong deadline) {
+  // Buffer contains space for the MojoHandleSignalsState
+  void* buffer_start = env->GetDirectBufferAddress(buffer);
+  DCHECK(buffer_start);
+  DCHECK_EQ(reinterpret_cast<const uintptr_t>(buffer_start) % 8, 0u);
+  DCHECK_EQ(sizeof(struct MojoHandleSignalsState),
             static_cast<size_t>(env->GetDirectBufferCapacity(buffer)));
-  return MojoQueryHandleSignalsState(mojo_handle, signals_state);
+  struct MojoHandleSignalsState* signals_state =
+      static_cast<struct MojoHandleSignalsState*>(buffer_start);
+  return MojoWait(mojo_handle, signals, deadline, signals_state);
 }
 
 static jint WriteMessage(JNIEnv* env,