From 47409d5a1d7cb7d69fdeeb86be32abcdcc53e93b Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Tue, 26 Oct 2021 14:09:14 -0700 Subject: [PATCH 1/4] Revert "Revert "Android Background Platform Channels (#29147)"" This reverts commit 7ed91e14ccc1f5ffc4cb13d1d37e27d370c8f7cd. --- ci/licenses_golden/licenses_flutter | 4 + shell/common/BUILD.gn | 1 + shell/common/platform_message_handler.h | 39 +++++ shell/common/platform_view.cc | 5 + shell/common/platform_view.h | 12 ++ shell/common/shell.cc | 24 ++- shell/common/shell.h | 7 + shell/common/shell_test.cc | 35 ++--- shell/common/shell_test.h | 4 +- shell/common/shell_unittests.cc | 70 +++++++++ shell/platform/android/BUILD.gn | 3 + shell/platform/android/android_shell_holder.h | 6 + .../android/android_shell_holder_unittests.cc | 39 +++++ .../flutter/embedding/engine/FlutterJNI.java | 94 +++++++++--- .../embedding/engine/dart/DartExecutor.java | 26 +++- .../embedding/engine/dart/DartMessenger.java | 142 +++++++++++++++--- .../engine/dart/PlatformMessageHandler.java | 5 +- .../engine/dart/PlatformTaskQueue.java | 19 +++ .../plugin/common/BasicMessageChannel.java | 24 ++- .../plugin/common/BinaryMessenger.java | 26 +++- .../flutter/plugin/common/EventChannel.java | 24 ++- .../flutter/plugin/common/MethodChannel.java | 23 ++- .../io/flutter/view/FlutterNativeView.java | 10 +- .../android/io/flutter/view/FlutterView.java | 10 +- .../platform_message_handler_android.cc | 64 ++++++++ .../platform_message_handler_android.h | 38 +++++ .../platform/android/platform_view_android.cc | 47 +----- .../platform/android/platform_view_android.h | 20 +-- .../android/platform_view_android_jni_impl.cc | 41 +++-- .../engine/dart/DartMessengerTest.java | 67 +++++++-- .../plugin/editing/TextInputPluginTest.java | 6 +- .../platform/PlatformViewsControllerTest.java | 68 +++++++-- .../MainActivity.java | 5 +- .../scenarios/SpawnedEngineActivity.java | 3 +- .../scenarios/TestableFlutterActivity.java | 3 +- 35 files changed, 832 insertions(+), 182 deletions(-) create mode 100644 shell/common/platform_message_handler.h create mode 100644 shell/platform/android/io/flutter/embedding/engine/dart/PlatformTaskQueue.java create mode 100644 shell/platform/android/platform_message_handler_android.cc create mode 100644 shell/platform/android/platform_message_handler_android.h diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index 8e2f603cf0303..af42c737cce60 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 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..8e3f77e448236 --- /dev/null +++ b/shell/common/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_COMMON_PLATFORM_MESSAGE_HANDLER_H_ +#define SHELL_COMMON_PLATFORM_MESSAGE_HANDLER_H_ + +#include + +#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. + /// 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 + +#endif // SHELL_COMMON_PLATFORM_MESSAGE_HANDLER_H_ 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 85e0a4fe8e6b9..b6555f6c11968 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,6 +811,17 @@ class PlatformView { virtual std::unique_ptr CreateSnapshotSurfaceProducer(); + //-------------------------------------------------------------------------- + /// @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; + protected: PlatformView::Delegate& delegate_; const TaskRunners task_runners_; diff --git a/shell/common/shell.cc b/shell/common/shell.cc index 24061c24210d8..df301e717fea9 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,13 +1193,17 @@ 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_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 { + if (view) { + view->HandlePlatformMessage(std::move(message)); + } + })); + } } void Shell::HandleEngineSkiaMessage(std::unique_ptr message) { @@ -1867,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 9fbdd5b1a423f..3884299d19294 100644 --- a/shell/common/shell.h +++ b/shell/common/shell.h @@ -395,6 +395,12 @@ 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; + private: using ServiceProtocolHandler = std::function 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/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 0288b6088ec7b..b76cc6560e564 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 @@ -3162,7 +3187,52 @@ TEST_F(ShellTest, UIWorkAfterOnPlatformViewDestroyed) { shell->GetTaskRunners().GetUITaskRunner(), [&ui_flush_latch]() { 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)); } diff --git a/shell/platform/android/BUILD.gn b/shell/platform/android/BUILD.gn index 2780a734ef590..862fc405f88fd 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_android.cc", + "platform_message_handler_android.h", "platform_message_response_android.cc", "platform_message_response_android.h", "platform_view_android.cc", @@ -186,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/android_shell_holder.h b/shell/platform/android/android_shell_holder.h index 9ea264f366722..062e7e45c4cff 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_android.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 shell_->GetPlatformMessageHandler(); + } + private: const flutter::Settings settings_; const std::shared_ptr jni_facade_; diff --git a/shell/platform/android/android_shell_holder_unittests.cc b/shell/platform/android/android_shell_holder_unittests.cc index 2d3bc3ef311e1..85cb352e005f9 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(response_id); +} } // namespace testing } // namespace flutter diff --git a/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java b/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java index 9f0648e49741f..ac484f21f4601 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); @@ -858,14 +875,33 @@ public void setPlatformMessageHandler(@Nullable PlatformMessageHandler platformM this.platformMessageHandler = platformMessageHandler; } - // Called by native. + 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); + } + + // Called by native on the ui thread. // 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, + final 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) @@ -931,16 +967,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(); } } @@ -949,21 +989,25 @@ 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."); } - 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(); } } 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 2822b4060fbcd..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,11 @@ 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; +import java.util.concurrent.ThreadFactory; import java.util.concurrent.atomic.AtomicBoolean; /** @@ -26,25 +31,104 @@ 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) { + @NonNull private final DartMessengerTaskQueue platformTaskQueue = new PlatformTaskQueue(); + + @NonNull private WeakHashMap createdTaskQueues; + + @NonNull private TaskQueueFactory taskQueueFactory; + + DartMessenger(@NonNull FlutterJNI flutterJNI, @NonNull TaskQueueFactory taskQueueFactory) { this.flutterJNI = flutterJNI; - this.messageHandlers = new HashMap<>(); + 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 DartMessengerTaskQueue taskQueue; + + HandlerInfo( + @NonNull BinaryMessenger.BinaryMessageHandler handler, + @Nullable DartMessengerTaskQueue taskQueue) { + this.handler = handler; + this.taskQueue = 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"); + }; + this.executor = Executors.newSingleThreadExecutor(threadFactory); + } + + @Override + public void dispatch(@NonNull Runnable runnable) { + executor.execute(runnable); + } + } + + @Override + public TaskQueue makeBackgroundTaskQueue() { + DartMessengerTaskQueue taskQueue = taskQueueFactory.makeBackgroundTaskQueue(); + TaskQueueToken token = new TaskQueueToken(); + createdTaskQueues.put(token, taskQueue); + return token; } @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 { + 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, handler); + messageHandlers.put(channel, new HandlerInfo(handler, dartMessengerTaskQueue)); } } @@ -72,20 +156,13 @@ public void send( } } - @Override - public void handleMessageFromDart( - @NonNull final String channel, @Nullable ByteBuffer message, final int replyId) { - Log.v(TAG, "Received message from Dart over channel '" + channel + "'"); - BinaryMessenger.BinaryMessageHandler handler = messageHandlers.get(channel); - 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)); - 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); - } + handlerInfo.handler.onMessage(message, new Reply(flutterJNI, replyId)); } catch (Exception ex) { Log.e(TAG, "Uncaught exception in binary message listener", ex); flutterJNI.invokePlatformMessageEmptyResponseCallback(replyId); @@ -98,6 +175,37 @@ public void handleMessageFromDart( } } + @Override + public void handleMessageFromDart( + @NonNull final String channel, + @Nullable ByteBuffer message, + final int replyId, + long messageData) { + // Called from the ui thread. + Log.v(TAG, "Received message from Dart over channel '" + channel + "'"); + @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 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/io/flutter/embedding/engine/dart/PlatformTaskQueue.java b/shell/platform/android/io/flutter/embedding/engine/dart/PlatformTaskQueue.java new file mode 100644 index 0000000000000..e90ab89e62223 --- /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; + +/** A BinaryMessenger.TaskQueue that posts to the platform thread (aka main thread). */ +public class PlatformTaskQueue implements DartMessenger.DartMessengerTaskQueue { + @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/io/flutter/plugin/common/BasicMessageChannel.java b/shell/platform/android/io/flutter/plugin/common/BasicMessageChannel.java index 55eb3109af5bb..fa9ffe2a90855 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,25 @@ 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 means execute on the platform thread. See also {@link + * BinaryMessenger#makeBackgroundTaskQueue()}. + */ + 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 +81,7 @@ public BasicMessageChannel( this.messenger = messenger; this.name = name; this.codec = codec; + this.taskQueue = taskQueue; } /** @@ -101,7 +122,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), taskQueue); } /** diff --git a/shell/platform/android/io/flutter/plugin/common/BinaryMessenger.java b/shell/platform/android/io/flutter/plugin/common/BinaryMessenger.java index fb1a22e6f4d3c..433a6a9bbeaa9 100644 --- a/shell/platform/android/io/flutter/plugin/common/BinaryMessenger.java +++ b/shell/platform/android/io/flutter/plugin/common/BinaryMessenger.java @@ -26,6 +26,24 @@ * @see EventChannel , which supports communication using event streams. */ public interface BinaryMessenger { + /** + * 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 {} + + /** + * 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(); + /** * Sends a binary message to the Flutter application. * @@ -62,9 +80,14 @@ public interface BinaryMessenger { * * @param channel the name {@link String} of the channel. * @param handler a {@link BinaryMessageHandler} to be invoked on incoming messages, or null. + * @param taskQueue a {@link BinaryMessenger.TaskQueue} that specifies what thread will execute + * the handler. Specifying null means execute on the platform thread. */ @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 +120,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..70bd7d56951c7 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,25 @@ 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 means execute on the platform thread. See also {@link + * BinaryMessenger#makeBackgroundTaskQueue()}. + */ + 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 +90,7 @@ public EventChannel(BinaryMessenger messenger, String name, MethodCodec codec) { this.messenger = messenger; this.name = name; this.codec = codec; + this.taskQueue = taskQueue; } /** @@ -84,7 +106,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), taskQueue); } /** diff --git a/shell/platform/android/io/flutter/plugin/common/MethodChannel.java b/shell/platform/android/io/flutter/plugin/common/MethodChannel.java index 901299b943019..66c92cf6a2497 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,25 @@ public MethodChannel(BinaryMessenger messenger, String name) { * @param codec a {@link MessageCodec}. */ 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 means execute on the platform thread. See also {@link + * BinaryMessenger#makeBackgroundTaskQueue()}. + */ + 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 +90,7 @@ public MethodChannel(BinaryMessenger messenger, String name, MethodCodec codec) this.messenger = messenger; this.name = name; this.codec = codec; + this.taskQueue = taskQueue; } /** @@ -117,7 +138,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_message_handler_android.cc b/shell/platform/android/platform_message_handler_android.cc new file mode 100644 index 0000000000000..2179611a2f90e --- /dev/null +++ b/shell/platform/android/platform_message_handler_android.cc @@ -0,0 +1,64 @@ + +#include "flutter/shell/platform/android/platform_message_handler_android.h" + +namespace flutter { + +PlatformMessageHandlerAndroid::PlatformMessageHandlerAndroid( + const std::shared_ptr& jni_facade) + : jni_facade_(jni_facade) {} + +void PlatformMessageHandlerAndroid::InvokePlatformMessageResponseCallback( + int response_id, + std::unique_ptr mapping) { + // 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. + 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->Complete(std::move(mapping)); +} + +void PlatformMessageHandlerAndroid::InvokePlatformMessageEmptyResponseCallback( + int response_id) { + // Called from any thread. + if (!response_id) { + 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); + } + message_response->CompleteEmpty(); +} + +// |PlatformView| +void PlatformMessageHandlerAndroid::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); +} + +} // namespace flutter diff --git a/shell/platform/android/platform_message_handler_android.h b/shell/platform/android/platform_message_handler_android.h new file mode 100644 index 0000000000000..dc36013db69c1 --- /dev/null +++ b/shell/platform/android/platform_message_handler_android.h @@ -0,0 +1,38 @@ +// 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/common/platform_message_handler.h" +#include "flutter/shell/platform/android/jni/platform_view_android_jni.h" + +namespace flutter { +class PlatformMessageHandlerAndroid : public PlatformMessageHandler { + public: + PlatformMessageHandlerAndroid( + const std::shared_ptr& jni_facade); + void HandlePlatformMessage(std::unique_ptr message) override; + void InvokePlatformMessageResponseCallback( + int response_id, + std::unique_ptr mapping) override; + + void InvokePlatformMessageEmptyResponseCallback(int response_id) override; + + 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 50b9d55c6ca4a..064bbf92ae668 100644 --- a/shell/platform/android/platform_view_android.cc +++ b/shell/platform/android/platform_view_android.cc @@ -73,7 +73,8 @@ PlatformViewAndroid::PlatformViewAndroid( : 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_(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_) { @@ -190,51 +191,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) { - if (!response_id) - return; - 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); - message_response->Complete( - std::make_unique(std::move(response))); -} - -void PlatformViewAndroid::InvokePlatformMessageEmptyResponseCallback( - JNIEnv* env, - jint response_id) { - if (!response_id) - return; - auto it = pending_responses_.find(response_id); - if (it == pending_responses_.end()) - return; - auto message_response = std::move(it->second); - pending_responses_.erase(it); - message_response->CompleteEmpty(); -} - // |PlatformView| void PlatformViewAndroid::HandlePlatformMessage( std::unique_ptr message) { - int response_id = next_response_id_++; - if (auto response = message->response()) { - pending_responses_[response_id] = response; - } - // This call can re-enter in InvokePlatformMessageXxxResponseCallback. - jni_facade_->FlutterViewHandlePlatformMessage(std::move(message), - response_id); - message = nullptr; + // Called from the ui thread. + 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 1bd959ef855dd..0ff654f0fba72 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_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" @@ -79,14 +80,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, @@ -116,6 +109,11 @@ class PlatformViewAndroid final : public PlatformView { return android_context_; } + std::shared_ptr GetPlatformMessageHandler() + const override { + return platform_message_handler_; + } + private: const std::shared_ptr jni_facade_; std::shared_ptr android_context_; @@ -124,11 +122,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::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 376ac155516d0..8b8494b3d3a6d 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, @@ -483,22 +490,21 @@ static void InvokePlatformMessageResponseCallback(JNIEnv* env, jint responseId, jobject message, jint position) { - ANDROID_SHELL_HOLDER->GetPlatformView() - ->InvokePlatformMessageResponseCallback(env, // - responseId, // - message, // - 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(responseId, std::move(mapping)); } static void InvokePlatformMessageEmptyResponseCallback(JNIEnv* env, jobject jcaller, jlong shell_holder, jint responseId) { - ANDROID_SHELL_HOLDER->GetPlatformView() - ->InvokePlatformMessageEmptyResponseCallback(env, // - responseId // - ); + ANDROID_SHELL_HOLDER->GetPlatformMessageHandler() + ->InvokePlatformMessageEmptyResponseCallback(responseId); } static void NotifyLowMemoryWarning(JNIEnv* env, @@ -638,6 +644,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 +830,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 +1113,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 +1129,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)); 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..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,6 +10,7 @@ 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; @@ -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 DartMessengerTaskQueue { + public void dispatch(Runnable runnable) { + runnable.run(); + } + } + @Test public void itHandlesErrors() { // Setup test. @@ -44,14 +53,14 @@ 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); - messenger.handleMessageFromDart("test", ByteBuffer.allocate(0), 0); + 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); currentThread.setUncaughtExceptionHandler(savedHandler); @@ -61,21 +70,22 @@ 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); + BinaryMessenger.TaskQueue taskQueue = messenger.makeBackgroundTaskQueue(); + messenger.setMessageHandler(channel, handler, taskQueue); 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]); } @@ -83,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; @@ -92,14 +102,15 @@ public void directByteBufferLimitZeroAfterUsage() { byteBuffers[0] = message; assertEquals(bufferSize, byteBuffers[0].limit()); }; - messenger.setMessageHandler(channel, handler); + BinaryMessenger.TaskQueue taskQueue = messenger.makeBackgroundTaskQueue(); + messenger.setMessageHandler(channel, handler, taskQueue); 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 +150,40 @@ 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, () -> synchronousTaskQueue); + 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); + verify(fakeFlutterJni).cleanupMessageData(eq(messageData)); + } + + @Test + 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"; + 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); + 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<>(); 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) { 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 4d4cc68037a8c3856b3c1d93090d10d4df8375e9 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Tue, 26 Oct 2021 14:52:53 -0700 Subject: [PATCH 2/4] Made this PR less of a breaking change by keeping setMessageHandler's signature the same --- .../embedding/engine/dart/DartExecutor.java | 28 +++++++--- .../embedding/engine/dart/DartMessenger.java | 36 ++++++++---- .../plugin/common/BasicMessageChannel.java | 4 +- .../plugin/common/BinaryMessenger.java | 18 ++++-- .../flutter/plugin/common/EventChannel.java | 3 +- .../flutter/plugin/common/MethodChannel.java | 3 +- .../io/flutter/view/FlutterNativeView.java | 10 +++- .../android/io/flutter/view/FlutterView.java | 10 +++- .../engine/dart/DartMessengerTest.java | 55 +++++++++++++++++-- .../plugin/editing/TextInputPluginTest.java | 6 +- .../MainActivity.java | 5 +- .../scenarios/SpawnedEngineActivity.java | 3 +- .../scenarios/TestableFlutterActivity.java | 3 +- 13 files changed, 135 insertions(+), 49 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 dc5de3da1ed4b..80f853c577b95 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, null); + dartMessenger.setMessageHandler("flutter/isolate", isolateChannelMessageHandler); this.binaryMessenger = new DefaultBinaryMessenger(dartMessenger); // The JNI might already be attached if coming from a spawned engine. If so, correctly report // that this DartExecutor is already running. @@ -200,10 +200,16 @@ public void send( @Override @UiThread public void setMessageHandler( - @NonNull String channel, - @Nullable BinaryMessenger.BinaryMessageHandler handler, - @Nullable TaskQueue taskQueue) { - binaryMessenger.setMessageHandler(channel, handler, taskQueue); + @NonNull String channel, @Nullable BinaryMessenger.BinaryMessageHandler handler) { + binaryMessenger.setMessageHandler(channel, handler); + } + + /** @deprecated Use {@link #getBinaryMessenger()} instead. */ + @Deprecated + @Override + @UiThread + public void setTaskQueue(@NonNull String channel, @Nullable TaskQueue taskQueue) { + binaryMessenger.setTaskQueue(channel, taskQueue); } // ------ END BinaryMessenger ----- @@ -427,10 +433,14 @@ public void send( @Override @UiThread public void setMessageHandler( - @NonNull String channel, - @Nullable BinaryMessenger.BinaryMessageHandler handler, - @Nullable TaskQueue taskQueue) { - messenger.setMessageHandler(channel, handler, taskQueue); + @NonNull String channel, @Nullable BinaryMessenger.BinaryMessageHandler handler) { + messenger.setMessageHandler(channel, handler); + } + + @Override + @UiThread + public void setTaskQueue(@NonNull String channel, @Nullable TaskQueue taskQueue) { + messenger.setTaskQueue(channel, 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 7e6b9706f22ef..5744ca37d04b5 100644 --- a/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java +++ b/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java @@ -34,6 +34,8 @@ class DartMessenger implements BinaryMessenger, PlatformMessageHandler { @NonNull private final ConcurrentHashMap messageHandlers; + @NonNull private final HashMap taskQueues; + @NonNull private final Map pendingReplies; private int nextReplyId = 1; @@ -49,6 +51,7 @@ class DartMessenger implements BinaryMessenger, PlatformMessageHandler { this.pendingReplies = new HashMap<>(); this.createdTaskQueues = new WeakHashMap(); this.taskQueueFactory = taskQueueFactory; + this.taskQueues = new HashMap<>(); } DartMessenger(@NonNull FlutterJNI flutterJNI) { @@ -112,26 +115,37 @@ public TaskQueue makeBackgroundTaskQueue() { @Override public void setMessageHandler( - @NonNull String channel, - @Nullable BinaryMessenger.BinaryMessageHandler handler, - @Nullable TaskQueue taskQueue) { + @NonNull String channel, @Nullable BinaryMessenger.BinaryMessageHandler handler) { 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)."); - } - } + DartMessengerTaskQueue dartMessengerTaskQueue = taskQueues.get(channel); Log.v(TAG, "Setting handler for channel '" + channel + "'"); messageHandlers.put(channel, new HandlerInfo(handler, dartMessengerTaskQueue)); } } + @Override + public void setTaskQueue(@NonNull String channel, @Nullable BinaryMessenger.TaskQueue taskQueue) { + DartMessengerTaskQueue dartMessengerTaskQueue = null; + if (taskQueue == null) { + taskQueues.remove(channel); + } else { + dartMessengerTaskQueue = createdTaskQueues.get(taskQueue); + if (dartMessengerTaskQueue == null) { + throw new IllegalArgumentException( + "Unrecognized TaskQueue, use BinaryMessenger to create your TaskQueue (ex makeBackgroundTaskQueue)."); + } + taskQueues.put(channel, dartMessengerTaskQueue); + } + HandlerInfo existingHandlerInfo = messageHandlers.get(channel); + if (existingHandlerInfo != null) { + messageHandlers.put( + channel, new HandlerInfo(existingHandlerInfo.handler, dartMessengerTaskQueue)); + } + } + @Override @UiThread public void send(@NonNull String channel, @NonNull ByteBuffer message) { diff --git a/shell/platform/android/io/flutter/plugin/common/BasicMessageChannel.java b/shell/platform/android/io/flutter/plugin/common/BasicMessageChannel.java index fa9ffe2a90855..de790f117a1cb 100644 --- a/shell/platform/android/io/flutter/plugin/common/BasicMessageChannel.java +++ b/shell/platform/android/io/flutter/plugin/common/BasicMessageChannel.java @@ -122,8 +122,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), taskQueue); + messenger.setMessageHandler(name, handler == null ? null : new IncomingMessageHandler(handler)); + messenger.setTaskQueue(name, handler == null ? null : taskQueue); } /** diff --git a/shell/platform/android/io/flutter/plugin/common/BinaryMessenger.java b/shell/platform/android/io/flutter/plugin/common/BinaryMessenger.java index 433a6a9bbeaa9..6cf4f6c6b17e8 100644 --- a/shell/platform/android/io/flutter/plugin/common/BinaryMessenger.java +++ b/shell/platform/android/io/flutter/plugin/common/BinaryMessenger.java @@ -80,14 +80,24 @@ 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. + */ + @UiThread + void setMessageHandler(@NonNull String channel, @Nullable BinaryMessageHandler handler); + + /** + * Registers a TaskQueue for a specified channel which controls the threading model to follow when + * invoking the message handler. + * + *

A null value for taskQueue means to execute the handler on the platform thread. + * + *

See also: {@link BinaryMessenger#makeBackgroundTaskQueue()} + * + * @param channel the name {@link String} of the channel. * @param taskQueue a {@link BinaryMessenger.TaskQueue} that specifies what thread will execute * the handler. Specifying null means execute on the platform thread. */ @UiThread - void setMessageHandler( - @NonNull String channel, - @Nullable BinaryMessageHandler handler, - @Nullable TaskQueue taskQueue); + void setTaskQueue(@NonNull String channel, @Nullable TaskQueue taskQueue); /** Handler for incoming binary messages from Flutter. */ interface BinaryMessageHandler { diff --git a/shell/platform/android/io/flutter/plugin/common/EventChannel.java b/shell/platform/android/io/flutter/plugin/common/EventChannel.java index 70bd7d56951c7..3c21f7ef4b9ff 100644 --- a/shell/platform/android/io/flutter/plugin/common/EventChannel.java +++ b/shell/platform/android/io/flutter/plugin/common/EventChannel.java @@ -106,7 +106,8 @@ public EventChannel( @UiThread public void setStreamHandler(final StreamHandler handler) { messenger.setMessageHandler( - name, handler == null ? null : new IncomingStreamRequestHandler(handler), taskQueue); + name, handler == null ? null : new IncomingStreamRequestHandler(handler)); + messenger.setTaskQueue(name, handler == null ? null : taskQueue); } /** diff --git a/shell/platform/android/io/flutter/plugin/common/MethodChannel.java b/shell/platform/android/io/flutter/plugin/common/MethodChannel.java index 66c92cf6a2497..32ef488eb3879 100644 --- a/shell/platform/android/io/flutter/plugin/common/MethodChannel.java +++ b/shell/platform/android/io/flutter/plugin/common/MethodChannel.java @@ -138,7 +138,8 @@ 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), taskQueue); + name, handler == null ? null : new IncomingMethodCallHandler(handler)); + messenger.setTaskQueue(name, handler == null ? null : taskQueue); } /** diff --git a/shell/platform/android/io/flutter/view/FlutterNativeView.java b/shell/platform/android/io/flutter/view/FlutterNativeView.java index 6e6ca763a0111..cc5286749d849 100644 --- a/shell/platform/android/io/flutter/view/FlutterNativeView.java +++ b/shell/platform/android/io/flutter/view/FlutterNativeView.java @@ -149,8 +149,14 @@ public void send(String channel, ByteBuffer message, BinaryReply callback) { @Override @UiThread - public void setMessageHandler(String channel, BinaryMessageHandler handler, TaskQueue taskQueue) { - dartExecutor.getBinaryMessenger().setMessageHandler(channel, handler, taskQueue); + public void setMessageHandler(String channel, BinaryMessageHandler handler) { + dartExecutor.getBinaryMessenger().setMessageHandler(channel, handler); + } + + @Override + @UiThread + public void setTaskQueue(String channel, TaskQueue taskQueue) { + dartExecutor.getBinaryMessenger().setTaskQueue(channel, 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 f63ce7add3f9f..9cdd53d0969d1 100644 --- a/shell/platform/android/io/flutter/view/FlutterView.java +++ b/shell/platform/android/io/flutter/view/FlutterView.java @@ -846,8 +846,14 @@ public void send(String channel, ByteBuffer message, BinaryReply callback) { @Override @UiThread - public void setMessageHandler(String channel, BinaryMessageHandler handler, TaskQueue taskQueue) { - mNativeView.setMessageHandler(channel, handler, taskQueue); + public void setMessageHandler(String channel, BinaryMessageHandler handler) { + mNativeView.setMessageHandler(channel, handler); + } + + @Override + @UiThread + public void setTaskQueue(String channel, TaskQueue taskQueue) { + mNativeView.setTaskQueue(channel, taskQueue); } /** Listener will be called on the Android UI thread once when Flutter renders the first frame. */ diff --git a/shell/platform/android/test/io/flutter/embedding/engine/dart/DartMessengerTest.java b/shell/platform/android/test/io/flutter/embedding/engine/dart/DartMessengerTest.java index 4e7a6fdb7c2e1..67653970f4b14 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 @@ -14,6 +14,7 @@ import io.flutter.plugin.common.BinaryMessenger; import io.flutter.plugin.common.BinaryMessenger.BinaryMessageHandler; import java.nio.ByteBuffer; +import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mockito; @@ -23,7 +24,7 @@ @Config(manifest = Config.NONE) @RunWith(RobolectricTestRunner.class) public class DartMessengerTest { - SynchronousTaskQueue synchronousTaskQueue = new SynchronousTaskQueue(); + SynchronousTaskQueue synchronousTaskQueue = null; private static class ReportingUncaughtExceptionHandler implements Thread.UncaughtExceptionHandler { @@ -36,9 +37,21 @@ public void uncaughtException(Thread t, Throwable e) { } private static class SynchronousTaskQueue implements DartMessengerTaskQueue { + private boolean didRun = false; + public void dispatch(Runnable runnable) { + didRun = true; runnable.run(); } + + public boolean getDidRun() { + return didRun; + } + } + + @Before + public void setUp() { + synchronousTaskQueue = new SynchronousTaskQueue(); } @Test @@ -59,7 +72,8 @@ public void itHandlesErrors() { .when(throwingHandler) .onMessage(any(ByteBuffer.class), any(DartMessenger.Reply.class)); BinaryMessenger.TaskQueue taskQueue = messenger.makeBackgroundTaskQueue(); - messenger.setMessageHandler("test", throwingHandler, taskQueue); + messenger.setMessageHandler("test", throwingHandler); + messenger.setTaskQueue("test", taskQueue); messenger.handleMessageFromDart("test", ByteBuffer.allocate(0), 0, 0); assertNotNull(reportingHandler.latestException); assertTrue(reportingHandler.latestException instanceof AssertionError); @@ -78,7 +92,8 @@ public void givesDirectByteBuffer() { wasDirect[0] = message.isDirect(); }; BinaryMessenger.TaskQueue taskQueue = messenger.makeBackgroundTaskQueue(); - messenger.setMessageHandler(channel, handler, taskQueue); + messenger.setMessageHandler(channel, handler); + messenger.setTaskQueue(channel, taskQueue); final ByteBuffer message = ByteBuffer.allocateDirect(4 * 2); message.rewind(); message.putChar('a'); @@ -87,6 +102,31 @@ public void givesDirectByteBuffer() { message.putChar('d'); messenger.handleMessageFromDart(channel, message, /*replyId=*/ 123, 0); assertTrue(wasDirect[0]); + assertTrue(synchronousTaskQueue.didRun); + } + + @Test + public void setTaskQueueFirst() { + // The same test as givesDirectByteBuffer, but calls setTaskQueue before setMessageHandler. + final FlutterJNI fakeFlutterJni = mock(FlutterJNI.class); + 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(); + }; + BinaryMessenger.TaskQueue taskQueue = messenger.makeBackgroundTaskQueue(); + messenger.setTaskQueue(channel, taskQueue); + messenger.setMessageHandler(channel, handler); + final ByteBuffer message = ByteBuffer.allocateDirect(4 * 2); + message.rewind(); + message.putChar('a'); + message.putChar('b'); + message.putChar('c'); + message.putChar('d'); + messenger.handleMessageFromDart(channel, message, /*replyId=*/ 123, 0); + assertTrue(synchronousTaskQueue.didRun); } @Test @@ -103,7 +143,8 @@ public void directByteBufferLimitZeroAfterUsage() { assertEquals(bufferSize, byteBuffers[0].limit()); }; BinaryMessenger.TaskQueue taskQueue = messenger.makeBackgroundTaskQueue(); - messenger.setMessageHandler(channel, handler, taskQueue); + messenger.setMessageHandler(channel, handler); + messenger.setTaskQueue(channel, taskQueue); final ByteBuffer message = ByteBuffer.allocateDirect(bufferSize); message.rewind(); message.putChar('a'); @@ -161,7 +202,8 @@ public void cleansUpMessageData() throws InterruptedException { (ByteBuffer message, BinaryMessenger.BinaryReply reply) -> { reply.reply(null); }; - messenger.setMessageHandler(channel, handler, taskQueue); + messenger.setMessageHandler(channel, handler); + messenger.setTaskQueue(channel, taskQueue); final ByteBuffer message = ByteBuffer.allocateDirect(4 * 2); int replyId = 1; long messageData = 1234; @@ -179,7 +221,8 @@ public void cleansUpMessageDataOnError() throws InterruptedException { (ByteBuffer message, BinaryMessenger.BinaryReply reply) -> { throw new RuntimeException("hello"); }; - messenger.setMessageHandler(channel, handler, taskQueue); + messenger.setMessageHandler(channel, handler); + messenger.setTaskQueue(channel, taskQueue); final ByteBuffer message = ByteBuffer.allocateDirect(4 * 2); int replyId = 1; long messageData = 1234; 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 c937721fe60e5..0c858342a35d7 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(), eq(null)); + .setMessageHandler(any(String.class), binaryMessageHandlerCaptor.capture()); 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(), eq(null)); + .setMessageHandler(any(String.class), binaryMessageHandlerCaptor.capture()); 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(), eq(null)); + .setMessageHandler(any(String.class), binaryMessageHandlerCaptor.capture()); JSONObject arguments = new JSONObject(); arguments.put("action", "actionCommand"); 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 57d62e93587c9..73001786ba308 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,10 +31,7 @@ public void onMessage(ByteBuffer message, final BinaryMessenger.BinaryReply call @Override public void configureFlutterEngine(@NonNull FlutterEngine flutterEngine) { - flutterEngine - .getDartExecutor() - .getBinaryMessenger() - .setMessageHandler("finish", finishHandler, null); + flutterEngine.getDartExecutor().getBinaryMessenger().setMessageHandler("finish", finishHandler); final boolean moved = moveTaskToBack(true); if (!moved) { 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 6599ed5437977..fbce67dede353 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,8 +22,7 @@ public FlutterEngine provideFlutterEngine(@NonNull Context context) { secondEngine .getDartExecutor() - .setMessageHandler( - "take_screenshot", (byteBuffer, binaryReply) -> notifyFlutterRendered(), null); + .setMessageHandler("take_screenshot", (byteBuffer, binaryReply) -> notifyFlutterRendered()); return secondEngine; } diff --git a/testing/scenario_app/android/app/src/main/java/dev/flutter/scenarios/TestableFlutterActivity.java b/testing/scenario_app/android/app/src/main/java/dev/flutter/scenarios/TestableFlutterActivity.java index 75a5240c70282..82ba2d95c5abd 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,8 +19,7 @@ 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(), null); + .setMessageHandler("take_screenshot", (byteBuffer, binaryReply) -> notifyFlutterRendered()); } protected void notifyFlutterRendered() { From 4622a04eef4ed2a7ea27f263b0e4a4f2be4d6551 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Tue, 26 Oct 2021 17:01:29 -0700 Subject: [PATCH 3/4] switched to overloading setMessageHandler --- .../embedding/engine/dart/DartExecutor.java | 14 +++-- .../embedding/engine/dart/DartMessenger.java | 40 ++++++-------- .../plugin/common/BasicMessageChannel.java | 9 ++- .../plugin/common/BinaryMessenger.java | 28 ++++++++-- .../flutter/plugin/common/EventChannel.java | 10 +++- .../flutter/plugin/common/MethodChannel.java | 10 +++- .../io/flutter/view/FlutterNativeView.java | 4 +- .../android/io/flutter/view/FlutterView.java | 4 +- .../engine/dart/DartMessengerTest.java | 55 ++----------------- 9 files changed, 79 insertions(+), 95 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 80f853c577b95..5580d7a5cd258 100644 --- a/shell/platform/android/io/flutter/embedding/engine/dart/DartExecutor.java +++ b/shell/platform/android/io/flutter/embedding/engine/dart/DartExecutor.java @@ -208,8 +208,11 @@ public void setMessageHandler( @Deprecated @Override @UiThread - public void setTaskQueue(@NonNull String channel, @Nullable TaskQueue taskQueue) { - binaryMessenger.setTaskQueue(channel, taskQueue); + public void setMessageHandler( + @NonNull String channel, + @Nullable BinaryMessenger.BinaryMessageHandler handler, + @Nullable TaskQueue taskQueue) { + binaryMessenger.setMessageHandler(channel, handler, taskQueue); } // ------ END BinaryMessenger ----- @@ -439,8 +442,11 @@ public void setMessageHandler( @Override @UiThread - public void setTaskQueue(@NonNull String channel, @Nullable TaskQueue taskQueue) { - messenger.setTaskQueue(channel, taskQueue); + public void setMessageHandler( + @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 5744ca37d04b5..c5faf03b5dd40 100644 --- a/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java +++ b/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java @@ -34,8 +34,6 @@ class DartMessenger implements BinaryMessenger, PlatformMessageHandler { @NonNull private final ConcurrentHashMap messageHandlers; - @NonNull private final HashMap taskQueues; - @NonNull private final Map pendingReplies; private int nextReplyId = 1; @@ -51,7 +49,6 @@ class DartMessenger implements BinaryMessenger, PlatformMessageHandler { this.pendingReplies = new HashMap<>(); this.createdTaskQueues = new WeakHashMap(); this.taskQueueFactory = taskQueueFactory; - this.taskQueues = new HashMap<>(); } DartMessenger(@NonNull FlutterJNI flutterJNI) { @@ -116,36 +113,31 @@ public TaskQueue makeBackgroundTaskQueue() { @Override public void setMessageHandler( @NonNull String channel, @Nullable BinaryMessenger.BinaryMessageHandler handler) { + setMessageHandler(channel, handler, null); + } + + @Override + public void setMessageHandler( + @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 { - DartMessengerTaskQueue dartMessengerTaskQueue = taskQueues.get(channel); + 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)); } } - @Override - public void setTaskQueue(@NonNull String channel, @Nullable BinaryMessenger.TaskQueue taskQueue) { - DartMessengerTaskQueue dartMessengerTaskQueue = null; - if (taskQueue == null) { - taskQueues.remove(channel); - } else { - dartMessengerTaskQueue = createdTaskQueues.get(taskQueue); - if (dartMessengerTaskQueue == null) { - throw new IllegalArgumentException( - "Unrecognized TaskQueue, use BinaryMessenger to create your TaskQueue (ex makeBackgroundTaskQueue)."); - } - taskQueues.put(channel, dartMessengerTaskQueue); - } - HandlerInfo existingHandlerInfo = messageHandlers.get(channel); - if (existingHandlerInfo != null) { - messageHandlers.put( - channel, new HandlerInfo(existingHandlerInfo.handler, dartMessengerTaskQueue)); - } - } - @Override @UiThread public void send(@NonNull String channel, @NonNull ByteBuffer message) { diff --git a/shell/platform/android/io/flutter/plugin/common/BasicMessageChannel.java b/shell/platform/android/io/flutter/plugin/common/BasicMessageChannel.java index de790f117a1cb..0427307d087ff 100644 --- a/shell/platform/android/io/flutter/plugin/common/BasicMessageChannel.java +++ b/shell/platform/android/io/flutter/plugin/common/BasicMessageChannel.java @@ -122,8 +122,13 @@ 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.setTaskQueue(name, handler == null ? null : taskQueue); + if (taskQueue != null) { + messenger.setMessageHandler( + name, handler == null ? null : new IncomingMessageHandler(handler), taskQueue); + } else { + messenger.setMessageHandler( + name, handler == null ? null : new IncomingMessageHandler(handler)); + } } /** diff --git a/shell/platform/android/io/flutter/plugin/common/BinaryMessenger.java b/shell/platform/android/io/flutter/plugin/common/BinaryMessenger.java index 6cf4f6c6b17e8..b2b32fe975bd9 100644 --- a/shell/platform/android/io/flutter/plugin/common/BinaryMessenger.java +++ b/shell/platform/android/io/flutter/plugin/common/BinaryMessenger.java @@ -42,7 +42,10 @@ public interface TaskQueue {} * serial. */ @UiThread - TaskQueue makeBackgroundTaskQueue(); + default TaskQueue makeBackgroundTaskQueue() { + // TODO(aaclarke): Remove default implementation when it is safe for Google Flutter users. + throw new UnsupportedOperationException("makeBackgroundTaskQueue not implemented."); + } /** * Sends a binary message to the Flutter application. @@ -85,19 +88,32 @@ public interface TaskQueue {} void setMessageHandler(@NonNull String channel, @Nullable BinaryMessageHandler handler); /** - * Registers a TaskQueue for a specified channel which controls the threading model to follow when - * invoking the message handler. + * Registers a handler to be invoked when the Flutter application sends a message to its host + * platform. * - *

A null value for taskQueue means to execute the handler on the platform thread. + *

Registration overwrites any previous registration for the same channel name. Use a null + * handler to deregister. * - *

See also: {@link BinaryMessenger#makeBackgroundTaskQueue()} + *

If no handler has been registered for a particular channel, any incoming message on that + * channel will be handled silently by sending a null reply. * * @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 means execute on the platform thread. */ @UiThread - void setTaskQueue(@NonNull String channel, @Nullable TaskQueue taskQueue); + default void setMessageHandler( + @NonNull String channel, + @Nullable BinaryMessageHandler handler, + @Nullable TaskQueue taskQueue) { + // TODO(aaclarke): Remove default implementation when it is safe for Google Flutter users. + if (taskQueue != null) { + throw new UnsupportedOperationException( + "setMessageHandler called with nonnull taskQueue is not supported."); + } + setMessageHandler(channel, handler); + } /** Handler for incoming binary messages from Flutter. */ interface BinaryMessageHandler { diff --git a/shell/platform/android/io/flutter/plugin/common/EventChannel.java b/shell/platform/android/io/flutter/plugin/common/EventChannel.java index 3c21f7ef4b9ff..2138d037f5697 100644 --- a/shell/platform/android/io/flutter/plugin/common/EventChannel.java +++ b/shell/platform/android/io/flutter/plugin/common/EventChannel.java @@ -105,9 +105,13 @@ public EventChannel( */ @UiThread public void setStreamHandler(final StreamHandler handler) { - messenger.setMessageHandler( - name, handler == null ? null : new IncomingStreamRequestHandler(handler)); - messenger.setTaskQueue(name, handler == null ? null : taskQueue); + if (taskQueue != null) { + messenger.setMessageHandler( + name, handler == null ? null : new IncomingStreamRequestHandler(handler), taskQueue); + } else { + messenger.setMessageHandler( + name, handler == null ? null : new IncomingStreamRequestHandler(handler)); + } } /** diff --git a/shell/platform/android/io/flutter/plugin/common/MethodChannel.java b/shell/platform/android/io/flutter/plugin/common/MethodChannel.java index 32ef488eb3879..615a4fbd069e3 100644 --- a/shell/platform/android/io/flutter/plugin/common/MethodChannel.java +++ b/shell/platform/android/io/flutter/plugin/common/MethodChannel.java @@ -137,9 +137,13 @@ 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)); - messenger.setTaskQueue(name, handler == null ? null : taskQueue); + if (taskQueue != null) { + messenger.setMessageHandler( + name, handler == null ? null : new IncomingMethodCallHandler(handler), taskQueue); + } else { + messenger.setMessageHandler( + name, handler == null ? null : new IncomingMethodCallHandler(handler)); + } } /** diff --git a/shell/platform/android/io/flutter/view/FlutterNativeView.java b/shell/platform/android/io/flutter/view/FlutterNativeView.java index cc5286749d849..c44a23c9982bf 100644 --- a/shell/platform/android/io/flutter/view/FlutterNativeView.java +++ b/shell/platform/android/io/flutter/view/FlutterNativeView.java @@ -155,8 +155,8 @@ public void setMessageHandler(String channel, BinaryMessageHandler handler) { @Override @UiThread - public void setTaskQueue(String channel, TaskQueue taskQueue) { - dartExecutor.getBinaryMessenger().setTaskQueue(channel, taskQueue); + 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 9cdd53d0969d1..317ec6562a1ca 100644 --- a/shell/platform/android/io/flutter/view/FlutterView.java +++ b/shell/platform/android/io/flutter/view/FlutterView.java @@ -852,8 +852,8 @@ public void setMessageHandler(String channel, BinaryMessageHandler handler) { @Override @UiThread - public void setTaskQueue(String channel, TaskQueue taskQueue) { - mNativeView.setTaskQueue(channel, taskQueue); + 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/test/io/flutter/embedding/engine/dart/DartMessengerTest.java b/shell/platform/android/test/io/flutter/embedding/engine/dart/DartMessengerTest.java index 67653970f4b14..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 @@ -14,7 +14,6 @@ import io.flutter.plugin.common.BinaryMessenger; import io.flutter.plugin.common.BinaryMessenger.BinaryMessageHandler; import java.nio.ByteBuffer; -import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mockito; @@ -24,7 +23,7 @@ @Config(manifest = Config.NONE) @RunWith(RobolectricTestRunner.class) public class DartMessengerTest { - SynchronousTaskQueue synchronousTaskQueue = null; + SynchronousTaskQueue synchronousTaskQueue = new SynchronousTaskQueue(); private static class ReportingUncaughtExceptionHandler implements Thread.UncaughtExceptionHandler { @@ -37,21 +36,9 @@ public void uncaughtException(Thread t, Throwable e) { } private static class SynchronousTaskQueue implements DartMessengerTaskQueue { - private boolean didRun = false; - public void dispatch(Runnable runnable) { - didRun = true; runnable.run(); } - - public boolean getDidRun() { - return didRun; - } - } - - @Before - public void setUp() { - synchronousTaskQueue = new SynchronousTaskQueue(); } @Test @@ -72,8 +59,7 @@ public void itHandlesErrors() { .when(throwingHandler) .onMessage(any(ByteBuffer.class), any(DartMessenger.Reply.class)); BinaryMessenger.TaskQueue taskQueue = messenger.makeBackgroundTaskQueue(); - messenger.setMessageHandler("test", throwingHandler); - messenger.setTaskQueue("test", taskQueue); + messenger.setMessageHandler("test", throwingHandler, taskQueue); messenger.handleMessageFromDart("test", ByteBuffer.allocate(0), 0, 0); assertNotNull(reportingHandler.latestException); assertTrue(reportingHandler.latestException instanceof AssertionError); @@ -92,8 +78,7 @@ public void givesDirectByteBuffer() { wasDirect[0] = message.isDirect(); }; BinaryMessenger.TaskQueue taskQueue = messenger.makeBackgroundTaskQueue(); - messenger.setMessageHandler(channel, handler); - messenger.setTaskQueue(channel, taskQueue); + messenger.setMessageHandler(channel, handler, taskQueue); final ByteBuffer message = ByteBuffer.allocateDirect(4 * 2); message.rewind(); message.putChar('a'); @@ -102,31 +87,6 @@ public void givesDirectByteBuffer() { message.putChar('d'); messenger.handleMessageFromDart(channel, message, /*replyId=*/ 123, 0); assertTrue(wasDirect[0]); - assertTrue(synchronousTaskQueue.didRun); - } - - @Test - public void setTaskQueueFirst() { - // The same test as givesDirectByteBuffer, but calls setTaskQueue before setMessageHandler. - final FlutterJNI fakeFlutterJni = mock(FlutterJNI.class); - 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(); - }; - BinaryMessenger.TaskQueue taskQueue = messenger.makeBackgroundTaskQueue(); - messenger.setTaskQueue(channel, taskQueue); - messenger.setMessageHandler(channel, handler); - final ByteBuffer message = ByteBuffer.allocateDirect(4 * 2); - message.rewind(); - message.putChar('a'); - message.putChar('b'); - message.putChar('c'); - message.putChar('d'); - messenger.handleMessageFromDart(channel, message, /*replyId=*/ 123, 0); - assertTrue(synchronousTaskQueue.didRun); } @Test @@ -143,8 +103,7 @@ public void directByteBufferLimitZeroAfterUsage() { assertEquals(bufferSize, byteBuffers[0].limit()); }; BinaryMessenger.TaskQueue taskQueue = messenger.makeBackgroundTaskQueue(); - messenger.setMessageHandler(channel, handler); - messenger.setTaskQueue(channel, taskQueue); + messenger.setMessageHandler(channel, handler, taskQueue); final ByteBuffer message = ByteBuffer.allocateDirect(bufferSize); message.rewind(); message.putChar('a'); @@ -202,8 +161,7 @@ public void cleansUpMessageData() throws InterruptedException { (ByteBuffer message, BinaryMessenger.BinaryReply reply) -> { reply.reply(null); }; - messenger.setMessageHandler(channel, handler); - messenger.setTaskQueue(channel, taskQueue); + messenger.setMessageHandler(channel, handler, taskQueue); final ByteBuffer message = ByteBuffer.allocateDirect(4 * 2); int replyId = 1; long messageData = 1234; @@ -221,8 +179,7 @@ public void cleansUpMessageDataOnError() throws InterruptedException { (ByteBuffer message, BinaryMessenger.BinaryReply reply) -> { throw new RuntimeException("hello"); }; - messenger.setMessageHandler(channel, handler); - messenger.setTaskQueue(channel, taskQueue); + messenger.setMessageHandler(channel, handler, taskQueue); final ByteBuffer message = ByteBuffer.allocateDirect(4 * 2); int replyId = 1; long messageData = 1234; From d59fb6bc28ad9c55a0cb04ad2111a6965634ccd3 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Wed, 27 Oct 2021 09:37:25 -0700 Subject: [PATCH 4/4] updated comments --- .../android/io/flutter/plugin/common/BasicMessageChannel.java | 3 +++ .../android/io/flutter/plugin/common/BinaryMessenger.java | 4 ++-- .../android/io/flutter/plugin/common/EventChannel.java | 3 +++ .../android/io/flutter/plugin/common/MethodChannel.java | 3 +++ 4 files changed, 11 insertions(+), 2 deletions(-) diff --git a/shell/platform/android/io/flutter/plugin/common/BasicMessageChannel.java b/shell/platform/android/io/flutter/plugin/common/BasicMessageChannel.java index 0427307d087ff..b1f06cb80c92f 100644 --- a/shell/platform/android/io/flutter/plugin/common/BasicMessageChannel.java +++ b/shell/platform/android/io/flutter/plugin/common/BasicMessageChannel.java @@ -122,6 +122,9 @@ public void send(@Nullable T message, @Nullable final Reply callback) { */ @UiThread public void setMessageHandler(@Nullable final MessageHandler handler) { + // We call the 2 parameter variant specifically to avoid breaking changes in + // mock verify calls. + // See https://github.com/flutter/flutter/issues/92582. if (taskQueue != null) { messenger.setMessageHandler( 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 b2b32fe975bd9..968075f880b31 100644 --- a/shell/platform/android/io/flutter/plugin/common/BinaryMessenger.java +++ b/shell/platform/android/io/flutter/plugin/common/BinaryMessenger.java @@ -43,7 +43,7 @@ public interface TaskQueue {} */ @UiThread default TaskQueue makeBackgroundTaskQueue() { - // TODO(aaclarke): Remove default implementation when it is safe for Google Flutter users. + // TODO(92582): Remove default implementation when it is safe for Google Flutter users. throw new UnsupportedOperationException("makeBackgroundTaskQueue not implemented."); } @@ -107,7 +107,7 @@ default void setMessageHandler( @NonNull String channel, @Nullable BinaryMessageHandler handler, @Nullable TaskQueue taskQueue) { - // TODO(aaclarke): Remove default implementation when it is safe for Google Flutter users. + // TODO(92582): Remove default implementation when it is safe for Google Flutter users. if (taskQueue != null) { throw new UnsupportedOperationException( "setMessageHandler called with nonnull taskQueue is not supported."); diff --git a/shell/platform/android/io/flutter/plugin/common/EventChannel.java b/shell/platform/android/io/flutter/plugin/common/EventChannel.java index 2138d037f5697..f8835f90904c4 100644 --- a/shell/platform/android/io/flutter/plugin/common/EventChannel.java +++ b/shell/platform/android/io/flutter/plugin/common/EventChannel.java @@ -105,6 +105,9 @@ public EventChannel( */ @UiThread public void setStreamHandler(final StreamHandler handler) { + // We call the 2 parameter variant specifically to avoid breaking changes in + // mock verify calls. + // See https://github.com/flutter/flutter/issues/92582. if (taskQueue != null) { messenger.setMessageHandler( 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 615a4fbd069e3..3c0149754fccb 100644 --- a/shell/platform/android/io/flutter/plugin/common/MethodChannel.java +++ b/shell/platform/android/io/flutter/plugin/common/MethodChannel.java @@ -137,6 +137,9 @@ public void invokeMethod(String method, @Nullable Object arguments, @Nullable Re */ @UiThread public void setMethodCallHandler(final @Nullable MethodCallHandler handler) { + // We call the 2 parameter variant specifically to avoid breaking changes in + // mock verify calls. + // See https://github.com/flutter/flutter/issues/92582. if (taskQueue != null) { messenger.setMessageHandler( name, handler == null ? null : new IncomingMethodCallHandler(handler), taskQueue);