Skip to content

Commit

Permalink
Mojo EDK: Introduce MojoQueryHandleSignalsState API
Browse files Browse the repository at this point in the history 
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-Original-Commit-Position: refs/heads/master@{#457315}
Committed: https://chromium.googlesource.com/chromium/src/+/853496a78ae997c2d8b80f3cd8fabf9423fb3361
Review-Url: https://codereview.chromium.org/2741033003
Cr-Commit-Position: refs/heads/master@{#457378}
  • Loading branch information
@krockot
krockot authored and Commit bot committed Mar 16, 2017
1 parent 2007632 commit c794954
Show file tree
Hide file tree
Showing 37 changed files with 631 additions and 785 deletions.
21 changes: 7 additions & 14 deletions content/child/web_data_consumer_handle_impl.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,20 +62,13 @@ 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 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;
}
// 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;
}

uint32_t size_to_pass = size;
Expand Down
11 changes: 4 additions & 7 deletions mojo/android/javatests/src/org/chromium/mojo/HandleMock.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
package org.chromium.mojo;

import org.chromium.mojo.system.Core;
import org.chromium.mojo.system.Core.WaitResult;
import org.chromium.mojo.system.Core.HandleSignalsState;
import org.chromium.mojo.system.DataPipe;
import org.chromium.mojo.system.DataPipe.ConsumerHandle;
import org.chromium.mojo.system.DataPipe.ProducerHandle;
Expand Down Expand Up @@ -35,14 +35,11 @@ public void close() {
}

/**
* @see Handle#wait(Core.HandleSignals, long)
* @see Handle#querySignalsState()
*/
@Override
public WaitResult wait(Core.HandleSignals signals, long deadline) {
// Do nothing.
WaitResult result = new WaitResult();
result.setMojoResult(MojoResult.OK);
return result;
public HandleSignalsState querySignalsState() {
return null;
}

/**
Expand Down
5 changes: 1 addition & 4 deletions mojo/android/javatests/src/org/chromium/mojo/bindings/RouterTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
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;
Expand Down Expand Up @@ -227,8 +226,6 @@ public void testDroppingReceiverWithoutUsingIt() {

// Confirm that the pipe was closed on the Router side.
HandleSignals closedFlag = HandleSignals.none().setPeerClosed(true);
WaitResult result = mHandle.wait(closedFlag, 0);
assertEquals(MojoResult.OK, result.getMojoResult());
assertEquals(closedFlag, result.getHandleSignalsState().getSatisfiedSignals());
assertEquals(closedFlag, mHandle.querySignalsState().getSatisfiedSignals());
}
}
126 changes: 2 additions & 124 deletions mojo/android/javatests/src/org/chromium/mojo/system/impl/CoreImplTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,6 @@
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;
Expand All @@ -30,7 +27,6 @@
import java.util.Random;
import java.util.concurrent.Executors;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.TimeUnit;

/**
* Testing the core API.
Expand Down Expand Up @@ -76,22 +72,6 @@ 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();

Expand Down Expand Up @@ -183,46 +163,6 @@ 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.
*/
Expand Down Expand Up @@ -274,53 +214,14 @@ 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());

// 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());
handles.first.close();
handles.second.close();
}

/**
Expand Down Expand Up @@ -540,29 +441,6 @@ 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
Expand Down
58 changes: 9 additions & 49 deletions mojo/android/system/core_impl.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,41 +26,6 @@ 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,
Expand Down Expand Up @@ -127,21 +92,16 @@ static jint Close(JNIEnv* env,
return MojoClose(mojo_handle);
}

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 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_cast<size_t>(env->GetDirectBufferCapacity(buffer)));
struct MojoHandleSignalsState* signals_state =
static_cast<struct MojoHandleSignalsState*>(buffer_start);
return MojoWait(mojo_handle, signals, deadline, signals_state);
return MojoQueryHandleSignalsState(mojo_handle, signals_state);
}

static jint WriteMessage(JNIEnv* env,
Expand Down
Loading

0 comments on commit c794954

Please sign in to comment.