From 45cc4b5a911072285505af9d83675465b7ff0100 Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Wed, 27 Oct 2021 23:54:18 -0700 Subject: [PATCH 01/20] Fix race condition introduced by background platform channels --- .../embedding/engine/dart/DartMessenger.java | 65 ++++++++++++++++--- 1 file changed, 55 insertions(+), 10 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 c5faf03b5dd40..ecd240f1223ad 100644 --- a/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java +++ b/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java @@ -7,6 +7,7 @@ import androidx.annotation.NonNull; import androidx.annotation.Nullable; import androidx.annotation.UiThread; +import com.google.common.util.concurrent.SettableFuture; import io.flutter.Log; import io.flutter.embedding.engine.FlutterJNI; import io.flutter.plugin.common.BinaryMessenger; @@ -32,8 +33,15 @@ class DartMessenger implements BinaryMessenger, PlatformMessageHandler { @NonNull private final FlutterJNI flutterJNI; + /** + * Maps a channel name to an object that contains the task queue and the handler associated with + * the channel. + */ @NonNull private final ConcurrentHashMap messageHandlers; + /** Maps a channel name to a future that resolves when the channel is defined. */ + @NonNull private final ConcurrentHashMap> definedChannels; + @NonNull private final Map pendingReplies; private int nextReplyId = 1; @@ -46,6 +54,7 @@ class DartMessenger implements BinaryMessenger, PlatformMessageHandler { DartMessenger(@NonNull FlutterJNI flutterJNI, @NonNull TaskQueueFactory taskQueueFactory) { this.flutterJNI = flutterJNI; this.messageHandlers = new ConcurrentHashMap<>(); + this.definedChannels = new ConcurrentHashMap<>(); this.pendingReplies = new HashMap<>(); this.createdTaskQueues = new WeakHashMap(); this.taskQueueFactory = taskQueueFactory; @@ -124,18 +133,22 @@ public void setMessageHandler( if (handler == null) { 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)."); - } + return; + } + 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, dartMessengerTaskQueue)); } + Log.v(TAG, "Setting handler for channel '" + channel + "'"); + if (!definedChannels.has(channel)) { + definedChannels.put(channel, SettableFuture.create()); + } + definedChannels.get(channel).set(Void.TYPE); + messageHandlers.put(channel, new HandlerInfo(handler, dartMessengerTaskQueue)); } @Override @@ -189,6 +202,38 @@ public void handleMessageFromDart( long messageData) { // Called from the ui thread. Log.v(TAG, "Received message from Dart over channel '" + channel + "'"); + + if (!messageHandlers.has(channel)) { + // Handle race condition situation. + // The channel is not defined when the Dart VM sends a message before the channels are + // registered. + // This is possible if the Dart VM starts before channel registration, and if the thread that + // registers the channels is busy or slow at registering the channel handlers. + // In such cases, wait for a limited time, and exit with error if the time is exceeded. + // + // This is effectively acting as a lock, so the current thread (Dart UI thread) is blocked + // until the lock is released. + if (!definedChannels.has(channel)) { + definedChannels.put(channel, SettableFuture.create()); + } + final SettableFuture future = definedChannels.get(channel); + try { + future.get(10, TimeUnit.SECONDS); + } catch (ExecutionException | InterruptedException ex) { + Log.e(TAG, "Undefined channel " + channel + ".", ex); + return; + } catch (TimeoutException ex) { + Log.e( + TAG, + "Channel " + + channel + + " was not defined in time. " + + "This likely means that the handler was never defined, or that the thread " + + "that defines the handler has been busy for an extended period of time.", + ex); + return; + } + } @Nullable final HandlerInfo handlerInfo = messageHandlers.get(channel); @Nullable final DartMessengerTaskQueue taskQueue = (handlerInfo != null) ? handlerInfo.taskQueue : null; From 0cab9a15965256c8d6c0943618cd75156db9513f Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Wed, 27 Oct 2021 23:55:52 -0700 Subject: [PATCH 02/20] Format --- .../io/flutter/embedding/engine/dart/DartMessenger.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java b/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java index ecd240f1223ad..52277f1b096ec 100644 --- a/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java +++ b/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java @@ -204,13 +204,13 @@ public void handleMessageFromDart( Log.v(TAG, "Received message from Dart over channel '" + channel + "'"); if (!messageHandlers.has(channel)) { - // Handle race condition situation. // The channel is not defined when the Dart VM sends a message before the channels are // registered. + // // This is possible if the Dart VM starts before channel registration, and if the thread that // registers the channels is busy or slow at registering the channel handlers. - // In such cases, wait for a limited time, and exit with error if the time is exceeded. // + // In such cases, wait for a limited time, and exit with error if the time is exceeded. // This is effectively acting as a lock, so the current thread (Dart UI thread) is blocked // until the lock is released. if (!definedChannels.has(channel)) { From ff024cab32c2559220645c12dced0dc578b1da4c Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Thu, 28 Oct 2021 15:11:07 -0700 Subject: [PATCH 03/20] Lock --- .../embedding/engine/dart/DartMessenger.java | 76 ++++++++++--------- 1 file changed, 40 insertions(+), 36 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 52277f1b096ec..a7c2e6c53be96 100644 --- a/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java +++ b/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java @@ -7,7 +7,6 @@ import androidx.annotation.NonNull; import androidx.annotation.Nullable; import androidx.annotation.UiThread; -import com.google.common.util.concurrent.SettableFuture; import io.flutter.Log; import io.flutter.embedding.engine.FlutterJNI; import io.flutter.plugin.common.BinaryMessenger; @@ -18,6 +17,7 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; +import java.util.concurrent.FutureTask; import java.util.concurrent.ThreadFactory; import java.util.concurrent.atomic.AtomicBoolean; @@ -40,7 +40,7 @@ class DartMessenger implements BinaryMessenger, PlatformMessageHandler { @NonNull private final ConcurrentHashMap messageHandlers; /** Maps a channel name to a future that resolves when the channel is defined. */ - @NonNull private final ConcurrentHashMap> definedChannels; + @NonNull private final ConcurrentHashMap> definedChannels; @NonNull private final Map pendingReplies; private int nextReplyId = 1; @@ -144,11 +144,13 @@ public void setMessageHandler( } } Log.v(TAG, "Setting handler for channel '" + channel + "'"); - if (!definedChannels.has(channel)) { - definedChannels.put(channel, SettableFuture.create()); + synchronized (this) { + if (!definedChannels.has(channel)) { + definedChannels.put(channel, SettableFuture.create()); + } + definedChannels.get(channel).set(Void.TYPE); + messageHandlers.put(channel, new HandlerInfo(handler, dartMessengerTaskQueue)); } - definedChannels.get(channel).set(Void.TYPE); - messageHandlers.put(channel, new HandlerInfo(handler, dartMessengerTaskQueue)); } @Override @@ -202,36 +204,38 @@ public void handleMessageFromDart( long messageData) { // Called from the ui thread. Log.v(TAG, "Received message from Dart over channel '" + channel + "'"); - - if (!messageHandlers.has(channel)) { - // The channel is not defined when the Dart VM sends a message before the channels are - // registered. - // - // This is possible if the Dart VM starts before channel registration, and if the thread that - // registers the channels is busy or slow at registering the channel handlers. - // - // In such cases, wait for a limited time, and exit with error if the time is exceeded. - // This is effectively acting as a lock, so the current thread (Dart UI thread) is blocked - // until the lock is released. - if (!definedChannels.has(channel)) { - definedChannels.put(channel, SettableFuture.create()); - } - final SettableFuture future = definedChannels.get(channel); - try { - future.get(10, TimeUnit.SECONDS); - } catch (ExecutionException | InterruptedException ex) { - Log.e(TAG, "Undefined channel " + channel + ".", ex); - return; - } catch (TimeoutException ex) { - Log.e( - TAG, - "Channel " - + channel - + " was not defined in time. " - + "This likely means that the handler was never defined, or that the thread " - + "that defines the handler has been busy for an extended period of time.", - ex); - return; + synchronized (this) { + if (!messageHandlers.has(channel)) { + // The channel is not defined when the Dart VM sends a message before the channels are + // registered. + // + // This is possible if the Dart VM starts before channel registration, and if the thread + // that + // registers the channels is busy or slow at registering the channel handlers. + // + // In such cases, wait for a limited time, and exit with error if the time is exceeded. + // This is effectively acting as a lock, so the current thread (Dart UI thread) is blocked + // until the lock is released. + if (!definedChannels.has(channel)) { + definedChannels.put(channel, SettableFuture.create()); + } + final SettableFuture future = definedChannels.get(channel); + try { + future.get(10, TimeUnit.SECONDS); + } catch (ExecutionException | InterruptedException ex) { + Log.e(TAG, "Undefined channel " + channel + ".", ex); + return; + } catch (TimeoutException ex) { + Log.e( + TAG, + "Channel " + + channel + + " was not defined in time. " + + "This likely means that the handler was never defined, or that the thread " + + "that defines the handler has been busy for an extended period of time.", + ex); + return; + } } } @Nullable final HandlerInfo handlerInfo = messageHandlers.get(channel); From 6b02fb06024d10ecc9fde735964737bd1d86ce04 Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Thu, 28 Oct 2021 23:24:57 -0700 Subject: [PATCH 04/20] WIP --- .../embedding/engine/dart/DartMessenger.java | 68 ++++++++----------- 1 file changed, 28 insertions(+), 40 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 a7c2e6c53be96..8b510579215c9 100644 --- a/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java +++ b/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java @@ -204,8 +204,32 @@ public void handleMessageFromDart( long messageData) { // Called from the ui thread. Log.v(TAG, "Received message from Dart over channel '" + channel + "'"); + final Runnable dispatch = () -> { + final HandlerInfo handlerInfo = messageHandlers.get(channel); + final DartMessengerTaskQueue taskQueue = (handlerInfo != null) ? handlerInfo.taskQueue : null; + Runnable myRunnable = + () -> { + 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); + } + }; + final DartMessengerTaskQueue nonnullTaskQueue = + taskQueue == null ? platformTaskQueue : taskQueue; + nonnullTaskQueue.dispatch(myRunnable); + }; + synchronized (this) { - if (!messageHandlers.has(channel)) { + if (messageHandlers.has(channel)) { + dispatch.run(); + } else { // The channel is not defined when the Dart VM sends a message before the channels are // registered. // @@ -217,48 +241,12 @@ public void handleMessageFromDart( // This is effectively acting as a lock, so the current thread (Dart UI thread) is blocked // until the lock is released. if (!definedChannels.has(channel)) { - definedChannels.put(channel, SettableFuture.create()); - } - final SettableFuture future = definedChannels.get(channel); - try { - future.get(10, TimeUnit.SECONDS); - } catch (ExecutionException | InterruptedException ex) { - Log.e(TAG, "Undefined channel " + channel + ".", ex); - return; - } catch (TimeoutException ex) { - Log.e( - TAG, - "Channel " - + channel - + " was not defined in time. " - + "This likely means that the handler was never defined, or that the thread " - + "that defines the handler has been busy for an extended period of time.", - ex); - return; + definedChannels.put(channel, new FutureTask(() -> { + + })); } } } - @Nullable final HandlerInfo handlerInfo = messageHandlers.get(channel); - @Nullable - final DartMessengerTaskQueue taskQueue = (handlerInfo != null) ? handlerInfo.taskQueue : null; - Runnable myRunnable = - () -> { - 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); - } - }; - @NonNull - final DartMessengerTaskQueue nonnullTaskQueue = - taskQueue == null ? platformTaskQueue : taskQueue; - nonnullTaskQueue.dispatch(myRunnable); } @Override From a48099704611ef6f85e0bfa5c680ac4e2c998b87 Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Fri, 29 Oct 2021 13:06:39 -0700 Subject: [PATCH 05/20] Rework queue --- .../embedding/engine/FlutterEngine.java | 1 + .../embedding/engine/dart/DartExecutor.java | 10 ++ .../embedding/engine/dart/DartMessenger.java | 130 ++++++++++++------ 3 files changed, 100 insertions(+), 41 deletions(-) diff --git a/shell/platform/android/io/flutter/embedding/engine/FlutterEngine.java b/shell/platform/android/io/flutter/embedding/engine/FlutterEngine.java index 828bb94586832..fce472eecb34a 100644 --- a/shell/platform/android/io/flutter/embedding/engine/FlutterEngine.java +++ b/shell/platform/android/io/flutter/embedding/engine/FlutterEngine.java @@ -352,6 +352,7 @@ public FlutterEngine( // loaded AndroidManifest config turn this feature on. if (automaticallyRegisterPlugins && flutterLoader.automaticallyRegisterPlugins()) { GeneratedPluginRegister.registerGeneratedPlugins(this); + dartExecutor.notifyPluginRegistration(); } } 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 5580d7a5cd258..b40b6442ab8ab 100644 --- a/shell/platform/android/io/flutter/embedding/engine/dart/DartExecutor.java +++ b/shell/platform/android/io/flutter/embedding/engine/dart/DartExecutor.java @@ -273,6 +273,16 @@ public void notifyLowMemoryWarning() { } } + /** + * Notifies that the plugins have been registered. + * + *

This informs the Dart messenger that all expected channel handlers have been registered. + */ + public void notifyPluginRegistration() { + dartMessenger.runDelayedTasks(); + dartMessenger.stopDelayedTaskQueue(); + } + /** * Configuration options that specify which Dart entrypoint function is executed and where to find * that entrypoint and other assets required for Dart execution. 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 8b510579215c9..ac1500a911728 100644 --- a/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java +++ b/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java @@ -12,12 +12,13 @@ import io.flutter.plugin.common.BinaryMessenger; import java.nio.ByteBuffer; import java.util.HashMap; +import java.util.LinkedList; +import java.util.List; import java.util.Map; import java.util.WeakHashMap; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; -import java.util.concurrent.FutureTask; import java.util.concurrent.ThreadFactory; import java.util.concurrent.atomic.AtomicBoolean; @@ -39,8 +40,13 @@ class DartMessenger implements BinaryMessenger, PlatformMessageHandler { */ @NonNull private final ConcurrentHashMap messageHandlers; - /** Maps a channel name to a future that resolves when the channel is defined. */ - @NonNull private final ConcurrentHashMap> definedChannels; + /** + * Maps a channel name to queue of task dispatchers. This queue is processed when the channel + * handler is registered. + */ + @NonNull private final ConcurrentHashMap> delayedTaskDispatcher; + + private boolean canDelayTasks = true; @NonNull private final Map pendingReplies; private int nextReplyId = 1; @@ -54,7 +60,7 @@ class DartMessenger implements BinaryMessenger, PlatformMessageHandler { DartMessenger(@NonNull FlutterJNI flutterJNI, @NonNull TaskQueueFactory taskQueueFactory) { this.flutterJNI = flutterJNI; this.messageHandlers = new ConcurrentHashMap<>(); - this.definedChannels = new ConcurrentHashMap<>(); + this.delayedTaskDispatcher = new ConcurrentHashMap<>(); this.pendingReplies = new HashMap<>(); this.createdTaskQueues = new WeakHashMap(); this.taskQueueFactory = taskQueueFactory; @@ -144,15 +150,56 @@ public void setMessageHandler( } } Log.v(TAG, "Setting handler for channel '" + channel + "'"); - synchronized (this) { - if (!definedChannels.has(channel)) { - definedChannels.put(channel, SettableFuture.create()); + messageHandlers.put(channel, new HandlerInfo(handler, dartMessengerTaskQueue)); + runDelayedTasksForChannel(channel); + } + + /** + * Runs the tasks that handle messages received from Dart for the provided channel name. + * + *

The channel may not have associated tasks if it was registered prior to reciving the first + * message from Dart. + * + * @param channel The channel name. + */ + public synchronized void runDelayedTasksForChannel(@NonNull String channel) { + if (!delayedTaskDispatcher.has(channel)) { + return; + } + final LinkedList list = delayedTaskDispatcher.get(channel); + delayedTaskDispatcher.remove(channel); + + while (!list.isEmpty()) { + final Runnable task = list.poll(); + try { + task.run(); + } catch (Exception ex) { + Log.e(TAG, "Exception thrown while running delayed task for channel '" + channel + "'."); } - definedChannels.get(channel).set(Void.TYPE); - messageHandlers.put(channel, new HandlerInfo(handler, dartMessengerTaskQueue)); } } + /** Runs all the tasks that handle messages received from Dart for the provided channel name. */ + public synchronized void runDelayedTasks() { + for (String channel : delayedTaskDispatcher.keySet()) { + runDelayedTasksForChannel(channel); + } + } + + /** + * Stops the ability to queue tasks when messages are received from Dart. + * + *

This should be called if there's no pending channel handler registration. For example, + * channels are typically registered during plugin registration, so it makes sense to stop the + * delayed task queue right at that point. + * + *

Once this is called, any future message from Dart that doesn't have an associated handler + * results in an error. + */ + public synchronized void stopDelayedTaskQueue() { + canDelayTasks = false; + } + @Override @UiThread public void send(@NonNull String channel, @NonNull ByteBuffer message) { @@ -204,47 +251,48 @@ public void handleMessageFromDart( long messageData) { // Called from the ui thread. Log.v(TAG, "Received message from Dart over channel '" + channel + "'"); - final Runnable dispatch = () -> { - final HandlerInfo handlerInfo = messageHandlers.get(channel); - final DartMessengerTaskQueue taskQueue = (handlerInfo != null) ? handlerInfo.taskQueue : null; - Runnable myRunnable = - () -> { - 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); - } - }; - final DartMessengerTaskQueue nonnullTaskQueue = - taskQueue == null ? platformTaskQueue : taskQueue; - nonnullTaskQueue.dispatch(myRunnable); - }; + final Runnable taskDispatcher = + () -> { + final HandlerInfo handlerInfo = messageHandlers.get(channel); + final DartMessengerTaskQueue taskQueue = + (handlerInfo != null) ? handlerInfo.taskQueue : null; + Runnable myRunnable = + () -> { + 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); + } + }; + final DartMessengerTaskQueue nonnullTaskQueue = + taskQueue == null ? platformTaskQueue : taskQueue; + nonnullTaskQueue.dispatch(myRunnable); + }; synchronized (this) { - if (messageHandlers.has(channel)) { - dispatch.run(); + if (!canDelayTasks || messageHandlers.has(channel)) { + taskDispatcher.run(); } else { // The channel is not defined when the Dart VM sends a message before the channels are // registered. // // This is possible if the Dart VM starts before channel registration, and if the thread - // that - // registers the channels is busy or slow at registering the channel handlers. + // that registers the channels is busy or slow at registering the channel handlers. // - // In such cases, wait for a limited time, and exit with error if the time is exceeded. - // This is effectively acting as a lock, so the current thread (Dart UI thread) is blocked - // until the lock is released. - if (!definedChannels.has(channel)) { - definedChannels.put(channel, new FutureTask(() -> { - - })); + // In such cases, the task dispatchers are queued, and processed when the channel is + // defined. + if (!delayedTaskDispatcher.has(channel)) { + delayedTaskDispatcher.put(channel, new LinkedList<>()); } + List delayedTaskQueue = delayedTaskDispatcher.get(channel); + delayedTaskQueue.add(taskDispatcher); } } } From 4bae8acc22056ee4105707e58a251b6c36d4536d Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Fri, 29 Oct 2021 13:14:16 -0700 Subject: [PATCH 06/20] Log error --- .../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 f1a52dd9254f8..633ad5a28b853 100644 --- a/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java +++ b/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java @@ -175,7 +175,7 @@ public synchronized void runDelayedTasksForChannel(@NonNull String channel) { try { task.run(); } catch (Exception ex) { - Log.e(TAG, "Exception thrown while running delayed task for channel '" + channel + "'."); + Log.e(TAG, "Exception thrown while running delayed task for channel '" + channel + "'", ex); } } } From 5f0cbf82ff0a71aa3b9dde5f001e7dca46f24b3d Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Fri, 29 Oct 2021 14:42:18 -0700 Subject: [PATCH 07/20] Flag --- .../embedding/engine/FlutterEngine.java | 1 - .../embedding/engine/dart/DartExecutor.java | 10 --- .../embedding/engine/dart/DartMessenger.java | 86 ++++++++++--------- 3 files changed, 47 insertions(+), 50 deletions(-) diff --git a/shell/platform/android/io/flutter/embedding/engine/FlutterEngine.java b/shell/platform/android/io/flutter/embedding/engine/FlutterEngine.java index fce472eecb34a..828bb94586832 100644 --- a/shell/platform/android/io/flutter/embedding/engine/FlutterEngine.java +++ b/shell/platform/android/io/flutter/embedding/engine/FlutterEngine.java @@ -352,7 +352,6 @@ public FlutterEngine( // loaded AndroidManifest config turn this feature on. if (automaticallyRegisterPlugins && flutterLoader.automaticallyRegisterPlugins()) { GeneratedPluginRegister.registerGeneratedPlugins(this); - dartExecutor.notifyPluginRegistration(); } } 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 bb1b8dbd35f2b..ecf410d0fc3b0 100644 --- a/shell/platform/android/io/flutter/embedding/engine/dart/DartExecutor.java +++ b/shell/platform/android/io/flutter/embedding/engine/dart/DartExecutor.java @@ -286,16 +286,6 @@ public void notifyLowMemoryWarning() { } } - /** - * Notifies that the plugins have been registered. - * - *

This informs the Dart messenger that all expected channel handlers have been registered. - */ - public void notifyPluginRegistration() { - dartMessenger.runDelayedTasks(); - dartMessenger.stopDelayedTaskQueue(); - } - /** * Configuration options that specify which Dart entrypoint function is executed and where to find * that entrypoint and other assets required for Dart execution. 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 633ad5a28b853..8533d3c5035e0 100644 --- a/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java +++ b/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java @@ -38,32 +38,38 @@ class DartMessenger implements BinaryMessenger, PlatformMessageHandler { /** * Maps a channel name to an object that contains the task queue and the handler associated with * the channel. + * + *

Reads and writes to this map must lock {@code handlersLock}. */ - @NonNull private final ConcurrentHashMap messageHandlers; + @NonNull + private final ConcurrentHashMap messageHandlers = new ConcurrentHashMap<>(); /** * Maps a channel name to queue of task dispatchers. This queue is processed when the channel * handler is registered. + * + *

Reads and writes to this map must lock {@code handlersLock}. */ - @NonNull private final ConcurrentHashMap> delayedTaskDispatcher; + @NonNull + private final ConcurrentHashMap> delayedTaskDispatcher = + new ConcurrentHashMap<>(); - private boolean canDelayTasks = true; + @NonNull private final Object handlersLock = new Object(); + private boolean canDelayTasks = false; - @NonNull private final Map pendingReplies; + @NonNull private final Map pendingReplies = new HashMap<>(); private int nextReplyId = 1; @NonNull private final DartMessengerTaskQueue platformTaskQueue = new PlatformTaskQueue(); - @NonNull private WeakHashMap createdTaskQueues; + @NonNull + private WeakHashMap createdTaskQueues = + new WeakHashMap(); @NonNull private TaskQueueFactory taskQueueFactory; DartMessenger(@NonNull FlutterJNI flutterJNI, @NonNull TaskQueueFactory taskQueueFactory) { this.flutterJNI = flutterJNI; - this.messageHandlers = new ConcurrentHashMap<>(); - this.delayedTaskDispatcher = new ConcurrentHashMap<>(); - this.pendingReplies = new HashMap<>(); - this.createdTaskQueues = new WeakHashMap(); this.taskQueueFactory = taskQueueFactory; } @@ -139,7 +145,9 @@ public void setMessageHandler( @Nullable TaskQueue taskQueue) { if (handler == null) { Log.v(TAG, "Removing handler for channel '" + channel + "'"); - messageHandlers.remove(channel); + synchronized (handlersLock) { + messageHandlers.remove(channel); + } return; } DartMessengerTaskQueue dartMessengerTaskQueue = null; @@ -151,7 +159,9 @@ public void setMessageHandler( } } Log.v(TAG, "Setting handler for channel '" + channel + "'"); - messageHandlers.put(channel, new HandlerInfo(handler, dartMessengerTaskQueue)); + synchronized (handlersLock) { + messageHandlers.put(channel, new HandlerInfo(handler, dartMessengerTaskQueue)); + } runDelayedTasksForChannel(channel); } @@ -163,13 +173,15 @@ public void setMessageHandler( * * @param channel The channel name. */ - public synchronized void runDelayedTasksForChannel(@NonNull String channel) { - if (!delayedTaskDispatcher.has(channel)) { - return; + public void runDelayedTasksForChannel(@NonNull String channel) { + List list; + synchronized (handlersLock) { + if (!delayedTaskDispatcher.contains(channel)) { + return; + } + list = delayedTaskDispatcher.get(channel); + delayedTaskDispatcher.remove(channel); } - final LinkedList list = delayedTaskDispatcher.get(channel); - delayedTaskDispatcher.remove(channel); - while (!list.isEmpty()) { final Runnable task = list.poll(); try { @@ -180,25 +192,15 @@ public synchronized void runDelayedTasksForChannel(@NonNull String channel) { } } - /** Runs all the tasks that handle messages received from Dart for the provided channel name. */ - public synchronized void runDelayedTasks() { - for (String channel : delayedTaskDispatcher.keySet()) { - runDelayedTasksForChannel(channel); - } - } - /** - * Stops the ability to queue tasks when messages are received from Dart. + * Enables the ability to queue tasks when messages are received from Dart. * - *

This should be called if there's no pending channel handler registration. For example, - * channels are typically registered during plugin registration, so it makes sense to stop the - * delayed task queue right at that point. - * - *

Once this is called, any future message from Dart that doesn't have an associated handler - * results in an error. + *

This is useful when there are pending channel handler registrations. For example, Dart may + * be initialized concurrently, and prior to the registration of the channel handlers. This + * implies that Dart may start sending messages while plugins are being registered. */ - public synchronized void stopDelayedTaskQueue() { - canDelayTasks = false; + public void enableDelayedTaskQueue() { + canDelayTasks = true; } @Override @@ -260,7 +262,10 @@ public void handleMessageFromDart( Log.v(TAG, "Received message from Dart over channel '" + channel + "'"); final Runnable taskDispatcher = () -> { - final HandlerInfo handlerInfo = messageHandlers.get(channel); + HandlerInfo handlerInfo; + synchronized (handlersLock) { + handlerInfo = messageHandlers.get(channel); + } final DartMessengerTaskQueue taskQueue = (handlerInfo != null) ? handlerInfo.taskQueue : null; Runnable myRunnable = @@ -285,10 +290,10 @@ public void handleMessageFromDart( nonnullTaskQueue.dispatch(myRunnable); }; - synchronized (this) { - if (!canDelayTasks || messageHandlers.has(channel)) { - taskDispatcher.run(); - } else { + boolean runNow = true; + synchronized (handlersLock) { + if (canDelayTasks && !messageHandlers.contains(channel)) { + runNow = false; // The channel is not defined when the Dart VM sends a message before the channels are // registered. // @@ -297,13 +302,16 @@ public void handleMessageFromDart( // // In such cases, the task dispatchers are queued, and processed when the channel is // defined. - if (!delayedTaskDispatcher.has(channel)) { + if (!delayedTaskDispatcher.contains(channel)) { delayedTaskDispatcher.put(channel, new LinkedList<>()); } List delayedTaskQueue = delayedTaskDispatcher.get(channel); delayedTaskQueue.add(taskDispatcher); } } + if (runNow) { + taskDispatcher.run(); + } } @Override From fcc5ae103a42de70720ed1611bfbd661867bc3ab Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Fri, 29 Oct 2021 14:48:20 -0700 Subject: [PATCH 08/20] edits --- .../io/flutter/embedding/engine/dart/DartMessenger.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java b/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java index 8533d3c5035e0..6b2b58c79481a 100644 --- a/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java +++ b/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java @@ -174,12 +174,12 @@ public void setMessageHandler( * @param channel The channel name. */ public void runDelayedTasksForChannel(@NonNull String channel) { - List list; + LinkedList list; synchronized (handlersLock) { if (!delayedTaskDispatcher.contains(channel)) { return; } - list = delayedTaskDispatcher.get(channel); + list = (LinkedList) delayedTaskDispatcher.get(channel); delayedTaskDispatcher.remove(channel); } while (!list.isEmpty()) { From c90f6ea877e66fa2bbbd319af1db65851d43d803 Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Fri, 29 Oct 2021 15:23:20 -0700 Subject: [PATCH 09/20] Simplify --- .../embedding/engine/dart/DartMessenger.java | 69 +++++++++---------- 1 file changed, 34 insertions(+), 35 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 6b2b58c79481a..31d81f4d693c5 100644 --- a/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java +++ b/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java @@ -252,6 +252,30 @@ private void invokeHandler( } } + private void dispatchMessageToQueue(HandlerInfo handlerInfo) { + final DartMessengerTaskQueue taskQueue = (handlerInfo != null) ? handlerInfo.taskQueue : null; + Runnable myRunnable = + () -> { + Trace.beginSection("DartMessenger#handleMessageFromDart on " + channel); + 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 { + Trace.endSection(); + // This is deleting the data underneath the message object. + flutterJNI.cleanupMessageData(messageData); + } + }; + final DartMessengerTaskQueue nonnullTaskQueue = + taskQueue == null ? platformTaskQueue : taskQueue; + nonnullTaskQueue.dispatch(myRunnable); + } + @Override public void handleMessageFromDart( @NonNull final String channel, @@ -260,40 +284,12 @@ public void handleMessageFromDart( long messageData) { // Called from the ui thread. Log.v(TAG, "Received message from Dart over channel '" + channel + "'"); - final Runnable taskDispatcher = - () -> { - HandlerInfo handlerInfo; - synchronized (handlersLock) { - handlerInfo = messageHandlers.get(channel); - } - final DartMessengerTaskQueue taskQueue = - (handlerInfo != null) ? handlerInfo.taskQueue : null; - Runnable myRunnable = - () -> { - Trace.beginSection("DartMessenger#handleMessageFromDart on " + channel); - 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 { - Trace.endSection(); - // This is deleting the data underneath the message object. - flutterJNI.cleanupMessageData(messageData); - } - }; - final DartMessengerTaskQueue nonnullTaskQueue = - taskQueue == null ? platformTaskQueue : taskQueue; - nonnullTaskQueue.dispatch(myRunnable); - }; - boolean runNow = true; + HandlerInfo handlerInfo; synchronized (handlersLock) { - if (canDelayTasks && !messageHandlers.contains(channel)) { - runNow = false; + if (messageHandlers.contains(channel)) { + handlerInfo = messageHandlers.get(channel); + } else if (canDelayTasks) { // The channel is not defined when the Dart VM sends a message before the channels are // registered. // @@ -306,11 +302,14 @@ public void handleMessageFromDart( delayedTaskDispatcher.put(channel, new LinkedList<>()); } List delayedTaskQueue = delayedTaskDispatcher.get(channel); - delayedTaskQueue.add(taskDispatcher); + delayedTaskQueue.add( + () -> { + dispatchMessageToQueue(messageHandlers.get(channel)); + }); } } - if (runNow) { - taskDispatcher.run(); + if (handlerInfo != null) { + dispatchMessageToQueue(handlerInfo); } } From 983943ac14c2af00cc9183c85752ae8396fb6417 Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Fri, 29 Oct 2021 15:24:31 -0700 Subject: [PATCH 10/20] Fix formatter --- .../io/flutter/embedding/engine/dart/DartMessenger.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java b/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java index 31d81f4d693c5..36379c5fde4f1 100644 --- a/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java +++ b/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java @@ -261,8 +261,7 @@ private void dispatchMessageToQueue(HandlerInfo handlerInfo) { 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. + // happens to be direct they will get a deterministic error. message.limit(0); } } finally { From db476b4c45157f3bc42f3b44d3857a62711c7f18 Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Fri, 29 Oct 2021 15:32:29 -0700 Subject: [PATCH 11/20] missing params --- .../embedding/engine/dart/DartMessenger.java | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java b/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java index 36379c5fde4f1..864cb28b940e4 100644 --- a/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java +++ b/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java @@ -252,7 +252,12 @@ private void invokeHandler( } } - private void dispatchMessageToQueue(HandlerInfo handlerInfo) { + private void dispatchMessageToQueue( + @NonNull String channel, + @Nullable HandlerInfo handlerInfo, + @Nullable ByteBuffer message, + int replyId, + long messageData) { final DartMessengerTaskQueue taskQueue = (handlerInfo != null) ? handlerInfo.taskQueue : null; Runnable myRunnable = () -> { @@ -277,10 +282,7 @@ private void dispatchMessageToQueue(HandlerInfo handlerInfo) { @Override public void handleMessageFromDart( - @NonNull final String channel, - @Nullable ByteBuffer message, - final int replyId, - long messageData) { + @NonNull String channel, @Nullable ByteBuffer message, int replyId, long messageData) { // Called from the ui thread. Log.v(TAG, "Received message from Dart over channel '" + channel + "'"); @@ -303,12 +305,12 @@ public void handleMessageFromDart( List delayedTaskQueue = delayedTaskDispatcher.get(channel); delayedTaskQueue.add( () -> { - dispatchMessageToQueue(messageHandlers.get(channel)); + dispatchMessageToQueue(messageHandlers.get(channel), message, replyId, messageData); }); } } if (handlerInfo != null) { - dispatchMessageToQueue(handlerInfo); + dispatchMessageToQueue(handlerInfo, message, replyId, messageData); } } From ea25323cffe7f5ece56b445c58688c1e03cd99ae Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Fri, 29 Oct 2021 16:04:58 -0700 Subject: [PATCH 12/20] Use container --- .../embedding/engine/dart/DartMessenger.java | 38 +++++++++++++------ 1 file changed, 26 insertions(+), 12 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 864cb28b940e4..f0b29c2f299db 100644 --- a/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java +++ b/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java @@ -51,7 +51,7 @@ class DartMessenger implements BinaryMessenger, PlatformMessageHandler { *

Reads and writes to this map must lock {@code handlersLock}. */ @NonNull - private final ConcurrentHashMap> delayedTaskDispatcher = + private final ConcurrentHashMap> delayedTaskDispatcher = new ConcurrentHashMap<>(); @NonNull private final Object handlersLock = new Object(); @@ -93,6 +93,10 @@ public DartMessengerTaskQueue makeBackgroundTaskQueue() { } } + /** + * Holds information about a platform handler, such as the task queue that processes messages from + * Dart. + */ private static class HandlerInfo { @NonNull public final BinaryMessenger.BinaryMessageHandler handler; @Nullable public final DartMessengerTaskQueue taskQueue; @@ -105,6 +109,22 @@ private static class HandlerInfo { } } + /** + * Holds information that allows to dispatch a Dart message to a platform handler when it becomes + * available. + */ + private static class DelayedMessageInfo { + @NonNull public final ByteBuffer message; + int replyId; + long messageData; + + DelayedMessageInfo(@NonNull ByteBuffer message, int replyId, long messageData) { + this.message = message; + this.replyId = replyId; + this.messageData = messageData; + } + } + private static class DefaultTaskQueue implements DartMessengerTaskQueue { @NonNull private final ExecutorService executor; @@ -174,7 +194,7 @@ public void setMessageHandler( * @param channel The channel name. */ public void runDelayedTasksForChannel(@NonNull String channel) { - LinkedList list; + LinkedList list; synchronized (handlersLock) { if (!delayedTaskDispatcher.contains(channel)) { return; @@ -183,12 +203,9 @@ public void runDelayedTasksForChannel(@NonNull String channel) { delayedTaskDispatcher.remove(channel); } while (!list.isEmpty()) { - final Runnable task = list.poll(); - try { - task.run(); - } catch (Exception ex) { - Log.e(TAG, "Exception thrown while running delayed task for channel '" + channel + "'", ex); - } + final DelayedMessageInfo info = list.poll(); + dispatchMessageToQueue( + channel, messageHandlers.get(channel), info.message, info.replyId, info.messageData); } } @@ -303,10 +320,7 @@ public void handleMessageFromDart( delayedTaskDispatcher.put(channel, new LinkedList<>()); } List delayedTaskQueue = delayedTaskDispatcher.get(channel); - delayedTaskQueue.add( - () -> { - dispatchMessageToQueue(messageHandlers.get(channel), message, replyId, messageData); - }); + delayedTaskQueue.add(new DelayedMessageInfo(message, replyId, messageData)); } } if (handlerInfo != null) { From 0b794ead7b200a2aff0c913d92c09b5cfa427413 Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Fri, 29 Oct 2021 18:59:56 -0700 Subject: [PATCH 13/20] tweaks --- .../embedding/engine/dart/DartMessenger.java | 68 +++++++------------ 1 file changed, 25 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 f0b29c2f299db..0c13d9e2e82fc 100644 --- a/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java +++ b/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java @@ -17,7 +17,6 @@ import java.util.List; import java.util.Map; import java.util.WeakHashMap; -import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.ThreadFactory; @@ -41,21 +40,17 @@ class DartMessenger implements BinaryMessenger, PlatformMessageHandler { * *

Reads and writes to this map must lock {@code handlersLock}. */ - @NonNull - private final ConcurrentHashMap messageHandlers = new ConcurrentHashMap<>(); + @NonNull private final Map messageHandlers = new HashMap<>(); /** - * Maps a channel name to queue of task dispatchers. This queue is processed when the channel - * handler is registered. + * Maps a channel name to an object that holds information about the incoming Dart message. * *

Reads and writes to this map must lock {@code handlersLock}. */ - @NonNull - private final ConcurrentHashMap> delayedTaskDispatcher = - new ConcurrentHashMap<>(); + @NonNull private final Map> bufferedMessages = new HashMap<>(); @NonNull private final Object handlersLock = new Object(); - private boolean canDelayTasks = false; + private boolean enableBufferingIncomingMessages = false; @NonNull private final Map pendingReplies = new HashMap<>(); private int nextReplyId = 1; @@ -113,12 +108,12 @@ private static class HandlerInfo { * Holds information that allows to dispatch a Dart message to a platform handler when it becomes * available. */ - private static class DelayedMessageInfo { + private static class BufferedMessageInfo { @NonNull public final ByteBuffer message; int replyId; long messageData; - DelayedMessageInfo(@NonNull ByteBuffer message, int replyId, long messageData) { + BufferedMessageInfo(@NonNull ByteBuffer message, int replyId, long messageData) { this.message = message; this.replyId = replyId; this.messageData = messageData; @@ -179,31 +174,17 @@ public void setMessageHandler( } } Log.v(TAG, "Setting handler for channel '" + channel + "'"); - synchronized (handlersLock) { - messageHandlers.put(channel, new HandlerInfo(handler, dartMessengerTaskQueue)); - } - runDelayedTasksForChannel(channel); - } - /** - * Runs the tasks that handle messages received from Dart for the provided channel name. - * - *

The channel may not have associated tasks if it was registered prior to reciving the first - * message from Dart. - * - * @param channel The channel name. - */ - public void runDelayedTasksForChannel(@NonNull String channel) { - LinkedList list; + LinkedList list; synchronized (handlersLock) { - if (!delayedTaskDispatcher.contains(channel)) { + messageHandlers.put(channel, new HandlerInfo(handler, dartMessengerTaskQueue)); + if (!bufferedMessages.containsKey(channel)) { return; } - list = (LinkedList) delayedTaskDispatcher.get(channel); - delayedTaskDispatcher.remove(channel); + list = (LinkedList) bufferedMessages.get(channel); + bufferedMessages.remove(channel); } - while (!list.isEmpty()) { - final DelayedMessageInfo info = list.poll(); + for (BufferedMessageInfo info : list) { dispatchMessageToQueue( channel, messageHandlers.get(channel), info.message, info.replyId, info.messageData); } @@ -216,8 +197,8 @@ public void runDelayedTasksForChannel(@NonNull String channel) { * be initialized concurrently, and prior to the registration of the channel handlers. This * implies that Dart may start sending messages while plugins are being registered. */ - public void enableDelayedTaskQueue() { - canDelayTasks = true; + public void enableBufferingIncomingMessages() { + enableBufferingIncomingMessages = true; } @Override @@ -287,9 +268,9 @@ private void dispatchMessageToQueue( message.limit(0); } } finally { - Trace.endSection(); // This is deleting the data underneath the message object. flutterJNI.cleanupMessageData(messageData); + Trace.endSection(); } }; final DartMessengerTaskQueue nonnullTaskQueue = @@ -304,10 +285,11 @@ public void handleMessageFromDart( Log.v(TAG, "Received message from Dart over channel '" + channel + "'"); HandlerInfo handlerInfo; + boolean messageDeferred; synchronized (handlersLock) { - if (messageHandlers.contains(channel)) { - handlerInfo = messageHandlers.get(channel); - } else if (canDelayTasks) { + handlerInfo = messageHandlers.get(channel); + messageDeferred = (enableBufferingIncomingMessages && handlerInfo == null); + if (messageDeferred) { // The channel is not defined when the Dart VM sends a message before the channels are // registered. // @@ -316,15 +298,15 @@ public void handleMessageFromDart( // // In such cases, the task dispatchers are queued, and processed when the channel is // defined. - if (!delayedTaskDispatcher.contains(channel)) { - delayedTaskDispatcher.put(channel, new LinkedList<>()); + if (!bufferedMessages.containsKey(channel)) { + bufferedMessages.put(channel, new LinkedList<>()); } - List delayedTaskQueue = delayedTaskDispatcher.get(channel); - delayedTaskQueue.add(new DelayedMessageInfo(message, replyId, messageData)); + List buffer = bufferedMessages.get(channel); + buffer.add(new BufferedMessageInfo(message, replyId, messageData)); } } - if (handlerInfo != null) { - dispatchMessageToQueue(handlerInfo, message, replyId, messageData); + if (!messageDeferred) { + dispatchMessageToQueue(channel, handlerInfo, message, replyId, messageData); } } From 4f193f2b2b0b577da89970b841161a70b87e9df9 Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Mon, 1 Nov 2021 17:39:22 -0700 Subject: [PATCH 14/20] Test --- .../engine/dart/DartMessengerTest.java | 58 +++++++++++++++++-- 1 file changed, 54 insertions(+), 4 deletions(-) diff --git a/shell/platform/android/test/io/flutter/embedding/engine/dart/DartMessengerTest.java b/shell/platform/android/test/io/flutter/embedding/engine/dart/DartMessengerTest.java index 4e7a6fdb7c2e1..b7d6944f503b8 100644 --- a/shell/platform/android/test/io/flutter/embedding/engine/dart/DartMessengerTest.java +++ b/shell/platform/android/test/io/flutter/embedding/engine/dart/DartMessengerTest.java @@ -1,13 +1,18 @@ package io.flutter.embedding.engine.dart; +import static android.os.Looper.getMainLooper; import static junit.framework.TestCase.assertEquals; import static junit.framework.TestCase.assertNotNull; import static junit.framework.TestCase.assertTrue; +import static org.junit.Assert.assertArrayEquals; import static org.mockito.Matchers.any; +import static org.mockito.Matchers.anyInt; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import static org.robolectric.Shadows.shadowOf; import io.flutter.embedding.engine.FlutterJNI; import io.flutter.embedding.engine.dart.DartMessenger.DartMessengerTaskQueue; @@ -16,6 +21,7 @@ import java.nio.ByteBuffer; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; import org.mockito.Mockito; import org.robolectric.RobolectricTestRunner; import org.robolectric.annotation.Config; @@ -155,8 +161,8 @@ public void replyIdIncrementsOnNullReply() { public void cleansUpMessageData() throws InterruptedException { final FlutterJNI fakeFlutterJni = mock(FlutterJNI.class); final DartMessenger messenger = new DartMessenger(fakeFlutterJni, () -> synchronousTaskQueue); - BinaryMessenger.TaskQueue taskQueue = messenger.makeBackgroundTaskQueue(); - String channel = "foobar"; + final BinaryMessenger.TaskQueue taskQueue = messenger.makeBackgroundTaskQueue(); + final String channel = "foobar"; BinaryMessenger.BinaryMessageHandler handler = (ByteBuffer message, BinaryMessenger.BinaryReply reply) -> { reply.reply(null); @@ -173,8 +179,8 @@ public void cleansUpMessageData() throws InterruptedException { public void cleansUpMessageDataOnError() throws InterruptedException { final FlutterJNI fakeFlutterJni = mock(FlutterJNI.class); final DartMessenger messenger = new DartMessenger(fakeFlutterJni, () -> synchronousTaskQueue); - BinaryMessenger.TaskQueue taskQueue = messenger.makeBackgroundTaskQueue(); - String channel = "foobar"; + final BinaryMessenger.TaskQueue taskQueue = messenger.makeBackgroundTaskQueue(); + final String channel = "foobar"; BinaryMessenger.BinaryMessageHandler handler = (ByteBuffer message, BinaryMessenger.BinaryReply reply) -> { throw new RuntimeException("hello"); @@ -186,4 +192,48 @@ public void cleansUpMessageDataOnError() throws InterruptedException { messenger.handleMessageFromDart(channel, message, replyId, messageData); verify(fakeFlutterJni).cleanupMessageData(eq(messageData)); } + + @Test + public void emptyResponseWhenHandlerIsNotSet() throws InterruptedException { + final FlutterJNI fakeFlutterJni = mock(FlutterJNI.class); + final DartMessenger messenger = new DartMessenger(fakeFlutterJni, () -> synchronousTaskQueue); + final String channel = "foobar"; + final ByteBuffer message = ByteBuffer.allocateDirect(4 * 2); + int replyId = 1; + long messageData = 1234; + messenger.handleMessageFromDart(channel, message, replyId, messageData); + shadowOf(getMainLooper()).idle(); + verify(fakeFlutterJni).invokePlatformMessageEmptyResponseCallback(replyId); + } + + @Test + public void buffersResponseWhenHandlerIsNotSet() throws InterruptedException { + final FlutterJNI fakeFlutterJni = mock(FlutterJNI.class); + final DartMessenger messenger = new DartMessenger(fakeFlutterJni, () -> synchronousTaskQueue); + final BinaryMessenger.TaskQueue taskQueue = messenger.makeBackgroundTaskQueue(); + final String channel = "foobar"; + final ByteBuffer message = ByteBuffer.allocateDirect(4 * 2); + int replyId = 1; + long messageData = 1234; + + messenger.enableBufferingIncomingMessages(); + messenger.handleMessageFromDart(channel, message, replyId, messageData); + + shadowOf(getMainLooper()).idle(); + verify(fakeFlutterJni, never()).invokePlatformMessageEmptyResponseCallback(eq(replyId)); + + final BinaryMessenger.BinaryMessageHandler handler = + (ByteBuffer msg, BinaryMessenger.BinaryReply reply) -> { + reply.reply(ByteBuffer.wrap("done".getBytes())); + }; + messenger.setMessageHandler(channel, handler, taskQueue); + + shadowOf(getMainLooper()).idle(); + verify(fakeFlutterJni, never()).invokePlatformMessageEmptyResponseCallback(eq(replyId)); + + final ArgumentCaptor response = ArgumentCaptor.forClass(ByteBuffer.class); + verify(fakeFlutterJni) + .invokePlatformMessageResponseCallback(anyInt(), response.capture(), anyInt()); + assertArrayEquals("done".getBytes(), response.getValue().array()); + } } From d36def96d50b7e577453848503aba4e9d68eb316 Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Mon, 1 Nov 2021 17:40:52 -0700 Subject: [PATCH 15/20] line --- .../test/io/flutter/embedding/engine/dart/DartMessengerTest.java | 1 + 1 file changed, 1 insertion(+) 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 b7d6944f503b8..00d712d2d0264 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 @@ -201,6 +201,7 @@ public void emptyResponseWhenHandlerIsNotSet() throws InterruptedException { final ByteBuffer message = ByteBuffer.allocateDirect(4 * 2); int replyId = 1; long messageData = 1234; + messenger.handleMessageFromDart(channel, message, replyId, messageData); shadowOf(getMainLooper()).idle(); verify(fakeFlutterJni).invokePlatformMessageEmptyResponseCallback(replyId); From 291e88ef08b7bd7ad5f7951012d563c0f604bc0d Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Mon, 1 Nov 2021 17:43:25 -0700 Subject: [PATCH 16/20] Add final --- .../engine/dart/DartMessengerTest.java | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/shell/platform/android/test/io/flutter/embedding/engine/dart/DartMessengerTest.java b/shell/platform/android/test/io/flutter/embedding/engine/dart/DartMessengerTest.java index 00d712d2d0264..08a322d6da41f 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 @@ -169,8 +169,8 @@ public void cleansUpMessageData() throws InterruptedException { }; messenger.setMessageHandler(channel, handler, taskQueue); final ByteBuffer message = ByteBuffer.allocateDirect(4 * 2); - int replyId = 1; - long messageData = 1234; + final int replyId = 1; + final long messageData = 1234; messenger.handleMessageFromDart(channel, message, replyId, messageData); verify(fakeFlutterJni).cleanupMessageData(eq(messageData)); } @@ -187,8 +187,9 @@ public void cleansUpMessageDataOnError() throws InterruptedException { }; messenger.setMessageHandler(channel, handler, taskQueue); final ByteBuffer message = ByteBuffer.allocateDirect(4 * 2); - int replyId = 1; - long messageData = 1234; + final int replyId = 1; + final long messageData = 1234; + messenger.handleMessageFromDart(channel, message, replyId, messageData); verify(fakeFlutterJni).cleanupMessageData(eq(messageData)); } @@ -199,8 +200,8 @@ public void emptyResponseWhenHandlerIsNotSet() throws InterruptedException { final DartMessenger messenger = new DartMessenger(fakeFlutterJni, () -> synchronousTaskQueue); final String channel = "foobar"; final ByteBuffer message = ByteBuffer.allocateDirect(4 * 2); - int replyId = 1; - long messageData = 1234; + final int replyId = 1; + final long messageData = 1234; messenger.handleMessageFromDart(channel, message, replyId, messageData); shadowOf(getMainLooper()).idle(); @@ -214,8 +215,8 @@ public void buffersResponseWhenHandlerIsNotSet() throws InterruptedException { final BinaryMessenger.TaskQueue taskQueue = messenger.makeBackgroundTaskQueue(); final String channel = "foobar"; final ByteBuffer message = ByteBuffer.allocateDirect(4 * 2); - int replyId = 1; - long messageData = 1234; + final int replyId = 1; + final long messageData = 1234; messenger.enableBufferingIncomingMessages(); messenger.handleMessageFromDart(channel, message, replyId, messageData); From d40a3b775032154571fc881904711b8ff588b5c5 Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Tue, 2 Nov 2021 17:30:50 -0700 Subject: [PATCH 17/20] disableBufferingIncomingMessages --- .../embedding/engine/dart/DartMessenger.java | 11 +++++- .../engine/dart/DartMessengerTest.java | 38 +++++++++++++++++++ 2 files changed, 48 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 0c13d9e2e82fc..a973fbcff79e9 100644 --- a/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java +++ b/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java @@ -191,7 +191,7 @@ public void setMessageHandler( } /** - * Enables the ability to queue tasks when messages are received from Dart. + * Enables the ability to queue messages received from Dart. * *

This is useful when there are pending channel handler registrations. For example, Dart may * be initialized concurrently, and prior to the registration of the channel handlers. This @@ -201,6 +201,15 @@ public void enableBufferingIncomingMessages() { enableBufferingIncomingMessages = true; } + /** + * Disables the ability to queue messages received from Dart. + * + *

This can be used after all pending channel handlers have been registered. + */ + public void disableBufferingIncomingMessages() { + enableBufferingIncomingMessages = false; + } + @Override @UiThread public void send(@NonNull String channel, @NonNull ByteBuffer message) { 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 08a322d6da41f..a8fba3b502d15 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 @@ -238,4 +238,42 @@ public void buffersResponseWhenHandlerIsNotSet() throws InterruptedException { .invokePlatformMessageResponseCallback(anyInt(), response.capture(), anyInt()); assertArrayEquals("done".getBytes(), response.getValue().array()); } + + @Test + public void emptyResponseWhenHandlerIsUnregistered() throws InterruptedException { + final FlutterJNI fakeFlutterJni = mock(FlutterJNI.class); + final DartMessenger messenger = new DartMessenger(fakeFlutterJni, () -> synchronousTaskQueue); + final BinaryMessenger.TaskQueue taskQueue = messenger.makeBackgroundTaskQueue(); + final String channel = "foobar"; + final ByteBuffer message = ByteBuffer.allocateDirect(4 * 2); + final int replyId = 1; + final long messageData = 1234; + + messenger.enableBufferingIncomingMessages(); + messenger.handleMessageFromDart(channel, message, replyId, messageData); + + shadowOf(getMainLooper()).idle(); + verify(fakeFlutterJni, never()).invokePlatformMessageEmptyResponseCallback(eq(replyId)); + + final BinaryMessenger.BinaryMessageHandler handler = + (ByteBuffer msg, BinaryMessenger.BinaryReply reply) -> { + reply.reply(ByteBuffer.wrap("done".getBytes())); + }; + messenger.setMessageHandler(channel, handler, taskQueue); + + shadowOf(getMainLooper()).idle(); + verify(fakeFlutterJni, never()).invokePlatformMessageEmptyResponseCallback(eq(replyId)); + + final ArgumentCaptor response = ArgumentCaptor.forClass(ByteBuffer.class); + verify(fakeFlutterJni) + .invokePlatformMessageResponseCallback(anyInt(), response.capture(), anyInt()); + assertArrayEquals("done".getBytes(), response.getValue().array()); + + messenger.disableBufferingIncomingMessages(); + messenger.setMessageHandler(channel, null, null); // Unregister handler. + + messenger.handleMessageFromDart(channel, message, replyId, messageData); + shadowOf(getMainLooper()).idle(); + verify(fakeFlutterJni).invokePlatformMessageEmptyResponseCallback(replyId); + } } From b14cdef150a3c358799241b58e599815352ad528 Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Wed, 3 Nov 2021 14:50:23 -0700 Subject: [PATCH 18/20] feedback --- .../embedding/engine/dart/DartExecutor.java | 12 ++++++ .../embedding/engine/dart/DartMessenger.java | 39 ++++++++++--------- .../plugin/common/BinaryMessenger.java | 16 ++++++++ .../io/flutter/view/FlutterNativeView.java | 6 +++ .../android/io/flutter/view/FlutterView.java | 6 +++ .../engine/dart/DartMessengerTest.java | 20 ++++++++++ 6 files changed, 80 insertions(+), 19 deletions(-) diff --git a/shell/platform/android/io/flutter/embedding/engine/dart/DartExecutor.java b/shell/platform/android/io/flutter/embedding/engine/dart/DartExecutor.java index ecf410d0fc3b0..13eb59cd0fa9f 100644 --- a/shell/platform/android/io/flutter/embedding/engine/dart/DartExecutor.java +++ b/shell/platform/android/io/flutter/embedding/engine/dart/DartExecutor.java @@ -286,6 +286,12 @@ public void notifyLowMemoryWarning() { } } + @Override + public void enableBufferingIncomingMessages() {} + + @Override + public void disableBufferingIncomingMessages() {} + /** * Configuration options that specify which Dart entrypoint function is executed and where to find * that entrypoint and other assets required for Dart execution. @@ -461,5 +467,11 @@ public void setMessageHandler( @Nullable TaskQueue taskQueue) { messenger.setMessageHandler(channel, handler, taskQueue); } + + @Override + public void enableBufferingIncomingMessages() {} + + @Override + public void disableBufferingIncomingMessages() {} } } 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 143aa13bf1cff..5c86498cb8a03 100644 --- a/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java +++ b/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java @@ -50,7 +50,7 @@ class DartMessenger implements BinaryMessenger, PlatformMessageHandler { @NonNull private final Map> bufferedMessages = new HashMap<>(); @NonNull private final Object handlersLock = new Object(); - private boolean enableBufferingIncomingMessages = false; + private AtomicBoolean enableBufferingIncomingMessages = new AtomicBoolean(false); @NonNull private final Map pendingReplies = new HashMap<>(); private int nextReplyId = 1; @@ -204,14 +204,13 @@ public void setMessageHandler( } Log.v(TAG, "Setting handler for channel '" + channel + "'"); - LinkedList list; + List list; synchronized (handlersLock) { messageHandlers.put(channel, new HandlerInfo(handler, dartMessengerTaskQueue)); if (!bufferedMessages.containsKey(channel)) { return; } - list = (LinkedList) bufferedMessages.get(channel); - bufferedMessages.remove(channel); + list = bufferedMessages.remove(channel); } for (BufferedMessageInfo info : list) { dispatchMessageToQueue( @@ -219,24 +218,26 @@ public void setMessageHandler( } } - /** - * Enables the ability to queue messages received from Dart. - * - *

This is useful when there are pending channel handler registrations. For example, Dart may - * be initialized concurrently, and prior to the registration of the channel handlers. This - * implies that Dart may start sending messages while plugins are being registered. - */ + @Override public void enableBufferingIncomingMessages() { - enableBufferingIncomingMessages = true; + enableBufferingIncomingMessages.set(true); } - /** - * Disables the ability to queue messages received from Dart. - * - *

This can be used after all pending channel handlers have been registered. - */ + @Override public void disableBufferingIncomingMessages() { - enableBufferingIncomingMessages = false; + if (!enableBufferingIncomingMessages.getAndSet(false)) { + return; + } + Map> pendingMessagesCopy = new HashMap<>(); + synchronized (handlersLock) { + pendingMessagesCopy = new HashMap(bufferedMessages); + bufferedMessages.clear(); + } + for (String channel : pendingMessagesCopy.keySet()) { + for (BufferedMessageInfo info : pendingMessagesCopy.get(channel)) { + dispatchMessageToQueue(channel, null, info.message, info.replyId, info.messageData); + } + } } @Override @@ -326,7 +327,7 @@ public void handleMessageFromDart( boolean messageDeferred; synchronized (handlersLock) { handlerInfo = messageHandlers.get(channel); - messageDeferred = (enableBufferingIncomingMessages && handlerInfo == null); + messageDeferred = (enableBufferingIncomingMessages.get() && handlerInfo == null); if (messageDeferred) { // The channel is not defined when the Dart VM sends a message before the channels are // registered. diff --git a/shell/platform/android/io/flutter/plugin/common/BinaryMessenger.java b/shell/platform/android/io/flutter/plugin/common/BinaryMessenger.java index 137d5bf2818a7..17ab47de4b635 100644 --- a/shell/platform/android/io/flutter/plugin/common/BinaryMessenger.java +++ b/shell/platform/android/io/flutter/plugin/common/BinaryMessenger.java @@ -116,6 +116,22 @@ default void setMessageHandler( setMessageHandler(channel, handler); } + /** + * Enables the ability to queue messages received from Dart. + * + *

This is useful when there are pending channel handler registrations. For example, Dart may + * be initialized concurrently, and prior to the registration of the channel handlers. This + * implies that Dart may start sending messages while plugins are being registered. + */ + void enableBufferingIncomingMessages(); + + /** + * Disables the ability to queue messages received from Dart. + * + *

This can be used after all pending channel handlers have been registered. + */ + void disableBufferingIncomingMessages(); + /** Handler for incoming binary messages from Flutter. */ interface BinaryMessageHandler { /** diff --git a/shell/platform/android/io/flutter/view/FlutterNativeView.java b/shell/platform/android/io/flutter/view/FlutterNativeView.java index 2cf589cf7e78c..ca4d6524b6aa6 100644 --- a/shell/platform/android/io/flutter/view/FlutterNativeView.java +++ b/shell/platform/android/io/flutter/view/FlutterNativeView.java @@ -159,6 +159,12 @@ public void setMessageHandler(String channel, BinaryMessageHandler handler, Task dartExecutor.getBinaryMessenger().setMessageHandler(channel, handler, taskQueue); } + @Override + public void enableBufferingIncomingMessages() {} + + @Override + public void disableBufferingIncomingMessages() {} + /*package*/ FlutterJNI getFlutterJNI() { return mFlutterJNI; } diff --git a/shell/platform/android/io/flutter/view/FlutterView.java b/shell/platform/android/io/flutter/view/FlutterView.java index 317ec6562a1ca..d4d14d30bbd8e 100644 --- a/shell/platform/android/io/flutter/view/FlutterView.java +++ b/shell/platform/android/io/flutter/view/FlutterView.java @@ -347,6 +347,12 @@ public void removeFirstFrameListener(FirstFrameListener listener) { mFirstFrameListeners.remove(listener); } + @Override + public void enableBufferingIncomingMessages() {} + + @Override + public void disableBufferingIncomingMessages() {} + /** * Reverts this back to the {@link SurfaceView} defaults, at the back of its window and opaque. */ 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 387ba1c253c97..b83e9a07457dd 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 @@ -244,6 +244,26 @@ public void buffersResponseWhenHandlerIsNotSet() throws InterruptedException { assertArrayEquals("done".getBytes(), response.getValue().array()); } + @Test + public void disableBufferingTriggersEmptyResponseForPendingMessages() + throws InterruptedException { + final FlutterJNI fakeFlutterJni = mock(FlutterJNI.class); + final DartMessenger messenger = new DartMessenger(fakeFlutterJni, () -> synchronousTaskQueue); + final String channel = "foobar"; + final ByteBuffer message = ByteBuffer.allocateDirect(4 * 2); + final int replyId = 1; + final long messageData = 1234; + + messenger.enableBufferingIncomingMessages(); + messenger.handleMessageFromDart(channel, message, replyId, messageData); + shadowOf(getMainLooper()).idle(); + verify(fakeFlutterJni, never()).invokePlatformMessageEmptyResponseCallback(replyId); + + messenger.disableBufferingIncomingMessages(); + shadowOf(getMainLooper()).idle(); + verify(fakeFlutterJni).invokePlatformMessageEmptyResponseCallback(replyId); + } + @Test public void emptyResponseWhenHandlerIsUnregistered() throws InterruptedException { final FlutterJNI fakeFlutterJni = mock(FlutterJNI.class); From 7d6dd55f565712665fc8d03fd0593f14acb1c0c1 Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Wed, 3 Nov 2021 14:55:09 -0700 Subject: [PATCH 19/20] final + annotation --- .../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 5c86498cb8a03..86532931c6199 100644 --- a/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java +++ b/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java @@ -50,7 +50,7 @@ class DartMessenger implements BinaryMessenger, PlatformMessageHandler { @NonNull private final Map> bufferedMessages = new HashMap<>(); @NonNull private final Object handlersLock = new Object(); - private AtomicBoolean enableBufferingIncomingMessages = new AtomicBoolean(false); + @NonNull private final AtomicBoolean enableBufferingIncomingMessages = new AtomicBoolean(false); @NonNull private final Map pendingReplies = new HashMap<>(); private int nextReplyId = 1; From 4120dbd6860bfcccb50f7bbd046cc682e3d26492 Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Thu, 4 Nov 2021 17:19:50 -0700 Subject: [PATCH 20/20] Jason feedback --- .../embedding/engine/dart/DartMessenger.java | 23 +++++++++---------- 1 file changed, 11 insertions(+), 12 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 86532931c6199..a37182251481b 100644 --- a/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java +++ b/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java @@ -47,7 +47,7 @@ class DartMessenger implements BinaryMessenger, PlatformMessageHandler { * *

Reads and writes to this map must lock {@code handlersLock}. */ - @NonNull private final Map> bufferedMessages = new HashMap<>(); + @NonNull private Map> bufferedMessages = new HashMap<>(); @NonNull private final Object handlersLock = new Object(); @NonNull private final AtomicBoolean enableBufferingIncomingMessages = new AtomicBoolean(false); @@ -207,10 +207,10 @@ public void setMessageHandler( List list; synchronized (handlersLock) { messageHandlers.put(channel, new HandlerInfo(handler, dartMessengerTaskQueue)); - if (!bufferedMessages.containsKey(channel)) { + list = bufferedMessages.remove(channel); + if (list == null) { return; } - list = bufferedMessages.remove(channel); } for (BufferedMessageInfo info : list) { dispatchMessageToQueue( @@ -225,17 +225,16 @@ public void enableBufferingIncomingMessages() { @Override public void disableBufferingIncomingMessages() { - if (!enableBufferingIncomingMessages.getAndSet(false)) { - return; - } - Map> pendingMessagesCopy = new HashMap<>(); + Map> pendingMessages; synchronized (handlersLock) { - pendingMessagesCopy = new HashMap(bufferedMessages); - bufferedMessages.clear(); + enableBufferingIncomingMessages.set(false); + pendingMessages = bufferedMessages; + bufferedMessages = new HashMap<>(); } - for (String channel : pendingMessagesCopy.keySet()) { - for (BufferedMessageInfo info : pendingMessagesCopy.get(channel)) { - dispatchMessageToQueue(channel, null, info.message, info.replyId, info.messageData); + for (Map.Entry> channel : pendingMessages.entrySet()) { + for (BufferedMessageInfo info : channel.getValue()) { + dispatchMessageToQueue( + channel.getKey(), null, info.message, info.replyId, info.messageData); } } }