From 3a5e448d228804e6a0fdabc7cb34cc2f2d29ae44 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 7 May 2021 16:19:23 -0700 Subject: [PATCH 01/19] moved to direct buffers --- .../flutter/embedding/engine/FlutterJNI.java | 4 ++-- .../embedding/engine/dart/DartMessenger.java | 9 ++++----- .../engine/dart/PlatformMessageHandler.java | 5 +++-- .../android/platform_view_android_jni_impl.cc | 20 +++++++++---------- .../engine/dart/DartMessengerTest.java | 2 +- .../platform/PlatformViewsControllerTest.java | 6 ++---- 6 files changed, 21 insertions(+), 25 deletions(-) diff --git a/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java b/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java index 17f918174af50..6b817e35aa565 100644 --- a/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java +++ b/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java @@ -814,7 +814,7 @@ public void setPlatformMessageHandler(@Nullable PlatformMessageHandler platformM @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 void handlePlatformMessage( // 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); } diff --git a/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java b/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java index fc03bb1f3c6f6..4e3e2fe3b795e 100644 --- a/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java +++ b/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java @@ -75,14 +75,13 @@ public void send( @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)); } catch (Exception ex) { Log.e(TAG, "Uncaught exception in binary message listener", ex); flutterJNI.invokePlatformMessageEmptyResponseCallback(replyId); @@ -96,13 +95,13 @@ public void handleMessageFromDart( } @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); } catch (Exception ex) { Log.e(TAG, "Uncaught exception in binary message reply handler", ex); } catch (Error err) { diff --git a/shell/platform/android/io/flutter/embedding/engine/dart/PlatformMessageHandler.java b/shell/platform/android/io/flutter/embedding/engine/dart/PlatformMessageHandler.java index ed8a5b044daad..22bb6f195666f 100644 --- a/shell/platform/android/io/flutter/embedding/engine/dart/PlatformMessageHandler.java +++ b/shell/platform/android/io/flutter/embedding/engine/dart/PlatformMessageHandler.java @@ -6,11 +6,12 @@ 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); } diff --git a/shell/platform/android/platform_view_android_jni_impl.cc b/shell/platform/android/platform_view_android_jni_impl.cc index 16e58e46ce280..fd79d1775b4a7 100644 --- a/shell/platform/android/platform_view_android_jni_impl.cc +++ b/shell/platform/android/platform_view_android_jni_impl.cc @@ -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,7 @@ 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 +1107,10 @@ void PlatformViewAndroidJNIImpl::FlutterViewHandlePlatformMessage( fml::jni::StringToJavaString(env, message->channel()); if (message->hasData()) { - fml::jni::ScopedJavaLocalRef message_array( - env, env->NewByteArray(message->data().GetSize())); - env->SetByteArrayRegion( - message_array.obj(), 0, message->data().GetSize(), - reinterpret_cast(message->data().GetMapping())); + fml::jni::ScopedJavaLocalRef message_array( + env, env->NewDirectByteBuffer( + const_cast(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 +1140,9 @@ void PlatformViewAndroidJNIImpl::FlutterViewHandlePlatformMessageResponse( nullptr); } else { // Convert the vector to a Java byte array. - fml::jni::ScopedJavaLocalRef data_array( - env, env->NewByteArray(data->GetSize())); - env->SetByteArrayRegion(data_array.obj(), 0, data->GetSize(), - reinterpret_cast(data->GetMapping())); + fml::jni::ScopedJavaLocalRef data_array( + env, env->NewDirectByteBuffer(const_cast(data->GetMapping()), + data->GetSize())); env->CallVoidMethod(java_object.obj(), g_handle_platform_message_response_method, responseId, diff --git a/shell/platform/android/test/io/flutter/embedding/engine/dart/DartMessengerTest.java b/shell/platform/android/test/io/flutter/embedding/engine/dart/DartMessengerTest.java index 3529448fc85b6..1500f5f7a3521 100644 --- a/shell/platform/android/test/io/flutter/embedding/engine/dart/DartMessengerTest.java +++ b/shell/platform/android/test/io/flutter/embedding/engine/dart/DartMessengerTest.java @@ -46,7 +46,7 @@ public void itHandlesErrors() { .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); diff --git a/shell/platform/android/test/io/flutter/plugin/platform/PlatformViewsControllerTest.java b/shell/platform/android/test/io/flutter/plugin/platform/PlatformViewsControllerTest.java index 1b75c81705f94..12db03678b61c 100644 --- a/shell/platform/android/test/io/flutter/plugin/platform/PlatformViewsControllerTest.java +++ b/shell/platform/android/test/io/flutter/plugin/platform/PlatformViewsControllerTest.java @@ -630,12 +630,10 @@ public void checkInputConnectionProxy__falseIfViewIsNull() { 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( From 985a4b84fc754b2c312c6ca8aa414198b78bcc66 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 21 May 2021 16:06:23 -0700 Subject: [PATCH 02/19] Added a warning about caching the ByteBuffer from MessageCodecs. --- .../android/io/flutter/plugin/common/MessageCodec.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/shell/platform/android/io/flutter/plugin/common/MessageCodec.java b/shell/platform/android/io/flutter/plugin/common/MessageCodec.java index 2e9d88718f9a7..db343e35742cb 100644 --- a/shell/platform/android/io/flutter/plugin/common/MessageCodec.java +++ b/shell/platform/android/io/flutter/plugin/common/MessageCodec.java @@ -26,6 +26,10 @@ public interface MessageCodec { /** * Decodes the specified message from binary. * + *

Warning: The ByteBuffer may be `direct`. Do not retain references to + * the ByteBuffer without checking `ByteBuffer.isDirect()`. See also: + * https://docs.oracle.com/javase/7/docs/api/java/nio/ByteBuffer.html + * * @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. From 64ba0112e486443ead38d264b7e8d0dbd4cc4c05 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 21 May 2021 16:18:59 -0700 Subject: [PATCH 03/19] fixed formatting --- .../android/io/flutter/plugin/common/MessageCodec.java | 4 ++-- shell/platform/android/platform_view_android_jni_impl.cc | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/shell/platform/android/io/flutter/plugin/common/MessageCodec.java b/shell/platform/android/io/flutter/plugin/common/MessageCodec.java index db343e35742cb..c3c222a3f0225 100644 --- a/shell/platform/android/io/flutter/plugin/common/MessageCodec.java +++ b/shell/platform/android/io/flutter/plugin/common/MessageCodec.java @@ -26,8 +26,8 @@ public interface MessageCodec { /** * Decodes the specified message from binary. * - *

Warning: The ByteBuffer may be `direct`. Do not retain references to - * the ByteBuffer without checking `ByteBuffer.isDirect()`. See also: + *

Warning: The ByteBuffer may be `direct`. Do not retain references to the ByteBuffer + * without checking `ByteBuffer.isDirect()`. See also: * https://docs.oracle.com/javase/7/docs/api/java/nio/ByteBuffer.html * * @param message the {@link ByteBuffer} message, possibly null. diff --git a/shell/platform/android/platform_view_android_jni_impl.cc b/shell/platform/android/platform_view_android_jni_impl.cc index fd79d1775b4a7..dc0cc7ec39836 100644 --- a/shell/platform/android/platform_view_android_jni_impl.cc +++ b/shell/platform/android/platform_view_android_jni_impl.cc @@ -836,7 +836,8 @@ bool RegisterApi(JNIEnv* env) { } g_handle_platform_message_response_method = env->GetMethodID( - g_flutter_jni_class->obj(), "handlePlatformMessageResponse", "(ILjava/nio/ByteBuffer;)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"; From 8945151df842d954fc98c671adde2c9122b6226a Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 21 May 2021 18:17:45 -0700 Subject: [PATCH 04/19] Allowed MessageCodec's to specify if they want direct ByteBuffers. --- .../platform/android/android_shell_holder.cc | 13 ++++ shell/platform/android/android_shell_holder.h | 6 ++ .../flutter/embedding/engine/FlutterJNI.java | 26 +++++++ .../embedding/engine/dart/DartExecutor.java | 14 ++-- .../embedding/engine/dart/DartMessenger.java | 6 +- .../plugin/common/BasicMessageChannel.java | 5 +- .../io/flutter/plugin/common/BinaryCodec.java | 17 ++++- .../plugin/common/BinaryMessenger.java | 12 +++- .../flutter/plugin/common/EventChannel.java | 4 +- .../plugin/common/JSONMessageCodec.java | 5 ++ .../flutter/plugin/common/MessageCodec.java | 15 +++- .../flutter/plugin/common/MethodChannel.java | 4 +- .../plugin/common/StandardMessageCodec.java | 5 ++ .../io/flutter/plugin/common/StringCodec.java | 5 ++ .../io/flutter/view/FlutterNativeView.java | 7 +- .../android/io/flutter/view/FlutterView.java | 5 +- shell/platform/android/jni/jni_mock.h | 10 +++ .../android/jni/platform_view_android_jni.h | 7 ++ .../android/platform_view_android_jni_impl.cc | 70 ++++++++++++++++++- .../android/platform_view_android_jni_impl.h | 10 +++ .../engine/dart/DartMessengerTest.java | 2 +- .../plugin/editing/TextInputPluginTest.java | 6 +- .../platform/PlatformViewsControllerTest.java | 7 ++ testing/run_tests.py | 8 +-- 24 files changed, 241 insertions(+), 28 deletions(-) diff --git a/shell/platform/android/android_shell_holder.cc b/shell/platform/android/android_shell_holder.cc index 60cb633e2f27f..590cbcad003d2 100644 --- a/shell/platform/android/android_shell_holder.cc +++ b/shell/platform/android/android_shell_holder.cc @@ -303,4 +303,17 @@ std::optional AndroidShellHolder::BuildRunConfiguration( } return config; } + +void AndroidShellHolder::ClearDirectByteBufferDecodingPreference( + const std::string& channel) { + jni_facade_->ClearDirectByteBufferDecodingPreference(channel); +} + +void AndroidShellHolder::SetDirectByteBufferDecodingPreference( + const std::string& channel, + bool wants_direct_byte_buffer_for_decoding) { + jni_facade_->SetDirectByteBufferDecodingPreference( + channel, wants_direct_byte_buffer_for_decoding); +} + } // namespace flutter diff --git a/shell/platform/android/android_shell_holder.h b/shell/platform/android/android_shell_holder.h index 39f0274fd49b5..62daed3676827 100644 --- a/shell/platform/android/android_shell_holder.h +++ b/shell/platform/android/android_shell_holder.h @@ -96,6 +96,12 @@ class AndroidShellHolder { void NotifyLowMemoryWarning(); + void ClearDirectByteBufferDecodingPreference(const std::string& channel); + + void SetDirectByteBufferDecodingPreference( + const std::string& channel, + bool wants_direct_byte_buffer_for_decoding); + private: const flutter::Settings settings_; const std::shared_ptr jni_facade_; diff --git a/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java b/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java index 6b817e35aa565..c8e6e9552dcdb 100644 --- a/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java +++ b/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java @@ -822,6 +822,32 @@ public void handlePlatformMessage( // (https://github.com/flutter/flutter/issues/25391) } + @SuppressWarnings("unused") + @VisibleForTesting + public void handleIndirectPlatformMessage( + @NonNull final String channel, ByteBuffer message, final int replyId) { + ByteBuffer indirectByteBuffer = ByteBuffer.allocate(message.capacity()); + indirectByteBuffer.put(message); + indirectByteBuffer.rewind(); + handlePlatformMessage(channel, indirectByteBuffer, replyId); + } + + private native void nativeClearDirectByteBufferDecodingPreference( + long nativeShellHolderId, String channel); + + public void clearDirectByteBufferDecodingPreference(@NonNull String channel) { + nativeClearDirectByteBufferDecodingPreference(nativeShellHolderId, channel); + } + + private native void nativeSetDirectByteBufferDecodingPreference( + long nativeShellHolderId, String channel, boolean wantsDirectByteBufferForDecoding); + + public void setDirectByteBufferDecodingPreference( + @NonNull String channel, boolean wantsDirectByteBufferForDecoding) { + nativeSetDirectByteBufferDecodingPreference( + nativeShellHolderId, channel, wantsDirectByteBufferForDecoding); + } + // Called by native to respond to a platform message that we sent. // TODO(mattcarroll): determine if reply is nonull or nullable @SuppressWarnings("unused") diff --git a/shell/platform/android/io/flutter/embedding/engine/dart/DartExecutor.java b/shell/platform/android/io/flutter/embedding/engine/dart/DartExecutor.java index e7aabf7c6ad0f..f0c6f99f95fba 100644 --- a/shell/platform/android/io/flutter/embedding/engine/dart/DartExecutor.java +++ b/shell/platform/android/io/flutter/embedding/engine/dart/DartExecutor.java @@ -59,7 +59,7 @@ public DartExecutor(@NonNull FlutterJNI flutterJNI, @NonNull AssetManager assetM this.flutterJNI = flutterJNI; this.assetManager = assetManager; this.dartMessenger = new DartMessenger(flutterJNI); - dartMessenger.setMessageHandler("flutter/isolate", isolateChannelMessageHandler); + dartMessenger.setMessageHandler("flutter/isolate", isolateChannelMessageHandler, true); this.binaryMessenger = new DefaultBinaryMessenger(dartMessenger); // The JNI might already be attached if coming from a spawned engine. If so, correctly report // that this DartExecutor is already running. @@ -192,8 +192,10 @@ public void send( @Override @UiThread public void setMessageHandler( - @NonNull String channel, @Nullable BinaryMessenger.BinaryMessageHandler handler) { - binaryMessenger.setMessageHandler(channel, handler); + @NonNull String channel, + @Nullable BinaryMessenger.BinaryMessageHandler handler, + boolean wantsDirectByteBufferForDecoding) { + binaryMessenger.setMessageHandler(channel, handler, wantsDirectByteBufferForDecoding); } // ------ END BinaryMessenger ----- @@ -413,8 +415,10 @@ public void send( @Override @UiThread public void setMessageHandler( - @NonNull String channel, @Nullable BinaryMessenger.BinaryMessageHandler handler) { - messenger.setMessageHandler(channel, handler); + @NonNull String channel, + @Nullable BinaryMessenger.BinaryMessageHandler handler, + boolean wantsDirectByteBufferForDecoding) { + messenger.setMessageHandler(channel, handler, wantsDirectByteBufferForDecoding); } } } diff --git a/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java b/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java index 4e3e2fe3b795e..f74a59304c50b 100644 --- a/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java +++ b/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java @@ -38,12 +38,16 @@ class DartMessenger implements BinaryMessenger, PlatformMessageHandler { @Override public void setMessageHandler( - @NonNull String channel, @Nullable BinaryMessenger.BinaryMessageHandler handler) { + @NonNull String channel, + @Nullable BinaryMessenger.BinaryMessageHandler handler, + boolean wantsDirectByteBufferForDecoding) { if (handler == null) { Log.v(TAG, "Removing handler for channel '" + channel + "'"); + flutterJNI.clearDirectByteBufferDecodingPreference(channel); messageHandlers.remove(channel); } else { Log.v(TAG, "Setting handler for channel '" + channel + "'"); + flutterJNI.setDirectByteBufferDecodingPreference(channel, wantsDirectByteBufferForDecoding); messageHandlers.put(channel, handler); } } diff --git a/shell/platform/android/io/flutter/plugin/common/BasicMessageChannel.java b/shell/platform/android/io/flutter/plugin/common/BasicMessageChannel.java index 55eb3109af5bb..f0e9eead514b6 100644 --- a/shell/platform/android/io/flutter/plugin/common/BasicMessageChannel.java +++ b/shell/platform/android/io/flutter/plugin/common/BasicMessageChannel.java @@ -101,7 +101,10 @@ public void send(@Nullable T message, @Nullable final Reply callback) { */ @UiThread public void setMessageHandler(@Nullable final MessageHandler handler) { - messenger.setMessageHandler(name, handler == null ? null : new IncomingMessageHandler(handler)); + messenger.setMessageHandler( + name, + handler == null ? null : new IncomingMessageHandler(handler), + codec.wantsDirectByteBufferForDecoding()); } /** diff --git a/shell/platform/android/io/flutter/plugin/common/BinaryCodec.java b/shell/platform/android/io/flutter/plugin/common/BinaryCodec.java index 348de24c2be4c..1f226673f7945 100644 --- a/shell/platform/android/io/flutter/plugin/common/BinaryCodec.java +++ b/shell/platform/android/io/flutter/plugin/common/BinaryCodec.java @@ -18,8 +18,23 @@ public final class BinaryCodec implements MessageCodec { // 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 calls decodeMessage with direct ByteBuffers for better performance. */ + public static final BinaryCodec INSTANCE_DIRECT = new BinaryCodec(true); - private BinaryCodec() {} + private final boolean wantsDirectByteBufferForDecoding; + + private BinaryCodec() { + this.wantsDirectByteBufferForDecoding = false; + } + + private BinaryCodec(boolean wantsDirectByteBufferForDecoding) { + this.wantsDirectByteBufferForDecoding = wantsDirectByteBufferForDecoding; + } + + @Override + public boolean wantsDirectByteBufferForDecoding() { + return wantsDirectByteBufferForDecoding; + } @Override public ByteBuffer encodeMessage(ByteBuffer message) { diff --git a/shell/platform/android/io/flutter/plugin/common/BinaryMessenger.java b/shell/platform/android/io/flutter/plugin/common/BinaryMessenger.java index fb1a22e6f4d3c..dcae2fc0230e8 100644 --- a/shell/platform/android/io/flutter/plugin/common/BinaryMessenger.java +++ b/shell/platform/android/io/flutter/plugin/common/BinaryMessenger.java @@ -4,6 +4,7 @@ package io.flutter.plugin.common; +import android.annotation.SuppressLint; import androidx.annotation.NonNull; import androidx.annotation.Nullable; import androidx.annotation.UiThread; @@ -50,6 +51,12 @@ public interface BinaryMessenger { @UiThread void send(@NonNull String channel, @Nullable ByteBuffer message, @Nullable BinaryReply callback); + @UiThread + @SuppressLint("NewApi") + default void setMessageHandler(@NonNull String channel, @Nullable BinaryMessageHandler handler) { + setMessageHandler(channel, handler, true); + } + /** * Registers a handler to be invoked when the Flutter application sends a message to its host * platform. @@ -64,7 +71,10 @@ public interface BinaryMessenger { * @param handler a {@link BinaryMessageHandler} to be invoked on incoming messages, or null. */ @UiThread - void setMessageHandler(@NonNull String channel, @Nullable BinaryMessageHandler handler); + void setMessageHandler( + @NonNull String channel, + @Nullable BinaryMessageHandler handler, + boolean wantsDirectByteBufferForDecoding); /** Handler for incoming binary messages from Flutter. */ interface BinaryMessageHandler { diff --git a/shell/platform/android/io/flutter/plugin/common/EventChannel.java b/shell/platform/android/io/flutter/plugin/common/EventChannel.java index 05dae1c01b3fd..c01b10807116e 100644 --- a/shell/platform/android/io/flutter/plugin/common/EventChannel.java +++ b/shell/platform/android/io/flutter/plugin/common/EventChannel.java @@ -84,7 +84,9 @@ public EventChannel(BinaryMessenger messenger, String name, MethodCodec codec) { @UiThread public void setStreamHandler(final StreamHandler handler) { messenger.setMessageHandler( - name, handler == null ? null : new IncomingStreamRequestHandler(handler)); + name, + handler == null ? null : new IncomingStreamRequestHandler(handler), + /*wantsDirectByteBufferForDecoding=*/ true); } /** diff --git a/shell/platform/android/io/flutter/plugin/common/JSONMessageCodec.java b/shell/platform/android/io/flutter/plugin/common/JSONMessageCodec.java index c6bf0e668b790..cb50c2a5eb06e 100644 --- a/shell/platform/android/io/flutter/plugin/common/JSONMessageCodec.java +++ b/shell/platform/android/io/flutter/plugin/common/JSONMessageCodec.java @@ -27,6 +27,11 @@ public final class JSONMessageCodec implements MessageCodec { private JSONMessageCodec() {} + @Override + public boolean wantsDirectByteBufferForDecoding() { + return true; + } + @Override public ByteBuffer encodeMessage(Object message) { if (message == null) { diff --git a/shell/platform/android/io/flutter/plugin/common/MessageCodec.java b/shell/platform/android/io/flutter/plugin/common/MessageCodec.java index c3c222a3f0225..5392ddf8bb352 100644 --- a/shell/platform/android/io/flutter/plugin/common/MessageCodec.java +++ b/shell/platform/android/io/flutter/plugin/common/MessageCodec.java @@ -13,6 +13,15 @@ *

Both operations throw {@link IllegalArgumentException}, if conversion fails. */ public interface MessageCodec { + + /** + * Controls the ByteBuffer parameter for `decodeMessage`. + * + * @return true if the MessageCodec wants the ByteBuffer parameter to decodeMessage to be a direct + * ByteBuffer. + */ + boolean wantsDirectByteBufferForDecoding(); + /** * Encodes the specified message into binary. * @@ -26,9 +35,9 @@ public interface MessageCodec { /** * Decodes the specified message from binary. * - *

Warning: The ByteBuffer may be `direct`. Do not retain references to the ByteBuffer - * without checking `ByteBuffer.isDirect()`. See also: - * https://docs.oracle.com/javase/7/docs/api/java/nio/ByteBuffer.html + *

Warning: The ByteBuffer may be `direct` if `wantsDirectByteBufferForDecoding` returns + * `true. Do not retain references to the ByteBuffer without checking `ByteBuffer.isDirect()`. See + * also: https://docs.oracle.com/javase/7/docs/api/java/nio/ByteBuffer.html * * @param message the {@link ByteBuffer} message, possibly null. * @return a T value representation of the bytes between the given buffer's current position and diff --git a/shell/platform/android/io/flutter/plugin/common/MethodChannel.java b/shell/platform/android/io/flutter/plugin/common/MethodChannel.java index 901299b943019..1fd6505c28722 100644 --- a/shell/platform/android/io/flutter/plugin/common/MethodChannel.java +++ b/shell/platform/android/io/flutter/plugin/common/MethodChannel.java @@ -117,7 +117,9 @@ public void invokeMethod(String method, @Nullable Object arguments, @Nullable Re @UiThread public void setMethodCallHandler(final @Nullable MethodCallHandler handler) { messenger.setMessageHandler( - name, handler == null ? null : new IncomingMethodCallHandler(handler)); + name, + handler == null ? null : new IncomingMethodCallHandler(handler), + /*wantsDirectByteBufferForDecoding=*/ true); } /** diff --git a/shell/platform/android/io/flutter/plugin/common/StandardMessageCodec.java b/shell/platform/android/io/flutter/plugin/common/StandardMessageCodec.java index e962fa6b3c0a8..6ee8a51a0757e 100644 --- a/shell/platform/android/io/flutter/plugin/common/StandardMessageCodec.java +++ b/shell/platform/android/io/flutter/plugin/common/StandardMessageCodec.java @@ -63,6 +63,11 @@ public class StandardMessageCodec implements MessageCodec { private static final String TAG = "StandardMessageCodec#"; public static final StandardMessageCodec INSTANCE = new StandardMessageCodec(); + @Override + public boolean wantsDirectByteBufferForDecoding() { + return true; + } + @Override public ByteBuffer encodeMessage(Object message) { if (message == null) { diff --git a/shell/platform/android/io/flutter/plugin/common/StringCodec.java b/shell/platform/android/io/flutter/plugin/common/StringCodec.java index dfcdbe68bd0fc..21fdffff7bfd7 100644 --- a/shell/platform/android/io/flutter/plugin/common/StringCodec.java +++ b/shell/platform/android/io/flutter/plugin/common/StringCodec.java @@ -20,6 +20,11 @@ public final class StringCodec implements MessageCodec { private StringCodec() {} + @Override + public boolean wantsDirectByteBufferForDecoding() { + return true; + } + @Override public ByteBuffer encodeMessage(String message) { if (message == null) { diff --git a/shell/platform/android/io/flutter/view/FlutterNativeView.java b/shell/platform/android/io/flutter/view/FlutterNativeView.java index 2b7afda518c1f..f9b6d431061bf 100644 --- a/shell/platform/android/io/flutter/view/FlutterNativeView.java +++ b/shell/platform/android/io/flutter/view/FlutterNativeView.java @@ -140,8 +140,11 @@ public void send(String channel, ByteBuffer message, BinaryReply callback) { @Override @UiThread - public void setMessageHandler(String channel, BinaryMessageHandler handler) { - dartExecutor.getBinaryMessenger().setMessageHandler(channel, handler); + public void setMessageHandler( + String channel, BinaryMessageHandler handler, boolean wantsDirectByteBufferForDecoding) { + dartExecutor + .getBinaryMessenger() + .setMessageHandler(channel, handler, wantsDirectByteBufferForDecoding); } /*package*/ FlutterJNI getFlutterJNI() { diff --git a/shell/platform/android/io/flutter/view/FlutterView.java b/shell/platform/android/io/flutter/view/FlutterView.java index 59711aabd7880..13769ef5e5137 100644 --- a/shell/platform/android/io/flutter/view/FlutterView.java +++ b/shell/platform/android/io/flutter/view/FlutterView.java @@ -863,8 +863,9 @@ public void send(String channel, ByteBuffer message, BinaryReply callback) { @Override @UiThread - public void setMessageHandler(String channel, BinaryMessageHandler handler) { - mNativeView.setMessageHandler(channel, handler); + public void setMessageHandler( + String channel, BinaryMessageHandler handler, boolean wantsDirectByteBufferForDecoding) { + mNativeView.setMessageHandler(channel, handler, wantsDirectByteBufferForDecoding); } /** Listener will be called on the Android UI thread once when Flutter renders the first frame. */ diff --git a/shell/platform/android/jni/jni_mock.h b/shell/platform/android/jni/jni_mock.h index 1a31db796a3ce..df32c6a6da1af 100644 --- a/shell/platform/android/jni/jni_mock.h +++ b/shell/platform/android/jni/jni_mock.h @@ -101,6 +101,16 @@ class JNIMock final : public PlatformViewAndroidJNI { RequestDartDeferredLibrary, (int loading_unit_id), (override)); + + MOCK_METHOD(void, + ClearDirectByteBufferDecodingPreference, + (const std::string& channel), + (override)); + + MOCK_METHOD(void, + SetDirectByteBufferDecodingPreference, + (const std::string&, bool), + (override)); }; } // namespace flutter diff --git a/shell/platform/android/jni/platform_view_android_jni.h b/shell/platform/android/jni/platform_view_android_jni.h index eae098a6b8871..702f45c58109f 100644 --- a/shell/platform/android/jni/platform_view_android_jni.h +++ b/shell/platform/android/jni/platform_view_android_jni.h @@ -197,6 +197,13 @@ class PlatformViewAndroidJNI { virtual double GetDisplayRefreshRate() = 0; virtual bool RequestDartDeferredLibrary(int loading_unit_id) = 0; + + virtual void ClearDirectByteBufferDecodingPreference( + const std::string& channel) = 0; + + virtual void SetDirectByteBufferDecodingPreference( + const std::string& channel, + bool wants_direct_byte_buffer_for_decoding) = 0; }; } // namespace flutter diff --git a/shell/platform/android/platform_view_android_jni_impl.cc b/shell/platform/android/platform_view_android_jni_impl.cc index dc0cc7ec39836..cdb68675374b6 100644 --- a/shell/platform/android/platform_view_android_jni_impl.cc +++ b/shell/platform/android/platform_view_android_jni_impl.cc @@ -85,6 +85,8 @@ static jmethodID g_long_constructor = nullptr; static jmethodID g_handle_platform_message_method = nullptr; +static jmethodID g_handle_indirect_platform_message_method = nullptr; + static jmethodID g_handle_platform_message_response_method = nullptr; static jmethodID g_update_semantics_method = nullptr; @@ -487,6 +489,26 @@ static void UnregisterTexture(JNIEnv* env, static_cast(texture_id)); } +static void ClearDirectByteBufferDecodingPreference(JNIEnv* env, + jobject jcaller, + jlong shell_holder, + jstring jchannel) { + auto channel = fml::jni::JavaStringToString(env, jchannel); + ANDROID_SHELL_HOLDER->ClearDirectByteBufferDecodingPreference(channel); +} + +static void SetDirectByteBufferDecodingPreference( + JNIEnv* env, + jobject jcaller, + jlong shell_holder, + jstring jchannel, + jboolean wants_direct_byte_buffer_for_decoding) { + auto channel = fml::jni::JavaStringToString(env, jchannel); + ANDROID_SHELL_HOLDER->SetDirectByteBufferDecodingPreference( + channel, // + wants_direct_byte_buffer_for_decoding); +} + static void InvokePlatformMessageResponseCallback(JNIEnv* env, jobject jcaller, jlong shell_holder, @@ -652,6 +674,18 @@ bool RegisterApi(JNIEnv* env) { .signature = "(JLjava/lang/String;Ljava/nio/ByteBuffer;II)V", .fnPtr = reinterpret_cast(&DispatchPlatformMessage), }, + { + .name = "nativeClearDirectByteBufferDecodingPreference", + .signature = "(JLjava/lang/String;)V", + .fnPtr = + reinterpret_cast(&ClearDirectByteBufferDecodingPreference), + }, + { + .name = "nativeSetDirectByteBufferDecodingPreference", + .signature = "(JLjava/lang/String;I)V", + .fnPtr = + reinterpret_cast(&SetDirectByteBufferDecodingPreference), + }, { .name = "nativeInvokePlatformMessageResponseCallback", .signature = "(JILjava/nio/ByteBuffer;I)V", @@ -826,6 +860,15 @@ bool RegisterApi(JNIEnv* env) { return false; } + g_handle_indirect_platform_message_method = env->GetMethodID( + g_flutter_jni_class->obj(), "handleIndirectPlatformMessage", + "(Ljava/lang/String;Ljava/nio/ByteBuffer;I)V"); + + if (g_handle_indirect_platform_message_method == nullptr) { + FML_LOG(ERROR) << "Could not locate handleIndirectPlatformMessage method"; + return false; + } + g_handle_platform_message_method = env->GetMethodID(g_flutter_jni_class->obj(), "handlePlatformMessage", "(Ljava/lang/String;Ljava/nio/ByteBuffer;I)V"); @@ -1094,6 +1137,21 @@ PlatformViewAndroidJNIImpl::PlatformViewAndroidJNIImpl( PlatformViewAndroidJNIImpl::~PlatformViewAndroidJNIImpl() = default; +void PlatformViewAndroidJNIImpl::ClearDirectByteBufferDecodingPreference( + const std::string& channel) { + channels_with_indirect_byte_buffer_decoding_.erase(channel); +} + +void PlatformViewAndroidJNIImpl::SetDirectByteBufferDecodingPreference( + const std::string& channel, + bool wants_direct_byte_buffer_for_decoding) { + if (wants_direct_byte_buffer_for_decoding) { + channels_with_indirect_byte_buffer_decoding_.erase(channel); + } else { + channels_with_indirect_byte_buffer_decoding_.emplace(std::move(channel)); + } +} + void PlatformViewAndroidJNIImpl::FlutterViewHandlePlatformMessage( std::unique_ptr message, int responseId) { @@ -1112,8 +1170,16 @@ void PlatformViewAndroidJNIImpl::FlutterViewHandlePlatformMessage( env, env->NewDirectByteBuffer( const_cast(message->data().GetMapping()), message->data().GetSize())); - env->CallVoidMethod(java_object.obj(), g_handle_platform_message_method, - java_channel.obj(), message_array.obj(), responseId); + if (!channels_with_indirect_byte_buffer_decoding_.empty() && + channels_with_indirect_byte_buffer_decoding_.find(message->channel()) != + channels_with_indirect_byte_buffer_decoding_.end()) { + env->CallVoidMethod(java_object.obj(), g_handle_platform_message_method, + java_channel.obj(), message_array.obj(), responseId); + } else { + env->CallVoidMethod(java_object.obj(), + g_handle_indirect_platform_message_method, + java_channel.obj(), message_array.obj(), responseId); + } } else { env->CallVoidMethod(java_object.obj(), g_handle_platform_message_method, java_channel.obj(), nullptr, responseId); diff --git a/shell/platform/android/platform_view_android_jni_impl.h b/shell/platform/android/platform_view_android_jni_impl.h index 34b80f07c6297..ad1f13af21e6d 100644 --- a/shell/platform/android/platform_view_android_jni_impl.h +++ b/shell/platform/android/platform_view_android_jni_impl.h @@ -5,6 +5,8 @@ #ifndef FLUTTER_SHELL_PLATFORM_ANDROID_PLATFORM_VIEW_ANDROID_JNI_IMPL_H_ #define FLUTTER_SHELL_PLATFORM_ANDROID_PLATFORM_VIEW_ANDROID_JNI_IMPL_H_ +#include + #include "flutter/fml/platform/android/jni_weak_ref.h" #include "flutter/shell/platform/android/jni/platform_view_android_jni.h" @@ -82,9 +84,17 @@ class PlatformViewAndroidJNIImpl final : public PlatformViewAndroidJNI { bool RequestDartDeferredLibrary(int loading_unit_id) override; + void ClearDirectByteBufferDecodingPreference( + const std::string& channel) override; + + void SetDirectByteBufferDecodingPreference( + const std::string& channel, + bool wants_direct_byte_buffer_for_decoding) override; + private: // Reference to FlutterJNI object. const fml::jni::JavaObjectWeakGlobalRef java_object_; + std::set channels_with_indirect_byte_buffer_decoding_; FML_DISALLOW_COPY_AND_ASSIGN(PlatformViewAndroidJNIImpl); }; diff --git a/shell/platform/android/test/io/flutter/embedding/engine/dart/DartMessengerTest.java b/shell/platform/android/test/io/flutter/embedding/engine/dart/DartMessengerTest.java index 1500f5f7a3521..aa6cd370467fc 100644 --- a/shell/platform/android/test/io/flutter/embedding/engine/dart/DartMessengerTest.java +++ b/shell/platform/android/test/io/flutter/embedding/engine/dart/DartMessengerTest.java @@ -45,7 +45,7 @@ public void itHandlesErrors() { .when(throwingHandler) .onMessage(any(ByteBuffer.class), any(DartMessenger.Reply.class)); - messenger.setMessageHandler("test", throwingHandler); + messenger.setMessageHandler("test", throwingHandler, true); messenger.handleMessageFromDart("test", ByteBuffer.allocate(0), 0); assertNotNull(reportingHandler.latestException); assertTrue(reportingHandler.latestException instanceof AssertionError); diff --git a/shell/platform/android/test/io/flutter/plugin/editing/TextInputPluginTest.java b/shell/platform/android/test/io/flutter/plugin/editing/TextInputPluginTest.java index 7e8436418ae96..f3c4ec7c87e7e 100644 --- a/shell/platform/android/test/io/flutter/plugin/editing/TextInputPluginTest.java +++ b/shell/platform/android/test/io/flutter/plugin/editing/TextInputPluginTest.java @@ -1000,7 +1000,7 @@ public void respondsToInputChannelMessages() { textInputChannel.setTextInputMethodHandler(mockHandler); verify(mockBinaryMessenger, times(1)) - .setMessageHandler(any(String.class), binaryMessageHandlerCaptor.capture()); + .setMessageHandler(any(String.class), binaryMessageHandlerCaptor.capture(), eq(true)); BinaryMessenger.BinaryMessageHandler binaryMessageHandler = binaryMessageHandlerCaptor.getValue(); @@ -1033,7 +1033,7 @@ public void sendAppPrivateCommand_dataIsEmpty() throws JSONException { new TextInputPlugin(testView, textInputChannel, mock(PlatformViewsController.class)); verify(mockBinaryMessenger, times(1)) - .setMessageHandler(any(String.class), binaryMessageHandlerCaptor.capture()); + .setMessageHandler(any(String.class), binaryMessageHandlerCaptor.capture(), eq(true)); JSONObject arguments = new JSONObject(); arguments.put("action", "actionCommand"); @@ -1064,7 +1064,7 @@ public void sendAppPrivateCommand_hasData() throws JSONException { new TextInputPlugin(testView, textInputChannel, mock(PlatformViewsController.class)); verify(mockBinaryMessenger, times(1)) - .setMessageHandler(any(String.class), binaryMessageHandlerCaptor.capture()); + .setMessageHandler(any(String.class), binaryMessageHandlerCaptor.capture(), eq(true)); JSONObject arguments = new JSONObject(); arguments.put("action", "actionCommand"); diff --git a/shell/platform/android/test/io/flutter/plugin/platform/PlatformViewsControllerTest.java b/shell/platform/android/test/io/flutter/plugin/platform/PlatformViewsControllerTest.java index 12db03678b61c..bd75a234e456d 100644 --- a/shell/platform/android/test/io/flutter/plugin/platform/PlatformViewsControllerTest.java +++ b/shell/platform/android/test/io/flutter/plugin/platform/PlatformViewsControllerTest.java @@ -758,6 +758,13 @@ public void invokePlatformMessageResponseCallback( public static SparseArray getResponses() { return replies; } + + @Implementation + public void clearDirectByteBufferDecodingPreference(String channel) {} + + @Implementation + public void setDirectByteBufferDecodingPreference( + String channel, boolean wantsDirectByteBufferForDecoding) {} } @Implements(SurfaceView.class) diff --git a/testing/run_tests.py b/testing/run_tests.py index 4f5226d5f539e..9d9cf1cf4de5e 100755 --- a/testing/run_tests.py +++ b/testing/run_tests.py @@ -367,10 +367,10 @@ def AssertExpectedJavaVersion(): """Checks that the user has Java 8 which is the supported Java version for Android""" EXPECTED_VERSION = '1.8' # `java -version` is output to stderr. https://bugs.java.com/bugdatabase/view_bug.do?bug_id=4380614 - version_output = subprocess.check_output(['java', '-version'], stderr=subprocess.STDOUT) - match = bool(re.compile('version "%s' % EXPECTED_VERSION).search(version_output)) - message = "JUnit tests need to be run with Java %s. Check the `java -version` on your PATH." % EXPECTED_VERSION - assert match, message + # version_output = subprocess.check_output(['java', '-version'], stderr=subprocess.STDOUT) + # match = bool(re.compile('version "%s' % EXPECTED_VERSION).search(version_output)) + # message = "JUnit tests need to be run with Java %s. Check the `java -version` on your PATH." % EXPECTED_VERSION + # assert match, message def AssertExpectedXcodeVersion(): From 3814d7d90b61f392aa368a42b38752fda97edbb1 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Tue, 25 May 2021 10:07:17 -0700 Subject: [PATCH 05/19] random fixes --- .../io/flutter/embedding/engine/dart/DartExecutor.java | 5 ++++- shell/platform/android/platform_view_android_jni_impl.cc | 2 +- testing/run_tests.py | 8 ++++---- .../dev/flutter/scenarios/TextPlatformViewFactory.java | 5 +++++ 4 files changed, 14 insertions(+), 6 deletions(-) diff --git a/shell/platform/android/io/flutter/embedding/engine/dart/DartExecutor.java b/shell/platform/android/io/flutter/embedding/engine/dart/DartExecutor.java index f0c6f99f95fba..5f286a2feb8b8 100644 --- a/shell/platform/android/io/flutter/embedding/engine/dart/DartExecutor.java +++ b/shell/platform/android/io/flutter/embedding/engine/dart/DartExecutor.java @@ -59,7 +59,10 @@ public DartExecutor(@NonNull FlutterJNI flutterJNI, @NonNull AssetManager assetM this.flutterJNI = flutterJNI; this.assetManager = assetManager; this.dartMessenger = new DartMessenger(flutterJNI); - dartMessenger.setMessageHandler("flutter/isolate", isolateChannelMessageHandler, true); + dartMessenger.setMessageHandler( + "flutter/isolate", + isolateChannelMessageHandler, + /*wantsDirectByteBufferForDecoding=*/ true); this.binaryMessenger = new DefaultBinaryMessenger(dartMessenger); // The JNI might already be attached if coming from a spawned engine. If so, correctly report // that this DartExecutor is already running. diff --git a/shell/platform/android/platform_view_android_jni_impl.cc b/shell/platform/android/platform_view_android_jni_impl.cc index cdb68675374b6..96770b7ab57e2 100644 --- a/shell/platform/android/platform_view_android_jni_impl.cc +++ b/shell/platform/android/platform_view_android_jni_impl.cc @@ -1148,7 +1148,7 @@ void PlatformViewAndroidJNIImpl::SetDirectByteBufferDecodingPreference( if (wants_direct_byte_buffer_for_decoding) { channels_with_indirect_byte_buffer_decoding_.erase(channel); } else { - channels_with_indirect_byte_buffer_decoding_.emplace(std::move(channel)); + channels_with_indirect_byte_buffer_decoding_.insert(channel); } } diff --git a/testing/run_tests.py b/testing/run_tests.py index 9d9cf1cf4de5e..4f5226d5f539e 100755 --- a/testing/run_tests.py +++ b/testing/run_tests.py @@ -367,10 +367,10 @@ def AssertExpectedJavaVersion(): """Checks that the user has Java 8 which is the supported Java version for Android""" EXPECTED_VERSION = '1.8' # `java -version` is output to stderr. https://bugs.java.com/bugdatabase/view_bug.do?bug_id=4380614 - # version_output = subprocess.check_output(['java', '-version'], stderr=subprocess.STDOUT) - # match = bool(re.compile('version "%s' % EXPECTED_VERSION).search(version_output)) - # message = "JUnit tests need to be run with Java %s. Check the `java -version` on your PATH." % EXPECTED_VERSION - # assert match, message + version_output = subprocess.check_output(['java', '-version'], stderr=subprocess.STDOUT) + match = bool(re.compile('version "%s' % EXPECTED_VERSION).search(version_output)) + message = "JUnit tests need to be run with Java %s. Check the `java -version` on your PATH." % EXPECTED_VERSION + assert match, message def AssertExpectedXcodeVersion(): diff --git a/testing/scenario_app/android/app/src/main/java/dev/flutter/scenarios/TextPlatformViewFactory.java b/testing/scenario_app/android/app/src/main/java/dev/flutter/scenarios/TextPlatformViewFactory.java index d5d52b14f7226..0e29464624364 100644 --- a/testing/scenario_app/android/app/src/main/java/dev/flutter/scenarios/TextPlatformViewFactory.java +++ b/testing/scenario_app/android/app/src/main/java/dev/flutter/scenarios/TextPlatformViewFactory.java @@ -16,6 +16,11 @@ public final class TextPlatformViewFactory extends PlatformViewFactory { TextPlatformViewFactory() { super( new MessageCodec() { + @Override + public boolean wantsDirectByteBufferForDecoding() { + return true; + } + @Nullable @Override public ByteBuffer encodeMessage(@Nullable Object o) { From 478451be9a7fae77dd2cb0badff72bb497d27b41 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Tue, 25 May 2021 10:33:19 -0700 Subject: [PATCH 06/19] removed 2 parameter setMessageHandler from binarymessenger --- .../io/flutter/plugin/common/BinaryMessenger.java | 9 ++------- .../java/dev/flutter/scenarios/EngineLaunchE2ETest.java | 3 ++- .../dev/flutter/scenarios/SpawnedEngineActivity.java | 3 ++- .../dev/flutter/scenarios/TestableFlutterActivity.java | 3 ++- 4 files changed, 8 insertions(+), 10 deletions(-) diff --git a/shell/platform/android/io/flutter/plugin/common/BinaryMessenger.java b/shell/platform/android/io/flutter/plugin/common/BinaryMessenger.java index dcae2fc0230e8..077daff6aa41b 100644 --- a/shell/platform/android/io/flutter/plugin/common/BinaryMessenger.java +++ b/shell/platform/android/io/flutter/plugin/common/BinaryMessenger.java @@ -4,7 +4,6 @@ package io.flutter.plugin.common; -import android.annotation.SuppressLint; import androidx.annotation.NonNull; import androidx.annotation.Nullable; import androidx.annotation.UiThread; @@ -51,12 +50,6 @@ public interface BinaryMessenger { @UiThread void send(@NonNull String channel, @Nullable ByteBuffer message, @Nullable BinaryReply callback); - @UiThread - @SuppressLint("NewApi") - default void setMessageHandler(@NonNull String channel, @Nullable BinaryMessageHandler handler) { - setMessageHandler(channel, handler, true); - } - /** * Registers a handler to be invoked when the Flutter application sends a message to its host * platform. @@ -69,6 +62,8 @@ default void setMessageHandler(@NonNull String channel, @Nullable BinaryMessageH * * @param channel the name {@link String} of the channel. * @param handler a {@link BinaryMessageHandler} to be invoked on incoming messages, or null. + * @param wantsDirectByteBufferForDecoding pass in `true` to make the ByteBuffer parameter to the + * `BinaryMessageHandler` a direct ByteBuffer. */ @UiThread void setMessageHandler( diff --git a/testing/scenario_app/android/app/src/androidTest/java/dev/flutter/scenarios/EngineLaunchE2ETest.java b/testing/scenario_app/android/app/src/androidTest/java/dev/flutter/scenarios/EngineLaunchE2ETest.java index c3e50feba9a2d..6ca5ede9e83ac 100644 --- a/testing/scenario_app/android/app/src/androidTest/java/dev/flutter/scenarios/EngineLaunchE2ETest.java +++ b/testing/scenario_app/android/app/src/androidTest/java/dev/flutter/scenarios/EngineLaunchE2ETest.java @@ -50,7 +50,8 @@ public void smokeTestEngineLaunch() throws Throwable { .getDartExecutor() .setMessageHandler( "waiting_for_status", - (byteBuffer, binaryReply) -> statusReceived.complete(Boolean.TRUE)); + (byteBuffer, binaryReply) -> statusReceived.complete(Boolean.TRUE), + true); // Launching the entrypoint will run the Dart code that sends the "waiting_for_status" platform // message. diff --git a/testing/scenario_app/android/app/src/main/java/dev/flutter/scenarios/SpawnedEngineActivity.java b/testing/scenario_app/android/app/src/main/java/dev/flutter/scenarios/SpawnedEngineActivity.java index 52ab54b8e3a9e..680c0069c5fb9 100644 --- a/testing/scenario_app/android/app/src/main/java/dev/flutter/scenarios/SpawnedEngineActivity.java +++ b/testing/scenario_app/android/app/src/main/java/dev/flutter/scenarios/SpawnedEngineActivity.java @@ -21,7 +21,8 @@ public FlutterEngine provideFlutterEngine(@NonNull Context context) { secondEngine .getDartExecutor() - .setMessageHandler("take_screenshot", (byteBuffer, binaryReply) -> notifyFlutterRendered()); + .setMessageHandler( + "take_screenshot", (byteBuffer, binaryReply) -> notifyFlutterRendered(), true); return secondEngine; } diff --git a/testing/scenario_app/android/app/src/main/java/dev/flutter/scenarios/TestableFlutterActivity.java b/testing/scenario_app/android/app/src/main/java/dev/flutter/scenarios/TestableFlutterActivity.java index 457d980c5ecae..793a16bf8fd49 100644 --- a/testing/scenario_app/android/app/src/main/java/dev/flutter/scenarios/TestableFlutterActivity.java +++ b/testing/scenario_app/android/app/src/main/java/dev/flutter/scenarios/TestableFlutterActivity.java @@ -17,7 +17,8 @@ public void configureFlutterEngine(FlutterEngine flutterEngine) { super.configureFlutterEngine(flutterEngine); flutterEngine .getDartExecutor() - .setMessageHandler("take_screenshot", (byteBuffer, binaryReply) -> notifyFlutterRendered()); + .setMessageHandler( + "take_screenshot", (byteBuffer, binaryReply) -> notifyFlutterRendered(), true); } protected void notifyFlutterRendered() { From 88cb2044958aa2512a970b1884ebd23387da1adb Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Tue, 25 May 2021 11:04:02 -0700 Subject: [PATCH 07/19] Added DartMessenger unit tests --- .../engine/dart/DartMessengerTest.java | 21 +++++++++++++++++++ testing/run_tests.py | 8 +++---- 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/shell/platform/android/test/io/flutter/embedding/engine/dart/DartMessengerTest.java b/shell/platform/android/test/io/flutter/embedding/engine/dart/DartMessengerTest.java index aa6cd370467fc..747b0a9bf27fa 100644 --- a/shell/platform/android/test/io/flutter/embedding/engine/dart/DartMessengerTest.java +++ b/shell/platform/android/test/io/flutter/embedding/engine/dart/DartMessengerTest.java @@ -4,8 +4,10 @@ import static junit.framework.TestCase.assertTrue; import static org.mockito.Matchers.any; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; 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; @@ -51,4 +53,23 @@ public void itHandlesErrors() { assertTrue(reportingHandler.latestException instanceof AssertionError); currentThread.setUncaughtExceptionHandler(savedHandler); } + + @Test + public void setsDirectByteBuffer() { + final FlutterJNI fakeFlutterJni = mock(FlutterJNI.class); + final DartMessenger messenger = new DartMessenger(fakeFlutterJni); + final String channel = "foo"; + final BinaryMessenger.BinaryMessageHandler handler = (byteBuffer, reply) -> {}; + messenger.setMessageHandler(channel, handler, true); + verify(fakeFlutterJni).setDirectByteBufferDecodingPreference(channel, true); + } + + @Test + public void clearsDirectByteBuffer() { + final FlutterJNI fakeFlutterJni = mock(FlutterJNI.class); + final DartMessenger messenger = new DartMessenger(fakeFlutterJni); + final String channel = "foo"; + messenger.setMessageHandler(channel, null, true); + verify(fakeFlutterJni).clearDirectByteBufferDecodingPreference(channel); + } } diff --git a/testing/run_tests.py b/testing/run_tests.py index 4f5226d5f539e..9d9cf1cf4de5e 100755 --- a/testing/run_tests.py +++ b/testing/run_tests.py @@ -367,10 +367,10 @@ def AssertExpectedJavaVersion(): """Checks that the user has Java 8 which is the supported Java version for Android""" EXPECTED_VERSION = '1.8' # `java -version` is output to stderr. https://bugs.java.com/bugdatabase/view_bug.do?bug_id=4380614 - version_output = subprocess.check_output(['java', '-version'], stderr=subprocess.STDOUT) - match = bool(re.compile('version "%s' % EXPECTED_VERSION).search(version_output)) - message = "JUnit tests need to be run with Java %s. Check the `java -version` on your PATH." % EXPECTED_VERSION - assert match, message + # version_output = subprocess.check_output(['java', '-version'], stderr=subprocess.STDOUT) + # match = bool(re.compile('version "%s' % EXPECTED_VERSION).search(version_output)) + # message = "JUnit tests need to be run with Java %s. Check the `java -version` on your PATH." % EXPECTED_VERSION + # assert match, message def AssertExpectedXcodeVersion(): From caa29560cbbe56052b01ba38c4f78d1dd5236c98 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Tue, 25 May 2021 12:29:36 -0700 Subject: [PATCH 08/19] got things working in integration tests --- .../io/flutter/embedding/engine/dart/DartMessenger.java | 4 +++- shell/platform/android/platform_view_android_jni_impl.cc | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java b/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java index f74a59304c50b..be10518a52718 100644 --- a/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java +++ b/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java @@ -47,7 +47,9 @@ public void setMessageHandler( messageHandlers.remove(channel); } else { Log.v(TAG, "Setting handler for channel '" + channel + "'"); - flutterJNI.setDirectByteBufferDecodingPreference(channel, wantsDirectByteBufferForDecoding); + if (!wantsDirectByteBufferForDecoding) { + flutterJNI.setDirectByteBufferDecodingPreference(channel, wantsDirectByteBufferForDecoding); + } messageHandlers.put(channel, handler); } } diff --git a/shell/platform/android/platform_view_android_jni_impl.cc b/shell/platform/android/platform_view_android_jni_impl.cc index 96770b7ab57e2..75e6bd97bc8a4 100644 --- a/shell/platform/android/platform_view_android_jni_impl.cc +++ b/shell/platform/android/platform_view_android_jni_impl.cc @@ -682,7 +682,7 @@ bool RegisterApi(JNIEnv* env) { }, { .name = "nativeSetDirectByteBufferDecodingPreference", - .signature = "(JLjava/lang/String;I)V", + .signature = "(JLjava/lang/String;Z)V", .fnPtr = reinterpret_cast(&SetDirectByteBufferDecodingPreference), }, From 258eed1436097a3f3f1b6f018d63295a7af9fa64 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Tue, 25 May 2021 13:14:40 -0700 Subject: [PATCH 09/19] fixed logical error that reversed direct buffers --- .../io/flutter/embedding/engine/dart/DartMessenger.java | 4 ++++ shell/platform/android/platform_view_android_jni_impl.cc | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java b/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java index be10518a52718..b1958a49af557 100644 --- a/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java +++ b/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java @@ -48,6 +48,10 @@ public void setMessageHandler( } else { Log.v(TAG, "Setting handler for channel '" + channel + "'"); if (!wantsDirectByteBufferForDecoding) { + if (!this.flutterJNI.isAttached()) { + throw new IllegalStateException( + "Only direct ByteBuffers are supported for channels whose message handlers are setup before jni is attached."); + } flutterJNI.setDirectByteBufferDecodingPreference(channel, wantsDirectByteBufferForDecoding); } messageHandlers.put(channel, handler); diff --git a/shell/platform/android/platform_view_android_jni_impl.cc b/shell/platform/android/platform_view_android_jni_impl.cc index 75e6bd97bc8a4..ae6feef9fd464 100644 --- a/shell/platform/android/platform_view_android_jni_impl.cc +++ b/shell/platform/android/platform_view_android_jni_impl.cc @@ -1170,8 +1170,8 @@ void PlatformViewAndroidJNIImpl::FlutterViewHandlePlatformMessage( env, env->NewDirectByteBuffer( const_cast(message->data().GetMapping()), message->data().GetSize())); - if (!channels_with_indirect_byte_buffer_decoding_.empty() && - channels_with_indirect_byte_buffer_decoding_.find(message->channel()) != + if (channels_with_indirect_byte_buffer_decoding_.empty() || + channels_with_indirect_byte_buffer_decoding_.find(message->channel()) == channels_with_indirect_byte_buffer_decoding_.end()) { env->CallVoidMethod(java_object.obj(), g_handle_platform_message_method, java_channel.obj(), message_array.obj(), responseId); From 3331e19bb3aa596de37467e1025a6d852d32b3df Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Tue, 25 May 2021 14:28:23 -0700 Subject: [PATCH 10/19] updated tests --- .../embedding/engine/dart/DartMessengerTest.java | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/shell/platform/android/test/io/flutter/embedding/engine/dart/DartMessengerTest.java b/shell/platform/android/test/io/flutter/embedding/engine/dart/DartMessengerTest.java index 747b0a9bf27fa..21b8f74b89eac 100644 --- a/shell/platform/android/test/io/flutter/embedding/engine/dart/DartMessengerTest.java +++ b/shell/platform/android/test/io/flutter/embedding/engine/dart/DartMessengerTest.java @@ -4,7 +4,9 @@ import static junit.framework.TestCase.assertTrue; import static org.mockito.Matchers.any; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; import io.flutter.embedding.engine.FlutterJNI; import io.flutter.plugin.common.BinaryMessenger; @@ -61,7 +63,19 @@ public void setsDirectByteBuffer() { final String channel = "foo"; final BinaryMessenger.BinaryMessageHandler handler = (byteBuffer, reply) -> {}; messenger.setMessageHandler(channel, handler, true); - verify(fakeFlutterJni).setDirectByteBufferDecodingPreference(channel, true); + verify(fakeFlutterJni, never()) + .setDirectByteBufferDecodingPreference(Mockito.anyString(), Mockito.anyBoolean()); + } + + @Test + public void setsIndirectByteBuffer() { + final FlutterJNI fakeFlutterJni = mock(FlutterJNI.class); + final DartMessenger messenger = new DartMessenger(fakeFlutterJni); + final String channel = "foo"; + final BinaryMessenger.BinaryMessageHandler handler = (byteBuffer, reply) -> {}; + when(fakeFlutterJni.isAttached()).thenReturn(true); + messenger.setMessageHandler(channel, handler, false); + verify(fakeFlutterJni).setDirectByteBufferDecodingPreference(channel, false); } @Test From 687a0b9096a9c876dbb2bf90f1899a633b923da7 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Tue, 25 May 2021 14:44:55 -0700 Subject: [PATCH 11/19] revert local changes to run_tests, updated docstrings --- .../io/flutter/plugin/common/BinaryCodec.java | 15 ++++++++++++++- .../io/flutter/plugin/common/MessageCodec.java | 7 +++++-- testing/run_tests.py | 8 ++++---- 3 files changed, 23 insertions(+), 7 deletions(-) diff --git a/shell/platform/android/io/flutter/plugin/common/BinaryCodec.java b/shell/platform/android/io/flutter/plugin/common/BinaryCodec.java index 1f226673f7945..4cc018923e964 100644 --- a/shell/platform/android/io/flutter/plugin/common/BinaryCodec.java +++ b/shell/platform/android/io/flutter/plugin/common/BinaryCodec.java @@ -18,7 +18,11 @@ public final class BinaryCodec implements MessageCodec { // 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 calls decodeMessage with direct ByteBuffers for better performance. */ + /** + * A BinaryCodec that calls `decodeMessage` with direct ByteBuffers for better performance. + * + * @see BinaryCodec.BinaryCodec(boolean) + */ public static final BinaryCodec INSTANCE_DIRECT = new BinaryCodec(true); private final boolean wantsDirectByteBufferForDecoding; @@ -27,6 +31,15 @@ private BinaryCodec() { this.wantsDirectByteBufferForDecoding = false; } + /** + * A constructor for BinaryCodec. + * + * @param wantsDirectByteBufferForDecoding `true` means that Flutter will send direct ByteBuffers + * to `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 wantsDirectByteBufferForDecoding) { this.wantsDirectByteBufferForDecoding = wantsDirectByteBufferForDecoding; } diff --git a/shell/platform/android/io/flutter/plugin/common/MessageCodec.java b/shell/platform/android/io/flutter/plugin/common/MessageCodec.java index 5392ddf8bb352..3299a1f673504 100644 --- a/shell/platform/android/io/flutter/plugin/common/MessageCodec.java +++ b/shell/platform/android/io/flutter/plugin/common/MessageCodec.java @@ -17,6 +17,7 @@ public interface MessageCodec { /** * Controls the ByteBuffer parameter for `decodeMessage`. * + * @see MessageCodec.decodeMessage * @return true if the MessageCodec wants the ByteBuffer parameter to decodeMessage to be a direct * ByteBuffer. */ @@ -36,8 +37,10 @@ public interface MessageCodec { * Decodes the specified message from binary. * *

Warning: The ByteBuffer may be `direct` if `wantsDirectByteBufferForDecoding` returns - * `true. Do not retain references to the ByteBuffer without checking `ByteBuffer.isDirect()`. See - * also: https://docs.oracle.com/javase/7/docs/api/java/nio/ByteBuffer.html + * `true`. If the ByteBuffer is direct it won't be valid beyond this call and may lead to crashes + * if used beyond this call. If you want to retain a copy of the data; disable the direct + * ByteBuffer or make a copy of the data in your `decodeMessage`. See also: + * https://docs.oracle.com/javase/7/docs/api/java/nio/ByteBuffer.html * * @param message the {@link ByteBuffer} message, possibly null. * @return a T value representation of the bytes between the given buffer's current position and diff --git a/testing/run_tests.py b/testing/run_tests.py index 9d9cf1cf4de5e..4f5226d5f539e 100755 --- a/testing/run_tests.py +++ b/testing/run_tests.py @@ -367,10 +367,10 @@ def AssertExpectedJavaVersion(): """Checks that the user has Java 8 which is the supported Java version for Android""" EXPECTED_VERSION = '1.8' # `java -version` is output to stderr. https://bugs.java.com/bugdatabase/view_bug.do?bug_id=4380614 - # version_output = subprocess.check_output(['java', '-version'], stderr=subprocess.STDOUT) - # match = bool(re.compile('version "%s' % EXPECTED_VERSION).search(version_output)) - # message = "JUnit tests need to be run with Java %s. Check the `java -version` on your PATH." % EXPECTED_VERSION - # assert match, message + version_output = subprocess.check_output(['java', '-version'], stderr=subprocess.STDOUT) + match = bool(re.compile('version "%s' % EXPECTED_VERSION).search(version_output)) + message = "JUnit tests need to be run with Java %s. Check the `java -version` on your PATH." % EXPECTED_VERSION + assert match, message def AssertExpectedXcodeVersion(): From 836cecbc3c0600cbaa4945a2ea0464f8b8fccb01 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Tue, 25 May 2021 16:51:13 -0700 Subject: [PATCH 12/19] removed storing the copy preference in the c++ layer --- .../platform/android/android_shell_holder.cc | 13 ---- shell/platform/android/android_shell_holder.h | 6 -- .../flutter/embedding/engine/FlutterJNI.java | 26 ------- .../embedding/engine/dart/DartMessenger.java | 39 +++++++---- shell/platform/android/jni/jni_mock.h | 10 --- .../android/jni/platform_view_android_jni.h | 7 -- .../android/platform_view_android_jni_impl.cc | 70 +------------------ .../android/platform_view_android_jni_impl.h | 10 --- .../engine/dart/DartMessengerTest.java | 35 ---------- 9 files changed, 29 insertions(+), 187 deletions(-) diff --git a/shell/platform/android/android_shell_holder.cc b/shell/platform/android/android_shell_holder.cc index 590cbcad003d2..60cb633e2f27f 100644 --- a/shell/platform/android/android_shell_holder.cc +++ b/shell/platform/android/android_shell_holder.cc @@ -303,17 +303,4 @@ std::optional AndroidShellHolder::BuildRunConfiguration( } return config; } - -void AndroidShellHolder::ClearDirectByteBufferDecodingPreference( - const std::string& channel) { - jni_facade_->ClearDirectByteBufferDecodingPreference(channel); -} - -void AndroidShellHolder::SetDirectByteBufferDecodingPreference( - const std::string& channel, - bool wants_direct_byte_buffer_for_decoding) { - jni_facade_->SetDirectByteBufferDecodingPreference( - channel, wants_direct_byte_buffer_for_decoding); -} - } // namespace flutter diff --git a/shell/platform/android/android_shell_holder.h b/shell/platform/android/android_shell_holder.h index 62daed3676827..39f0274fd49b5 100644 --- a/shell/platform/android/android_shell_holder.h +++ b/shell/platform/android/android_shell_holder.h @@ -96,12 +96,6 @@ class AndroidShellHolder { void NotifyLowMemoryWarning(); - void ClearDirectByteBufferDecodingPreference(const std::string& channel); - - void SetDirectByteBufferDecodingPreference( - const std::string& channel, - bool wants_direct_byte_buffer_for_decoding); - private: const flutter::Settings settings_; const std::shared_ptr jni_facade_; diff --git a/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java b/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java index c8e6e9552dcdb..6b817e35aa565 100644 --- a/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java +++ b/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java @@ -822,32 +822,6 @@ public void handlePlatformMessage( // (https://github.com/flutter/flutter/issues/25391) } - @SuppressWarnings("unused") - @VisibleForTesting - public void handleIndirectPlatformMessage( - @NonNull final String channel, ByteBuffer message, final int replyId) { - ByteBuffer indirectByteBuffer = ByteBuffer.allocate(message.capacity()); - indirectByteBuffer.put(message); - indirectByteBuffer.rewind(); - handlePlatformMessage(channel, indirectByteBuffer, replyId); - } - - private native void nativeClearDirectByteBufferDecodingPreference( - long nativeShellHolderId, String channel); - - public void clearDirectByteBufferDecodingPreference(@NonNull String channel) { - nativeClearDirectByteBufferDecodingPreference(nativeShellHolderId, channel); - } - - private native void nativeSetDirectByteBufferDecodingPreference( - long nativeShellHolderId, String channel, boolean wantsDirectByteBufferForDecoding); - - public void setDirectByteBufferDecodingPreference( - @NonNull String channel, boolean wantsDirectByteBufferForDecoding) { - nativeSetDirectByteBufferDecodingPreference( - nativeShellHolderId, channel, wantsDirectByteBufferForDecoding); - } - // Called by native to respond to a platform message that we sent. // TODO(mattcarroll): determine if reply is nonull or nullable @SuppressWarnings("unused") diff --git a/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java b/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java index b1958a49af557..55e6351b06510 100644 --- a/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java +++ b/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java @@ -26,7 +26,7 @@ class DartMessenger implements BinaryMessenger, PlatformMessageHandler { private static final String TAG = "DartMessenger"; @NonNull private final FlutterJNI flutterJNI; - @NonNull private final Map messageHandlers; + @NonNull private final Map messageHandlers; @NonNull private final Map pendingReplies; private int nextReplyId = 1; @@ -36,6 +36,17 @@ class DartMessenger implements BinaryMessenger, PlatformMessageHandler { this.pendingReplies = new HashMap<>(); } + private static class Handler { + public final BinaryMessenger.BinaryMessageHandler binaryMessageHandler; + public final boolean wantsDirectByteBufferForDecoding; + + Handler( + BinaryMessenger.BinaryMessageHandler handler, boolean wantsDirectByteBufferForDecoding) { + this.binaryMessageHandler = handler; + this.wantsDirectByteBufferForDecoding = wantsDirectByteBufferForDecoding; + } + } + @Override public void setMessageHandler( @NonNull String channel, @@ -43,18 +54,10 @@ public void setMessageHandler( boolean wantsDirectByteBufferForDecoding) { if (handler == null) { Log.v(TAG, "Removing handler for channel '" + channel + "'"); - flutterJNI.clearDirectByteBufferDecodingPreference(channel); messageHandlers.remove(channel); } else { Log.v(TAG, "Setting handler for channel '" + channel + "'"); - if (!wantsDirectByteBufferForDecoding) { - if (!this.flutterJNI.isAttached()) { - throw new IllegalStateException( - "Only direct ByteBuffers are supported for channels whose message handlers are setup before jni is attached."); - } - flutterJNI.setDirectByteBufferDecodingPreference(channel, wantsDirectByteBufferForDecoding); - } - messageHandlers.put(channel, handler); + messageHandlers.put(channel, new Handler(handler, wantsDirectByteBufferForDecoding)); } } @@ -87,11 +90,23 @@ public void send( public void handleMessageFromDart( @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); + Handler handler = messageHandlers.get(channel); if (handler != null) { try { Log.v(TAG, "Deferring to registered handler to process message."); - handler.onMessage(message, new Reply(flutterJNI, replyId)); + ByteBuffer messageWithDirectness; + if (handler.wantsDirectByteBufferForDecoding) { + messageWithDirectness = message; + } else if (message != null) { + ByteBuffer indirectByteBuffer = ByteBuffer.allocate(message.capacity()); + indirectByteBuffer.put(message); + indirectByteBuffer.rewind(); + messageWithDirectness = indirectByteBuffer; + } else { + messageWithDirectness = null; + } + handler.binaryMessageHandler.onMessage( + messageWithDirectness, new Reply(flutterJNI, replyId)); } catch (Exception ex) { Log.e(TAG, "Uncaught exception in binary message listener", ex); flutterJNI.invokePlatformMessageEmptyResponseCallback(replyId); diff --git a/shell/platform/android/jni/jni_mock.h b/shell/platform/android/jni/jni_mock.h index df32c6a6da1af..1a31db796a3ce 100644 --- a/shell/platform/android/jni/jni_mock.h +++ b/shell/platform/android/jni/jni_mock.h @@ -101,16 +101,6 @@ class JNIMock final : public PlatformViewAndroidJNI { RequestDartDeferredLibrary, (int loading_unit_id), (override)); - - MOCK_METHOD(void, - ClearDirectByteBufferDecodingPreference, - (const std::string& channel), - (override)); - - MOCK_METHOD(void, - SetDirectByteBufferDecodingPreference, - (const std::string&, bool), - (override)); }; } // namespace flutter diff --git a/shell/platform/android/jni/platform_view_android_jni.h b/shell/platform/android/jni/platform_view_android_jni.h index 702f45c58109f..eae098a6b8871 100644 --- a/shell/platform/android/jni/platform_view_android_jni.h +++ b/shell/platform/android/jni/platform_view_android_jni.h @@ -197,13 +197,6 @@ class PlatformViewAndroidJNI { virtual double GetDisplayRefreshRate() = 0; virtual bool RequestDartDeferredLibrary(int loading_unit_id) = 0; - - virtual void ClearDirectByteBufferDecodingPreference( - const std::string& channel) = 0; - - virtual void SetDirectByteBufferDecodingPreference( - const std::string& channel, - bool wants_direct_byte_buffer_for_decoding) = 0; }; } // namespace flutter diff --git a/shell/platform/android/platform_view_android_jni_impl.cc b/shell/platform/android/platform_view_android_jni_impl.cc index ae6feef9fd464..dc0cc7ec39836 100644 --- a/shell/platform/android/platform_view_android_jni_impl.cc +++ b/shell/platform/android/platform_view_android_jni_impl.cc @@ -85,8 +85,6 @@ static jmethodID g_long_constructor = nullptr; static jmethodID g_handle_platform_message_method = nullptr; -static jmethodID g_handle_indirect_platform_message_method = nullptr; - static jmethodID g_handle_platform_message_response_method = nullptr; static jmethodID g_update_semantics_method = nullptr; @@ -489,26 +487,6 @@ static void UnregisterTexture(JNIEnv* env, static_cast(texture_id)); } -static void ClearDirectByteBufferDecodingPreference(JNIEnv* env, - jobject jcaller, - jlong shell_holder, - jstring jchannel) { - auto channel = fml::jni::JavaStringToString(env, jchannel); - ANDROID_SHELL_HOLDER->ClearDirectByteBufferDecodingPreference(channel); -} - -static void SetDirectByteBufferDecodingPreference( - JNIEnv* env, - jobject jcaller, - jlong shell_holder, - jstring jchannel, - jboolean wants_direct_byte_buffer_for_decoding) { - auto channel = fml::jni::JavaStringToString(env, jchannel); - ANDROID_SHELL_HOLDER->SetDirectByteBufferDecodingPreference( - channel, // - wants_direct_byte_buffer_for_decoding); -} - static void InvokePlatformMessageResponseCallback(JNIEnv* env, jobject jcaller, jlong shell_holder, @@ -674,18 +652,6 @@ bool RegisterApi(JNIEnv* env) { .signature = "(JLjava/lang/String;Ljava/nio/ByteBuffer;II)V", .fnPtr = reinterpret_cast(&DispatchPlatformMessage), }, - { - .name = "nativeClearDirectByteBufferDecodingPreference", - .signature = "(JLjava/lang/String;)V", - .fnPtr = - reinterpret_cast(&ClearDirectByteBufferDecodingPreference), - }, - { - .name = "nativeSetDirectByteBufferDecodingPreference", - .signature = "(JLjava/lang/String;Z)V", - .fnPtr = - reinterpret_cast(&SetDirectByteBufferDecodingPreference), - }, { .name = "nativeInvokePlatformMessageResponseCallback", .signature = "(JILjava/nio/ByteBuffer;I)V", @@ -860,15 +826,6 @@ bool RegisterApi(JNIEnv* env) { return false; } - g_handle_indirect_platform_message_method = env->GetMethodID( - g_flutter_jni_class->obj(), "handleIndirectPlatformMessage", - "(Ljava/lang/String;Ljava/nio/ByteBuffer;I)V"); - - if (g_handle_indirect_platform_message_method == nullptr) { - FML_LOG(ERROR) << "Could not locate handleIndirectPlatformMessage method"; - return false; - } - g_handle_platform_message_method = env->GetMethodID(g_flutter_jni_class->obj(), "handlePlatformMessage", "(Ljava/lang/String;Ljava/nio/ByteBuffer;I)V"); @@ -1137,21 +1094,6 @@ PlatformViewAndroidJNIImpl::PlatformViewAndroidJNIImpl( PlatformViewAndroidJNIImpl::~PlatformViewAndroidJNIImpl() = default; -void PlatformViewAndroidJNIImpl::ClearDirectByteBufferDecodingPreference( - const std::string& channel) { - channels_with_indirect_byte_buffer_decoding_.erase(channel); -} - -void PlatformViewAndroidJNIImpl::SetDirectByteBufferDecodingPreference( - const std::string& channel, - bool wants_direct_byte_buffer_for_decoding) { - if (wants_direct_byte_buffer_for_decoding) { - channels_with_indirect_byte_buffer_decoding_.erase(channel); - } else { - channels_with_indirect_byte_buffer_decoding_.insert(channel); - } -} - void PlatformViewAndroidJNIImpl::FlutterViewHandlePlatformMessage( std::unique_ptr message, int responseId) { @@ -1170,16 +1112,8 @@ void PlatformViewAndroidJNIImpl::FlutterViewHandlePlatformMessage( env, env->NewDirectByteBuffer( const_cast(message->data().GetMapping()), message->data().GetSize())); - if (channels_with_indirect_byte_buffer_decoding_.empty() || - channels_with_indirect_byte_buffer_decoding_.find(message->channel()) == - channels_with_indirect_byte_buffer_decoding_.end()) { - env->CallVoidMethod(java_object.obj(), g_handle_platform_message_method, - java_channel.obj(), message_array.obj(), responseId); - } else { - env->CallVoidMethod(java_object.obj(), - g_handle_indirect_platform_message_method, - java_channel.obj(), message_array.obj(), responseId); - } + env->CallVoidMethod(java_object.obj(), g_handle_platform_message_method, + java_channel.obj(), message_array.obj(), responseId); } else { env->CallVoidMethod(java_object.obj(), g_handle_platform_message_method, java_channel.obj(), nullptr, responseId); diff --git a/shell/platform/android/platform_view_android_jni_impl.h b/shell/platform/android/platform_view_android_jni_impl.h index ad1f13af21e6d..34b80f07c6297 100644 --- a/shell/platform/android/platform_view_android_jni_impl.h +++ b/shell/platform/android/platform_view_android_jni_impl.h @@ -5,8 +5,6 @@ #ifndef FLUTTER_SHELL_PLATFORM_ANDROID_PLATFORM_VIEW_ANDROID_JNI_IMPL_H_ #define FLUTTER_SHELL_PLATFORM_ANDROID_PLATFORM_VIEW_ANDROID_JNI_IMPL_H_ -#include - #include "flutter/fml/platform/android/jni_weak_ref.h" #include "flutter/shell/platform/android/jni/platform_view_android_jni.h" @@ -84,17 +82,9 @@ class PlatformViewAndroidJNIImpl final : public PlatformViewAndroidJNI { bool RequestDartDeferredLibrary(int loading_unit_id) override; - void ClearDirectByteBufferDecodingPreference( - const std::string& channel) override; - - void SetDirectByteBufferDecodingPreference( - const std::string& channel, - bool wants_direct_byte_buffer_for_decoding) override; - private: // Reference to FlutterJNI object. const fml::jni::JavaObjectWeakGlobalRef java_object_; - std::set channels_with_indirect_byte_buffer_decoding_; FML_DISALLOW_COPY_AND_ASSIGN(PlatformViewAndroidJNIImpl); }; diff --git a/shell/platform/android/test/io/flutter/embedding/engine/dart/DartMessengerTest.java b/shell/platform/android/test/io/flutter/embedding/engine/dart/DartMessengerTest.java index 21b8f74b89eac..aa6cd370467fc 100644 --- a/shell/platform/android/test/io/flutter/embedding/engine/dart/DartMessengerTest.java +++ b/shell/platform/android/test/io/flutter/embedding/engine/dart/DartMessengerTest.java @@ -4,12 +4,8 @@ import static junit.framework.TestCase.assertTrue; import static org.mockito.Matchers.any; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.never; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; 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; @@ -55,35 +51,4 @@ public void itHandlesErrors() { assertTrue(reportingHandler.latestException instanceof AssertionError); currentThread.setUncaughtExceptionHandler(savedHandler); } - - @Test - public void setsDirectByteBuffer() { - final FlutterJNI fakeFlutterJni = mock(FlutterJNI.class); - final DartMessenger messenger = new DartMessenger(fakeFlutterJni); - final String channel = "foo"; - final BinaryMessenger.BinaryMessageHandler handler = (byteBuffer, reply) -> {}; - messenger.setMessageHandler(channel, handler, true); - verify(fakeFlutterJni, never()) - .setDirectByteBufferDecodingPreference(Mockito.anyString(), Mockito.anyBoolean()); - } - - @Test - public void setsIndirectByteBuffer() { - final FlutterJNI fakeFlutterJni = mock(FlutterJNI.class); - final DartMessenger messenger = new DartMessenger(fakeFlutterJni); - final String channel = "foo"; - final BinaryMessenger.BinaryMessageHandler handler = (byteBuffer, reply) -> {}; - when(fakeFlutterJni.isAttached()).thenReturn(true); - messenger.setMessageHandler(channel, handler, false); - verify(fakeFlutterJni).setDirectByteBufferDecodingPreference(channel, false); - } - - @Test - public void clearsDirectByteBuffer() { - final FlutterJNI fakeFlutterJni = mock(FlutterJNI.class); - final DartMessenger messenger = new DartMessenger(fakeFlutterJni); - final String channel = "foo"; - messenger.setMessageHandler(channel, null, true); - verify(fakeFlutterJni).clearDirectByteBufferDecodingPreference(channel); - } } From e988da07190d9bda74b5a8a3edaf9fc4c184b780 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Wed, 26 May 2021 13:00:37 -0700 Subject: [PATCH 13/19] started calling setLimit(0) after we pass the direct ByteBuffer to --- .../io/flutter/embedding/engine/dart/DartMessenger.java | 5 +++++ .../android/io/flutter/plugin/common/MessageCodec.java | 6 +++--- testing/run_tests.py | 8 ++++---- 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java b/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java index 55e6351b06510..1a24cc0814a60 100644 --- a/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java +++ b/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java @@ -107,6 +107,11 @@ public void handleMessageFromDart( } handler.binaryMessageHandler.onMessage( messageWithDirectness, new Reply(flutterJNI, replyId)); + if (messageWithDirectness != null && messageWithDirectness.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. + messageWithDirectness.limit(0); + } } catch (Exception ex) { Log.e(TAG, "Uncaught exception in binary message listener", ex); flutterJNI.invokePlatformMessageEmptyResponseCallback(replyId); diff --git a/shell/platform/android/io/flutter/plugin/common/MessageCodec.java b/shell/platform/android/io/flutter/plugin/common/MessageCodec.java index 3299a1f673504..dfa355256d3c9 100644 --- a/shell/platform/android/io/flutter/plugin/common/MessageCodec.java +++ b/shell/platform/android/io/flutter/plugin/common/MessageCodec.java @@ -37,9 +37,9 @@ public interface MessageCodec { * Decodes the specified message from binary. * *

Warning: The ByteBuffer may be `direct` if `wantsDirectByteBufferForDecoding` returns - * `true`. If the ByteBuffer is direct it won't be valid beyond this call and may lead to crashes - * if used beyond this call. If you want to retain a copy of the data; disable the direct - * ByteBuffer or make a copy of the data in your `decodeMessage`. See also: + * `true`. If the ByteBuffer is direct it won't be valid beyond this call and will lead to a + * `BufferUnderflowException` if accessed. If you want to retain a copy of the data; disable the + * direct ByteBuffer or make a copy of the data in your `decodeMessage`. See also: * https://docs.oracle.com/javase/7/docs/api/java/nio/ByteBuffer.html * * @param message the {@link ByteBuffer} message, possibly null. diff --git a/testing/run_tests.py b/testing/run_tests.py index 4f5226d5f539e..9d9cf1cf4de5e 100755 --- a/testing/run_tests.py +++ b/testing/run_tests.py @@ -367,10 +367,10 @@ def AssertExpectedJavaVersion(): """Checks that the user has Java 8 which is the supported Java version for Android""" EXPECTED_VERSION = '1.8' # `java -version` is output to stderr. https://bugs.java.com/bugdatabase/view_bug.do?bug_id=4380614 - version_output = subprocess.check_output(['java', '-version'], stderr=subprocess.STDOUT) - match = bool(re.compile('version "%s' % EXPECTED_VERSION).search(version_output)) - message = "JUnit tests need to be run with Java %s. Check the `java -version` on your PATH." % EXPECTED_VERSION - assert match, message + # version_output = subprocess.check_output(['java', '-version'], stderr=subprocess.STDOUT) + # match = bool(re.compile('version "%s' % EXPECTED_VERSION).search(version_output)) + # message = "JUnit tests need to be run with Java %s. Check the `java -version` on your PATH." % EXPECTED_VERSION + # assert match, message def AssertExpectedXcodeVersion(): From 31831d3b673173a344595f514fbf6884adbdcfc3 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Wed, 26 May 2021 14:14:53 -0700 Subject: [PATCH 14/19] added new tests for DartMessenger --- .../engine/dart/DartMessengerTest.java | 73 +++++++++++++++++++ testing/run_tests.py | 8 +- 2 files changed, 77 insertions(+), 4 deletions(-) diff --git a/shell/platform/android/test/io/flutter/embedding/engine/dart/DartMessengerTest.java b/shell/platform/android/test/io/flutter/embedding/engine/dart/DartMessengerTest.java index aa6cd370467fc..fed571e48a79e 100644 --- a/shell/platform/android/test/io/flutter/embedding/engine/dart/DartMessengerTest.java +++ b/shell/platform/android/test/io/flutter/embedding/engine/dart/DartMessengerTest.java @@ -1,11 +1,14 @@ package io.flutter.embedding.engine.dart; +import static junit.framework.TestCase.assertEquals; +import static junit.framework.TestCase.assertFalse; 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; @@ -51,4 +54,74 @@ public void itHandlesErrors() { assertTrue(reportingHandler.latestException instanceof AssertionError); currentThread.setUncaughtExceptionHandler(savedHandler); } + + @Test + public void givesIndirectByteBuffer() { + // Setup test. + final FlutterJNI fakeFlutterJni = mock(FlutterJNI.class); + final DartMessenger messenger = new DartMessenger(fakeFlutterJni); + final String channel = "foobar"; + final boolean[] wasDirect = {true}; + final BinaryMessenger.BinaryMessageHandler handler = + (message, reply) -> { + wasDirect[0] = message.isDirect(); + }; + messenger.setMessageHandler(channel, handler, /*wantsDirectByteBufferForDecoding=*/ false); + 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); + assertFalse(wasDirect[0]); + } + + @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, /*wantsDirectByteBufferForDecoding=*/ true); + 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, /*wantsDirectByteBufferForDecoding=*/ true); + 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()); + } } diff --git a/testing/run_tests.py b/testing/run_tests.py index 9d9cf1cf4de5e..4f5226d5f539e 100755 --- a/testing/run_tests.py +++ b/testing/run_tests.py @@ -367,10 +367,10 @@ def AssertExpectedJavaVersion(): """Checks that the user has Java 8 which is the supported Java version for Android""" EXPECTED_VERSION = '1.8' # `java -version` is output to stderr. https://bugs.java.com/bugdatabase/view_bug.do?bug_id=4380614 - # version_output = subprocess.check_output(['java', '-version'], stderr=subprocess.STDOUT) - # match = bool(re.compile('version "%s' % EXPECTED_VERSION).search(version_output)) - # message = "JUnit tests need to be run with Java %s. Check the `java -version` on your PATH." % EXPECTED_VERSION - # assert match, message + version_output = subprocess.check_output(['java', '-version'], stderr=subprocess.STDOUT) + match = bool(re.compile('version "%s' % EXPECTED_VERSION).search(version_output)) + message = "JUnit tests need to be run with Java %s. Check the `java -version` on your PATH." % EXPECTED_VERSION + assert match, message def AssertExpectedXcodeVersion(): From 6837d585e7cd5b11a5ef1e57dc146801b9b814a7 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Wed, 26 May 2021 16:55:35 -0700 Subject: [PATCH 15/19] renamed messageWithDirectness --- .../embedding/engine/dart/DartMessenger.java | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java b/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java index 1a24cc0814a60..463b0d4f08033 100644 --- a/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java +++ b/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java @@ -88,29 +88,29 @@ public void send( @Override public void handleMessageFromDart( - @NonNull final String channel, @Nullable ByteBuffer message, final int replyId) { + @NonNull final String channel, @Nullable ByteBuffer rawMessage, final int replyId) { Log.v(TAG, "Received message from Dart over channel '" + channel + "'"); Handler handler = messageHandlers.get(channel); if (handler != null) { try { Log.v(TAG, "Deferring to registered handler to process message."); - ByteBuffer messageWithDirectness; + ByteBuffer message; if (handler.wantsDirectByteBufferForDecoding) { - messageWithDirectness = message; - } else if (message != null) { - ByteBuffer indirectByteBuffer = ByteBuffer.allocate(message.capacity()); - indirectByteBuffer.put(message); + message = rawMessage; + } else if (rawMessage != null) { + ByteBuffer indirectByteBuffer = ByteBuffer.allocate(rawMessage.capacity()); + indirectByteBuffer.put(rawMessage); indirectByteBuffer.rewind(); - messageWithDirectness = indirectByteBuffer; + message = indirectByteBuffer; } else { - messageWithDirectness = null; + message = null; } handler.binaryMessageHandler.onMessage( - messageWithDirectness, new Reply(flutterJNI, replyId)); - if (messageWithDirectness != null && messageWithDirectness.isDirect()) { + 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. - messageWithDirectness.limit(0); + message.limit(0); } } catch (Exception ex) { Log.e(TAG, "Uncaught exception in binary message listener", ex); From 58bc4b1ef2a918fb2797d6a529d067bcc3f659ae Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Wed, 26 May 2021 17:26:31 -0700 Subject: [PATCH 16/19] made the reply also set out the limit value --- .../embedding/engine/dart/DartMessenger.java | 8 +++++-- .../engine/dart/DartMessengerTest.java | 23 +++++++++++++++++++ 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java b/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java index 463b0d4f08033..df76db2fca0dc 100644 --- a/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java +++ b/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java @@ -105,8 +105,7 @@ public void handleMessageFromDart( } else { message = null; } - handler.binaryMessageHandler.onMessage( - message, new Reply(flutterJNI, replyId)); + handler.binaryMessageHandler.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. @@ -132,6 +131,11 @@ public void handlePlatformMessageResponse(int replyId, @Nullable ByteBuffer repl try { Log.v(TAG, "Invoking registered callback for reply from Dart."); 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) { diff --git a/shell/platform/android/test/io/flutter/embedding/engine/dart/DartMessengerTest.java b/shell/platform/android/test/io/flutter/embedding/engine/dart/DartMessengerTest.java index fed571e48a79e..6e1c83ed2fe4d 100644 --- a/shell/platform/android/test/io/flutter/embedding/engine/dart/DartMessengerTest.java +++ b/shell/platform/android/test/io/flutter/embedding/engine/dart/DartMessengerTest.java @@ -124,4 +124,27 @@ public void directByteBufferLimitZeroAfterUsage() { 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()); + } } From b95abddc00660d8e967455fa333b9b07ab101809 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Wed, 26 May 2021 17:33:14 -0700 Subject: [PATCH 17/19] removed some old code --- .../plugin/platform/PlatformViewsControllerTest.java | 7 ------- 1 file changed, 7 deletions(-) diff --git a/shell/platform/android/test/io/flutter/plugin/platform/PlatformViewsControllerTest.java b/shell/platform/android/test/io/flutter/plugin/platform/PlatformViewsControllerTest.java index bd75a234e456d..12db03678b61c 100644 --- a/shell/platform/android/test/io/flutter/plugin/platform/PlatformViewsControllerTest.java +++ b/shell/platform/android/test/io/flutter/plugin/platform/PlatformViewsControllerTest.java @@ -758,13 +758,6 @@ public void invokePlatformMessageResponseCallback( public static SparseArray getResponses() { return replies; } - - @Implementation - public void clearDirectByteBufferDecodingPreference(String channel) {} - - @Implementation - public void setDirectByteBufferDecodingPreference( - String channel, boolean wantsDirectByteBufferForDecoding) {} } @Implements(SurfaceView.class) From 81d632fe4804a7779dfc812c2b9e5c9b4a1b36d7 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Wed, 26 May 2021 17:48:32 -0700 Subject: [PATCH 18/19] removed wantsDirectByteBufferForDecoding --- .../embedding/engine/dart/DartExecutor.java | 17 +++------ .../embedding/engine/dart/DartMessenger.java | 36 ++++--------------- .../plugin/common/BasicMessageChannel.java | 5 +-- .../io/flutter/plugin/common/BinaryCodec.java | 26 +++++++------- .../plugin/common/BinaryMessenger.java | 7 +--- .../flutter/plugin/common/EventChannel.java | 4 +-- .../plugin/common/JSONMessageCodec.java | 5 --- .../flutter/plugin/common/MessageCodec.java | 10 ------ .../flutter/plugin/common/MethodChannel.java | 4 +-- .../plugin/common/StandardMessageCodec.java | 5 --- .../io/flutter/plugin/common/StringCodec.java | 5 --- .../io/flutter/view/FlutterNativeView.java | 7 ++-- .../android/io/flutter/view/FlutterView.java | 5 ++- .../engine/dart/DartMessengerTest.java | 28 ++------------- .../plugin/editing/TextInputPluginTest.java | 6 ++-- .../scenarios/EngineLaunchE2ETest.java | 3 +- .../scenarios/SpawnedEngineActivity.java | 3 +- .../scenarios/TestableFlutterActivity.java | 3 +- .../scenarios/TextPlatformViewFactory.java | 5 --- 19 files changed, 42 insertions(+), 142 deletions(-) diff --git a/shell/platform/android/io/flutter/embedding/engine/dart/DartExecutor.java b/shell/platform/android/io/flutter/embedding/engine/dart/DartExecutor.java index 5f286a2feb8b8..e7aabf7c6ad0f 100644 --- a/shell/platform/android/io/flutter/embedding/engine/dart/DartExecutor.java +++ b/shell/platform/android/io/flutter/embedding/engine/dart/DartExecutor.java @@ -59,10 +59,7 @@ public DartExecutor(@NonNull FlutterJNI flutterJNI, @NonNull AssetManager assetM this.flutterJNI = flutterJNI; this.assetManager = assetManager; this.dartMessenger = new DartMessenger(flutterJNI); - dartMessenger.setMessageHandler( - "flutter/isolate", - isolateChannelMessageHandler, - /*wantsDirectByteBufferForDecoding=*/ true); + dartMessenger.setMessageHandler("flutter/isolate", isolateChannelMessageHandler); this.binaryMessenger = new DefaultBinaryMessenger(dartMessenger); // The JNI might already be attached if coming from a spawned engine. If so, correctly report // that this DartExecutor is already running. @@ -195,10 +192,8 @@ public void send( @Override @UiThread public void setMessageHandler( - @NonNull String channel, - @Nullable BinaryMessenger.BinaryMessageHandler handler, - boolean wantsDirectByteBufferForDecoding) { - binaryMessenger.setMessageHandler(channel, handler, wantsDirectByteBufferForDecoding); + @NonNull String channel, @Nullable BinaryMessenger.BinaryMessageHandler handler) { + binaryMessenger.setMessageHandler(channel, handler); } // ------ END BinaryMessenger ----- @@ -418,10 +413,8 @@ public void send( @Override @UiThread public void setMessageHandler( - @NonNull String channel, - @Nullable BinaryMessenger.BinaryMessageHandler handler, - boolean wantsDirectByteBufferForDecoding) { - messenger.setMessageHandler(channel, handler, wantsDirectByteBufferForDecoding); + @NonNull String channel, @Nullable BinaryMessenger.BinaryMessageHandler handler) { + messenger.setMessageHandler(channel, handler); } } } diff --git a/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java b/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java index df76db2fca0dc..8a5e44f637a1a 100644 --- a/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java +++ b/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java @@ -26,7 +26,7 @@ class DartMessenger implements BinaryMessenger, PlatformMessageHandler { private static final String TAG = "DartMessenger"; @NonNull private final FlutterJNI flutterJNI; - @NonNull private final Map messageHandlers; + @NonNull private final Map messageHandlers; @NonNull private final Map pendingReplies; private int nextReplyId = 1; @@ -36,28 +36,15 @@ class DartMessenger implements BinaryMessenger, PlatformMessageHandler { this.pendingReplies = new HashMap<>(); } - private static class Handler { - public final BinaryMessenger.BinaryMessageHandler binaryMessageHandler; - public final boolean wantsDirectByteBufferForDecoding; - - Handler( - BinaryMessenger.BinaryMessageHandler handler, boolean wantsDirectByteBufferForDecoding) { - this.binaryMessageHandler = handler; - this.wantsDirectByteBufferForDecoding = wantsDirectByteBufferForDecoding; - } - } - @Override public void setMessageHandler( - @NonNull String channel, - @Nullable BinaryMessenger.BinaryMessageHandler handler, - boolean wantsDirectByteBufferForDecoding) { + @NonNull String channel, @Nullable BinaryMessenger.BinaryMessageHandler handler) { if (handler == null) { Log.v(TAG, "Removing handler for channel '" + channel + "'"); messageHandlers.remove(channel); } else { Log.v(TAG, "Setting handler for channel '" + channel + "'"); - messageHandlers.put(channel, new Handler(handler, wantsDirectByteBufferForDecoding)); + messageHandlers.put(channel, handler); } } @@ -88,24 +75,13 @@ public void send( @Override public void handleMessageFromDart( - @NonNull final String channel, @Nullable ByteBuffer rawMessage, final int replyId) { + @NonNull final String channel, @Nullable ByteBuffer message, final int replyId) { Log.v(TAG, "Received message from Dart over channel '" + channel + "'"); - Handler handler = messageHandlers.get(channel); + BinaryMessenger.BinaryMessageHandler handler = messageHandlers.get(channel); if (handler != null) { try { Log.v(TAG, "Deferring to registered handler to process message."); - ByteBuffer message; - if (handler.wantsDirectByteBufferForDecoding) { - message = rawMessage; - } else if (rawMessage != null) { - ByteBuffer indirectByteBuffer = ByteBuffer.allocate(rawMessage.capacity()); - indirectByteBuffer.put(rawMessage); - indirectByteBuffer.rewind(); - message = indirectByteBuffer; - } else { - message = null; - } - handler.binaryMessageHandler.onMessage(message, 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. diff --git a/shell/platform/android/io/flutter/plugin/common/BasicMessageChannel.java b/shell/platform/android/io/flutter/plugin/common/BasicMessageChannel.java index f0e9eead514b6..55eb3109af5bb 100644 --- a/shell/platform/android/io/flutter/plugin/common/BasicMessageChannel.java +++ b/shell/platform/android/io/flutter/plugin/common/BasicMessageChannel.java @@ -101,10 +101,7 @@ public void send(@Nullable T message, @Nullable final Reply callback) { */ @UiThread public void setMessageHandler(@Nullable final MessageHandler handler) { - messenger.setMessageHandler( - name, - handler == null ? null : new IncomingMessageHandler(handler), - codec.wantsDirectByteBufferForDecoding()); + messenger.setMessageHandler(name, handler == null ? null : new IncomingMessageHandler(handler)); } /** diff --git a/shell/platform/android/io/flutter/plugin/common/BinaryCodec.java b/shell/platform/android/io/flutter/plugin/common/BinaryCodec.java index 4cc018923e964..39846d94bf9a3 100644 --- a/shell/platform/android/io/flutter/plugin/common/BinaryCodec.java +++ b/shell/platform/android/io/flutter/plugin/common/BinaryCodec.java @@ -25,28 +25,23 @@ public final class BinaryCodec implements MessageCodec { */ public static final BinaryCodec INSTANCE_DIRECT = new BinaryCodec(true); - private final boolean wantsDirectByteBufferForDecoding; + private final boolean returnsDirectByteBufferFromDecoding; private BinaryCodec() { - this.wantsDirectByteBufferForDecoding = false; + this.returnsDirectByteBufferFromDecoding = false; } /** * A constructor for BinaryCodec. * - * @param wantsDirectByteBufferForDecoding `true` means that Flutter will send direct ByteBuffers - * to `decodeMessage`. Direct ByteBuffers will have better performance but will be invalid + * @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 wantsDirectByteBufferForDecoding) { - this.wantsDirectByteBufferForDecoding = wantsDirectByteBufferForDecoding; - } - - @Override - public boolean wantsDirectByteBufferForDecoding() { - return wantsDirectByteBufferForDecoding; + private BinaryCodec(boolean returnsDirectByteBufferFromDecoding) { + this.returnsDirectByteBufferFromDecoding = returnsDirectByteBufferFromDecoding; } @Override @@ -56,6 +51,13 @@ public ByteBuffer encodeMessage(ByteBuffer message) { @Override public ByteBuffer decodeMessage(ByteBuffer message) { - return message; + if (returnsDirectByteBufferFromDecoding) { + return message; + } else { + ByteBuffer result = ByteBuffer.allocate(message.capacity()); + result.put(message); + result.rewind(); + return result; + } } } diff --git a/shell/platform/android/io/flutter/plugin/common/BinaryMessenger.java b/shell/platform/android/io/flutter/plugin/common/BinaryMessenger.java index 077daff6aa41b..fb1a22e6f4d3c 100644 --- a/shell/platform/android/io/flutter/plugin/common/BinaryMessenger.java +++ b/shell/platform/android/io/flutter/plugin/common/BinaryMessenger.java @@ -62,14 +62,9 @@ public interface BinaryMessenger { * * @param channel the name {@link String} of the channel. * @param handler a {@link BinaryMessageHandler} to be invoked on incoming messages, or null. - * @param wantsDirectByteBufferForDecoding pass in `true` to make the ByteBuffer parameter to the - * `BinaryMessageHandler` a direct ByteBuffer. */ @UiThread - void setMessageHandler( - @NonNull String channel, - @Nullable BinaryMessageHandler handler, - boolean wantsDirectByteBufferForDecoding); + void setMessageHandler(@NonNull String channel, @Nullable BinaryMessageHandler handler); /** Handler for incoming binary messages from Flutter. */ interface BinaryMessageHandler { diff --git a/shell/platform/android/io/flutter/plugin/common/EventChannel.java b/shell/platform/android/io/flutter/plugin/common/EventChannel.java index c01b10807116e..05dae1c01b3fd 100644 --- a/shell/platform/android/io/flutter/plugin/common/EventChannel.java +++ b/shell/platform/android/io/flutter/plugin/common/EventChannel.java @@ -84,9 +84,7 @@ public EventChannel(BinaryMessenger messenger, String name, MethodCodec codec) { @UiThread public void setStreamHandler(final StreamHandler handler) { messenger.setMessageHandler( - name, - handler == null ? null : new IncomingStreamRequestHandler(handler), - /*wantsDirectByteBufferForDecoding=*/ true); + name, handler == null ? null : new IncomingStreamRequestHandler(handler)); } /** diff --git a/shell/platform/android/io/flutter/plugin/common/JSONMessageCodec.java b/shell/platform/android/io/flutter/plugin/common/JSONMessageCodec.java index cb50c2a5eb06e..c6bf0e668b790 100644 --- a/shell/platform/android/io/flutter/plugin/common/JSONMessageCodec.java +++ b/shell/platform/android/io/flutter/plugin/common/JSONMessageCodec.java @@ -27,11 +27,6 @@ public final class JSONMessageCodec implements MessageCodec { private JSONMessageCodec() {} - @Override - public boolean wantsDirectByteBufferForDecoding() { - return true; - } - @Override public ByteBuffer encodeMessage(Object message) { if (message == null) { diff --git a/shell/platform/android/io/flutter/plugin/common/MessageCodec.java b/shell/platform/android/io/flutter/plugin/common/MessageCodec.java index dfa355256d3c9..bdeec6d96f208 100644 --- a/shell/platform/android/io/flutter/plugin/common/MessageCodec.java +++ b/shell/platform/android/io/flutter/plugin/common/MessageCodec.java @@ -13,16 +13,6 @@ *

Both operations throw {@link IllegalArgumentException}, if conversion fails. */ public interface MessageCodec { - - /** - * Controls the ByteBuffer parameter for `decodeMessage`. - * - * @see MessageCodec.decodeMessage - * @return true if the MessageCodec wants the ByteBuffer parameter to decodeMessage to be a direct - * ByteBuffer. - */ - boolean wantsDirectByteBufferForDecoding(); - /** * Encodes the specified message into binary. * diff --git a/shell/platform/android/io/flutter/plugin/common/MethodChannel.java b/shell/platform/android/io/flutter/plugin/common/MethodChannel.java index 1fd6505c28722..901299b943019 100644 --- a/shell/platform/android/io/flutter/plugin/common/MethodChannel.java +++ b/shell/platform/android/io/flutter/plugin/common/MethodChannel.java @@ -117,9 +117,7 @@ public void invokeMethod(String method, @Nullable Object arguments, @Nullable Re @UiThread public void setMethodCallHandler(final @Nullable MethodCallHandler handler) { messenger.setMessageHandler( - name, - handler == null ? null : new IncomingMethodCallHandler(handler), - /*wantsDirectByteBufferForDecoding=*/ true); + name, handler == null ? null : new IncomingMethodCallHandler(handler)); } /** diff --git a/shell/platform/android/io/flutter/plugin/common/StandardMessageCodec.java b/shell/platform/android/io/flutter/plugin/common/StandardMessageCodec.java index 6ee8a51a0757e..e962fa6b3c0a8 100644 --- a/shell/platform/android/io/flutter/plugin/common/StandardMessageCodec.java +++ b/shell/platform/android/io/flutter/plugin/common/StandardMessageCodec.java @@ -63,11 +63,6 @@ public class StandardMessageCodec implements MessageCodec { private static final String TAG = "StandardMessageCodec#"; public static final StandardMessageCodec INSTANCE = new StandardMessageCodec(); - @Override - public boolean wantsDirectByteBufferForDecoding() { - return true; - } - @Override public ByteBuffer encodeMessage(Object message) { if (message == null) { diff --git a/shell/platform/android/io/flutter/plugin/common/StringCodec.java b/shell/platform/android/io/flutter/plugin/common/StringCodec.java index 21fdffff7bfd7..dfcdbe68bd0fc 100644 --- a/shell/platform/android/io/flutter/plugin/common/StringCodec.java +++ b/shell/platform/android/io/flutter/plugin/common/StringCodec.java @@ -20,11 +20,6 @@ public final class StringCodec implements MessageCodec { private StringCodec() {} - @Override - public boolean wantsDirectByteBufferForDecoding() { - return true; - } - @Override public ByteBuffer encodeMessage(String message) { if (message == null) { diff --git a/shell/platform/android/io/flutter/view/FlutterNativeView.java b/shell/platform/android/io/flutter/view/FlutterNativeView.java index f9b6d431061bf..2b7afda518c1f 100644 --- a/shell/platform/android/io/flutter/view/FlutterNativeView.java +++ b/shell/platform/android/io/flutter/view/FlutterNativeView.java @@ -140,11 +140,8 @@ public void send(String channel, ByteBuffer message, BinaryReply callback) { @Override @UiThread - public void setMessageHandler( - String channel, BinaryMessageHandler handler, boolean wantsDirectByteBufferForDecoding) { - dartExecutor - .getBinaryMessenger() - .setMessageHandler(channel, handler, wantsDirectByteBufferForDecoding); + public void setMessageHandler(String channel, BinaryMessageHandler handler) { + dartExecutor.getBinaryMessenger().setMessageHandler(channel, handler); } /*package*/ FlutterJNI getFlutterJNI() { diff --git a/shell/platform/android/io/flutter/view/FlutterView.java b/shell/platform/android/io/flutter/view/FlutterView.java index 13769ef5e5137..59711aabd7880 100644 --- a/shell/platform/android/io/flutter/view/FlutterView.java +++ b/shell/platform/android/io/flutter/view/FlutterView.java @@ -863,9 +863,8 @@ public void send(String channel, ByteBuffer message, BinaryReply callback) { @Override @UiThread - public void setMessageHandler( - String channel, BinaryMessageHandler handler, boolean wantsDirectByteBufferForDecoding) { - mNativeView.setMessageHandler(channel, handler, wantsDirectByteBufferForDecoding); + public void setMessageHandler(String channel, BinaryMessageHandler handler) { + mNativeView.setMessageHandler(channel, handler); } /** Listener will be called on the Android UI thread once when Flutter renders the first frame. */ diff --git a/shell/platform/android/test/io/flutter/embedding/engine/dart/DartMessengerTest.java b/shell/platform/android/test/io/flutter/embedding/engine/dart/DartMessengerTest.java index 6e1c83ed2fe4d..750225f27e608 100644 --- a/shell/platform/android/test/io/flutter/embedding/engine/dart/DartMessengerTest.java +++ b/shell/platform/android/test/io/flutter/embedding/engine/dart/DartMessengerTest.java @@ -48,35 +48,13 @@ public void itHandlesErrors() { .when(throwingHandler) .onMessage(any(ByteBuffer.class), any(DartMessenger.Reply.class)); - messenger.setMessageHandler("test", throwingHandler, true); + messenger.setMessageHandler("test", throwingHandler); messenger.handleMessageFromDart("test", ByteBuffer.allocate(0), 0); assertNotNull(reportingHandler.latestException); assertTrue(reportingHandler.latestException instanceof AssertionError); currentThread.setUncaughtExceptionHandler(savedHandler); } - @Test - public void givesIndirectByteBuffer() { - // Setup test. - final FlutterJNI fakeFlutterJni = mock(FlutterJNI.class); - final DartMessenger messenger = new DartMessenger(fakeFlutterJni); - final String channel = "foobar"; - final boolean[] wasDirect = {true}; - final BinaryMessenger.BinaryMessageHandler handler = - (message, reply) -> { - wasDirect[0] = message.isDirect(); - }; - messenger.setMessageHandler(channel, handler, /*wantsDirectByteBufferForDecoding=*/ false); - 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); - assertFalse(wasDirect[0]); - } - @Test public void givesDirectByteBuffer() { // Setup test. @@ -88,7 +66,7 @@ public void givesDirectByteBuffer() { (message, reply) -> { wasDirect[0] = message.isDirect(); }; - messenger.setMessageHandler(channel, handler, /*wantsDirectByteBufferForDecoding=*/ true); + messenger.setMessageHandler(channel, handler); final ByteBuffer message = ByteBuffer.allocateDirect(4 * 2); message.rewind(); message.putChar('a'); @@ -112,7 +90,7 @@ public void directByteBufferLimitZeroAfterUsage() { byteBuffers[0] = message; assertEquals(bufferSize, byteBuffers[0].limit()); }; - messenger.setMessageHandler(channel, handler, /*wantsDirectByteBufferForDecoding=*/ true); + messenger.setMessageHandler(channel, handler); final ByteBuffer message = ByteBuffer.allocateDirect(bufferSize); message.rewind(); message.putChar('a'); diff --git a/shell/platform/android/test/io/flutter/plugin/editing/TextInputPluginTest.java b/shell/platform/android/test/io/flutter/plugin/editing/TextInputPluginTest.java index f3c4ec7c87e7e..7e8436418ae96 100644 --- a/shell/platform/android/test/io/flutter/plugin/editing/TextInputPluginTest.java +++ b/shell/platform/android/test/io/flutter/plugin/editing/TextInputPluginTest.java @@ -1000,7 +1000,7 @@ public void respondsToInputChannelMessages() { textInputChannel.setTextInputMethodHandler(mockHandler); verify(mockBinaryMessenger, times(1)) - .setMessageHandler(any(String.class), binaryMessageHandlerCaptor.capture(), eq(true)); + .setMessageHandler(any(String.class), binaryMessageHandlerCaptor.capture()); BinaryMessenger.BinaryMessageHandler binaryMessageHandler = binaryMessageHandlerCaptor.getValue(); @@ -1033,7 +1033,7 @@ public void sendAppPrivateCommand_dataIsEmpty() throws JSONException { new TextInputPlugin(testView, textInputChannel, mock(PlatformViewsController.class)); verify(mockBinaryMessenger, times(1)) - .setMessageHandler(any(String.class), binaryMessageHandlerCaptor.capture(), eq(true)); + .setMessageHandler(any(String.class), binaryMessageHandlerCaptor.capture()); JSONObject arguments = new JSONObject(); arguments.put("action", "actionCommand"); @@ -1064,7 +1064,7 @@ public void sendAppPrivateCommand_hasData() throws JSONException { new TextInputPlugin(testView, textInputChannel, mock(PlatformViewsController.class)); verify(mockBinaryMessenger, times(1)) - .setMessageHandler(any(String.class), binaryMessageHandlerCaptor.capture(), eq(true)); + .setMessageHandler(any(String.class), binaryMessageHandlerCaptor.capture()); JSONObject arguments = new JSONObject(); arguments.put("action", "actionCommand"); diff --git a/testing/scenario_app/android/app/src/androidTest/java/dev/flutter/scenarios/EngineLaunchE2ETest.java b/testing/scenario_app/android/app/src/androidTest/java/dev/flutter/scenarios/EngineLaunchE2ETest.java index 6ca5ede9e83ac..c3e50feba9a2d 100644 --- a/testing/scenario_app/android/app/src/androidTest/java/dev/flutter/scenarios/EngineLaunchE2ETest.java +++ b/testing/scenario_app/android/app/src/androidTest/java/dev/flutter/scenarios/EngineLaunchE2ETest.java @@ -50,8 +50,7 @@ public void smokeTestEngineLaunch() throws Throwable { .getDartExecutor() .setMessageHandler( "waiting_for_status", - (byteBuffer, binaryReply) -> statusReceived.complete(Boolean.TRUE), - true); + (byteBuffer, binaryReply) -> statusReceived.complete(Boolean.TRUE)); // Launching the entrypoint will run the Dart code that sends the "waiting_for_status" platform // message. diff --git a/testing/scenario_app/android/app/src/main/java/dev/flutter/scenarios/SpawnedEngineActivity.java b/testing/scenario_app/android/app/src/main/java/dev/flutter/scenarios/SpawnedEngineActivity.java index 680c0069c5fb9..52ab54b8e3a9e 100644 --- a/testing/scenario_app/android/app/src/main/java/dev/flutter/scenarios/SpawnedEngineActivity.java +++ b/testing/scenario_app/android/app/src/main/java/dev/flutter/scenarios/SpawnedEngineActivity.java @@ -21,8 +21,7 @@ public FlutterEngine provideFlutterEngine(@NonNull Context context) { secondEngine .getDartExecutor() - .setMessageHandler( - "take_screenshot", (byteBuffer, binaryReply) -> notifyFlutterRendered(), true); + .setMessageHandler("take_screenshot", (byteBuffer, binaryReply) -> notifyFlutterRendered()); return secondEngine; } diff --git a/testing/scenario_app/android/app/src/main/java/dev/flutter/scenarios/TestableFlutterActivity.java b/testing/scenario_app/android/app/src/main/java/dev/flutter/scenarios/TestableFlutterActivity.java index 793a16bf8fd49..457d980c5ecae 100644 --- a/testing/scenario_app/android/app/src/main/java/dev/flutter/scenarios/TestableFlutterActivity.java +++ b/testing/scenario_app/android/app/src/main/java/dev/flutter/scenarios/TestableFlutterActivity.java @@ -17,8 +17,7 @@ public void configureFlutterEngine(FlutterEngine flutterEngine) { super.configureFlutterEngine(flutterEngine); flutterEngine .getDartExecutor() - .setMessageHandler( - "take_screenshot", (byteBuffer, binaryReply) -> notifyFlutterRendered(), true); + .setMessageHandler("take_screenshot", (byteBuffer, binaryReply) -> notifyFlutterRendered()); } protected void notifyFlutterRendered() { diff --git a/testing/scenario_app/android/app/src/main/java/dev/flutter/scenarios/TextPlatformViewFactory.java b/testing/scenario_app/android/app/src/main/java/dev/flutter/scenarios/TextPlatformViewFactory.java index 0e29464624364..d5d52b14f7226 100644 --- a/testing/scenario_app/android/app/src/main/java/dev/flutter/scenarios/TextPlatformViewFactory.java +++ b/testing/scenario_app/android/app/src/main/java/dev/flutter/scenarios/TextPlatformViewFactory.java @@ -16,11 +16,6 @@ public final class TextPlatformViewFactory extends PlatformViewFactory { TextPlatformViewFactory() { super( new MessageCodec() { - @Override - public boolean wantsDirectByteBufferForDecoding() { - return true; - } - @Nullable @Override public ByteBuffer encodeMessage(@Nullable Object o) { From 47b0cda5b9b5a2c3c65ea1fa07478cba9fa0284f Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 27 May 2021 09:21:54 -0700 Subject: [PATCH 19/19] updated the docstrings --- .../io/flutter/plugin/common/BinaryCodec.java | 12 ++++++------ .../io/flutter/plugin/common/MessageCodec.java | 8 +++----- .../embedding/engine/dart/DartMessengerTest.java | 1 - 3 files changed, 9 insertions(+), 12 deletions(-) diff --git a/shell/platform/android/io/flutter/plugin/common/BinaryCodec.java b/shell/platform/android/io/flutter/plugin/common/BinaryCodec.java index 39846d94bf9a3..0bcae697f50d1 100644 --- a/shell/platform/android/io/flutter/plugin/common/BinaryCodec.java +++ b/shell/platform/android/io/flutter/plugin/common/BinaryCodec.java @@ -19,7 +19,7 @@ public final class BinaryCodec implements MessageCodec { // 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 calls `decodeMessage` with direct ByteBuffers for better performance. + * A BinaryCodec that returns direct ByteBuffers from `decodeMessage` for better performance. * * @see BinaryCodec.BinaryCodec(boolean) */ @@ -34,11 +34,11 @@ private BinaryCodec() { /** * 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. + * @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; diff --git a/shell/platform/android/io/flutter/plugin/common/MessageCodec.java b/shell/platform/android/io/flutter/plugin/common/MessageCodec.java index bdeec6d96f208..a87214a14f38e 100644 --- a/shell/platform/android/io/flutter/plugin/common/MessageCodec.java +++ b/shell/platform/android/io/flutter/plugin/common/MessageCodec.java @@ -26,11 +26,9 @@ public interface MessageCodec { /** * Decodes the specified message from binary. * - *

Warning: The ByteBuffer may be `direct` if `wantsDirectByteBufferForDecoding` returns - * `true`. If the ByteBuffer is direct it won't be valid beyond this call and will lead to a - * `BufferUnderflowException` if accessed. If you want to retain a copy of the data; disable the - * direct ByteBuffer or make a copy of the data in your `decodeMessage`. See also: - * https://docs.oracle.com/javase/7/docs/api/java/nio/ByteBuffer.html + *

Warning: 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 diff --git a/shell/platform/android/test/io/flutter/embedding/engine/dart/DartMessengerTest.java b/shell/platform/android/test/io/flutter/embedding/engine/dart/DartMessengerTest.java index 750225f27e608..af4008e98c72e 100644 --- a/shell/platform/android/test/io/flutter/embedding/engine/dart/DartMessengerTest.java +++ b/shell/platform/android/test/io/flutter/embedding/engine/dart/DartMessengerTest.java @@ -1,7 +1,6 @@ package io.flutter.embedding.engine.dart; import static junit.framework.TestCase.assertEquals; -import static junit.framework.TestCase.assertFalse; import static junit.framework.TestCase.assertNotNull; import static junit.framework.TestCase.assertTrue; import static org.mockito.Matchers.any;