From 9034cb91b2bbc54af1d8d0a1b9fd674307b59f1d Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Tue, 12 Oct 2021 11:29:16 -0700 Subject: [PATCH 01/21] Implemented background platform channels for Android part 1: moved dispatch to embedder --- shell/common/platform_view.h | 4 +++ shell/common/shell.cc | 20 +++++++++------ .../flutter/embedding/engine/FlutterJNI.java | 13 ++++++++-- .../embedding/engine/dart/DartMessenger.java | 25 +++++++++++++++++-- .../engine/dart/PlatformMessageHandler.java | 5 +++- .../platform/android/platform_view_android.h | 4 +++ .../android/platform_view_android_jni_impl.cc | 22 +++++++++++++--- 7 files changed, 78 insertions(+), 15 deletions(-) diff --git a/shell/common/platform_view.h b/shell/common/platform_view.h index 74baf6d2930ec..274362f34ea39 100644 --- a/shell/common/platform_view.h +++ b/shell/common/platform_view.h @@ -810,6 +810,10 @@ class PlatformView { virtual std::unique_ptr CreateSnapshotSurfaceProducer(); + virtual bool DoesHandlePlatformMessagesOnPlatformThread() const { + return true; + } + protected: PlatformView::Delegate& delegate_; const TaskRunners task_runners_; diff --git a/shell/common/shell.cc b/shell/common/shell.cc index 24061c24210d8..2941ee32580d3 100644 --- a/shell/common/shell.cc +++ b/shell/common/shell.cc @@ -1192,13 +1192,19 @@ void Shell::OnEngineHandlePlatformMessage( return; } - task_runners_.GetPlatformTaskRunner()->PostTask( - fml::MakeCopyable([view = platform_view_->GetWeakPtr(), - message = std::move(message)]() mutable { - if (view) { - view->HandlePlatformMessage(std::move(message)); - } - })); + if (platform_view_->DoesHandlePlatformMessagesOnPlatformThread()) { + task_runners_.GetPlatformTaskRunner()->PostTask( + fml::MakeCopyable([view = platform_view_->GetWeakPtr(), + message = std::move(message)]() mutable { + if (view) { + view->HandlePlatformMessage(std::move(message)); + } + })); + } else { + if (platform_view_) { + platform_view_->HandlePlatformMessage(std::move(message)); + } + } } void Shell::HandleEngineSkiaMessage(std::unique_ptr message) { diff --git a/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java b/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java index 4ac8740641541..e9da03fc7465b 100644 --- a/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java +++ b/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java @@ -858,14 +858,23 @@ public void setPlatformMessageHandler(@Nullable PlatformMessageHandler platformM this.platformMessageHandler = platformMessageHandler; } + private native void nativeCleanupMessageData(long messageData); + + public void cleanupMessageData(long messageData) { + // This doesn't rely on being attached like other methods. + nativeCleanupMessageData(messageData); + } + // Called by native. // TODO(mattcarroll): determine if message is nonull or nullable @SuppressWarnings("unused") @VisibleForTesting public void handlePlatformMessage( - @NonNull final String channel, ByteBuffer message, final int replyId) { + @NonNull final String channel, ByteBuffer message, final int replyId, long messageData) { if (platformMessageHandler != null) { - platformMessageHandler.handleMessageFromDart(channel, message, replyId); + platformMessageHandler.handleMessageFromDart(channel, message, replyId, messageData); + } else { + nativeCleanupMessageData(messageData); } // TODO(mattcarroll): log dropped messages when in debug mode // (https://github.com/flutter/flutter/issues/25391) 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 2822b4060fbcd..19da9599b2d0a 100644 --- a/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java +++ b/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java @@ -4,6 +4,8 @@ package io.flutter.embedding.engine.dart; +import android.os.Handler; +import android.os.Looper; import androidx.annotation.NonNull; import androidx.annotation.Nullable; import androidx.annotation.UiThread; @@ -72,8 +74,7 @@ public void send( } } - @Override - public void handleMessageFromDart( + private void handleMessageFromDartPlatformThread( @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); @@ -98,6 +99,26 @@ public void handleMessageFromDart( } } + @Override + public void handleMessageFromDart( + @NonNull final String channel, + @Nullable ByteBuffer message, + final int replyId, + long messageData) { + // TODO(gaaclarke): Dispatch to a TaskQueue other than the platform channel if specified by the + // handler. + Handler mainHandler = new Handler(Looper.getMainLooper()); + Runnable myRunnable = + () -> { + try { + handleMessageFromDartPlatformThread(channel, message, replyId); + } finally { + flutterJNI.cleanupMessageData(messageData); + } + }; + mainHandler.post(myRunnable); + } + @Override public void handlePlatformMessageResponse(int replyId, @Nullable ByteBuffer reply) { Log.v(TAG, "Received message reply from Dart."); 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 22bb6f195666f..36a7a9d7eb52c 100644 --- a/shell/platform/android/io/flutter/embedding/engine/dart/PlatformMessageHandler.java +++ b/shell/platform/android/io/flutter/embedding/engine/dart/PlatformMessageHandler.java @@ -11,7 +11,10 @@ /** Handler that receives messages from Dart code. */ public interface PlatformMessageHandler { void handleMessageFromDart( - @NonNull final String channel, @Nullable ByteBuffer message, final int replyId); + @NonNull final String channel, + @Nullable ByteBuffer message, + final int replyId, + long messageData); void handlePlatformMessageResponse(int replyId, @Nullable ByteBuffer reply); } diff --git a/shell/platform/android/platform_view_android.h b/shell/platform/android/platform_view_android.h index 1bd959ef855dd..79f39ba0b71d9 100644 --- a/shell/platform/android/platform_view_android.h +++ b/shell/platform/android/platform_view_android.h @@ -116,6 +116,10 @@ class PlatformViewAndroid final : public PlatformView { return android_context_; } + bool DoesHandlePlatformMessagesOnPlatformThread() const override { + return false; + } + private: const std::shared_ptr jni_facade_; std::shared_ptr android_context_; diff --git a/shell/platform/android/platform_view_android_jni_impl.cc b/shell/platform/android/platform_view_android_jni_impl.cc index 376ac155516d0..4248761b52936 100644 --- a/shell/platform/android/platform_view_android_jni_impl.cc +++ b/shell/platform/android/platform_view_android_jni_impl.cc @@ -405,6 +405,13 @@ static void DispatchEmptyPlatformMessage(JNIEnv* env, ); } +static void CleanupMessageData(JNIEnv* env, + jobject jcaller, + jlong message_data) { + // Called from any thread. + free(reinterpret_cast(message_data)); +} + static void DispatchPointerDataPacket(JNIEnv* env, jobject jcaller, jlong shell_holder, @@ -638,6 +645,11 @@ bool RegisterApi(JNIEnv* env) { .signature = "(JLjava/lang/String;I)V", .fnPtr = reinterpret_cast(&DispatchEmptyPlatformMessage), }, + { + .name = "nativeCleanupMessageData", + .signature = "(J)V", + .fnPtr = reinterpret_cast(&CleanupMessageData), + }, { .name = "nativeDispatchPlatformMessage", .signature = "(JLjava/lang/String;Ljava/nio/ByteBuffer;II)V", @@ -819,7 +831,7 @@ bool RegisterApi(JNIEnv* env) { g_handle_platform_message_method = env->GetMethodID(g_flutter_jni_class->obj(), "handlePlatformMessage", - "(Ljava/lang/String;Ljava/nio/ByteBuffer;I)V"); + "(Ljava/lang/String;Ljava/nio/ByteBuffer;IJ)V"); if (g_handle_platform_message_method == nullptr) { FML_LOG(ERROR) << "Could not locate handlePlatformMessage method"; @@ -1102,6 +1114,7 @@ PlatformViewAndroidJNIImpl::~PlatformViewAndroidJNIImpl() = default; void PlatformViewAndroidJNIImpl::FlutterViewHandlePlatformMessage( std::unique_ptr message, int responseId) { + // Called from the ui thread. JNIEnv* env = fml::jni::AttachCurrentThread(); auto java_object = java_object_.get(env); @@ -1117,11 +1130,14 @@ void PlatformViewAndroidJNIImpl::FlutterViewHandlePlatformMessage( env, env->NewDirectByteBuffer( const_cast(message->data().GetMapping()), message->data().GetSize())); + // Message data is deleted in CleanupMessageData. + fml::MallocMapping mapping = message->releaseData(); env->CallVoidMethod(java_object.obj(), g_handle_platform_message_method, - java_channel.obj(), message_array.obj(), responseId); + java_channel.obj(), message_array.obj(), responseId, + mapping.Release()); } else { env->CallVoidMethod(java_object.obj(), g_handle_platform_message_method, - java_channel.obj(), nullptr, responseId); + java_channel.obj(), nullptr, responseId, nullptr); } FML_CHECK(fml::jni::CheckException(env)); From 68c41df94deee539014e80e804398ac66b44f0d9 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Tue, 12 Oct 2021 16:01:24 -0700 Subject: [PATCH 02/21] Part 2: Introduced TaskQueue API --- shell/common/platform_view.h | 2 + .../flutter/embedding/engine/FlutterJNI.java | 3 +- .../embedding/engine/dart/DartExecutor.java | 26 +++++-- .../embedding/engine/dart/DartMessenger.java | 76 +++++++++++++++---- .../plugin/common/BasicMessageChannel.java | 3 +- .../plugin/common/BinaryMessenger.java | 13 +++- .../flutter/plugin/common/EventChannel.java | 2 +- .../flutter/plugin/common/MethodChannel.java | 12 ++- .../io/flutter/view/FlutterNativeView.java | 10 ++- .../android/io/flutter/view/FlutterView.java | 10 ++- .../platform/android/platform_view_android.cc | 16 +++- .../platform/android/platform_view_android.h | 1 + .../android/platform_view_android_jni_impl.cc | 2 + .../scenarios/SpawnedEngineActivity.java | 3 +- .../scenarios/TestableFlutterActivity.java | 3 +- 15 files changed, 147 insertions(+), 35 deletions(-) diff --git a/shell/common/platform_view.h b/shell/common/platform_view.h index 274362f34ea39..0817d01c43395 100644 --- a/shell/common/platform_view.h +++ b/shell/common/platform_view.h @@ -810,6 +810,8 @@ class PlatformView { virtual std::unique_ptr CreateSnapshotSurfaceProducer(); + // This is temporary until all embedders start handling thread dispatching to + // allow messages to be handled on background threads. virtual bool DoesHandlePlatformMessagesOnPlatformThread() const { return true; } diff --git a/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java b/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java index e9da03fc7465b..a5044d4a2f2d9 100644 --- a/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java +++ b/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java @@ -958,10 +958,9 @@ private native void nativeInvokePlatformMessageEmptyResponseCallback( long nativeShellHolderId, int responseId); // TODO(mattcarroll): differentiate between channel responses and platform responses. - @UiThread public void invokePlatformMessageResponseCallback( int responseId, @NonNull ByteBuffer message, int position) { - ensureRunningOnMainThread(); + // Called on any thread. if (!message.isDirect()) { throw new IllegalArgumentException("Expected a direct ByteBuffer."); } 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 511888112ccf5..dc5de3da1ed4b 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, null); 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. @@ -168,6 +168,14 @@ public BinaryMessenger getBinaryMessenger() { } // ------ START BinaryMessenger (Deprecated: use getBinaryMessenger() instead) ----- + /** @deprecated Use {@link #getBinaryMessenger()} instead. */ + @Deprecated + @UiThread + @Override + public TaskQueue makeBackgroundTaskQueue() { + return binaryMessenger.makeBackgroundTaskQueue(); + } + /** @deprecated Use {@link #getBinaryMessenger()} instead. */ @Deprecated @Override @@ -192,8 +200,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, + @Nullable TaskQueue taskQueue) { + binaryMessenger.setMessageHandler(channel, handler, taskQueue); } // ------ END BinaryMessenger ----- @@ -371,6 +381,10 @@ private DefaultBinaryMessenger(@NonNull DartMessenger messenger) { this.messenger = messenger; } + public TaskQueue makeBackgroundTaskQueue() { + return messenger.makeBackgroundTaskQueue(); + } + /** * Sends the given {@code message} from Android to Dart over the given {@code channel}. * @@ -413,8 +427,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, + @Nullable TaskQueue taskQueue) { + messenger.setMessageHandler(channel, handler, taskQueue); } } } 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 19da9599b2d0a..ca386ad91b3ff 100644 --- a/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java +++ b/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java @@ -15,6 +15,9 @@ import java.nio.ByteBuffer; import java.util.HashMap; import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; import java.util.concurrent.atomic.AtomicBoolean; /** @@ -28,25 +31,60 @@ class DartMessenger implements BinaryMessenger, PlatformMessageHandler { private static final String TAG = "DartMessenger"; @NonNull private final FlutterJNI flutterJNI; - @NonNull private final Map messageHandlers; + + @NonNull private final ConcurrentHashMap messageHandlers; + @NonNull private final Map pendingReplies; private int nextReplyId = 1; DartMessenger(@NonNull FlutterJNI flutterJNI) { this.flutterJNI = flutterJNI; - this.messageHandlers = new HashMap<>(); + this.messageHandlers = new ConcurrentHashMap<>(); this.pendingReplies = new HashMap<>(); } + private static class HandlerInfo { + @NonNull public final BinaryMessenger.BinaryMessageHandler handler; + @Nullable public final TaskQueue taskQueue; + + HandlerInfo( + @NonNull BinaryMessenger.BinaryMessageHandler handler, @Nullable TaskQueue taskQueue) { + this.handler = handler; + this.taskQueue = taskQueue; + } + } + + private static class DefaultTaskQueue implements TaskQueue { + @NonNull private final ExecutorService executor; + + DefaultTaskQueue() { + // TODO(gaaclarke): Use a shared thread pool with serial queues instead of + // making a thread for each TaskQueue. + this.executor = Executors.newSingleThreadExecutor(); + } + + @Override + public void dispatch(@NonNull Runnable runnable) { + executor.submit(runnable); + } + } + + @Override + public TaskQueue makeBackgroundTaskQueue() { + return new DefaultTaskQueue(); + } + @Override public void setMessageHandler( - @NonNull String channel, @Nullable BinaryMessenger.BinaryMessageHandler handler) { + @NonNull String channel, + @Nullable BinaryMessenger.BinaryMessageHandler handler, + @Nullable TaskQueue taskQueue) { 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, handler); + messageHandlers.put(channel, new HandlerInfo(handler, taskQueue)); } } @@ -74,14 +112,13 @@ public void send( } } - private void handleMessageFromDartPlatformThread( - @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) { + private void invokeHandler( + @Nullable HandlerInfo handlerInfo, @Nullable ByteBuffer message, final int replyId) { + // Called from any thread. + if (handlerInfo != null) { try { Log.v(TAG, "Deferring to registered handler to process message."); - handler.onMessage(message, new Reply(flutterJNI, replyId)); + handlerInfo.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. @@ -105,18 +142,27 @@ public void handleMessageFromDart( @Nullable ByteBuffer message, final int replyId, long messageData) { - // TODO(gaaclarke): Dispatch to a TaskQueue other than the platform channel if specified by the - // handler. - Handler mainHandler = new Handler(Looper.getMainLooper()); + // Called from the ui thread. + Log.v(TAG, "Received message from Dart over channel '" + channel + "'"); + @Nullable HandlerInfo handlerInfo = messageHandlers.get(channel); + @Nullable TaskQueue taskQueue = null; + if (handlerInfo != null) { + taskQueue = handlerInfo.taskQueue; + } Runnable myRunnable = () -> { try { - handleMessageFromDartPlatformThread(channel, message, replyId); + invokeHandler(handlerInfo, message, replyId); } finally { flutterJNI.cleanupMessageData(messageData); } }; - mainHandler.post(myRunnable); + if (taskQueue == null) { + Handler mainHandler = new Handler(Looper.getMainLooper()); + mainHandler.post(myRunnable); + } else { + taskQueue.dispatch(myRunnable); + } } @Override diff --git a/shell/platform/android/io/flutter/plugin/common/BasicMessageChannel.java b/shell/platform/android/io/flutter/plugin/common/BasicMessageChannel.java index 55eb3109af5bb..24256b139c3cd 100644 --- a/shell/platform/android/io/flutter/plugin/common/BasicMessageChannel.java +++ b/shell/platform/android/io/flutter/plugin/common/BasicMessageChannel.java @@ -101,7 +101,8 @@ 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), null); } /** diff --git a/shell/platform/android/io/flutter/plugin/common/BinaryMessenger.java b/shell/platform/android/io/flutter/plugin/common/BinaryMessenger.java index fb1a22e6f4d3c..796e9eede380b 100644 --- a/shell/platform/android/io/flutter/plugin/common/BinaryMessenger.java +++ b/shell/platform/android/io/flutter/plugin/common/BinaryMessenger.java @@ -26,6 +26,13 @@ * @see EventChannel , which supports communication using event streams. */ public interface BinaryMessenger { + public interface TaskQueue { + void dispatch(@NonNull Runnable runnable); + } + + @UiThread + TaskQueue makeBackgroundTaskQueue(); + /** * Sends a binary message to the Flutter application. * @@ -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, + @Nullable TaskQueue taskQueue); /** Handler for incoming binary messages from Flutter. */ interface BinaryMessageHandler { @@ -97,7 +107,6 @@ interface BinaryReply { * outgoing replies must place the reply bytes between position zero and current position. * Reply receivers can read from the buffer directly. */ - @UiThread void reply(@Nullable ByteBuffer reply); } } diff --git a/shell/platform/android/io/flutter/plugin/common/EventChannel.java b/shell/platform/android/io/flutter/plugin/common/EventChannel.java index 984b251b29d65..0c2d65272d3bf 100644 --- a/shell/platform/android/io/flutter/plugin/common/EventChannel.java +++ b/shell/platform/android/io/flutter/plugin/common/EventChannel.java @@ -84,7 +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)); + name, handler == null ? null : new IncomingStreamRequestHandler(handler), null); } /** diff --git a/shell/platform/android/io/flutter/plugin/common/MethodChannel.java b/shell/platform/android/io/flutter/plugin/common/MethodChannel.java index 901299b943019..7c3d91cba7ca5 100644 --- a/shell/platform/android/io/flutter/plugin/common/MethodChannel.java +++ b/shell/platform/android/io/flutter/plugin/common/MethodChannel.java @@ -35,6 +35,7 @@ public class MethodChannel { private final BinaryMessenger messenger; private final String name; private final MethodCodec codec; + private final BinaryMessenger.TaskQueue taskQueue; /** * Creates a new channel associated with the specified {@link BinaryMessenger} and with the @@ -56,6 +57,14 @@ public MethodChannel(BinaryMessenger messenger, String name) { * @param codec a {@link MessageCodec}. */ public MethodChannel(BinaryMessenger messenger, String name, MethodCodec codec) { + this(messenger, name, codec, null); + } + + public MethodChannel( + BinaryMessenger messenger, + String name, + MethodCodec codec, + @Nullable BinaryMessenger.TaskQueue taskQueue) { if (BuildConfig.DEBUG) { if (messenger == null) { Log.e(TAG, "Parameter messenger must not be null."); @@ -70,6 +79,7 @@ public MethodChannel(BinaryMessenger messenger, String name, MethodCodec codec) this.messenger = messenger; this.name = name; this.codec = codec; + this.taskQueue = taskQueue; } /** @@ -117,7 +127,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)); + name, handler == null ? null : new IncomingMethodCallHandler(handler), taskQueue); } /** diff --git a/shell/platform/android/io/flutter/view/FlutterNativeView.java b/shell/platform/android/io/flutter/view/FlutterNativeView.java index 63005dc8bb534..6e6ca763a0111 100644 --- a/shell/platform/android/io/flutter/view/FlutterNativeView.java +++ b/shell/platform/android/io/flutter/view/FlutterNativeView.java @@ -124,6 +124,12 @@ public static String getObservatoryUri() { return FlutterJNI.getObservatoryUri(); } + @Override + @UiThread + public TaskQueue makeBackgroundTaskQueue() { + return dartExecutor.getBinaryMessenger().makeBackgroundTaskQueue(); + } + @Override @UiThread public void send(String channel, ByteBuffer message) { @@ -143,8 +149,8 @@ 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, TaskQueue taskQueue) { + dartExecutor.getBinaryMessenger().setMessageHandler(channel, handler, taskQueue); } /*package*/ FlutterJNI getFlutterJNI() { diff --git a/shell/platform/android/io/flutter/view/FlutterView.java b/shell/platform/android/io/flutter/view/FlutterView.java index 4876f1b67c04c..f63ce7add3f9f 100644 --- a/shell/platform/android/io/flutter/view/FlutterView.java +++ b/shell/platform/android/io/flutter/view/FlutterView.java @@ -822,6 +822,12 @@ public PointerIcon getSystemPointerIcon(int type) { return PointerIcon.getSystemIcon(getContext(), type); } + @Override + @UiThread + public TaskQueue makeBackgroundTaskQueue() { + return null; + } + @Override @UiThread public void send(String channel, ByteBuffer message) { @@ -840,8 +846,8 @@ 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, TaskQueue taskQueue) { + mNativeView.setMessageHandler(channel, handler, taskQueue); } /** Listener will be called on the Android UI thread once when Flutter renders the first frame. */ diff --git a/shell/platform/android/platform_view_android.cc b/shell/platform/android/platform_view_android.cc index 50b9d55c6ca4a..c99fe5259f082 100644 --- a/shell/platform/android/platform_view_android.cc +++ b/shell/platform/android/platform_view_android.cc @@ -195,8 +195,13 @@ void PlatformViewAndroid::InvokePlatformMessageResponseCallback( jint response_id, jobject java_response_data, jint java_response_position) { - if (!response_id) + // Called from any thread. + if (!response_id) { return; + } + // TODO(gaaclarke): Move the jump to the ui thread here from + // PlatformMessageResponseDart so we won't need to use a mutex anymore. + std::unique_lock lock(pending_responses_mutex_); auto it = pending_responses_.find(response_id); if (it == pending_responses_.end()) return; @@ -207,6 +212,7 @@ void PlatformViewAndroid::InvokePlatformMessageResponseCallback( response_data, response_data + java_response_position); auto message_response = std::move(it->second); pending_responses_.erase(it); + lock.unlock(); message_response->Complete( std::make_unique(std::move(response))); } @@ -214,21 +220,27 @@ void PlatformViewAndroid::InvokePlatformMessageResponseCallback( void PlatformViewAndroid::InvokePlatformMessageEmptyResponseCallback( JNIEnv* env, jint response_id) { - if (!response_id) + // Called from any thread. + if (!response_id) { return; + } + std::unique_lock lock(pending_responses_mutex_); auto it = pending_responses_.find(response_id); if (it == pending_responses_.end()) return; auto message_response = std::move(it->second); pending_responses_.erase(it); + lock.unlock(); message_response->CompleteEmpty(); } // |PlatformView| void PlatformViewAndroid::HandlePlatformMessage( std::unique_ptr message) { + // Called from the ui thread. int response_id = next_response_id_++; if (auto response = message->response()) { + std::lock_guard lock(pending_responses_mutex_); pending_responses_[response_id] = response; } // This call can re-enter in InvokePlatformMessageXxxResponseCallback. diff --git a/shell/platform/android/platform_view_android.h b/shell/platform/android/platform_view_android.h index 79f39ba0b71d9..8021bc142d191 100644 --- a/shell/platform/android/platform_view_android.h +++ b/shell/platform/android/platform_view_android.h @@ -133,6 +133,7 @@ class PlatformViewAndroid final : public PlatformView { int next_response_id_ = 1; std::unordered_map> pending_responses_; + std::mutex pending_responses_mutex_; // |PlatformView| void UpdateSemantics( diff --git a/shell/platform/android/platform_view_android_jni_impl.cc b/shell/platform/android/platform_view_android_jni_impl.cc index 4248761b52936..1de34ae23c468 100644 --- a/shell/platform/android/platform_view_android_jni_impl.cc +++ b/shell/platform/android/platform_view_android_jni_impl.cc @@ -490,6 +490,8 @@ static void InvokePlatformMessageResponseCallback(JNIEnv* env, jint responseId, jobject message, jint position) { + // NOTE(gaaclarke): Crash here because substantiating weakptr on thread other + // than platform thread. ANDROID_SHELL_HOLDER->GetPlatformView() ->InvokePlatformMessageResponseCallback(env, // responseId, // 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 fbce67dede353..6599ed5437977 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 @@ -22,7 +22,8 @@ public FlutterEngine provideFlutterEngine(@NonNull Context context) { secondEngine .getDartExecutor() - .setMessageHandler("take_screenshot", (byteBuffer, binaryReply) -> notifyFlutterRendered()); + .setMessageHandler( + "take_screenshot", (byteBuffer, binaryReply) -> notifyFlutterRendered(), null); 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 82ba2d95c5abd..75a5240c70282 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 @@ -19,7 +19,8 @@ public void configureFlutterEngine(@NonNull FlutterEngine flutterEngine) { // registration will fail and print a scary exception in the logs. flutterEngine .getDartExecutor() - .setMessageHandler("take_screenshot", (byteBuffer, binaryReply) -> notifyFlutterRendered()); + .setMessageHandler( + "take_screenshot", (byteBuffer, binaryReply) -> notifyFlutterRendered(), null); } protected void notifyFlutterRendered() { From c8de4a12fdae815148c0052ef23ab8dd45c2025c Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Wed, 13 Oct 2021 16:20:43 -0700 Subject: [PATCH 03/21] Part 3: Introduced PlatformMessageHandler --- shell/platform/android/BUILD.gn | 2 + .../platform/android/android_shell_holder.cc | 18 +++-- shell/platform/android/android_shell_holder.h | 7 ++ .../android/platform_message_handler.cc | 69 +++++++++++++++++++ .../android/platform_message_handler.h | 39 +++++++++++ .../platform/android/platform_view_android.cc | 66 +++--------------- .../platform/android/platform_view_android.h | 29 +++----- .../android/platform_view_android_jni_impl.cc | 6 +- 8 files changed, 149 insertions(+), 87 deletions(-) create mode 100644 shell/platform/android/platform_message_handler.cc create mode 100644 shell/platform/android/platform_message_handler.h diff --git a/shell/platform/android/BUILD.gn b/shell/platform/android/BUILD.gn index 2780a734ef590..cda93ede8883f 100644 --- a/shell/platform/android/BUILD.gn +++ b/shell/platform/android/BUILD.gn @@ -79,6 +79,8 @@ source_set("flutter_shell_native_src") { "flutter_main.cc", "flutter_main.h", "library_loader.cc", + "platform_message_handler.cc", + "platform_message_handler.h", "platform_message_response_android.cc", "platform_message_response_android.h", "platform_view_android.cc", diff --git a/shell/platform/android/android_shell_holder.cc b/shell/platform/android/android_shell_holder.cc index b85db32a6d1cc..73e8ed2dda010 100644 --- a/shell/platform/android/android_shell_holder.cc +++ b/shell/platform/android/android_shell_holder.cc @@ -40,7 +40,9 @@ static PlatformData GetDefaultPlatformData() { AndroidShellHolder::AndroidShellHolder( flutter::Settings settings, std::shared_ptr jni_facade) - : settings_(std::move(settings)), jni_facade_(jni_facade) { + : settings_(std::move(settings)), + jni_facade_(jni_facade), + platform_message_handler_(new PlatformMessageHandler(jni_facade)) { static size_t thread_host_count = 1; auto thread_label = std::to_string(thread_host_count++); @@ -51,15 +53,16 @@ AndroidShellHolder::AndroidShellHolder( fml::WeakPtr weak_platform_view; Shell::CreateCallback on_create_platform_view = - [&jni_facade, &weak_platform_view](Shell& shell) { + [&jni_facade, &weak_platform_view, + platform_message_handler = platform_message_handler_](Shell& shell) { std::unique_ptr platform_view_android; platform_view_android = std::make_unique( shell, // delegate shell.GetTaskRunners(), // task runners jni_facade, // JNI interop shell.GetSettings() - .enable_software_rendering // use software rendering - ); + .enable_software_rendering, // use software rendering + platform_message_handler); weak_platform_view = platform_view_android->GetWeakPtr(); auto display = Display(jni_facade->GetDisplayRefreshRate()); shell.OnDisplayUpdates(DisplayUpdateType::kStartup, {display}); @@ -200,14 +203,15 @@ std::unique_ptr AndroidShellHolder::Spawn( // This is a synchronous call, so the captures don't have race checks. Shell::CreateCallback on_create_platform_view = - [&jni_facade, android_context, &weak_platform_view](Shell& shell) { + [&jni_facade, android_context, &weak_platform_view, + platform_message_handler = platform_message_handler_](Shell& shell) { std::unique_ptr platform_view_android; platform_view_android = std::make_unique( shell, // delegate shell.GetTaskRunners(), // task runners jni_facade, // JNI interop - android_context // Android context - ); + android_context, // Android context + platform_message_handler); weak_platform_view = platform_view_android->GetWeakPtr(); auto display = Display(jni_facade->GetDisplayRefreshRate()); shell.OnDisplayUpdates(DisplayUpdateType::kStartup, {display}); diff --git a/shell/platform/android/android_shell_holder.h b/shell/platform/android/android_shell_holder.h index 9ea264f366722..8eeb10bf320cd 100644 --- a/shell/platform/android/android_shell_holder.h +++ b/shell/platform/android/android_shell_holder.h @@ -16,6 +16,7 @@ #include "flutter/shell/common/shell.h" #include "flutter/shell/common/thread_host.h" #include "flutter/shell/platform/android/jni/platform_view_android_jni.h" +#include "flutter/shell/platform/android/platform_message_handler.h" #include "flutter/shell/platform/android/platform_view_android.h" namespace flutter { @@ -96,6 +97,11 @@ class AndroidShellHolder { void NotifyLowMemoryWarning(); + const std::shared_ptr& GetPlatformMessageHandler() + const { + return platform_message_handler_; + } + private: const flutter::Settings settings_; const std::shared_ptr jni_facade_; @@ -105,6 +111,7 @@ class AndroidShellHolder { bool is_valid_ = false; uint64_t next_pointer_flow_id_ = 0; std::shared_ptr asset_manager_; + std::shared_ptr platform_message_handler_; //---------------------------------------------------------------------------- /// @brief Constructor with its components injected. diff --git a/shell/platform/android/platform_message_handler.cc b/shell/platform/android/platform_message_handler.cc new file mode 100644 index 0000000000000..6e19dca43dcc9 --- /dev/null +++ b/shell/platform/android/platform_message_handler.cc @@ -0,0 +1,69 @@ + +#include "flutter/shell/platform/android/platform_message_handler.h" + +namespace flutter { + +PlatformMessageHandler::PlatformMessageHandler( + const std::shared_ptr& jni_facade) + : jni_facade_(jni_facade) {} + +void PlatformMessageHandler::InvokePlatformMessageResponseCallback( + JNIEnv* env, + jint response_id, + jobject java_response_data, + jint java_response_position) { + // Called from any thread. + if (!response_id) { + return; + } + // TODO(gaaclarke): Move the jump to the ui thread here from + // PlatformMessageResponseDart so we won't need to use a mutex anymore. + std::unique_lock lock(pending_responses_mutex_); + auto it = pending_responses_.find(response_id); + if (it == pending_responses_.end()) + return; + uint8_t* response_data = + static_cast(env->GetDirectBufferAddress(java_response_data)); + FML_DCHECK(response_data != nullptr); + std::vector response = std::vector( + response_data, response_data + java_response_position); + auto message_response = std::move(it->second); + pending_responses_.erase(it); + lock.unlock(); + message_response->Complete( + std::make_unique(std::move(response))); +} + +void PlatformMessageHandler::InvokePlatformMessageEmptyResponseCallback( + JNIEnv* env, + jint response_id) { + // Called from any thread. + if (!response_id) { + return; + } + std::unique_lock lock(pending_responses_mutex_); + auto it = pending_responses_.find(response_id); + if (it == pending_responses_.end()) + return; + auto message_response = std::move(it->second); + pending_responses_.erase(it); + lock.unlock(); + message_response->CompleteEmpty(); +} + +// |PlatformView| +void PlatformMessageHandler::HandlePlatformMessage( + std::unique_ptr message) { + // Called from the ui thread. + int response_id = next_response_id_++; + if (auto response = message->response()) { + std::lock_guard lock(pending_responses_mutex_); + pending_responses_[response_id] = response; + } + // This call can re-enter in InvokePlatformMessageXxxResponseCallback. + jni_facade_->FlutterViewHandlePlatformMessage(std::move(message), + response_id); + message = nullptr; +} + +} // namespace flutter diff --git a/shell/platform/android/platform_message_handler.h b/shell/platform/android/platform_message_handler.h new file mode 100644 index 0000000000000..d906c270fb091 --- /dev/null +++ b/shell/platform/android/platform_message_handler.h @@ -0,0 +1,39 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef SHELL_PLATFORM_ANDROID_PLATFORM_MESSAGE_HANDLER_H_ +#define SHELL_PLATFORM_ANDROID_PLATFORM_MESSAGE_HANDLER_H_ + +#include +#include +#include +#include + +#include "flutter/lib/ui/window/platform_message.h" +#include "flutter/shell/platform/android/jni/platform_view_android_jni.h" + +namespace flutter { +class PlatformMessageHandler { + public: + PlatformMessageHandler( + const std::shared_ptr& jni_facade); + void HandlePlatformMessage(std::unique_ptr message); + void InvokePlatformMessageResponseCallback(JNIEnv* env, + jint response_id, + jobject java_response_data, + jint java_response_position); + + void InvokePlatformMessageEmptyResponseCallback(JNIEnv* env, + jint response_id); + + private: + const std::shared_ptr jni_facade_; + int next_response_id_ = 1; + std::unordered_map> + pending_responses_; + std::mutex pending_responses_mutex_; +}; +} // namespace flutter + +#endif diff --git a/shell/platform/android/platform_view_android.cc b/shell/platform/android/platform_view_android.cc index c99fe5259f082..23bdf11c3a1b2 100644 --- a/shell/platform/android/platform_view_android.cc +++ b/shell/platform/android/platform_view_android.cc @@ -59,21 +59,25 @@ PlatformViewAndroid::PlatformViewAndroid( PlatformView::Delegate& delegate, flutter::TaskRunners task_runners, std::shared_ptr jni_facade, - bool use_software_rendering) + bool use_software_rendering, + const std::shared_ptr& platform_message_handler) : PlatformViewAndroid(delegate, std::move(task_runners), std::move(jni_facade), - CreateAndroidContext(use_software_rendering)) {} + CreateAndroidContext(use_software_rendering), + platform_message_handler) {} PlatformViewAndroid::PlatformViewAndroid( PlatformView::Delegate& delegate, flutter::TaskRunners task_runners, const std::shared_ptr& jni_facade, - const std::shared_ptr& android_context) + const std::shared_ptr& android_context, + const std::shared_ptr& platform_message_handler) : PlatformView(delegate, std::move(task_runners)), jni_facade_(jni_facade), android_context_(std::move(android_context)), - platform_view_android_delegate_(jni_facade) { + platform_view_android_delegate_(jni_facade), + platform_message_handler_(platform_message_handler) { // TODO(dnfield): always create a pbuffer surface for background use to // resolve https://github.com/flutter/flutter/issues/73675 if (android_context_) { @@ -190,63 +194,11 @@ void PlatformViewAndroid::DispatchEmptyPlatformMessage(JNIEnv* env, std::move(response))); } -void PlatformViewAndroid::InvokePlatformMessageResponseCallback( - JNIEnv* env, - jint response_id, - jobject java_response_data, - jint java_response_position) { - // Called from any thread. - if (!response_id) { - return; - } - // TODO(gaaclarke): Move the jump to the ui thread here from - // PlatformMessageResponseDart so we won't need to use a mutex anymore. - std::unique_lock lock(pending_responses_mutex_); - auto it = pending_responses_.find(response_id); - if (it == pending_responses_.end()) - return; - uint8_t* response_data = - static_cast(env->GetDirectBufferAddress(java_response_data)); - FML_DCHECK(response_data != nullptr); - std::vector response = std::vector( - response_data, response_data + java_response_position); - auto message_response = std::move(it->second); - pending_responses_.erase(it); - lock.unlock(); - message_response->Complete( - std::make_unique(std::move(response))); -} - -void PlatformViewAndroid::InvokePlatformMessageEmptyResponseCallback( - JNIEnv* env, - jint response_id) { - // Called from any thread. - if (!response_id) { - return; - } - std::unique_lock lock(pending_responses_mutex_); - auto it = pending_responses_.find(response_id); - if (it == pending_responses_.end()) - return; - auto message_response = std::move(it->second); - pending_responses_.erase(it); - lock.unlock(); - message_response->CompleteEmpty(); -} - // |PlatformView| void PlatformViewAndroid::HandlePlatformMessage( std::unique_ptr message) { // Called from the ui thread. - int response_id = next_response_id_++; - if (auto response = message->response()) { - std::lock_guard lock(pending_responses_mutex_); - pending_responses_[response_id] = response; - } - // This call can re-enter in InvokePlatformMessageXxxResponseCallback. - jni_facade_->FlutterViewHandlePlatformMessage(std::move(message), - response_id); - message = nullptr; + platform_message_handler_->HandlePlatformMessage(std::move(message)); } // |PlatformView| diff --git a/shell/platform/android/platform_view_android.h b/shell/platform/android/platform_view_android.h index 8021bc142d191..f20422937123f 100644 --- a/shell/platform/android/platform_view_android.h +++ b/shell/platform/android/platform_view_android.h @@ -17,6 +17,7 @@ #include "flutter/shell/common/snapshot_surface_producer.h" #include "flutter/shell/platform/android/context/android_context.h" #include "flutter/shell/platform/android/jni/platform_view_android_jni.h" +#include "flutter/shell/platform/android/platform_message_handler.h" #include "flutter/shell/platform/android/platform_view_android_delegate/platform_view_android_delegate.h" #include "flutter/shell/platform/android/surface/android_native_window.h" #include "flutter/shell/platform/android/surface/android_surface.h" @@ -41,10 +42,12 @@ class PlatformViewAndroid final : public PlatformView { public: static bool Register(JNIEnv* env); - PlatformViewAndroid(PlatformView::Delegate& delegate, - flutter::TaskRunners task_runners, - std::shared_ptr jni_facade, - bool use_software_rendering); + PlatformViewAndroid( + PlatformView::Delegate& delegate, + flutter::TaskRunners task_runners, + std::shared_ptr jni_facade, + bool use_software_rendering, + const std::shared_ptr& platform_message_handler); //---------------------------------------------------------------------------- /// @brief Creates a new PlatformViewAndroid but using an existing @@ -55,7 +58,8 @@ class PlatformViewAndroid final : public PlatformView { PlatformView::Delegate& delegate, flutter::TaskRunners task_runners, const std::shared_ptr& jni_facade, - const std::shared_ptr& android_context); + const std::shared_ptr& android_context, + const std::shared_ptr& platform_message_handler); ~PlatformViewAndroid() override; @@ -79,14 +83,6 @@ class PlatformViewAndroid final : public PlatformView { std::string name, jint response_id); - void InvokePlatformMessageResponseCallback(JNIEnv* env, - jint response_id, - jobject java_response_data, - jint java_response_position); - - void InvokePlatformMessageEmptyResponseCallback(JNIEnv* env, - jint response_id); - void DispatchSemanticsAction(JNIEnv* env, jint id, jint action, @@ -128,12 +124,7 @@ class PlatformViewAndroid final : public PlatformView { PlatformViewAndroidDelegate platform_view_android_delegate_; std::unique_ptr android_surface_; - - // We use id 0 to mean that no response is expected. - int next_response_id_ = 1; - std::unordered_map> - pending_responses_; - std::mutex pending_responses_mutex_; + std::shared_ptr platform_message_handler_; // |PlatformView| void UpdateSemantics( diff --git a/shell/platform/android/platform_view_android_jni_impl.cc b/shell/platform/android/platform_view_android_jni_impl.cc index 1de34ae23c468..311d16c81c42b 100644 --- a/shell/platform/android/platform_view_android_jni_impl.cc +++ b/shell/platform/android/platform_view_android_jni_impl.cc @@ -490,9 +490,7 @@ static void InvokePlatformMessageResponseCallback(JNIEnv* env, jint responseId, jobject message, jint position) { - // NOTE(gaaclarke): Crash here because substantiating weakptr on thread other - // than platform thread. - ANDROID_SHELL_HOLDER->GetPlatformView() + ANDROID_SHELL_HOLDER->GetPlatformMessageHandler() ->InvokePlatformMessageResponseCallback(env, // responseId, // message, // @@ -504,7 +502,7 @@ static void InvokePlatformMessageEmptyResponseCallback(JNIEnv* env, jobject jcaller, jlong shell_holder, jint responseId) { - ANDROID_SHELL_HOLDER->GetPlatformView() + ANDROID_SHELL_HOLDER->GetPlatformMessageHandler() ->InvokePlatformMessageEmptyResponseCallback(env, // responseId // ); From 19bd38cd005299111996a35e75007167c44bb236 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 15 Oct 2021 11:28:12 -0700 Subject: [PATCH 04/21] introduced PlatformMessageHandler, renamed PlatformMessageHandler to PlatformMessageHandlerAndroid This eliminates the potential race condition where we are calling into a platform view when it is being deallocated (That may not be possible in practice but this is safer). --- shell/common/BUILD.gn | 1 + shell/common/platform_message_handler.h | 21 ++++++++++++++++ shell/common/platform_view.h | 9 +++---- shell/common/shell.cc | 9 ++++--- shell/common/shell.h | 1 + shell/platform/android/BUILD.gn | 4 ++-- .../platform/android/android_shell_holder.cc | 2 +- shell/platform/android/android_shell_holder.h | 8 +++---- ...cc => platform_message_handler_android.cc} | 10 ++++---- ...r.h => platform_message_handler_android.h} | 7 +++--- .../platform/android/platform_view_android.cc | 6 +++-- .../platform/android/platform_view_android.h | 24 ++++++++++--------- 12 files changed, 65 insertions(+), 37 deletions(-) create mode 100644 shell/common/platform_message_handler.h rename shell/platform/android/{platform_message_handler.cc => platform_message_handler_android.cc} (87%) rename shell/platform/android/{platform_message_handler.h => platform_message_handler_android.h} (88%) diff --git a/shell/common/BUILD.gn b/shell/common/BUILD.gn index b8d00101df091..0a4ba228c98b5 100644 --- a/shell/common/BUILD.gn +++ b/shell/common/BUILD.gn @@ -73,6 +73,7 @@ source_set("common") { "engine.h", "pipeline.cc", "pipeline.h", + "platform_message_handler.h", "platform_view.cc", "platform_view.h", "pointer_data_dispatcher.cc", diff --git a/shell/common/platform_message_handler.h b/shell/common/platform_message_handler.h new file mode 100644 index 0000000000000..d1d3838eec690 --- /dev/null +++ b/shell/common/platform_message_handler.h @@ -0,0 +1,21 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef SHELL_COMMON_PLATFORM_MESSAGE_HANDLER_H_ +#define SHELL_COMMON_PLATFORM_MESSAGE_HANDLER_H_ + +#include + +#include "flutter/lib/ui/window/platform_message.h" + +namespace flutter { +class PlatformMessageHandler { + public: + virtual ~PlatformMessageHandler() = default; + virtual void HandlePlatformMessage( + std::unique_ptr message) = 0; +}; +} // namespace flutter + +#endif // SHELL_COMMON_PLATFORM_MESSAGE_HANDLER_H_ diff --git a/shell/common/platform_view.h b/shell/common/platform_view.h index 0817d01c43395..f02e4d45142a5 100644 --- a/shell/common/platform_view.h +++ b/shell/common/platform_view.h @@ -22,6 +22,7 @@ #include "flutter/lib/ui/window/pointer_data_packet.h" #include "flutter/lib/ui/window/pointer_data_packet_converter.h" #include "flutter/lib/ui/window/viewport_metrics.h" +#include "flutter/shell/common/platform_message_handler.h" #include "flutter/shell/common/pointer_data_dispatcher.h" #include "flutter/shell/common/vsync_waiter.h" #include "third_party/skia/include/core/SkSize.h" @@ -810,10 +811,10 @@ class PlatformView { virtual std::unique_ptr CreateSnapshotSurfaceProducer(); - // This is temporary until all embedders start handling thread dispatching to - // allow messages to be handled on background threads. - virtual bool DoesHandlePlatformMessagesOnPlatformThread() const { - return true; + // Returning nullptr means send message to the PlatformView. + virtual std::shared_ptr GetPlatformMessageHandler() + const { + return nullptr; } protected: diff --git a/shell/common/shell.cc b/shell/common/shell.cc index 2941ee32580d3..b9b42d31212ef 100644 --- a/shell/common/shell.cc +++ b/shell/common/shell.cc @@ -625,6 +625,7 @@ bool Shell::Setup(std::unique_ptr platform_view, } platform_view_ = std::move(platform_view); + platform_message_handler_ = platform_view_->GetPlatformMessageHandler(); engine_ = std::move(engine); rasterizer_ = std::move(rasterizer); io_manager_ = std::move(io_manager); @@ -1192,7 +1193,9 @@ void Shell::OnEngineHandlePlatformMessage( return; } - if (platform_view_->DoesHandlePlatformMessagesOnPlatformThread()) { + if (platform_message_handler_) { + platform_message_handler_->HandlePlatformMessage(std::move(message)); + } else { task_runners_.GetPlatformTaskRunner()->PostTask( fml::MakeCopyable([view = platform_view_->GetWeakPtr(), message = std::move(message)]() mutable { @@ -1200,10 +1203,6 @@ void Shell::OnEngineHandlePlatformMessage( view->HandlePlatformMessage(std::move(message)); } })); - } else { - if (platform_view_) { - platform_view_->HandlePlatformMessage(std::move(message)); - } } } diff --git a/shell/common/shell.h b/shell/common/shell.h index 9fbdd5b1a423f..96497629697f6 100644 --- a/shell/common/shell.h +++ b/shell/common/shell.h @@ -412,6 +412,7 @@ class Shell final : public PlatformView::Delegate, std::unique_ptr io_manager_; // on IO task runner std::shared_ptr is_gpu_disabled_sync_switch_; std::shared_ptr volatile_path_tracker_; + std::shared_ptr platform_message_handler_; fml::WeakPtr weak_engine_; // to be shared across threads fml::TaskRunnerAffineWeakPtr diff --git a/shell/platform/android/BUILD.gn b/shell/platform/android/BUILD.gn index cda93ede8883f..c2f1d88130f05 100644 --- a/shell/platform/android/BUILD.gn +++ b/shell/platform/android/BUILD.gn @@ -79,8 +79,8 @@ source_set("flutter_shell_native_src") { "flutter_main.cc", "flutter_main.h", "library_loader.cc", - "platform_message_handler.cc", - "platform_message_handler.h", + "platform_message_handler_android.cc", + "platform_message_handler_android.h", "platform_message_response_android.cc", "platform_message_response_android.h", "platform_view_android.cc", diff --git a/shell/platform/android/android_shell_holder.cc b/shell/platform/android/android_shell_holder.cc index 73e8ed2dda010..2517508ffcf03 100644 --- a/shell/platform/android/android_shell_holder.cc +++ b/shell/platform/android/android_shell_holder.cc @@ -42,7 +42,7 @@ AndroidShellHolder::AndroidShellHolder( std::shared_ptr jni_facade) : settings_(std::move(settings)), jni_facade_(jni_facade), - platform_message_handler_(new PlatformMessageHandler(jni_facade)) { + platform_message_handler_(new PlatformMessageHandlerAndroid(jni_facade)) { static size_t thread_host_count = 1; auto thread_label = std::to_string(thread_host_count++); diff --git a/shell/platform/android/android_shell_holder.h b/shell/platform/android/android_shell_holder.h index 8eeb10bf320cd..e4e75055db61a 100644 --- a/shell/platform/android/android_shell_holder.h +++ b/shell/platform/android/android_shell_holder.h @@ -16,7 +16,7 @@ #include "flutter/shell/common/shell.h" #include "flutter/shell/common/thread_host.h" #include "flutter/shell/platform/android/jni/platform_view_android_jni.h" -#include "flutter/shell/platform/android/platform_message_handler.h" +#include "flutter/shell/platform/android/platform_message_handler_android.h" #include "flutter/shell/platform/android/platform_view_android.h" namespace flutter { @@ -97,8 +97,8 @@ class AndroidShellHolder { void NotifyLowMemoryWarning(); - const std::shared_ptr& GetPlatformMessageHandler() - const { + const std::shared_ptr& + GetPlatformMessageHandler() const { return platform_message_handler_; } @@ -111,7 +111,7 @@ class AndroidShellHolder { bool is_valid_ = false; uint64_t next_pointer_flow_id_ = 0; std::shared_ptr asset_manager_; - std::shared_ptr platform_message_handler_; + std::shared_ptr platform_message_handler_; //---------------------------------------------------------------------------- /// @brief Constructor with its components injected. diff --git a/shell/platform/android/platform_message_handler.cc b/shell/platform/android/platform_message_handler_android.cc similarity index 87% rename from shell/platform/android/platform_message_handler.cc rename to shell/platform/android/platform_message_handler_android.cc index 6e19dca43dcc9..5b84ebb7b8f07 100644 --- a/shell/platform/android/platform_message_handler.cc +++ b/shell/platform/android/platform_message_handler_android.cc @@ -1,13 +1,13 @@ -#include "flutter/shell/platform/android/platform_message_handler.h" +#include "flutter/shell/platform/android/platform_message_handler_android.h" namespace flutter { -PlatformMessageHandler::PlatformMessageHandler( +PlatformMessageHandlerAndroid::PlatformMessageHandlerAndroid( const std::shared_ptr& jni_facade) : jni_facade_(jni_facade) {} -void PlatformMessageHandler::InvokePlatformMessageResponseCallback( +void PlatformMessageHandlerAndroid::InvokePlatformMessageResponseCallback( JNIEnv* env, jint response_id, jobject java_response_data, @@ -34,7 +34,7 @@ void PlatformMessageHandler::InvokePlatformMessageResponseCallback( std::make_unique(std::move(response))); } -void PlatformMessageHandler::InvokePlatformMessageEmptyResponseCallback( +void PlatformMessageHandlerAndroid::InvokePlatformMessageEmptyResponseCallback( JNIEnv* env, jint response_id) { // Called from any thread. @@ -52,7 +52,7 @@ void PlatformMessageHandler::InvokePlatformMessageEmptyResponseCallback( } // |PlatformView| -void PlatformMessageHandler::HandlePlatformMessage( +void PlatformMessageHandlerAndroid::HandlePlatformMessage( std::unique_ptr message) { // Called from the ui thread. int response_id = next_response_id_++; diff --git a/shell/platform/android/platform_message_handler.h b/shell/platform/android/platform_message_handler_android.h similarity index 88% rename from shell/platform/android/platform_message_handler.h rename to shell/platform/android/platform_message_handler_android.h index d906c270fb091..3d2f4536ed667 100644 --- a/shell/platform/android/platform_message_handler.h +++ b/shell/platform/android/platform_message_handler_android.h @@ -11,14 +11,15 @@ #include #include "flutter/lib/ui/window/platform_message.h" +#include "flutter/shell/common/platform_message_handler.h" #include "flutter/shell/platform/android/jni/platform_view_android_jni.h" namespace flutter { -class PlatformMessageHandler { +class PlatformMessageHandlerAndroid : public PlatformMessageHandler { public: - PlatformMessageHandler( + PlatformMessageHandlerAndroid( const std::shared_ptr& jni_facade); - void HandlePlatformMessage(std::unique_ptr message); + void HandlePlatformMessage(std::unique_ptr message) override; void InvokePlatformMessageResponseCallback(JNIEnv* env, jint response_id, jobject java_response_data, diff --git a/shell/platform/android/platform_view_android.cc b/shell/platform/android/platform_view_android.cc index 23bdf11c3a1b2..8016e25db4746 100644 --- a/shell/platform/android/platform_view_android.cc +++ b/shell/platform/android/platform_view_android.cc @@ -60,7 +60,8 @@ PlatformViewAndroid::PlatformViewAndroid( flutter::TaskRunners task_runners, std::shared_ptr jni_facade, bool use_software_rendering, - const std::shared_ptr& platform_message_handler) + const std::shared_ptr& + platform_message_handler) : PlatformViewAndroid(delegate, std::move(task_runners), std::move(jni_facade), @@ -72,7 +73,8 @@ PlatformViewAndroid::PlatformViewAndroid( flutter::TaskRunners task_runners, const std::shared_ptr& jni_facade, const std::shared_ptr& android_context, - const std::shared_ptr& platform_message_handler) + const std::shared_ptr& + platform_message_handler) : PlatformView(delegate, std::move(task_runners)), jni_facade_(jni_facade), android_context_(std::move(android_context)), diff --git a/shell/platform/android/platform_view_android.h b/shell/platform/android/platform_view_android.h index f20422937123f..b992d1c8c1ca8 100644 --- a/shell/platform/android/platform_view_android.h +++ b/shell/platform/android/platform_view_android.h @@ -17,7 +17,7 @@ #include "flutter/shell/common/snapshot_surface_producer.h" #include "flutter/shell/platform/android/context/android_context.h" #include "flutter/shell/platform/android/jni/platform_view_android_jni.h" -#include "flutter/shell/platform/android/platform_message_handler.h" +#include "flutter/shell/platform/android/platform_message_handler_android.h" #include "flutter/shell/platform/android/platform_view_android_delegate/platform_view_android_delegate.h" #include "flutter/shell/platform/android/surface/android_native_window.h" #include "flutter/shell/platform/android/surface/android_surface.h" @@ -42,12 +42,12 @@ class PlatformViewAndroid final : public PlatformView { public: static bool Register(JNIEnv* env); - PlatformViewAndroid( - PlatformView::Delegate& delegate, - flutter::TaskRunners task_runners, - std::shared_ptr jni_facade, - bool use_software_rendering, - const std::shared_ptr& platform_message_handler); + PlatformViewAndroid(PlatformView::Delegate& delegate, + flutter::TaskRunners task_runners, + std::shared_ptr jni_facade, + bool use_software_rendering, + const std::shared_ptr& + platform_message_handler); //---------------------------------------------------------------------------- /// @brief Creates a new PlatformViewAndroid but using an existing @@ -59,7 +59,8 @@ class PlatformViewAndroid final : public PlatformView { flutter::TaskRunners task_runners, const std::shared_ptr& jni_facade, const std::shared_ptr& android_context, - const std::shared_ptr& platform_message_handler); + const std::shared_ptr& + platform_message_handler); ~PlatformViewAndroid() override; @@ -112,8 +113,9 @@ class PlatformViewAndroid final : public PlatformView { return android_context_; } - bool DoesHandlePlatformMessagesOnPlatformThread() const override { - return false; + std::shared_ptr GetPlatformMessageHandler() + const override { + return platform_message_handler_; } private: @@ -124,7 +126,7 @@ class PlatformViewAndroid final : public PlatformView { PlatformViewAndroidDelegate platform_view_android_delegate_; std::unique_ptr android_surface_; - std::shared_ptr platform_message_handler_; + std::shared_ptr platform_message_handler_; // |PlatformView| void UpdateSemantics( From fb4402e019f0f7d50eb4de6f640e22cdd4777336 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 15 Oct 2021 13:28:53 -0700 Subject: [PATCH 05/21] Made it safe to invoke the reply on background threads by protecting attaching, deatching and invoking the reply with locks. --- .../flutter/embedding/engine/FlutterJNI.java | 66 +++++++++++++------ 1 file changed, 46 insertions(+), 20 deletions(-) diff --git a/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java b/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java index a5044d4a2f2d9..aec00bca83b01 100644 --- a/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java +++ b/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java @@ -41,6 +41,7 @@ import java.util.Locale; import java.util.Set; import java.util.concurrent.CopyOnWriteArraySet; +import java.util.concurrent.locks.ReentrantReadWriteLock; /** * Interface between Flutter embedding's Java code and Flutter engine's C/C++ code. @@ -98,6 +99,12 @@ @Keep public class FlutterJNI { private static final String TAG = "FlutterJNI"; + // This serializes the invocation of platform message responses and the + // attachment and detachment of the shell holder. This ensures that we don't + // detach FlutterJNI on the platform thread while a background thread invokes + // a message response. Typically accessing the shell holder happens on the + // platform thread and doesn't require locking. + private ReentrantReadWriteLock shellHolderLock = new ReentrantReadWriteLock(); // BEGIN Methods related to loading for FlutterLoader. /** @@ -304,7 +311,12 @@ public boolean isAttached() { public void attachToNative() { ensureRunningOnMainThread(); ensureNotAttachedToNative(); - nativeShellHolderId = performNativeAttach(this); + shellHolderLock.writeLock().lock(); + try { + nativeShellHolderId = performNativeAttach(this); + } finally { + shellHolderLock.writeLock().unlock(); + } } @VisibleForTesting @@ -368,8 +380,13 @@ private native FlutterJNI nativeSpawn( public void detachFromNativeAndReleaseResources() { ensureRunningOnMainThread(); ensureAttachedToNative(); - nativeDestroy(nativeShellHolderId); - nativeShellHolderId = null; + shellHolderLock.writeLock().lock(); + try { + nativeDestroy(nativeShellHolderId); + nativeShellHolderId = null; + } finally { + shellHolderLock.writeLock().unlock(); + } } private native void nativeDestroy(long nativeShellHolderId); @@ -940,16 +957,20 @@ private native void nativeDispatchPlatformMessage( int responseId); // TODO(mattcarroll): differentiate between channel responses and platform responses. - @UiThread public void invokePlatformMessageEmptyResponseCallback(int responseId) { - ensureRunningOnMainThread(); - if (isAttached()) { - nativeInvokePlatformMessageEmptyResponseCallback(nativeShellHolderId, responseId); - } else { - Log.w( - TAG, - "Tried to send a platform message response, but FlutterJNI was detached from native C++. Could not send. Response ID: " - + responseId); + // Called on any thread. + shellHolderLock.readLock().lock(); + try { + if (isAttached()) { + nativeInvokePlatformMessageEmptyResponseCallback(nativeShellHolderId, responseId); + } else { + Log.w( + TAG, + "Tried to send a platform message response, but FlutterJNI was detached from native C++. Could not send. Response ID: " + + responseId); + } + } finally { + shellHolderLock.readLock().unlock(); } } @@ -964,14 +985,19 @@ public void invokePlatformMessageResponseCallback( if (!message.isDirect()) { throw new IllegalArgumentException("Expected a direct ByteBuffer."); } - if (isAttached()) { - nativeInvokePlatformMessageResponseCallback( - nativeShellHolderId, responseId, message, position); - } else { - Log.w( - TAG, - "Tried to send a platform message response, but FlutterJNI was detached from native C++. Could not send. Response ID: " - + responseId); + shellHolderLock.readLock().lock(); + try { + if (isAttached()) { + nativeInvokePlatformMessageResponseCallback( + nativeShellHolderId, responseId, message, position); + } else { + Log.w( + TAG, + "Tried to send a platform message response, but FlutterJNI was detached from native C++. Could not send. Response ID: " + + responseId); + } + } finally { + shellHolderLock.readLock().unlock(); } } From 2471665be27b2c4233e6438fd856dca77d612df1 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 15 Oct 2021 13:50:49 -0700 Subject: [PATCH 06/21] jasons feedback --- .../platform/android/android_shell_holder.cc | 2 +- .../embedding/engine/dart/DartMessenger.java | 25 ++++++++++---- .../platform_message_handler_android.cc | 34 +++++++++++-------- 3 files changed, 38 insertions(+), 23 deletions(-) diff --git a/shell/platform/android/android_shell_holder.cc b/shell/platform/android/android_shell_holder.cc index 2517508ffcf03..8d5c14902dc97 100644 --- a/shell/platform/android/android_shell_holder.cc +++ b/shell/platform/android/android_shell_holder.cc @@ -211,7 +211,7 @@ std::unique_ptr AndroidShellHolder::Spawn( shell.GetTaskRunners(), // task runners jni_facade, // JNI interop android_context, // Android context - platform_message_handler); + std::make_shared(jni_facade)); weak_platform_view = platform_view_android->GetWeakPtr(); auto display = Display(jni_facade->GetDisplayRefreshRate()); shell.OnDisplayUpdates(DisplayUpdateType::kStartup, {display}); 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 ca386ad91b3ff..11d201da67b5a 100644 --- a/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java +++ b/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java @@ -37,6 +37,8 @@ class DartMessenger implements BinaryMessenger, PlatformMessageHandler { @NonNull private final Map pendingReplies; private int nextReplyId = 1; + @NonNull private final TaskQueue platformTaskQueue = new PlatformTaskQueue(); + DartMessenger(@NonNull FlutterJNI flutterJNI) { this.flutterJNI = flutterJNI; this.messageHandlers = new ConcurrentHashMap<>(); @@ -69,6 +71,15 @@ public void dispatch(@NonNull Runnable runnable) { } } + private static class PlatformTaskQueue implements TaskQueue { + @NonNull private final Handler handler = new Handler(Looper.getMainLooper()); + + @Override + public void dispatch(@NonNull Runnable runnable) { + handler.post(runnable); + } + } + @Override public TaskQueue makeBackgroundTaskQueue() { return new DefaultTaskQueue(); @@ -119,11 +130,6 @@ private void invokeHandler( try { Log.v(TAG, "Deferring to registered handler to process message."); handlerInfo.handler.onMessage(message, new Reply(flutterJNI, replyId)); - if (message != null && message.isDirect()) { - // This ensures that if a user retains an instance to the ByteBuffer and it happens to - // be direct they will get a deterministic error. - message.limit(0); - } } catch (Exception ex) { Log.e(TAG, "Uncaught exception in binary message listener", ex); flutterJNI.invokePlatformMessageEmptyResponseCallback(replyId); @@ -153,13 +159,18 @@ public void handleMessageFromDart( () -> { try { invokeHandler(handlerInfo, message, replyId); + if (message != null && message.isDirect()) { + // This ensures that if a user retains an instance to the ByteBuffer and it happens to + // be direct they will get a deterministic error. + message.limit(0); + } } finally { + // This is deleting the data underneath the message object. flutterJNI.cleanupMessageData(messageData); } }; if (taskQueue == null) { - Handler mainHandler = new Handler(Looper.getMainLooper()); - mainHandler.post(myRunnable); + platformTaskQueue.dispatch(myRunnable); } else { taskQueue.dispatch(myRunnable); } diff --git a/shell/platform/android/platform_message_handler_android.cc b/shell/platform/android/platform_message_handler_android.cc index 5b84ebb7b8f07..0be67396c7739 100644 --- a/shell/platform/android/platform_message_handler_android.cc +++ b/shell/platform/android/platform_message_handler_android.cc @@ -18,18 +18,21 @@ void PlatformMessageHandlerAndroid::InvokePlatformMessageResponseCallback( } // TODO(gaaclarke): Move the jump to the ui thread here from // PlatformMessageResponseDart so we won't need to use a mutex anymore. - std::unique_lock lock(pending_responses_mutex_); - auto it = pending_responses_.find(response_id); - if (it == pending_responses_.end()) - return; + fml::RefPtr message_response; + { + std::lock_guard lock(pending_responses_mutex_); + auto it = pending_responses_.find(response_id); + if (it == pending_responses_.end()) + return; + message_response = std::move(it->second); + pending_responses_.erase(it); + } + uint8_t* response_data = static_cast(env->GetDirectBufferAddress(java_response_data)); FML_DCHECK(response_data != nullptr); std::vector response = std::vector( response_data, response_data + java_response_position); - auto message_response = std::move(it->second); - pending_responses_.erase(it); - lock.unlock(); message_response->Complete( std::make_unique(std::move(response))); } @@ -41,13 +44,15 @@ void PlatformMessageHandlerAndroid::InvokePlatformMessageEmptyResponseCallback( if (!response_id) { return; } - std::unique_lock lock(pending_responses_mutex_); - auto it = pending_responses_.find(response_id); - if (it == pending_responses_.end()) - return; - auto message_response = std::move(it->second); - pending_responses_.erase(it); - lock.unlock(); + fml::RefPtr message_response; + { + std::lock_guard lock(pending_responses_mutex_); + auto it = pending_responses_.find(response_id); + if (it == pending_responses_.end()) + return; + message_response = std::move(it->second); + pending_responses_.erase(it); + } message_response->CompleteEmpty(); } @@ -63,7 +68,6 @@ void PlatformMessageHandlerAndroid::HandlePlatformMessage( // This call can re-enter in InvokePlatformMessageXxxResponseCallback. jni_facade_->FlutterViewHandlePlatformMessage(std::move(message), response_id); - message = nullptr; } } // namespace flutter From fb4f77f913580e8caf687f5ae817b89d2cf1cfe7 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 15 Oct 2021 17:33:46 -0700 Subject: [PATCH 07/21] added android embedder test --- .../android/android_shell_holder_unittests.cc | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/shell/platform/android/android_shell_holder_unittests.cc b/shell/platform/android/android_shell_holder_unittests.cc index 2d3bc3ef311e1..5ac4171bb720a 100644 --- a/shell/platform/android/android_shell_holder_unittests.cc +++ b/shell/platform/android/android_shell_holder_unittests.cc @@ -7,6 +7,7 @@ namespace flutter { namespace testing { namespace { class MockPlatformViewAndroidJNI : public PlatformViewAndroidJNI { + public: MOCK_METHOD2(FlutterViewHandlePlatformMessage, void(std::unique_ptr message, int responseId)); @@ -51,6 +52,15 @@ class MockPlatformViewAndroidJNI : public PlatformViewAndroidJNI { MOCK_METHOD0(GetDisplayRefreshRate, double()); MOCK_METHOD1(RequestDartDeferredLibrary, bool(int loading_unit_id)); }; + +class MockPlatformMessageResponse : public PlatformMessageResponse { + public: + static fml::RefPtr Create() { + return fml::AdoptRef(new MockPlatformMessageResponse()); + } + MOCK_METHOD1(Complete, void(std::unique_ptr data)); + MOCK_METHOD0(CompleteEmpty, void()); +}; } // namespace TEST(AndroidShellHolder, Create) { @@ -65,5 +75,34 @@ TEST(AndroidShellHolder, Create) { nullptr, /*is_fake_window=*/true); holder->GetPlatformView()->NotifyCreated(window); } + +TEST(AndroidShellHolder, HandlePlatformMessage) { + Settings settings; + settings.enable_software_rendering = false; + auto jni = std::make_shared(); + auto holder = std::make_unique(settings, jni); + EXPECT_NE(holder.get(), nullptr); + EXPECT_TRUE(holder->IsValid()); + EXPECT_NE(holder->GetPlatformView().get(), nullptr); + auto window = fml::MakeRefCounted( + nullptr, /*is_fake_window=*/true); + holder->GetPlatformView()->NotifyCreated(window); + EXPECT_TRUE(holder->GetPlatformMessageHandler()); + size_t data_size = 4; + fml::MallocMapping bytes = + fml::MallocMapping(static_cast(malloc(data_size)), data_size); + fml::RefPtr response = + MockPlatformMessageResponse::Create(); + auto message = std::make_unique( + /*channel=*/"foo", /*data=*/std::move(bytes), /*response=*/response); + int response_id = 1; + EXPECT_CALL(*jni, + FlutterViewHandlePlatformMessage(::testing::_, response_id)); + EXPECT_CALL(*response, CompleteEmpty()); + holder->GetPlatformMessageHandler()->HandlePlatformMessage( + std::move(message)); + holder->GetPlatformMessageHandler() + ->InvokePlatformMessageEmptyResponseCallback(nullptr, response_id); +} } // namespace testing } // namespace flutter From 9b3453d7ba941d0da7db7d7dac189c177fc95055 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 15 Oct 2021 18:06:58 -0700 Subject: [PATCH 08/21] 1) removed javaism from the platform_message_handler 2) remove it's ownership from android_shell_holder, now just shell and the platform_view have a reference --- shell/common/platform_message_handler.h | 5 +++++ shell/common/shell.h | 5 +++++ shell/platform/android/android_shell_holder.cc | 18 +++++++----------- shell/platform/android/android_shell_holder.h | 7 +++---- .../android/android_shell_holder_unittests.cc | 2 +- .../platform_message_handler_android.cc | 17 ++++------------- .../android/platform_message_handler_android.h | 10 ++++------ .../platform/android/platform_view_android.cc | 13 ++++--------- shell/platform/android/platform_view_android.h | 8 ++------ .../android/platform_view_android_jni_impl.cc | 15 +++++++-------- 10 files changed, 42 insertions(+), 58 deletions(-) diff --git a/shell/common/platform_message_handler.h b/shell/common/platform_message_handler.h index d1d3838eec690..edc0fd211b28d 100644 --- a/shell/common/platform_message_handler.h +++ b/shell/common/platform_message_handler.h @@ -15,6 +15,11 @@ class PlatformMessageHandler { virtual ~PlatformMessageHandler() = default; virtual void HandlePlatformMessage( std::unique_ptr message) = 0; + virtual void InvokePlatformMessageResponseCallback( + int response_id, + std::unique_ptr mapping) = 0; + + virtual void InvokePlatformMessageEmptyResponseCallback(int response_id) = 0; }; } // namespace flutter diff --git a/shell/common/shell.h b/shell/common/shell.h index 96497629697f6..6d914329b4455 100644 --- a/shell/common/shell.h +++ b/shell/common/shell.h @@ -395,6 +395,11 @@ class Shell final : public PlatformView::Delegate, /// @see `CreateCompatibleGenerator` void RegisterImageDecoder(ImageGeneratorFactory factory, int32_t priority); + const std::shared_ptr& GetPlatformMessageHandler() + const { + return platform_message_handler_; + } + private: using ServiceProtocolHandler = std::function jni_facade) - : settings_(std::move(settings)), - jni_facade_(jni_facade), - platform_message_handler_(new PlatformMessageHandlerAndroid(jni_facade)) { + : settings_(std::move(settings)), jni_facade_(jni_facade) { static size_t thread_host_count = 1; auto thread_label = std::to_string(thread_host_count++); @@ -53,16 +51,15 @@ AndroidShellHolder::AndroidShellHolder( fml::WeakPtr weak_platform_view; Shell::CreateCallback on_create_platform_view = - [&jni_facade, &weak_platform_view, - platform_message_handler = platform_message_handler_](Shell& shell) { + [&jni_facade, &weak_platform_view](Shell& shell) { std::unique_ptr platform_view_android; platform_view_android = std::make_unique( shell, // delegate shell.GetTaskRunners(), // task runners jni_facade, // JNI interop shell.GetSettings() - .enable_software_rendering, // use software rendering - platform_message_handler); + .enable_software_rendering // use software rendering + ); weak_platform_view = platform_view_android->GetWeakPtr(); auto display = Display(jni_facade->GetDisplayRefreshRate()); shell.OnDisplayUpdates(DisplayUpdateType::kStartup, {display}); @@ -203,15 +200,14 @@ std::unique_ptr AndroidShellHolder::Spawn( // This is a synchronous call, so the captures don't have race checks. Shell::CreateCallback on_create_platform_view = - [&jni_facade, android_context, &weak_platform_view, - platform_message_handler = platform_message_handler_](Shell& shell) { + [&jni_facade, android_context, &weak_platform_view](Shell& shell) { std::unique_ptr platform_view_android; platform_view_android = std::make_unique( shell, // delegate shell.GetTaskRunners(), // task runners jni_facade, // JNI interop - android_context, // Android context - std::make_shared(jni_facade)); + android_context // Android context + ); weak_platform_view = platform_view_android->GetWeakPtr(); auto display = Display(jni_facade->GetDisplayRefreshRate()); shell.OnDisplayUpdates(DisplayUpdateType::kStartup, {display}); diff --git a/shell/platform/android/android_shell_holder.h b/shell/platform/android/android_shell_holder.h index e4e75055db61a..062e7e45c4cff 100644 --- a/shell/platform/android/android_shell_holder.h +++ b/shell/platform/android/android_shell_holder.h @@ -97,9 +97,9 @@ class AndroidShellHolder { void NotifyLowMemoryWarning(); - const std::shared_ptr& - GetPlatformMessageHandler() const { - return platform_message_handler_; + const std::shared_ptr& GetPlatformMessageHandler() + const { + return shell_->GetPlatformMessageHandler(); } private: @@ -111,7 +111,6 @@ class AndroidShellHolder { bool is_valid_ = false; uint64_t next_pointer_flow_id_ = 0; std::shared_ptr asset_manager_; - std::shared_ptr platform_message_handler_; //---------------------------------------------------------------------------- /// @brief Constructor with its components injected. diff --git a/shell/platform/android/android_shell_holder_unittests.cc b/shell/platform/android/android_shell_holder_unittests.cc index 5ac4171bb720a..85cb352e005f9 100644 --- a/shell/platform/android/android_shell_holder_unittests.cc +++ b/shell/platform/android/android_shell_holder_unittests.cc @@ -102,7 +102,7 @@ TEST(AndroidShellHolder, HandlePlatformMessage) { holder->GetPlatformMessageHandler()->HandlePlatformMessage( std::move(message)); holder->GetPlatformMessageHandler() - ->InvokePlatformMessageEmptyResponseCallback(nullptr, response_id); + ->InvokePlatformMessageEmptyResponseCallback(response_id); } } // namespace testing } // namespace flutter diff --git a/shell/platform/android/platform_message_handler_android.cc b/shell/platform/android/platform_message_handler_android.cc index 0be67396c7739..2179611a2f90e 100644 --- a/shell/platform/android/platform_message_handler_android.cc +++ b/shell/platform/android/platform_message_handler_android.cc @@ -8,10 +8,8 @@ PlatformMessageHandlerAndroid::PlatformMessageHandlerAndroid( : jni_facade_(jni_facade) {} void PlatformMessageHandlerAndroid::InvokePlatformMessageResponseCallback( - JNIEnv* env, - jint response_id, - jobject java_response_data, - jint java_response_position) { + int response_id, + std::unique_ptr mapping) { // Called from any thread. if (!response_id) { return; @@ -28,18 +26,11 @@ void PlatformMessageHandlerAndroid::InvokePlatformMessageResponseCallback( pending_responses_.erase(it); } - uint8_t* response_data = - static_cast(env->GetDirectBufferAddress(java_response_data)); - FML_DCHECK(response_data != nullptr); - std::vector response = std::vector( - response_data, response_data + java_response_position); - message_response->Complete( - std::make_unique(std::move(response))); + message_response->Complete(std::move(mapping)); } void PlatformMessageHandlerAndroid::InvokePlatformMessageEmptyResponseCallback( - JNIEnv* env, - jint response_id) { + int response_id) { // Called from any thread. if (!response_id) { return; diff --git a/shell/platform/android/platform_message_handler_android.h b/shell/platform/android/platform_message_handler_android.h index 3d2f4536ed667..dc36013db69c1 100644 --- a/shell/platform/android/platform_message_handler_android.h +++ b/shell/platform/android/platform_message_handler_android.h @@ -20,13 +20,11 @@ class PlatformMessageHandlerAndroid : public PlatformMessageHandler { PlatformMessageHandlerAndroid( const std::shared_ptr& jni_facade); void HandlePlatformMessage(std::unique_ptr message) override; - void InvokePlatformMessageResponseCallback(JNIEnv* env, - jint response_id, - jobject java_response_data, - jint java_response_position); + void InvokePlatformMessageResponseCallback( + int response_id, + std::unique_ptr mapping) override; - void InvokePlatformMessageEmptyResponseCallback(JNIEnv* env, - jint response_id); + void InvokePlatformMessageEmptyResponseCallback(int response_id) override; private: const std::shared_ptr jni_facade_; diff --git a/shell/platform/android/platform_view_android.cc b/shell/platform/android/platform_view_android.cc index 8016e25db4746..064bbf92ae668 100644 --- a/shell/platform/android/platform_view_android.cc +++ b/shell/platform/android/platform_view_android.cc @@ -59,27 +59,22 @@ PlatformViewAndroid::PlatformViewAndroid( PlatformView::Delegate& delegate, flutter::TaskRunners task_runners, std::shared_ptr jni_facade, - bool use_software_rendering, - const std::shared_ptr& - platform_message_handler) + bool use_software_rendering) : PlatformViewAndroid(delegate, std::move(task_runners), std::move(jni_facade), - CreateAndroidContext(use_software_rendering), - platform_message_handler) {} + CreateAndroidContext(use_software_rendering)) {} PlatformViewAndroid::PlatformViewAndroid( PlatformView::Delegate& delegate, flutter::TaskRunners task_runners, const std::shared_ptr& jni_facade, - const std::shared_ptr& android_context, - const std::shared_ptr& - platform_message_handler) + const std::shared_ptr& android_context) : PlatformView(delegate, std::move(task_runners)), jni_facade_(jni_facade), android_context_(std::move(android_context)), platform_view_android_delegate_(jni_facade), - platform_message_handler_(platform_message_handler) { + platform_message_handler_(new PlatformMessageHandlerAndroid(jni_facade)) { // TODO(dnfield): always create a pbuffer surface for background use to // resolve https://github.com/flutter/flutter/issues/73675 if (android_context_) { diff --git a/shell/platform/android/platform_view_android.h b/shell/platform/android/platform_view_android.h index b992d1c8c1ca8..0ff654f0fba72 100644 --- a/shell/platform/android/platform_view_android.h +++ b/shell/platform/android/platform_view_android.h @@ -45,9 +45,7 @@ class PlatformViewAndroid final : public PlatformView { PlatformViewAndroid(PlatformView::Delegate& delegate, flutter::TaskRunners task_runners, std::shared_ptr jni_facade, - bool use_software_rendering, - const std::shared_ptr& - platform_message_handler); + bool use_software_rendering); //---------------------------------------------------------------------------- /// @brief Creates a new PlatformViewAndroid but using an existing @@ -58,9 +56,7 @@ class PlatformViewAndroid final : public PlatformView { PlatformView::Delegate& delegate, flutter::TaskRunners task_runners, const std::shared_ptr& jni_facade, - const std::shared_ptr& android_context, - const std::shared_ptr& - platform_message_handler); + const std::shared_ptr& android_context); ~PlatformViewAndroid() override; diff --git a/shell/platform/android/platform_view_android_jni_impl.cc b/shell/platform/android/platform_view_android_jni_impl.cc index 311d16c81c42b..8b8494b3d3a6d 100644 --- a/shell/platform/android/platform_view_android_jni_impl.cc +++ b/shell/platform/android/platform_view_android_jni_impl.cc @@ -490,12 +490,13 @@ static void InvokePlatformMessageResponseCallback(JNIEnv* env, jint responseId, jobject message, jint position) { + uint8_t* response_data = + static_cast(env->GetDirectBufferAddress(message)); + FML_DCHECK(response_data != nullptr); + auto mapping = std::make_unique( + fml::MallocMapping::Copy(response_data, response_data + position)); ANDROID_SHELL_HOLDER->GetPlatformMessageHandler() - ->InvokePlatformMessageResponseCallback(env, // - responseId, // - message, // - position // - ); + ->InvokePlatformMessageResponseCallback(responseId, std::move(mapping)); } static void InvokePlatformMessageEmptyResponseCallback(JNIEnv* env, @@ -503,9 +504,7 @@ static void InvokePlatformMessageEmptyResponseCallback(JNIEnv* env, jlong shell_holder, jint responseId) { ANDROID_SHELL_HOLDER->GetPlatformMessageHandler() - ->InvokePlatformMessageEmptyResponseCallback(env, // - responseId // - ); + ->InvokePlatformMessageEmptyResponseCallback(responseId); } static void NotifyLowMemoryWarning(JNIEnv* env, From d8b1dbe04e0dd5580c100f5438a8d648286dfcd2 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Mon, 18 Oct 2021 11:13:54 -0700 Subject: [PATCH 09/21] added shell unittest --- shell/common/shell_test.cc | 35 +++++++++-------- shell/common/shell_test.h | 4 +- shell/common/shell_unittests.cc | 70 +++++++++++++++++++++++++++++++++ 3 files changed, 91 insertions(+), 18 deletions(-) diff --git a/shell/common/shell_test.cc b/shell/common/shell_test.cc index b5cc9c8e3ae52..b986d580c080f 100644 --- a/shell/common/shell_test.cc +++ b/shell/common/shell_test.cc @@ -325,7 +325,8 @@ std::unique_ptr ShellTest::CreateShell( std::shared_ptr shell_test_external_view_embedder, bool is_gpu_disabled, - ShellTestPlatformView::BackendType rendering_backend) { + ShellTestPlatformView::BackendType rendering_backend, + Shell::CreateCallback platform_view_create_callback) { const auto vsync_clock = std::make_shared(); CreateVsyncWaiter create_vsync_waiter = [&]() { @@ -338,21 +339,21 @@ std::unique_ptr ShellTest::CreateShell( } }; - Shell::CreateCallback platfrom_view_create_callback = - [vsync_clock, // - &create_vsync_waiter, // - shell_test_external_view_embedder, // - rendering_backend // - ](Shell& shell) { - return ShellTestPlatformView::Create( - shell, // - shell.GetTaskRunners(), // - vsync_clock, // - std::move(create_vsync_waiter), // - rendering_backend, // - shell_test_external_view_embedder // - ); - }; + if (!platform_view_create_callback) { + platform_view_create_callback = [vsync_clock, // + &create_vsync_waiter, // + shell_test_external_view_embedder, // + rendering_backend // + ](Shell& shell) { + return ShellTestPlatformView::Create(shell, // + shell.GetTaskRunners(), // + vsync_clock, // + std::move(create_vsync_waiter), // + rendering_backend, // + shell_test_external_view_embedder // + ); + }; + } Shell::CreateCallback rasterizer_create_callback = [](Shell& shell) { return std::make_unique(shell); }; @@ -360,7 +361,7 @@ std::unique_ptr ShellTest::CreateShell( return Shell::Create(flutter::PlatformData(), // task_runners, // settings, // - platfrom_view_create_callback, // + platform_view_create_callback, // rasterizer_create_callback, // is_gpu_disabled // ); diff --git a/shell/common/shell_test.h b/shell/common/shell_test.h index 1524c150e377a..4a41060c75ee9 100644 --- a/shell/common/shell_test.h +++ b/shell/common/shell_test.h @@ -43,7 +43,9 @@ class ShellTest : public FixtureTest { shell_test_external_view_embedder = nullptr, bool is_gpu_disabled = false, ShellTestPlatformView::BackendType rendering_backend = - ShellTestPlatformView::BackendType::kDefaultBackend); + ShellTestPlatformView::BackendType::kDefaultBackend, + Shell::CreateCallback platform_view_create_callback = + nullptr); void DestroyShell(std::unique_ptr shell); void DestroyShell(std::unique_ptr shell, TaskRunners task_runners); TaskRunners GetTaskRunnersForFixture(); diff --git a/shell/common/shell_unittests.cc b/shell/common/shell_unittests.cc index 0e38e3f6329de..4f4ca440fbdd6 100644 --- a/shell/common/shell_unittests.cc +++ b/shell/common/shell_unittests.cc @@ -45,6 +45,10 @@ namespace flutter { namespace testing { + +using ::testing::_; +using ::testing::Return; + namespace { class MockPlatformViewDelegate : public PlatformView::Delegate { MOCK_METHOD1(OnPlatformViewCreated, void(std::unique_ptr surface)); @@ -119,6 +123,27 @@ class MockPlatformView : public PlatformView { MockPlatformView(MockPlatformViewDelegate& delegate, TaskRunners task_runners) : PlatformView(delegate, task_runners) {} MOCK_METHOD0(CreateRenderingSurface, std::unique_ptr()); + MOCK_CONST_METHOD0(GetPlatformMessageHandler, + std::shared_ptr()); +}; + +class MockPlatformMessageHandler : public PlatformMessageHandler { + public: + MOCK_METHOD1(HandlePlatformMessage, + void(std::unique_ptr message)); + MOCK_METHOD2(InvokePlatformMessageResponseCallback, + void(int response_id, std::unique_ptr mapping)); + MOCK_METHOD1(InvokePlatformMessageEmptyResponseCallback, + void(int response_id)); +}; + +class MockPlatformMessageResponse : public PlatformMessageResponse { + public: + static fml::RefPtr Create() { + return fml::AdoptRef(new MockPlatformMessageResponse()); + } + MOCK_METHOD1(Complete, void(std::unique_ptr data)); + MOCK_METHOD0(CompleteEmpty, void()); }; } // namespace @@ -3156,7 +3181,52 @@ TEST_F(ShellTest, UIWorkAfterOnPlatformViewDestroyed) { ui_flush_latch.Signal(); }); ui_flush_latch.Wait(); + DestroyShell(std::move(shell)); +} +TEST_F(ShellTest, UsesPlatformMessageHandler) { + TaskRunners task_runners = GetTaskRunnersForFixture(); + auto settings = CreateSettingsForFixture(); + MockPlatformViewDelegate platform_view_delegate; + auto platform_message_handler = + std::make_shared(); + int message_id = 1; + EXPECT_CALL(*platform_message_handler, HandlePlatformMessage(_)); + EXPECT_CALL(*platform_message_handler, + InvokePlatformMessageEmptyResponseCallback(message_id)); + Shell::CreateCallback platform_view_create_callback = + [&platform_view_delegate, task_runners, + platform_message_handler](flutter::Shell& shell) { + auto result = std::make_unique(platform_view_delegate, + task_runners); + EXPECT_CALL(*result, GetPlatformMessageHandler()) + .WillOnce(Return(platform_message_handler)); + return result; + }; + auto shell = CreateShell( + /*settings=*/std::move(settings), + /*task_runners=*/task_runners, + /*simulate_vsync=*/false, + /*shell_test_external_view_embedder=*/nullptr, + /*is_gpu_disabled=*/false, + /*rendering_backend=*/ + ShellTestPlatformView::BackendType::kDefaultBackend, + /*platform_view_create_callback=*/platform_view_create_callback); + + EXPECT_EQ(platform_message_handler, shell->GetPlatformMessageHandler()); + PostSync(task_runners.GetUITaskRunner(), [&shell]() { + size_t data_size = 4; + fml::MallocMapping bytes = + fml::MallocMapping(static_cast(malloc(data_size)), data_size); + fml::RefPtr response = + MockPlatformMessageResponse::Create(); + auto message = std::make_unique( + /*channel=*/"foo", /*data=*/std::move(bytes), /*response=*/response); + (static_cast(shell.get())) + ->OnEngineHandlePlatformMessage(std::move(message)); + }); + shell->GetPlatformMessageHandler() + ->InvokePlatformMessageEmptyResponseCallback(message_id); DestroyShell(std::move(shell)); } From 3ded42ef561b3b8ee774dc6677662a975d661ec8 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Mon, 18 Oct 2021 11:52:28 -0700 Subject: [PATCH 10/21] added java unittests --- shell/platform/android/BUILD.gn | 1 + .../embedding/engine/dart/DartMessenger.java | 13 +--- .../engine/dart/PlatformTaskQueue.java | 19 +++++ .../engine/dart/DartMessengerTest.java | 69 +++++++++++++++++-- .../plugin/editing/TextInputPluginTest.java | 6 +- .../platform/PlatformViewsControllerTest.java | 68 ++++++++++++++---- 6 files changed, 140 insertions(+), 36 deletions(-) create mode 100644 shell/platform/android/io/flutter/embedding/engine/dart/PlatformTaskQueue.java diff --git a/shell/platform/android/BUILD.gn b/shell/platform/android/BUILD.gn index c2f1d88130f05..862fc405f88fd 100644 --- a/shell/platform/android/BUILD.gn +++ b/shell/platform/android/BUILD.gn @@ -188,6 +188,7 @@ android_java_sources = [ "io/flutter/embedding/engine/dart/DartExecutor.java", "io/flutter/embedding/engine/dart/DartMessenger.java", "io/flutter/embedding/engine/dart/PlatformMessageHandler.java", + "io/flutter/embedding/engine/dart/PlatformTaskQueue.java", "io/flutter/embedding/engine/deferredcomponents/DeferredComponentManager.java", "io/flutter/embedding/engine/deferredcomponents/PlayStoreDeferredComponentManager.java", "io/flutter/embedding/engine/loader/ApplicationInfoLoader.java", 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 11d201da67b5a..db8c8060b6837 100644 --- a/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java +++ b/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java @@ -4,8 +4,6 @@ package io.flutter.embedding.engine.dart; -import android.os.Handler; -import android.os.Looper; import androidx.annotation.NonNull; import androidx.annotation.Nullable; import androidx.annotation.UiThread; @@ -27,7 +25,7 @@ * *

See {@link PlatformMessageHandler}, which handles messages to Android from Dart */ -class DartMessenger implements BinaryMessenger, PlatformMessageHandler { +public class DartMessenger implements BinaryMessenger, PlatformMessageHandler { private static final String TAG = "DartMessenger"; @NonNull private final FlutterJNI flutterJNI; @@ -71,15 +69,6 @@ public void dispatch(@NonNull Runnable runnable) { } } - private static class PlatformTaskQueue implements TaskQueue { - @NonNull private final Handler handler = new Handler(Looper.getMainLooper()); - - @Override - public void dispatch(@NonNull Runnable runnable) { - handler.post(runnable); - } - } - @Override public TaskQueue makeBackgroundTaskQueue() { return new DefaultTaskQueue(); diff --git a/shell/platform/android/io/flutter/embedding/engine/dart/PlatformTaskQueue.java b/shell/platform/android/io/flutter/embedding/engine/dart/PlatformTaskQueue.java new file mode 100644 index 0000000000000..5f7a5501803f2 --- /dev/null +++ b/shell/platform/android/io/flutter/embedding/engine/dart/PlatformTaskQueue.java @@ -0,0 +1,19 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +package io.flutter.embedding.engine.dart; + +import android.os.Handler; +import android.os.Looper; +import androidx.annotation.NonNull; +import io.flutter.plugin.common.BinaryMessenger; + +public class PlatformTaskQueue implements BinaryMessenger.TaskQueue { + @NonNull private final Handler handler = new Handler(Looper.getMainLooper()); + + @Override + public void dispatch(@NonNull Runnable runnable) { + handler.post(runnable); + } +} 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 5e1922103afbf..952b66e890b08 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 @@ -13,6 +13,7 @@ import io.flutter.plugin.common.BinaryMessenger; import io.flutter.plugin.common.BinaryMessenger.BinaryMessageHandler; import java.nio.ByteBuffer; +import java.util.concurrent.CountDownLatch; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mockito; @@ -22,6 +23,8 @@ @Config(manifest = Config.NONE) @RunWith(RobolectricTestRunner.class) public class DartMessengerTest { + SynchronousTaskQueue synchronousTaskQueue = new SynchronousTaskQueue(); + private static class ReportingUncaughtExceptionHandler implements Thread.UncaughtExceptionHandler { public Throwable latestException; @@ -32,6 +35,12 @@ public void uncaughtException(Thread t, Throwable e) { } } + private static class SynchronousTaskQueue implements BinaryMessenger.TaskQueue { + public void dispatch(Runnable runnable) { + runnable.run(); + } + } + @Test public void itHandlesErrors() { // Setup test. @@ -50,8 +59,8 @@ public void itHandlesErrors() { .when(throwingHandler) .onMessage(any(ByteBuffer.class), any(DartMessenger.Reply.class)); - messenger.setMessageHandler("test", throwingHandler); - messenger.handleMessageFromDart("test", ByteBuffer.allocate(0), 0); + messenger.setMessageHandler("test", throwingHandler, synchronousTaskQueue); + messenger.handleMessageFromDart("test", ByteBuffer.allocate(0), 0, 0); assertNotNull(reportingHandler.latestException); assertTrue(reportingHandler.latestException instanceof AssertionError); currentThread.setUncaughtExceptionHandler(savedHandler); @@ -68,14 +77,14 @@ public void givesDirectByteBuffer() { (message, reply) -> { wasDirect[0] = message.isDirect(); }; - messenger.setMessageHandler(channel, handler); + messenger.setMessageHandler(channel, handler, synchronousTaskQueue); 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); + messenger.handleMessageFromDart(channel, message, /*replyId=*/ 123, 0); assertTrue(wasDirect[0]); } @@ -92,14 +101,14 @@ public void directByteBufferLimitZeroAfterUsage() { byteBuffers[0] = message; assertEquals(bufferSize, byteBuffers[0].limit()); }; - messenger.setMessageHandler(channel, handler); + messenger.setMessageHandler(channel, handler, synchronousTaskQueue); 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); + messenger.handleMessageFromDart(channel, message, /*replyId=*/ 123, 0); assertNotNull(byteBuffers[0]); assertTrue(byteBuffers[0].isDirect()); assertEquals(0, byteBuffers[0].limit()); @@ -139,4 +148,52 @@ public void replyIdIncrementsOnNullReply() { messenger.send(channel, null, null); verify(fakeFlutterJni, times(1)).dispatchEmptyPlatformMessage(eq("foobar"), eq(2)); } + + @Test + public void cleansUpMessageData() throws InterruptedException { + final FlutterJNI fakeFlutterJni = mock(FlutterJNI.class); + final DartMessenger messenger = new DartMessenger(fakeFlutterJni); + BinaryMessenger.TaskQueue taskQueue = messenger.makeBackgroundTaskQueue(); + String channel = "foobar"; + BinaryMessenger.BinaryMessageHandler handler = + (ByteBuffer message, BinaryMessenger.BinaryReply reply) -> { + reply.reply(null); + }; + messenger.setMessageHandler(channel, handler, taskQueue); + final ByteBuffer message = ByteBuffer.allocateDirect(4 * 2); + int replyId = 1; + long messageData = 1234; + messenger.handleMessageFromDart(channel, message, replyId, messageData); + final CountDownLatch latch = new CountDownLatch(1); + taskQueue.dispatch( + () -> { + latch.countDown(); + }); + latch.await(); + verify(fakeFlutterJni).cleanupMessageData(eq(messageData)); + } + + @Test + public void cleansUpMessageDataOnError() throws InterruptedException { + final FlutterJNI fakeFlutterJni = mock(FlutterJNI.class); + final DartMessenger messenger = new DartMessenger(fakeFlutterJni); + BinaryMessenger.TaskQueue taskQueue = messenger.makeBackgroundTaskQueue(); + String channel = "foobar"; + BinaryMessenger.BinaryMessageHandler handler = + (ByteBuffer message, BinaryMessenger.BinaryReply reply) -> { + throw new RuntimeException("hello"); + }; + messenger.setMessageHandler(channel, handler, taskQueue); + final ByteBuffer message = ByteBuffer.allocateDirect(4 * 2); + int replyId = 1; + long messageData = 1234; + messenger.handleMessageFromDart(channel, message, replyId, messageData); + final CountDownLatch latch = new CountDownLatch(1); + taskQueue.dispatch( + () -> { + latch.countDown(); + }); + latch.await(); + verify(fakeFlutterJni).cleanupMessageData(eq(messageData)); + } } 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 0c858342a35d7..c937721fe60e5 100644 --- a/shell/platform/android/test/io/flutter/plugin/editing/TextInputPluginTest.java +++ b/shell/platform/android/test/io/flutter/plugin/editing/TextInputPluginTest.java @@ -1884,7 +1884,7 @@ public void respondsToInputChannelMessages() { textInputChannel.setTextInputMethodHandler(mockHandler); verify(mockBinaryMessenger, times(1)) - .setMessageHandler(any(String.class), binaryMessageHandlerCaptor.capture()); + .setMessageHandler(any(String.class), binaryMessageHandlerCaptor.capture(), eq(null)); BinaryMessenger.BinaryMessageHandler binaryMessageHandler = binaryMessageHandlerCaptor.getValue(); @@ -1917,7 +1917,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(null)); JSONObject arguments = new JSONObject(); arguments.put("action", "actionCommand"); @@ -1948,7 +1948,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(null)); 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 21c392879d688..38c919cae33dc 100644 --- a/shell/platform/android/test/io/flutter/plugin/platform/PlatformViewsControllerTest.java +++ b/shell/platform/android/test/io/flutter/plugin/platform/PlatformViewsControllerTest.java @@ -218,7 +218,7 @@ public void itUsesActionEventTypeFromMotionEventForHybridPlatformViews() { } @Test - @Config(shadows = {ShadowFlutterJNI.class}) + @Config(shadows = {ShadowFlutterJNI.class, ShadowPlatformTaskQueue.class}) public void getPlatformViewById__hybridComposition() { PlatformViewsController platformViewsController = new PlatformViewsController(); @@ -246,7 +246,7 @@ public void getPlatformViewById__hybridComposition() { } @Test - @Config(shadows = {ShadowFlutterJNI.class}) + @Config(shadows = {ShadowFlutterJNI.class, ShadowPlatformTaskQueue.class}) public void createPlatformViewMessage__initializesAndroidView() { PlatformViewsController platformViewsController = new PlatformViewsController(); @@ -268,7 +268,7 @@ public void createPlatformViewMessage__initializesAndroidView() { } @Test - @Config(shadows = {ShadowFlutterJNI.class}) + @Config(shadows = {ShadowFlutterJNI.class, ShadowPlatformTaskQueue.class}) public void createPlatformViewMessage__throwsIfViewIsNull() { PlatformViewsController platformViewsController = new PlatformViewsController(); @@ -296,7 +296,7 @@ public void createPlatformViewMessage__throwsIfViewIsNull() { } @Test - @Config(shadows = {ShadowFlutterJNI.class}) + @Config(shadows = {ShadowFlutterJNI.class, ShadowPlatformTaskQueue.class}) public void onDetachedFromJNI_clearsPlatformViewContext() { PlatformViewsController platformViewsController = new PlatformViewsController(); @@ -326,7 +326,7 @@ public void onDetachedFromJNI_clearsPlatformViewContext() { } @Test - @Config(shadows = {ShadowFlutterJNI.class}) + @Config(shadows = {ShadowFlutterJNI.class, ShadowPlatformTaskQueue.class}) public void onPreEngineRestart_clearsPlatformViewContext() { PlatformViewsController platformViewsController = new PlatformViewsController(); @@ -356,7 +356,7 @@ public void onPreEngineRestart_clearsPlatformViewContext() { } @Test - @Config(shadows = {ShadowFlutterJNI.class}) + @Config(shadows = {ShadowFlutterJNI.class, ShadowPlatformTaskQueue.class}) public void createPlatformViewMessage__throwsIfViewHasParent() { PlatformViewsController platformViewsController = new PlatformViewsController(); @@ -386,7 +386,7 @@ public void createPlatformViewMessage__throwsIfViewHasParent() { } @Test - @Config(shadows = {ShadowFlutterJNI.class}) + @Config(shadows = {ShadowFlutterJNI.class, ShadowPlatformTaskQueue.class}) public void disposeAndroidView__hybridComposition() { PlatformViewsController platformViewsController = new PlatformViewsController(); @@ -427,7 +427,12 @@ public void disposeAndroidView__hybridComposition() { } @Test - @Config(shadows = {ShadowFlutterSurfaceView.class, ShadowFlutterJNI.class}) + @Config( + shadows = { + ShadowFlutterSurfaceView.class, + ShadowFlutterJNI.class, + ShadowPlatformTaskQueue.class + }) public void onEndFrame__destroysOverlaySurfaceAfterFrameOnFlutterSurfaceView() { final PlatformViewsController platformViewsController = new PlatformViewsController(); @@ -525,7 +530,12 @@ public void onEndFrame__removesPlatformView() { } @Test - @Config(shadows = {ShadowFlutterSurfaceView.class, ShadowFlutterJNI.class}) + @Config( + shadows = { + ShadowFlutterSurfaceView.class, + ShadowFlutterJNI.class, + ShadowPlatformTaskQueue.class + }) public void onEndFrame__removesPlatformViewParent() { final PlatformViewsController platformViewsController = new PlatformViewsController(); @@ -563,7 +573,12 @@ public void onEndFrame__removesPlatformViewParent() { } @Test - @Config(shadows = {ShadowFlutterSurfaceView.class, ShadowFlutterJNI.class}) + @Config( + shadows = { + ShadowFlutterSurfaceView.class, + ShadowFlutterJNI.class, + ShadowPlatformTaskQueue.class + }) public void detach__destroysOverlaySurfaces() { final PlatformViewsController platformViewsController = new PlatformViewsController(); @@ -695,7 +710,7 @@ public void checkInputConnectionProxy__falseIfViewIsNull() { } @Test - @Config(shadows = {ShadowFlutterJNI.class}) + @Config(shadows = {ShadowFlutterJNI.class, ShadowPlatformTaskQueue.class}) public void convertPlatformViewRenderSurfaceAsDefault() { final PlatformViewsController platformViewsController = new PlatformViewsController(); @@ -741,7 +756,7 @@ public void convertPlatformViewRenderSurfaceAsDefault() { } @Test - @Config(shadows = {ShadowFlutterJNI.class}) + @Config(shadows = {ShadowFlutterJNI.class, ShadowPlatformTaskQueue.class}) public void dontConverRenderSurfaceWhenFlagIsTrue() { final PlatformViewsController platformViewsController = new PlatformViewsController(); @@ -812,7 +827,10 @@ private static void createPlatformView( new MethodCall("create", platformViewCreateArguments); jni.handlePlatformMessage( - "flutter/platform_views", encodeMethodCall(platformCreateMethodCall), /*replyId=*/ 0); + "flutter/platform_views", + encodeMethodCall(platformCreateMethodCall), + /*replyId=*/ 0, + /*messageData=*/ 0); } private static void disposePlatformView( @@ -826,7 +844,10 @@ private static void disposePlatformView( new MethodCall("dispose", platformViewDisposeArguments); jni.handlePlatformMessage( - "flutter/platform_views", encodeMethodCall(platformDisposeMethodCall), /*replyId=*/ 0); + "flutter/platform_views", + encodeMethodCall(platformDisposeMethodCall), + /*replyId=*/ 0, + /*messageData=*/ 0); } private static void synchronizeToNativeViewHierarchy( @@ -835,7 +856,10 @@ private static void synchronizeToNativeViewHierarchy( final MethodCall convertMethodCall = new MethodCall("synchronizeToNativeViewHierarchy", yes); jni.handlePlatformMessage( - "flutter/platform_views", encodeMethodCall(convertMethodCall), /*replyId=*/ 0); + "flutter/platform_views", + encodeMethodCall(convertMethodCall), + /*replyId=*/ 0, + /*messageData=*/ 0); } private static FlutterView attach( @@ -901,6 +925,20 @@ public FlutterImageView createImageView() { return view; } + /** + * For convenience when writing tests, this allows us to make fake messages from Flutter via + * Platform Channels. Typically those calls happen on the ui thread which dispatches to the + * platform thread. Since tests run on the platform thread it makes it difficult to test without + * this, but isn't technically required. + */ + @Implements(io.flutter.embedding.engine.dart.PlatformTaskQueue.class) + public static class ShadowPlatformTaskQueue { + @Implementation + public void dispatch(Runnable runnable) { + runnable.run(); + } + } + @Implements(FlutterJNI.class) public static class ShadowFlutterJNI { private static SparseArray replies = new SparseArray<>(); From a3e5eb5a87909b1f1c591718699051ad41a18665 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Mon, 18 Oct 2021 15:51:30 -0700 Subject: [PATCH 11/21] some random cleanup --- .../dev/flutter/android_background_image/MainActivity.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/testing/android_background_image/android/app/src/main/java/dev/flutter/android_background_image/MainActivity.java b/testing/android_background_image/android/app/src/main/java/dev/flutter/android_background_image/MainActivity.java index 73001786ba308..57d62e93587c9 100644 --- a/testing/android_background_image/android/app/src/main/java/dev/flutter/android_background_image/MainActivity.java +++ b/testing/android_background_image/android/app/src/main/java/dev/flutter/android_background_image/MainActivity.java @@ -31,7 +31,10 @@ public void onMessage(ByteBuffer message, final BinaryMessenger.BinaryReply call @Override public void configureFlutterEngine(@NonNull FlutterEngine flutterEngine) { - flutterEngine.getDartExecutor().getBinaryMessenger().setMessageHandler("finish", finishHandler); + flutterEngine + .getDartExecutor() + .getBinaryMessenger() + .setMessageHandler("finish", finishHandler, null); final boolean moved = moveTaskToBack(true); if (!moved) { From 30ca22e7b494e53aa3d16d15de086ff373dda8f5 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Tue, 19 Oct 2021 10:22:04 -0700 Subject: [PATCH 12/21] updated licenses --- ci/licenses_golden/licenses_flutter | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index 97f2e2fa5c669..da283c6333236 100755 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -708,6 +708,7 @@ FILE: ../../../flutter/shell/common/persistent_cache_unittests.cc FILE: ../../../flutter/shell/common/pipeline.cc FILE: ../../../flutter/shell/common/pipeline.h FILE: ../../../flutter/shell/common/pipeline_unittests.cc +FILE: ../../../flutter/shell/common/platform_message_handler.h FILE: ../../../flutter/shell/common/platform_view.cc FILE: ../../../flutter/shell/common/platform_view.h FILE: ../../../flutter/shell/common/pointer_data_dispatcher.cc @@ -841,6 +842,7 @@ FILE: ../../../flutter/shell/platform/android/io/flutter/embedding/engine/Flutte FILE: ../../../flutter/shell/platform/android/io/flutter/embedding/engine/dart/DartExecutor.java FILE: ../../../flutter/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java FILE: ../../../flutter/shell/platform/android/io/flutter/embedding/engine/dart/PlatformMessageHandler.java +FILE: ../../../flutter/shell/platform/android/io/flutter/embedding/engine/dart/PlatformTaskQueue.java FILE: ../../../flutter/shell/platform/android/io/flutter/embedding/engine/deferredcomponents/DeferredComponentManager.java FILE: ../../../flutter/shell/platform/android/io/flutter/embedding/engine/deferredcomponents/PlayStoreDeferredComponentManager.java FILE: ../../../flutter/shell/platform/android/io/flutter/embedding/engine/loader/ApplicationInfoLoader.java @@ -938,6 +940,8 @@ FILE: ../../../flutter/shell/platform/android/jni/jni_mock_unittest.cc FILE: ../../../flutter/shell/platform/android/jni/platform_view_android_jni.cc FILE: ../../../flutter/shell/platform/android/jni/platform_view_android_jni.h FILE: ../../../flutter/shell/platform/android/library_loader.cc +FILE: ../../../flutter/shell/platform/android/platform_message_handler_android.cc +FILE: ../../../flutter/shell/platform/android/platform_message_handler_android.h FILE: ../../../flutter/shell/platform/android/platform_message_response_android.cc FILE: ../../../flutter/shell/platform/android/platform_message_response_android.h FILE: ../../../flutter/shell/platform/android/platform_view_android.cc From 734f5d8711a4317c663fe40972d5d32bee7e10bd Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Tue, 19 Oct 2021 11:13:50 -0700 Subject: [PATCH 13/21] added docstrings --- shell/common/platform_message_handler.h | 10 ++++++++ shell/common/platform_view.cc | 5 ++++ shell/common/platform_view.h | 13 +++++++---- shell/common/shell.cc | 5 ++++ shell/common/shell.h | 7 +++--- .../flutter/embedding/engine/FlutterJNI.java | 7 ++++++ .../embedding/engine/dart/DartMessenger.java | 2 +- .../engine/dart/PlatformTaskQueue.java | 1 + .../plugin/common/BasicMessageChannel.java | 22 +++++++++++++++++- .../plugin/common/BinaryMessenger.java | 3 +++ .../flutter/plugin/common/EventChannel.java | 23 ++++++++++++++++++- .../flutter/plugin/common/MethodChannel.java | 10 ++++++++ 12 files changed, 98 insertions(+), 10 deletions(-) diff --git a/shell/common/platform_message_handler.h b/shell/common/platform_message_handler.h index edc0fd211b28d..daa75b25a3e26 100644 --- a/shell/common/platform_message_handler.h +++ b/shell/common/platform_message_handler.h @@ -10,15 +10,25 @@ #include "flutter/lib/ui/window/platform_message.h" namespace flutter { + +/// An interface over the ability to handle PlatformMessages that are being sent +/// from Flutter to the host platform. class PlatformMessageHandler { public: virtual ~PlatformMessageHandler() = default; + + /// Ultimately sends the PlatformMessage to the host platform. virtual void HandlePlatformMessage( std::unique_ptr message) = 0; + + /// Performs the return procedure for an associated call to + /// HandlePlatformMessage. virtual void InvokePlatformMessageResponseCallback( int response_id, std::unique_ptr mapping) = 0; + /// Performs the return procedure for an associated call to + /// HandlePlatformMessage where there is no return value. virtual void InvokePlatformMessageEmptyResponseCallback(int response_id) = 0; }; } // namespace flutter diff --git a/shell/common/platform_view.cc b/shell/common/platform_view.cc index cd3fada30afb4..2752a6e5dab28 100644 --- a/shell/common/platform_view.cc +++ b/shell/common/platform_view.cc @@ -184,4 +184,9 @@ PlatformView::CreateSnapshotSurfaceProducer() { return nullptr; } +std::shared_ptr +PlatformView::GetPlatformMessageHandler() const { + return nullptr; +} + } // namespace flutter diff --git a/shell/common/platform_view.h b/shell/common/platform_view.h index f02e4d45142a5..f0e925fc72c26 100644 --- a/shell/common/platform_view.h +++ b/shell/common/platform_view.h @@ -811,11 +811,16 @@ class PlatformView { virtual std::unique_ptr CreateSnapshotSurfaceProducer(); - // Returning nullptr means send message to the PlatformView. + //-------------------------------------------------------------------------- + /// @brief Specifies a delegate that will receive PlatformMessages from + /// Flutter to the host platform. + /// + /// @details If this returns `null` that means PlatformMessages should be sent + /// to the PlatformView. That is to protect legacy behavior, any embedder + /// that wants to support executing Platform Channel handlers on background + /// threads should be returing a thread-safe PlatformMessageHandler instead. virtual std::shared_ptr GetPlatformMessageHandler() - const { - return nullptr; - } + const; protected: PlatformView::Delegate& delegate_; diff --git a/shell/common/shell.cc b/shell/common/shell.cc index b9b42d31212ef..df301e717fea9 100644 --- a/shell/common/shell.cc +++ b/shell/common/shell.cc @@ -1872,4 +1872,9 @@ fml::TimePoint Shell::GetCurrentTimePoint() { return fml::TimePoint::Now(); } +const std::shared_ptr& +Shell::GetPlatformMessageHandler() const { + return platform_message_handler_; +} + } // namespace flutter diff --git a/shell/common/shell.h b/shell/common/shell.h index 6d914329b4455..3884299d19294 100644 --- a/shell/common/shell.h +++ b/shell/common/shell.h @@ -395,10 +395,11 @@ class Shell final : public PlatformView::Delegate, /// @see `CreateCompatibleGenerator` void RegisterImageDecoder(ImageGeneratorFactory factory, int32_t priority); + //---------------------------------------------------------------------------- + /// @brief Returns the delegate object that handles PlatformMessage's from + /// Flutter to the host platform (and its responses). const std::shared_ptr& GetPlatformMessageHandler() - const { - return platform_message_handler_; - } + const; private: using ServiceProtocolHandler = diff --git a/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java b/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java index aec00bca83b01..c1e36359ae866 100644 --- a/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java +++ b/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java @@ -877,6 +877,13 @@ public void setPlatformMessageHandler(@Nullable PlatformMessageHandler platformM private native void nativeCleanupMessageData(long messageData); + /** + * Destroys the resources provided sent to `handlePlatformMessage`. + * + *

This can be called on any thread. + * + * @param messageData the argument sent to handlePlatformMessage. + */ public void cleanupMessageData(long messageData) { // This doesn't rely on being attached like other methods. nativeCleanupMessageData(messageData); 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 db8c8060b6837..afa9bdfc22eea 100644 --- a/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java +++ b/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java @@ -25,7 +25,7 @@ * *

See {@link PlatformMessageHandler}, which handles messages to Android from Dart */ -public class DartMessenger implements BinaryMessenger, PlatformMessageHandler { +class DartMessenger implements BinaryMessenger, PlatformMessageHandler { private static final String TAG = "DartMessenger"; @NonNull private final FlutterJNI flutterJNI; diff --git a/shell/platform/android/io/flutter/embedding/engine/dart/PlatformTaskQueue.java b/shell/platform/android/io/flutter/embedding/engine/dart/PlatformTaskQueue.java index 5f7a5501803f2..6d75836bed752 100644 --- a/shell/platform/android/io/flutter/embedding/engine/dart/PlatformTaskQueue.java +++ b/shell/platform/android/io/flutter/embedding/engine/dart/PlatformTaskQueue.java @@ -9,6 +9,7 @@ import androidx.annotation.NonNull; import io.flutter.plugin.common.BinaryMessenger; +/** A BinaryMessenger.TaskQueue that posts to the platform thread (aka main thread). */ public class PlatformTaskQueue implements BinaryMessenger.TaskQueue { @NonNull private final Handler handler = new Handler(Looper.getMainLooper()); diff --git a/shell/platform/android/io/flutter/plugin/common/BasicMessageChannel.java b/shell/platform/android/io/flutter/plugin/common/BasicMessageChannel.java index 24256b139c3cd..ce8470c4a6b34 100644 --- a/shell/platform/android/io/flutter/plugin/common/BasicMessageChannel.java +++ b/shell/platform/android/io/flutter/plugin/common/BasicMessageChannel.java @@ -36,6 +36,7 @@ public final class BasicMessageChannel { @NonNull private final BinaryMessenger messenger; @NonNull private final String name; @NonNull private final MessageCodec codec; + @Nullable private final BinaryMessenger.TaskQueue taskQueue; /** * Creates a new channel associated with the specified {@link BinaryMessenger} and with the @@ -47,6 +48,24 @@ public final class BasicMessageChannel { */ public BasicMessageChannel( @NonNull BinaryMessenger messenger, @NonNull String name, @NonNull MessageCodec codec) { + this(messenger, name, codec, null); + } + + /** + * Creates a new channel associated with the specified {@link BinaryMessenger} and with the + * specified name and {@link MessageCodec}. + * + * @param messenger a {@link BinaryMessenger}. + * @param name a channel name String. + * @param codec a {@link MessageCodec}. + * @param taskQueue a {@link BinaryMessenger.TaskQueue} that specifies what thread will execute + * the handler. Specifying null mean execute on the platform thread. + */ + public BasicMessageChannel( + @NonNull BinaryMessenger messenger, + @NonNull String name, + @NonNull MessageCodec codec, + BinaryMessenger.TaskQueue taskQueue) { if (BuildConfig.DEBUG) { if (messenger == null) { Log.e(TAG, "Parameter messenger must not be null."); @@ -61,6 +80,7 @@ public BasicMessageChannel( this.messenger = messenger; this.name = name; this.codec = codec; + this.taskQueue = taskQueue; } /** @@ -102,7 +122,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), null); + name, handler == null ? null : new IncomingMessageHandler(handler), taskQueue); } /** diff --git a/shell/platform/android/io/flutter/plugin/common/BinaryMessenger.java b/shell/platform/android/io/flutter/plugin/common/BinaryMessenger.java index 796e9eede380b..cae99197f53ee 100644 --- a/shell/platform/android/io/flutter/plugin/common/BinaryMessenger.java +++ b/shell/platform/android/io/flutter/plugin/common/BinaryMessenger.java @@ -26,6 +26,7 @@ * @see EventChannel , which supports communication using event streams. */ public interface BinaryMessenger { + /** An abstraction over dispatching a Runnable to be executed on some thread. */ public interface TaskQueue { void dispatch(@NonNull Runnable runnable); } @@ -69,6 +70,8 @@ public interface TaskQueue { * * @param channel the name {@link String} of the channel. * @param handler a {@link BinaryMessageHandler} to be invoked on incoming messages, or null. + * @param taskQueue a {@link BinaryMessenger.TaskQueue} that specifies what thread will execute + * the handler. Specifying null mean execute on the platform thread. */ @UiThread void setMessageHandler( diff --git a/shell/platform/android/io/flutter/plugin/common/EventChannel.java b/shell/platform/android/io/flutter/plugin/common/EventChannel.java index 0c2d65272d3bf..5b584582c1937 100644 --- a/shell/platform/android/io/flutter/plugin/common/EventChannel.java +++ b/shell/platform/android/io/flutter/plugin/common/EventChannel.java @@ -4,6 +4,7 @@ package io.flutter.plugin.common; +import androidx.annotation.Nullable; import androidx.annotation.UiThread; import io.flutter.BuildConfig; import io.flutter.Log; @@ -34,6 +35,7 @@ public final class EventChannel { private final BinaryMessenger messenger; private final String name; private final MethodCodec codec; + @Nullable private final BinaryMessenger.TaskQueue taskQueue; /** * Creates a new channel associated with the specified {@link BinaryMessenger} and with the @@ -55,6 +57,24 @@ public EventChannel(BinaryMessenger messenger, String name) { * @param codec a {@link MessageCodec}. */ public EventChannel(BinaryMessenger messenger, String name, MethodCodec codec) { + this(messenger, name, codec, null); + } + + /** + * Creates a new channel associated with the specified {@link BinaryMessenger} and with the + * specified name and {@link MethodCodec}. + * + * @param messenger a {@link BinaryMessenger}. + * @param name a channel name String. + * @param codec a {@link MessageCodec}. + * @param taskQueue a {@link BinaryMessenger.TaskQueue} that specifies what thread will execute + * the handler. Specifying null mean execute on the platform thread. + */ + public EventChannel( + BinaryMessenger messenger, + String name, + MethodCodec codec, + BinaryMessenger.TaskQueue taskQueue) { if (BuildConfig.DEBUG) { if (messenger == null) { Log.e(TAG, "Parameter messenger must not be null."); @@ -69,6 +89,7 @@ public EventChannel(BinaryMessenger messenger, String name, MethodCodec codec) { this.messenger = messenger; this.name = name; this.codec = codec; + this.taskQueue = taskQueue; } /** @@ -84,7 +105,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), null); + name, handler == null ? null : new IncomingStreamRequestHandler(handler), taskQueue); } /** diff --git a/shell/platform/android/io/flutter/plugin/common/MethodChannel.java b/shell/platform/android/io/flutter/plugin/common/MethodChannel.java index 7c3d91cba7ca5..00ce7614e1ed0 100644 --- a/shell/platform/android/io/flutter/plugin/common/MethodChannel.java +++ b/shell/platform/android/io/flutter/plugin/common/MethodChannel.java @@ -60,6 +60,16 @@ public MethodChannel(BinaryMessenger messenger, String name, MethodCodec codec) this(messenger, name, codec, null); } + /** + * Creates a new channel associated with the specified {@link BinaryMessenger} and with the + * specified name and {@link MethodCodec}. + * + * @param messenger a {@link BinaryMessenger}. + * @param name a channel name String. + * @param codec a {@link MessageCodec}. + * @param taskQueue a {@link BinaryMessenger.TaskQueue} that specifies what thread will execute + * the handler. Specifying null mean execute on the platform thread. + */ public MethodChannel( BinaryMessenger messenger, String name, From a700e43c6a0a6d531b249f1d30cf33c93e4640a1 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Tue, 19 Oct 2021 11:56:50 -0700 Subject: [PATCH 14/21] added missing docstring --- .../android/io/flutter/plugin/common/BinaryMessenger.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/shell/platform/android/io/flutter/plugin/common/BinaryMessenger.java b/shell/platform/android/io/flutter/plugin/common/BinaryMessenger.java index cae99197f53ee..83e692797a78f 100644 --- a/shell/platform/android/io/flutter/plugin/common/BinaryMessenger.java +++ b/shell/platform/android/io/flutter/plugin/common/BinaryMessenger.java @@ -31,6 +31,12 @@ public interface TaskQueue { void dispatch(@NonNull Runnable runnable); } + /** + * Creates a TaskQueue that executes the tasks serially on a background thread. + * + *

There is no guarantee that the tasks will execute on the same thread, just that execution is + * serial. + */ @UiThread TaskQueue makeBackgroundTaskQueue(); From 7fa94f2e738799041e762fceee23be5a64076c51 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Tue, 19 Oct 2021 15:48:34 -0700 Subject: [PATCH 15/21] added missing threading information in the docstring --- shell/common/platform_message_handler.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/shell/common/platform_message_handler.h b/shell/common/platform_message_handler.h index daa75b25a3e26..8e3f77e448236 100644 --- a/shell/common/platform_message_handler.h +++ b/shell/common/platform_message_handler.h @@ -18,17 +18,20 @@ class PlatformMessageHandler { virtual ~PlatformMessageHandler() = default; /// Ultimately sends the PlatformMessage to the host platform. + /// This method is invoked on the ui thread. virtual void HandlePlatformMessage( std::unique_ptr message) = 0; /// Performs the return procedure for an associated call to /// HandlePlatformMessage. + /// This method should be thread-safe and able to be invoked on any thread. virtual void InvokePlatformMessageResponseCallback( int response_id, std::unique_ptr mapping) = 0; /// Performs the return procedure for an associated call to /// HandlePlatformMessage where there is no return value. + /// This method should be thread-safe and able to be invoked on any thread. virtual void InvokePlatformMessageEmptyResponseCallback(int response_id) = 0; }; } // namespace flutter From 0140577bc0c0e904775d2be3314f88265187ce5b Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Tue, 19 Oct 2021 16:47:56 -0700 Subject: [PATCH 16/21] handful of feedback for dan --- .../android/io/flutter/embedding/engine/FlutterJNI.java | 5 ++++- .../io/flutter/plugin/common/BasicMessageChannel.java | 2 +- .../android/io/flutter/plugin/common/BinaryMessenger.java | 2 +- .../android/io/flutter/plugin/common/EventChannel.java | 2 +- .../android/io/flutter/plugin/common/MethodChannel.java | 2 +- 5 files changed, 8 insertions(+), 5 deletions(-) diff --git a/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java b/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java index c1e36359ae866..61dec933a8a3a 100644 --- a/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java +++ b/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java @@ -894,7 +894,10 @@ public void cleanupMessageData(long messageData) { @SuppressWarnings("unused") @VisibleForTesting public void handlePlatformMessage( - @NonNull final String channel, ByteBuffer message, final int replyId, long messageData) { + @NonNull final String channel, + ByteBuffer message, + final int replyId, + final long messageData) { if (platformMessageHandler != null) { platformMessageHandler.handleMessageFromDart(channel, message, replyId, messageData); } else { diff --git a/shell/platform/android/io/flutter/plugin/common/BasicMessageChannel.java b/shell/platform/android/io/flutter/plugin/common/BasicMessageChannel.java index ce8470c4a6b34..6f67a5407431e 100644 --- a/shell/platform/android/io/flutter/plugin/common/BasicMessageChannel.java +++ b/shell/platform/android/io/flutter/plugin/common/BasicMessageChannel.java @@ -59,7 +59,7 @@ public BasicMessageChannel( * @param name a channel name String. * @param codec a {@link MessageCodec}. * @param taskQueue a {@link BinaryMessenger.TaskQueue} that specifies what thread will execute - * the handler. Specifying null mean execute on the platform thread. + * the handler. Specifying null means execute on the platform thread. */ public BasicMessageChannel( @NonNull BinaryMessenger messenger, diff --git a/shell/platform/android/io/flutter/plugin/common/BinaryMessenger.java b/shell/platform/android/io/flutter/plugin/common/BinaryMessenger.java index 83e692797a78f..de454ab8d80b9 100644 --- a/shell/platform/android/io/flutter/plugin/common/BinaryMessenger.java +++ b/shell/platform/android/io/flutter/plugin/common/BinaryMessenger.java @@ -77,7 +77,7 @@ public interface TaskQueue { * @param channel the name {@link String} of the channel. * @param handler a {@link BinaryMessageHandler} to be invoked on incoming messages, or null. * @param taskQueue a {@link BinaryMessenger.TaskQueue} that specifies what thread will execute - * the handler. Specifying null mean execute on the platform thread. + * the handler. Specifying null means execute on the platform thread. */ @UiThread void setMessageHandler( diff --git a/shell/platform/android/io/flutter/plugin/common/EventChannel.java b/shell/platform/android/io/flutter/plugin/common/EventChannel.java index 5b584582c1937..e0cb4758b5355 100644 --- a/shell/platform/android/io/flutter/plugin/common/EventChannel.java +++ b/shell/platform/android/io/flutter/plugin/common/EventChannel.java @@ -68,7 +68,7 @@ public EventChannel(BinaryMessenger messenger, String name, MethodCodec codec) { * @param name a channel name String. * @param codec a {@link MessageCodec}. * @param taskQueue a {@link BinaryMessenger.TaskQueue} that specifies what thread will execute - * the handler. Specifying null mean execute on the platform thread. + * the handler. Specifying null means execute on the platform thread. */ public EventChannel( BinaryMessenger messenger, diff --git a/shell/platform/android/io/flutter/plugin/common/MethodChannel.java b/shell/platform/android/io/flutter/plugin/common/MethodChannel.java index 00ce7614e1ed0..be59ad8c929cb 100644 --- a/shell/platform/android/io/flutter/plugin/common/MethodChannel.java +++ b/shell/platform/android/io/flutter/plugin/common/MethodChannel.java @@ -68,7 +68,7 @@ public MethodChannel(BinaryMessenger messenger, String name, MethodCodec codec) * @param name a channel name String. * @param codec a {@link MessageCodec}. * @param taskQueue a {@link BinaryMessenger.TaskQueue} that specifies what thread will execute - * the handler. Specifying null mean execute on the platform thread. + * the handler. Specifying null means execute on the platform thread. */ public MethodChannel( BinaryMessenger messenger, From b9a87c6e883da0362bdadefc82ce89bf8742f33a Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Wed, 20 Oct 2021 09:44:29 -0700 Subject: [PATCH 17/21] jason's feedback --- .../io/flutter/embedding/engine/FlutterJNI.java | 2 +- .../embedding/engine/dart/DartMessenger.java | 14 ++++---------- 2 files changed, 5 insertions(+), 11 deletions(-) diff --git a/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java b/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java index 61dec933a8a3a..982b818a1df81 100644 --- a/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java +++ b/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java @@ -889,7 +889,7 @@ public void cleanupMessageData(long messageData) { nativeCleanupMessageData(messageData); } - // Called by native. + // Called by native on the ui thread. // TODO(mattcarroll): determine if message is nonull or nullable @SuppressWarnings("unused") @VisibleForTesting 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 afa9bdfc22eea..2e2bb70590d91 100644 --- a/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java +++ b/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java @@ -139,11 +139,8 @@ public void handleMessageFromDart( long messageData) { // Called from the ui thread. Log.v(TAG, "Received message from Dart over channel '" + channel + "'"); - @Nullable HandlerInfo handlerInfo = messageHandlers.get(channel); - @Nullable TaskQueue taskQueue = null; - if (handlerInfo != null) { - taskQueue = handlerInfo.taskQueue; - } + @Nullable final HandlerInfo handlerInfo = messageHandlers.get(channel); + @Nullable final TaskQueue taskQueue = (handlerInfo != null) ? handlerInfo.taskQueue : null; Runnable myRunnable = () -> { try { @@ -158,11 +155,8 @@ public void handleMessageFromDart( flutterJNI.cleanupMessageData(messageData); } }; - if (taskQueue == null) { - platformTaskQueue.dispatch(myRunnable); - } else { - taskQueue.dispatch(myRunnable); - } + @NonNull final TaskQueue nonnullTaskQueue = taskQueue == null ? platformTaskQueue : taskQueue; + nonnullTaskQueue.dispatch(myRunnable); } @Override From 76d6c8f9e4fff6d874c4e5917094959664885972 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Wed, 20 Oct 2021 10:07:30 -0700 Subject: [PATCH 18/21] started naming generated threads --- .../io/flutter/embedding/engine/dart/DartMessenger.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) 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 2e2bb70590d91..0643c56f7acba 100644 --- a/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java +++ b/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java @@ -16,6 +16,7 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; +import java.util.concurrent.ThreadFactory; import java.util.concurrent.atomic.AtomicBoolean; /** @@ -60,7 +61,10 @@ private static class DefaultTaskQueue implements TaskQueue { DefaultTaskQueue() { // TODO(gaaclarke): Use a shared thread pool with serial queues instead of // making a thread for each TaskQueue. - this.executor = Executors.newSingleThreadExecutor(); + ThreadFactory threadFactory = (Runnable runnable) -> { + return new Thread(runnable, "DartMessenger.DefaultTaskQueue"); + }; + this.executor = Executors.newSingleThreadExecutor(threadFactory); } @Override From 56b20239a25421567c262f6cb2aaa15846555e5b Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Wed, 20 Oct 2021 13:44:12 -0700 Subject: [PATCH 19/21] submit -> execute --- .../android/io/flutter/embedding/engine/dart/DartMessenger.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 0643c56f7acba..164a3bf29e7a3 100644 --- a/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java +++ b/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java @@ -69,7 +69,7 @@ private static class DefaultTaskQueue implements TaskQueue { @Override public void dispatch(@NonNull Runnable runnable) { - executor.submit(runnable); + executor.execute(runnable); } } From 69c47f6b8ad6a021f86465563cdd3c359eca639e Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Wed, 20 Oct 2021 15:00:31 -0700 Subject: [PATCH 20/21] blocked users from using their own taskqueues --- .../embedding/engine/dart/DartMessenger.java | 67 +++++++++++++++---- .../engine/dart/PlatformTaskQueue.java | 3 +- .../plugin/common/BasicMessageChannel.java | 3 +- .../plugin/common/BinaryMessenger.java | 4 +- .../flutter/plugin/common/EventChannel.java | 3 +- .../flutter/plugin/common/MethodChannel.java | 3 +- .../engine/dart/DartMessengerTest.java | 36 ++++------ 7 files changed, 76 insertions(+), 43 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 164a3bf29e7a3..7e6b9706f22ef 100644 --- a/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java +++ b/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java @@ -13,6 +13,7 @@ import java.nio.ByteBuffer; import java.util.HashMap; import java.util.Map; +import java.util.WeakHashMap; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; @@ -36,34 +37,62 @@ class DartMessenger implements BinaryMessenger, PlatformMessageHandler { @NonNull private final Map pendingReplies; private int nextReplyId = 1; - @NonNull private final TaskQueue platformTaskQueue = new PlatformTaskQueue(); + @NonNull private final DartMessengerTaskQueue platformTaskQueue = new PlatformTaskQueue(); - DartMessenger(@NonNull FlutterJNI flutterJNI) { + @NonNull private WeakHashMap createdTaskQueues; + + @NonNull private TaskQueueFactory taskQueueFactory; + + DartMessenger(@NonNull FlutterJNI flutterJNI, @NonNull TaskQueueFactory taskQueueFactory) { this.flutterJNI = flutterJNI; this.messageHandlers = new ConcurrentHashMap<>(); this.pendingReplies = new HashMap<>(); + this.createdTaskQueues = new WeakHashMap(); + this.taskQueueFactory = taskQueueFactory; + } + + DartMessenger(@NonNull FlutterJNI flutterJNI) { + this(flutterJNI, new DefaultTaskQueueFactory()); + } + + private static class TaskQueueToken implements TaskQueue {} + + interface DartMessengerTaskQueue { + void dispatch(@NonNull Runnable runnable); + } + + interface TaskQueueFactory { + DartMessengerTaskQueue makeBackgroundTaskQueue(); + } + + private static class DefaultTaskQueueFactory implements TaskQueueFactory { + public DartMessengerTaskQueue makeBackgroundTaskQueue() { + return new DefaultTaskQueue(); + } } private static class HandlerInfo { @NonNull public final BinaryMessenger.BinaryMessageHandler handler; - @Nullable public final TaskQueue taskQueue; + @Nullable public final DartMessengerTaskQueue taskQueue; HandlerInfo( - @NonNull BinaryMessenger.BinaryMessageHandler handler, @Nullable TaskQueue taskQueue) { + @NonNull BinaryMessenger.BinaryMessageHandler handler, + @Nullable DartMessengerTaskQueue taskQueue) { this.handler = handler; this.taskQueue = taskQueue; } } - private static class DefaultTaskQueue implements TaskQueue { + private static class DefaultTaskQueue implements DartMessengerTaskQueue { @NonNull private final ExecutorService executor; DefaultTaskQueue() { // TODO(gaaclarke): Use a shared thread pool with serial queues instead of // making a thread for each TaskQueue. - ThreadFactory threadFactory = (Runnable runnable) -> { - return new Thread(runnable, "DartMessenger.DefaultTaskQueue"); - }; + ThreadFactory threadFactory = + (Runnable runnable) -> { + return new Thread(runnable, "DartMessenger.DefaultTaskQueue"); + }; this.executor = Executors.newSingleThreadExecutor(threadFactory); } @@ -75,7 +104,10 @@ public void dispatch(@NonNull Runnable runnable) { @Override public TaskQueue makeBackgroundTaskQueue() { - return new DefaultTaskQueue(); + DartMessengerTaskQueue taskQueue = taskQueueFactory.makeBackgroundTaskQueue(); + TaskQueueToken token = new TaskQueueToken(); + createdTaskQueues.put(token, taskQueue); + return token; } @Override @@ -87,8 +119,16 @@ public void setMessageHandler( Log.v(TAG, "Removing handler for channel '" + channel + "'"); messageHandlers.remove(channel); } else { + DartMessengerTaskQueue dartMessengerTaskQueue = null; + if (taskQueue != null) { + dartMessengerTaskQueue = createdTaskQueues.get(taskQueue); + if (dartMessengerTaskQueue == null) { + throw new IllegalArgumentException( + "Unrecognized TaskQueue, use BinaryMessenger to create your TaskQueue (ex makeBackgroundTaskQueue)."); + } + } Log.v(TAG, "Setting handler for channel '" + channel + "'"); - messageHandlers.put(channel, new HandlerInfo(handler, taskQueue)); + messageHandlers.put(channel, new HandlerInfo(handler, dartMessengerTaskQueue)); } } @@ -144,7 +184,8 @@ public void handleMessageFromDart( // Called from the ui thread. Log.v(TAG, "Received message from Dart over channel '" + channel + "'"); @Nullable final HandlerInfo handlerInfo = messageHandlers.get(channel); - @Nullable final TaskQueue taskQueue = (handlerInfo != null) ? handlerInfo.taskQueue : null; + @Nullable + final DartMessengerTaskQueue taskQueue = (handlerInfo != null) ? handlerInfo.taskQueue : null; Runnable myRunnable = () -> { try { @@ -159,7 +200,9 @@ public void handleMessageFromDart( flutterJNI.cleanupMessageData(messageData); } }; - @NonNull final TaskQueue nonnullTaskQueue = taskQueue == null ? platformTaskQueue : taskQueue; + @NonNull + final DartMessengerTaskQueue nonnullTaskQueue = + taskQueue == null ? platformTaskQueue : taskQueue; nonnullTaskQueue.dispatch(myRunnable); } diff --git a/shell/platform/android/io/flutter/embedding/engine/dart/PlatformTaskQueue.java b/shell/platform/android/io/flutter/embedding/engine/dart/PlatformTaskQueue.java index 6d75836bed752..e90ab89e62223 100644 --- a/shell/platform/android/io/flutter/embedding/engine/dart/PlatformTaskQueue.java +++ b/shell/platform/android/io/flutter/embedding/engine/dart/PlatformTaskQueue.java @@ -7,10 +7,9 @@ import android.os.Handler; import android.os.Looper; import androidx.annotation.NonNull; -import io.flutter.plugin.common.BinaryMessenger; /** A BinaryMessenger.TaskQueue that posts to the platform thread (aka main thread). */ -public class PlatformTaskQueue implements BinaryMessenger.TaskQueue { +public class PlatformTaskQueue implements DartMessenger.DartMessengerTaskQueue { @NonNull private final Handler handler = new Handler(Looper.getMainLooper()); @Override diff --git a/shell/platform/android/io/flutter/plugin/common/BasicMessageChannel.java b/shell/platform/android/io/flutter/plugin/common/BasicMessageChannel.java index 6f67a5407431e..fa9ffe2a90855 100644 --- a/shell/platform/android/io/flutter/plugin/common/BasicMessageChannel.java +++ b/shell/platform/android/io/flutter/plugin/common/BasicMessageChannel.java @@ -59,7 +59,8 @@ public BasicMessageChannel( * @param name a channel name String. * @param codec a {@link MessageCodec}. * @param taskQueue a {@link BinaryMessenger.TaskQueue} that specifies what thread will execute - * the handler. Specifying null means execute on the platform thread. + * the handler. Specifying null means execute on the platform thread. See also {@link + * BinaryMessenger#makeBackgroundTaskQueue()}. */ public BasicMessageChannel( @NonNull BinaryMessenger messenger, diff --git a/shell/platform/android/io/flutter/plugin/common/BinaryMessenger.java b/shell/platform/android/io/flutter/plugin/common/BinaryMessenger.java index de454ab8d80b9..32f1bfda47bb2 100644 --- a/shell/platform/android/io/flutter/plugin/common/BinaryMessenger.java +++ b/shell/platform/android/io/flutter/plugin/common/BinaryMessenger.java @@ -27,9 +27,7 @@ */ public interface BinaryMessenger { /** An abstraction over dispatching a Runnable to be executed on some thread. */ - public interface TaskQueue { - void dispatch(@NonNull Runnable runnable); - } + public interface TaskQueue {} /** * Creates a TaskQueue that executes the tasks serially on a background thread. diff --git a/shell/platform/android/io/flutter/plugin/common/EventChannel.java b/shell/platform/android/io/flutter/plugin/common/EventChannel.java index e0cb4758b5355..70bd7d56951c7 100644 --- a/shell/platform/android/io/flutter/plugin/common/EventChannel.java +++ b/shell/platform/android/io/flutter/plugin/common/EventChannel.java @@ -68,7 +68,8 @@ public EventChannel(BinaryMessenger messenger, String name, MethodCodec codec) { * @param name a channel name String. * @param codec a {@link MessageCodec}. * @param taskQueue a {@link BinaryMessenger.TaskQueue} that specifies what thread will execute - * the handler. Specifying null means execute on the platform thread. + * the handler. Specifying null means execute on the platform thread. See also {@link + * BinaryMessenger#makeBackgroundTaskQueue()}. */ public EventChannel( BinaryMessenger messenger, diff --git a/shell/platform/android/io/flutter/plugin/common/MethodChannel.java b/shell/platform/android/io/flutter/plugin/common/MethodChannel.java index be59ad8c929cb..66c92cf6a2497 100644 --- a/shell/platform/android/io/flutter/plugin/common/MethodChannel.java +++ b/shell/platform/android/io/flutter/plugin/common/MethodChannel.java @@ -68,7 +68,8 @@ public MethodChannel(BinaryMessenger messenger, String name, MethodCodec codec) * @param name a channel name String. * @param codec a {@link MessageCodec}. * @param taskQueue a {@link BinaryMessenger.TaskQueue} that specifies what thread will execute - * the handler. Specifying null means execute on the platform thread. + * the handler. Specifying null means execute on the platform thread. See also {@link + * BinaryMessenger#makeBackgroundTaskQueue()}. */ public MethodChannel( BinaryMessenger messenger, 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 952b66e890b08..4e7a6fdb7c2e1 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 @@ -10,10 +10,10 @@ import static org.mockito.Mockito.verify; import io.flutter.embedding.engine.FlutterJNI; +import io.flutter.embedding.engine.dart.DartMessenger.DartMessengerTaskQueue; import io.flutter.plugin.common.BinaryMessenger; import io.flutter.plugin.common.BinaryMessenger.BinaryMessageHandler; import java.nio.ByteBuffer; -import java.util.concurrent.CountDownLatch; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mockito; @@ -35,7 +35,7 @@ public void uncaughtException(Thread t, Throwable e) { } } - private static class SynchronousTaskQueue implements BinaryMessenger.TaskQueue { + private static class SynchronousTaskQueue implements DartMessengerTaskQueue { public void dispatch(Runnable runnable) { runnable.run(); } @@ -53,13 +53,13 @@ public void itHandlesErrors() { currentThread.setUncaughtExceptionHandler(reportingHandler); // Create object under test. - final DartMessenger messenger = new DartMessenger(fakeFlutterJni); + final DartMessenger messenger = new DartMessenger(fakeFlutterJni, () -> synchronousTaskQueue); final BinaryMessageHandler throwingHandler = mock(BinaryMessageHandler.class); Mockito.doThrow(AssertionError.class) .when(throwingHandler) .onMessage(any(ByteBuffer.class), any(DartMessenger.Reply.class)); - - messenger.setMessageHandler("test", throwingHandler, synchronousTaskQueue); + BinaryMessenger.TaskQueue taskQueue = messenger.makeBackgroundTaskQueue(); + messenger.setMessageHandler("test", throwingHandler, taskQueue); messenger.handleMessageFromDart("test", ByteBuffer.allocate(0), 0, 0); assertNotNull(reportingHandler.latestException); assertTrue(reportingHandler.latestException instanceof AssertionError); @@ -70,14 +70,15 @@ public void itHandlesErrors() { public void givesDirectByteBuffer() { // Setup test. final FlutterJNI fakeFlutterJni = mock(FlutterJNI.class); - final DartMessenger messenger = new DartMessenger(fakeFlutterJni); + final DartMessenger messenger = new DartMessenger(fakeFlutterJni, () -> synchronousTaskQueue); final String channel = "foobar"; final boolean[] wasDirect = {false}; final BinaryMessenger.BinaryMessageHandler handler = (message, reply) -> { wasDirect[0] = message.isDirect(); }; - messenger.setMessageHandler(channel, handler, synchronousTaskQueue); + BinaryMessenger.TaskQueue taskQueue = messenger.makeBackgroundTaskQueue(); + messenger.setMessageHandler(channel, handler, taskQueue); final ByteBuffer message = ByteBuffer.allocateDirect(4 * 2); message.rewind(); message.putChar('a'); @@ -92,7 +93,7 @@ public void givesDirectByteBuffer() { public void directByteBufferLimitZeroAfterUsage() { // Setup test. final FlutterJNI fakeFlutterJni = mock(FlutterJNI.class); - final DartMessenger messenger = new DartMessenger(fakeFlutterJni); + final DartMessenger messenger = new DartMessenger(fakeFlutterJni, () -> synchronousTaskQueue); final String channel = "foobar"; final ByteBuffer[] byteBuffers = {null}; final int bufferSize = 4 * 2; @@ -101,7 +102,8 @@ public void directByteBufferLimitZeroAfterUsage() { byteBuffers[0] = message; assertEquals(bufferSize, byteBuffers[0].limit()); }; - messenger.setMessageHandler(channel, handler, synchronousTaskQueue); + BinaryMessenger.TaskQueue taskQueue = messenger.makeBackgroundTaskQueue(); + messenger.setMessageHandler(channel, handler, taskQueue); final ByteBuffer message = ByteBuffer.allocateDirect(bufferSize); message.rewind(); message.putChar('a'); @@ -152,7 +154,7 @@ public void replyIdIncrementsOnNullReply() { @Test public void cleansUpMessageData() throws InterruptedException { final FlutterJNI fakeFlutterJni = mock(FlutterJNI.class); - final DartMessenger messenger = new DartMessenger(fakeFlutterJni); + final DartMessenger messenger = new DartMessenger(fakeFlutterJni, () -> synchronousTaskQueue); BinaryMessenger.TaskQueue taskQueue = messenger.makeBackgroundTaskQueue(); String channel = "foobar"; BinaryMessenger.BinaryMessageHandler handler = @@ -164,19 +166,13 @@ public void cleansUpMessageData() throws InterruptedException { int replyId = 1; long messageData = 1234; messenger.handleMessageFromDart(channel, message, replyId, messageData); - final CountDownLatch latch = new CountDownLatch(1); - taskQueue.dispatch( - () -> { - latch.countDown(); - }); - latch.await(); verify(fakeFlutterJni).cleanupMessageData(eq(messageData)); } @Test public void cleansUpMessageDataOnError() throws InterruptedException { final FlutterJNI fakeFlutterJni = mock(FlutterJNI.class); - final DartMessenger messenger = new DartMessenger(fakeFlutterJni); + final DartMessenger messenger = new DartMessenger(fakeFlutterJni, () -> synchronousTaskQueue); BinaryMessenger.TaskQueue taskQueue = messenger.makeBackgroundTaskQueue(); String channel = "foobar"; BinaryMessenger.BinaryMessageHandler handler = @@ -188,12 +184,6 @@ public void cleansUpMessageDataOnError() throws InterruptedException { int replyId = 1; long messageData = 1234; messenger.handleMessageFromDart(channel, message, replyId, messageData); - final CountDownLatch latch = new CountDownLatch(1); - taskQueue.dispatch( - () -> { - latch.countDown(); - }); - latch.await(); verify(fakeFlutterJni).cleanupMessageData(eq(messageData)); } } From 141520f5f1a189feb1b93e59690203c9354b0727 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 21 Oct 2021 14:14:53 -0700 Subject: [PATCH 21/21] updated docstring --- .../android/io/flutter/plugin/common/BinaryMessenger.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/shell/platform/android/io/flutter/plugin/common/BinaryMessenger.java b/shell/platform/android/io/flutter/plugin/common/BinaryMessenger.java index 32f1bfda47bb2..433a6a9bbeaa9 100644 --- a/shell/platform/android/io/flutter/plugin/common/BinaryMessenger.java +++ b/shell/platform/android/io/flutter/plugin/common/BinaryMessenger.java @@ -26,7 +26,13 @@ * @see EventChannel , which supports communication using event streams. */ public interface BinaryMessenger { - /** An abstraction over dispatching a Runnable to be executed on some thread. */ + /** + * An abstraction over the threading policy used to invoke message handlers. + * + *

These are generated by calling methods like {@link + * BinaryMessenger#makeBackgroundTaskQueue()} and can be passed into platform channels' + * constructors to control the threading policy for handling platform channels' messages. + */ public interface TaskQueue {} /**