Reland: "android platform channels: moved to direct buffers for c <-> java interop" (flutter/engine#26515)

This commit is contained in:
gaaclarke 2021-06-01 11:19:02 -07:00 committed by GitHub
parent b18ffe565c
commit 1881ea7655
11 changed files with 166 additions and 29 deletions

View File

@ -487,6 +487,7 @@ action("robolectric_tests") {
"test/io/flutter/embedding/engine/systemchannels/PlatformChannelTest.java",
"test/io/flutter/embedding/engine/systemchannels/RestorationChannelTest.java",
"test/io/flutter/external/FlutterLaunchTests.java",
"test/io/flutter/plugin/common/BinaryCodecTest.java",
"test/io/flutter/plugin/common/StandardMessageCodecTest.java",
"test/io/flutter/plugin/common/StandardMethodCodecTest.java",
"test/io/flutter/plugin/editing/InputConnectionAdaptorTest.java",

View File

@ -814,7 +814,7 @@ public class FlutterJNI {
@SuppressWarnings("unused")
@VisibleForTesting
public void handlePlatformMessage(
@NonNull final String channel, byte[] message, final int replyId) {
@NonNull final String channel, ByteBuffer message, final int replyId) {
if (platformMessageHandler != null) {
platformMessageHandler.handleMessageFromDart(channel, message, replyId);
}
@ -825,7 +825,7 @@ public class FlutterJNI {
// Called by native to respond to a platform message that we sent.
// TODO(mattcarroll): determine if reply is nonull or nullable
@SuppressWarnings("unused")
private void handlePlatformMessageResponse(int replyId, byte[] reply) {
private void handlePlatformMessageResponse(int replyId, ByteBuffer reply) {
if (platformMessageHandler != null) {
platformMessageHandler.handlePlatformMessageResponse(replyId, reply);
}

View File

@ -75,14 +75,18 @@ class DartMessenger implements BinaryMessenger, PlatformMessageHandler {
@Override
public void handleMessageFromDart(
@NonNull final String channel, @Nullable byte[] message, final int replyId) {
@NonNull final String channel, @Nullable ByteBuffer message, final int replyId) {
Log.v(TAG, "Received message from Dart over channel '" + channel + "'");
BinaryMessenger.BinaryMessageHandler handler = messageHandlers.get(channel);
if (handler != null) {
try {
Log.v(TAG, "Deferring to registered handler to process message.");
final ByteBuffer buffer = (message == null ? null : ByteBuffer.wrap(message));
handler.onMessage(buffer, new Reply(flutterJNI, replyId));
handler.onMessage(message, new Reply(flutterJNI, replyId));
if (message != null && message.isDirect()) {
// This ensures that if a user retains an instance to the ByteBuffer and it happens to
// be direct they will get a deterministic error.
message.limit(0);
}
} catch (Exception ex) {
Log.e(TAG, "Uncaught exception in binary message listener", ex);
flutterJNI.invokePlatformMessageEmptyResponseCallback(replyId);
@ -96,13 +100,18 @@ class DartMessenger implements BinaryMessenger, PlatformMessageHandler {
}
@Override
public void handlePlatformMessageResponse(int replyId, @Nullable byte[] reply) {
public void handlePlatformMessageResponse(int replyId, @Nullable ByteBuffer reply) {
Log.v(TAG, "Received message reply from Dart.");
BinaryMessenger.BinaryReply callback = pendingReplies.remove(replyId);
if (callback != null) {
try {
Log.v(TAG, "Invoking registered callback for reply from Dart.");
callback.reply(reply == null ? null : ByteBuffer.wrap(reply));
callback.reply(reply);
if (reply != null && reply.isDirect()) {
// This ensures that if a user retains an instance to the ByteBuffer and it happens to
// be direct they will get a deterministic error.
reply.limit(0);
}
} catch (Exception ex) {
Log.e(TAG, "Uncaught exception in binary message reply handler", ex);
} catch (Error err) {

View File

@ -6,11 +6,12 @@ package io.flutter.embedding.engine.dart;
import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
import java.nio.ByteBuffer;
/** Handler that receives messages from Dart code. */
public interface PlatformMessageHandler {
void handleMessageFromDart(
@NonNull final String channel, @Nullable byte[] message, final int replyId);
@NonNull final String channel, @Nullable ByteBuffer message, final int replyId);
void handlePlatformMessageResponse(int replyId, @Nullable byte[] reply);
void handlePlatformMessageResponse(int replyId, @Nullable ByteBuffer reply);
}

View File

@ -4,6 +4,7 @@
package io.flutter.plugin.common;
import androidx.annotation.Nullable;
import java.nio.ByteBuffer;
/**
@ -18,16 +19,48 @@ import java.nio.ByteBuffer;
public final class BinaryCodec implements MessageCodec<ByteBuffer> {
// This codec must match the Dart codec of the same name in package flutter/services.
public static final BinaryCodec INSTANCE = new BinaryCodec();
/**
* A BinaryCodec that returns direct ByteBuffers from `decodeMessage` for better performance.
*
* @see BinaryCodec.BinaryCodec(boolean)
*/
public static final BinaryCodec INSTANCE_DIRECT = new BinaryCodec(true);
private BinaryCodec() {}
private final boolean returnsDirectByteBufferFromDecoding;
private BinaryCodec() {
this.returnsDirectByteBufferFromDecoding = false;
}
/**
* A constructor for BinaryCodec.
*
* @param returnsDirectByteBufferFromDecoding `true` means that the Codec will return direct
* ByteBuffers from `decodeMessage`. Direct ByteBuffers will have better performance but will
* be invalid beyond the scope of the `decodeMessage` call. `false` means Flutter will copy
* the encoded message to Java's memory, so the ByteBuffer will be valid beyond the
* decodeMessage call, at the cost of a copy.
*/
private BinaryCodec(boolean returnsDirectByteBufferFromDecoding) {
this.returnsDirectByteBufferFromDecoding = returnsDirectByteBufferFromDecoding;
}
@Override
public ByteBuffer encodeMessage(ByteBuffer message) {
public ByteBuffer encodeMessage(@Nullable ByteBuffer message) {
return message;
}
@Override
public ByteBuffer decodeMessage(ByteBuffer message) {
return message;
public ByteBuffer decodeMessage(@Nullable ByteBuffer message) {
if (message == null) {
return message;
} else if (returnsDirectByteBufferFromDecoding) {
return message;
} else {
ByteBuffer result = ByteBuffer.allocate(message.capacity());
result.put(message);
result.rewind();
return result;
}
}
}

View File

@ -26,6 +26,10 @@ public interface MessageCodec<T> {
/**
* Decodes the specified message from binary.
*
* <p><b>Warning:</b> The ByteBuffer is "direct" and it won't be valid beyond this call. Storing
* the ByteBuffer and using it later and will lead to a {@code java.nio.BufferUnderflowException}.
* If you want to retain the data you'll need to copy it.
*
* @param message the {@link ByteBuffer} message, possibly null.
* @return a T value representation of the bytes between the given buffer's current position and
* its limit, or null, if message is null.

View File

@ -828,7 +828,7 @@ bool RegisterApi(JNIEnv* env) {
g_handle_platform_message_method =
env->GetMethodID(g_flutter_jni_class->obj(), "handlePlatformMessage",
"(Ljava/lang/String;[BI)V");
"(Ljava/lang/String;Ljava/nio/ByteBuffer;I)V");
if (g_handle_platform_message_method == nullptr) {
FML_LOG(ERROR) << "Could not locate handlePlatformMessage method";
@ -836,7 +836,8 @@ bool RegisterApi(JNIEnv* env) {
}
g_handle_platform_message_response_method = env->GetMethodID(
g_flutter_jni_class->obj(), "handlePlatformMessageResponse", "(I[B)V");
g_flutter_jni_class->obj(), "handlePlatformMessageResponse",
"(ILjava/nio/ByteBuffer;)V");
if (g_handle_platform_message_response_method == nullptr) {
FML_LOG(ERROR) << "Could not locate handlePlatformMessageResponse method";
@ -1107,11 +1108,10 @@ void PlatformViewAndroidJNIImpl::FlutterViewHandlePlatformMessage(
fml::jni::StringToJavaString(env, message->channel());
if (message->hasData()) {
fml::jni::ScopedJavaLocalRef<jbyteArray> message_array(
env, env->NewByteArray(message->data().GetSize()));
env->SetByteArrayRegion(
message_array.obj(), 0, message->data().GetSize(),
reinterpret_cast<const jbyte*>(message->data().GetMapping()));
fml::jni::ScopedJavaLocalRef<jobject> message_array(
env, env->NewDirectByteBuffer(
const_cast<uint8_t*>(message->data().GetMapping()),
message->data().GetSize()));
env->CallVoidMethod(java_object.obj(), g_handle_platform_message_method,
java_channel.obj(), message_array.obj(), responseId);
} else {
@ -1141,10 +1141,9 @@ void PlatformViewAndroidJNIImpl::FlutterViewHandlePlatformMessageResponse(
nullptr);
} else {
// Convert the vector to a Java byte array.
fml::jni::ScopedJavaLocalRef<jbyteArray> data_array(
env, env->NewByteArray(data->GetSize()));
env->SetByteArrayRegion(data_array.obj(), 0, data->GetSize(),
reinterpret_cast<const jbyte*>(data->GetMapping()));
fml::jni::ScopedJavaLocalRef<jobject> data_array(
env, env->NewDirectByteBuffer(const_cast<uint8_t*>(data->GetMapping()),
data->GetSize()));
env->CallVoidMethod(java_object.obj(),
g_handle_platform_message_response_method, responseId,

View File

@ -30,6 +30,7 @@ import io.flutter.embedding.engine.systemchannels.KeyEventChannelTest;
import io.flutter.embedding.engine.systemchannels.PlatformChannelTest;
import io.flutter.embedding.engine.systemchannels.RestorationChannelTest;
import io.flutter.external.FlutterLaunchTests;
import io.flutter.plugin.common.BinaryCodecTest;
import io.flutter.plugin.common.StandardMessageCodecTest;
import io.flutter.plugin.common.StandardMethodCodecTest;
import io.flutter.plugin.editing.InputConnectionAdaptorTest;
@ -53,6 +54,7 @@ import test.io.flutter.embedding.engine.PluginComponentTest;
AccessibilityBridgeTest.class,
AndroidKeyProcessorTest.class,
ApplicationInfoLoaderTest.class,
BinaryCodecTest.class,
DartExecutorTest.class,
DartMessengerTest.class,
FlutterActivityAndFragmentDelegateTest.class,

View File

@ -1,11 +1,13 @@
package io.flutter.embedding.engine.dart;
import static junit.framework.TestCase.assertEquals;
import static junit.framework.TestCase.assertNotNull;
import static junit.framework.TestCase.assertTrue;
import static org.mockito.Matchers.any;
import static org.mockito.Mockito.mock;
import io.flutter.embedding.engine.FlutterJNI;
import io.flutter.plugin.common.BinaryMessenger;
import io.flutter.plugin.common.BinaryMessenger.BinaryMessageHandler;
import java.nio.ByteBuffer;
import org.junit.Test;
@ -46,9 +48,80 @@ public class DartMessengerTest {
.onMessage(any(ByteBuffer.class), any(DartMessenger.Reply.class));
messenger.setMessageHandler("test", throwingHandler);
messenger.handleMessageFromDart("test", new byte[] {}, 0);
messenger.handleMessageFromDart("test", ByteBuffer.allocate(0), 0);
assertNotNull(reportingHandler.latestException);
assertTrue(reportingHandler.latestException instanceof AssertionError);
currentThread.setUncaughtExceptionHandler(savedHandler);
}
@Test
public void givesDirectByteBuffer() {
// Setup test.
final FlutterJNI fakeFlutterJni = mock(FlutterJNI.class);
final DartMessenger messenger = new DartMessenger(fakeFlutterJni);
final String channel = "foobar";
final boolean[] wasDirect = {false};
final BinaryMessenger.BinaryMessageHandler handler =
(message, reply) -> {
wasDirect[0] = message.isDirect();
};
messenger.setMessageHandler(channel, handler);
final ByteBuffer message = ByteBuffer.allocateDirect(4 * 2);
message.rewind();
message.putChar('a');
message.putChar('b');
message.putChar('c');
message.putChar('d');
messenger.handleMessageFromDart(channel, message, /*replyId=*/ 123);
assertTrue(wasDirect[0]);
}
@Test
public void directByteBufferLimitZeroAfterUsage() {
// Setup test.
final FlutterJNI fakeFlutterJni = mock(FlutterJNI.class);
final DartMessenger messenger = new DartMessenger(fakeFlutterJni);
final String channel = "foobar";
final ByteBuffer[] byteBuffers = {null};
final int bufferSize = 4 * 2;
final BinaryMessenger.BinaryMessageHandler handler =
(message, reply) -> {
byteBuffers[0] = message;
assertEquals(bufferSize, byteBuffers[0].limit());
};
messenger.setMessageHandler(channel, handler);
final ByteBuffer message = ByteBuffer.allocateDirect(bufferSize);
message.rewind();
message.putChar('a');
message.putChar('b');
message.putChar('c');
message.putChar('d');
messenger.handleMessageFromDart(channel, message, /*replyId=*/ 123);
assertNotNull(byteBuffers[0]);
assertTrue(byteBuffers[0].isDirect());
assertEquals(0, byteBuffers[0].limit());
}
@Test
public void directByteBufferLimitZeroAfterReply() {
// Setup test.
final FlutterJNI fakeFlutterJni = mock(FlutterJNI.class);
final DartMessenger messenger = new DartMessenger(fakeFlutterJni);
final ByteBuffer message = ByteBuffer.allocateDirect(4 * 2);
final String channel = "foobar";
message.rewind();
message.putChar('a');
message.putChar('b');
message.putChar('c');
message.putChar('d');
final ByteBuffer[] byteBuffers = {null};
BinaryMessenger.BinaryReply callback =
(reply) -> {
assertTrue(reply.isDirect());
byteBuffers[0] = reply;
};
messenger.send(channel, null, callback);
messenger.handlePlatformMessageResponse(1, message);
assertEquals(0, byteBuffers[0].limit());
}
}

View File

@ -0,0 +1,17 @@
package io.flutter.plugin.common;
import static org.junit.Assert.assertNull;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.robolectric.RobolectricTestRunner;
import org.robolectric.annotation.Config;
@Config(manifest = Config.NONE)
@RunWith(RobolectricTestRunner.class)
public class BinaryCodecTest {
@Test
public void decodeNull() {
assertNull(BinaryCodec.INSTANCE.decodeMessage(null));
}
}

View File

@ -630,12 +630,10 @@ public class PlatformViewsControllerTest {
assertFalse(shouldProxying);
}
private static byte[] encodeMethodCall(MethodCall call) {
private static ByteBuffer encodeMethodCall(MethodCall call) {
final ByteBuffer buffer = StandardMethodCodec.INSTANCE.encodeMethodCall(call);
buffer.rewind();
final byte[] dest = new byte[buffer.remaining()];
buffer.get(dest);
return dest;
return buffer;
}
private static void createPlatformView(