From 5183c565331e3e43144eb42b15a25c8261702dc2 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Thu, 18 Nov 2021 17:53:28 -0800 Subject: [PATCH] [Embedder] Send key data through message channel (flutter/engine#29795) This PR changes how embedder API's SendKeyData sends ui.KeyData to the framework. The packets are now sent over the existing platform messenger, reusing the entirety of its code path and functionalities while keeping the embedder API unchanged --- engine/src/flutter/lib/ui/hooks.dart | 5 - .../flutter/lib/ui/platform_dispatcher.dart | 43 +++++---- .../lib/ui/window/platform_configuration.cc | 38 -------- .../lib/ui/window/platform_configuration.h | 40 -------- engine/src/flutter/lib/ui/window/window.cc | 19 ---- engine/src/flutter/lib/ui/window/window.h | 8 -- .../src/flutter/runtime/runtime_controller.cc | 14 --- .../src/flutter/runtime/runtime_controller.h | 14 --- engine/src/flutter/shell/common/engine.cc | 8 -- engine/src/flutter/shell/common/engine.h | 15 --- .../src/flutter/shell/common/platform_view.cc | 6 -- .../src/flutter/shell/common/platform_view.h | 25 ----- engine/src/flutter/shell/common/shell.cc | 17 ---- engine/src/flutter/shell/common/shell.h | 5 - .../flutter/shell/common/shell_unittests.cc | 4 - .../ios/framework/Source/FlutterEngine.mm | 16 +++- .../Source/FlutterEnginePlatformViewTest.mm | 2 - .../Source/FlutterPlatformViewsTest.mm | 2 - .../Source/accessibility_bridge_test.mm | 2 - .../platform/darwin/ios/platform_view_ios.h | 3 - .../shell/platform/embedder/embedder.cc | 88 +++++++++++++++-- .../platform/embedder/embedder_engine.cc | 16 ---- .../shell/platform/embedder/embedder_engine.h | 16 ---- .../platform/embedder/fixtures/main.dart | 21 ++++ .../embedder/tests/embedder_unittests.cc | 95 ++++++++++++++++++- .../fuchsia/flutter/platform_view_unittest.cc | 4 - 26 files changed, 228 insertions(+), 298 deletions(-) diff --git a/engine/src/flutter/lib/ui/hooks.dart b/engine/src/flutter/lib/ui/hooks.dart index 8ef8b66e66..6a03d9ec4c 100644 --- a/engine/src/flutter/lib/ui/hooks.dart +++ b/engine/src/flutter/lib/ui/hooks.dart @@ -88,11 +88,6 @@ void _dispatchPointerDataPacket(ByteData packet) { PlatformDispatcher.instance._dispatchPointerDataPacket(packet); } -@pragma('vm:entry-point') -void _dispatchKeyData(ByteData packet, int responseId) { - PlatformDispatcher.instance._dispatchKeyData(packet, responseId); -} - @pragma('vm:entry-point') void _dispatchSemanticsAction(int id, int action, ByteData? args) { PlatformDispatcher.instance._dispatchSemanticsAction(id, action, args); diff --git a/engine/src/flutter/lib/ui/platform_dispatcher.dart b/engine/src/flutter/lib/ui/platform_dispatcher.dart index cc0bdcdfb1..7ddc906bae 100644 --- a/engine/src/flutter/lib/ui/platform_dispatcher.dart +++ b/engine/src/flutter/lib/ui/platform_dispatcher.dart @@ -28,9 +28,6 @@ typedef TimingsCallback = void Function(List timings); /// Signature for [PlatformDispatcher.onPointerDataPacket]. typedef PointerDataPacketCallback = void Function(PointerDataPacket packet); -// Signature for the response to KeyDataCallback. -typedef _KeyDataResponseCallback = void Function(int responseId, bool handled); - /// Signature for [PlatformDispatcher.onKeyData]. /// /// The callback should return true if the key event has been handled by the @@ -59,6 +56,11 @@ typedef PlatformConfigurationChangedCallback = void Function(PlatformConfigurati // A gesture setting value that indicates it has not been set by the engine. const double _kUnsetGestureSetting = -1.0; +// A message channel to receive KeyData from the platform. +// +// See embedder.cc::kFlutterKeyDataChannel for more information. +const String _kFlutterKeyDataChannel = 'flutter/keydata'; + /// Platform event dispatcher singleton. /// /// The most basic interface to the host operating system's interface. @@ -349,9 +351,19 @@ class PlatformDispatcher { return PointerDataPacket(data: data); } - /// Called by [_dispatchKeyData]. - void _respondToKeyData(int responseId, bool handled) - native 'PlatformConfiguration_respondToKeyData'; + static ChannelCallback _keyDataListener(KeyDataCallback onKeyData, Zone zone) => + (ByteData? packet, PlatformMessageResponseCallback callback) { + _invoke1( + (KeyData keyData) { + final bool handled = onKeyData(keyData); + final Uint8List response = Uint8List(1); + response[0] = handled ? 1 : 0; + callback(response.buffer.asByteData()); + }, + zone, + _unpackKeyData(packet!), + ); + }; /// A callback that is invoked when key data is available. /// @@ -362,22 +374,13 @@ class PlatformDispatcher { /// framework and should not be propagated further. KeyDataCallback? get onKeyData => _onKeyData; KeyDataCallback? _onKeyData; - Zone _onKeyDataZone = Zone.root; set onKeyData(KeyDataCallback? callback) { _onKeyData = callback; - _onKeyDataZone = Zone.current; - } - - // Called from the engine, via hooks.dart - void _dispatchKeyData(ByteData packet, int responseId) { - _invoke2( - (KeyData data, _KeyDataResponseCallback callback) { - callback(responseId, onKeyData != null && onKeyData!(data)); - }, - _onKeyDataZone, - _unpackKeyData(packet), - _respondToKeyData, - ); + if (callback != null) { + channelBuffers.setListener(_kFlutterKeyDataChannel, _keyDataListener(callback, Zone.current)); + } else { + channelBuffers.clearListener(_kFlutterKeyDataChannel); + } } // If this value changes, update the encoding code in the following files: diff --git a/engine/src/flutter/lib/ui/window/platform_configuration.cc b/engine/src/flutter/lib/ui/window/platform_configuration.cc index 47d6331e40..3cebfb5d10 100644 --- a/engine/src/flutter/lib/ui/window/platform_configuration.cc +++ b/engine/src/flutter/lib/ui/window/platform_configuration.cc @@ -181,15 +181,6 @@ void GetPersistentIsolateData(Dart_NativeArguments args) { persistent_isolate_data->GetSize())); } -void RespondToKeyData(Dart_Handle window, int response_id, bool handled) { - UIDartState::Current()->platform_configuration()->CompleteKeyDataResponse( - response_id, handled); -} - -void _RespondToKeyData(Dart_NativeArguments args) { - tonic::DartCallStatic(&RespondToKeyData, args); -} - Dart_Handle ToByteData(const fml::Mapping& buffer) { return tonic::DartByteData::Create(buffer.GetMapping(), buffer.GetSize()); } @@ -360,13 +351,6 @@ void PlatformConfiguration::DispatchSemanticsAction(int32_t id, args_handle})); } -uint64_t PlatformConfiguration::RegisterKeyDataResponse( - KeyDataResponse callback) { - uint64_t response_id = next_key_response_id_++; - pending_key_responses_[response_id] = std::move(callback); - return response_id; -} - void PlatformConfiguration::BeginFrame(fml::TimePoint frameTime, uint64_t frame_number) { std::shared_ptr dart_state = @@ -444,27 +428,6 @@ void PlatformConfiguration::CompletePlatformMessageResponse( response->Complete(std::make_unique(std::move(data))); } -void PlatformConfiguration::CompleteKeyDataResponse(uint64_t response_id, - bool handled) { - if (response_id == 0) { - return; - } - auto it = pending_key_responses_.find(response_id); - FML_DCHECK(it != pending_key_responses_.end()); - if (it == pending_key_responses_.end()) { - return; - } - KeyDataResponse callback = std::move(it->second); - pending_key_responses_.erase(it); - // The key result callback must be called on the platform thread. This is - // mainly because iOS needs to block the platform message loop with a nested - // loop to respond to key events synchronously, and if the callback is called - // from another thread, it won't stop the nested message loop, causing a - // deadlock. - UIDartState::Current()->GetTaskRunners().GetPlatformTaskRunner()->PostTask( - [handled, callback]() { callback(handled); }); -} - Dart_Handle ComputePlatformResolvedLocale(Dart_Handle supportedLocalesHandle) { std::vector supportedLocales = tonic::DartConverter>::FromDart( @@ -495,7 +458,6 @@ void PlatformConfiguration::RegisterNatives( true}, {"PlatformConfiguration_respondToPlatformMessage", _RespondToPlatformMessage, 3, true}, - {"PlatformConfiguration_respondToKeyData", _RespondToKeyData, 3, true}, {"PlatformConfiguration_render", Render, 3, true}, {"PlatformConfiguration_updateSemantics", UpdateSemantics, 2, true}, {"PlatformConfiguration_setIsolateDebugName", SetIsolateDebugName, 2, diff --git a/engine/src/flutter/lib/ui/window/platform_configuration.h b/engine/src/flutter/lib/ui/window/platform_configuration.h index 8bba4bd675..d42f8639a9 100644 --- a/engine/src/flutter/lib/ui/window/platform_configuration.h +++ b/engine/src/flutter/lib/ui/window/platform_configuration.h @@ -23,8 +23,6 @@ class FontCollection; class PlatformMessage; class Scene; -typedef std::function KeyDataResponse; - //-------------------------------------------------------------------------- /// @brief An enum for defining the different kinds of accessibility features /// that can be enabled by the platform. @@ -327,24 +325,6 @@ class PlatformConfiguration final { SemanticsAction action, fml::MallocMapping args); - //---------------------------------------------------------------------------- - /// @brief Registers a callback to be invoked when the framework has - /// decided whether to handle an event. This callback originates - /// in the platform view and has been forwarded through the engine - /// to here. - /// - /// This method will move and store the `callback`, associate it - /// with a self-incrementing identifier, the response ID, then - /// return the ID, which is typically used by - /// Window::DispatchKeyDataPacket. - /// - /// @param[in] callback The callback to be registered. - /// - /// @return The response ID to be associated with the callback. Using this - /// ID in CompleteKeyDataResponse will invoke the callback. - /// - uint64_t RegisterKeyDataResponse(KeyDataResponse callback); - //---------------------------------------------------------------------------- /// @brief Notifies the framework that it is time to begin working on a /// new frame previously scheduled via a call to @@ -439,21 +419,6 @@ class PlatformConfiguration final { /// void CompletePlatformMessageEmptyResponse(int response_id); - //---------------------------------------------------------------------------- - /// @brief Responds to a previously registered key data message from the - /// framework to the engine. - /// - /// For each response_id, this method should be called exactly - /// once. Responding to a response_id that has not been registered - /// or has been invoked will lead to a fatal error. - /// - /// @param[in] response_id The unique id that identifies the original platform - /// message to respond to, created by - /// RegisterKeyDataResponse. - /// @param[in] handled Whether the key data is handled. - /// - void CompleteKeyDataResponse(uint64_t response_id, bool handled); - private: PlatformConfigurationClient* client_; tonic::DartPersistentValue update_locales_; @@ -462,7 +427,6 @@ class PlatformConfiguration final { tonic::DartPersistentValue update_semantics_enabled_; tonic::DartPersistentValue update_accessibility_features_; tonic::DartPersistentValue dispatch_platform_message_; - tonic::DartPersistentValue dispatch_key_message_; tonic::DartPersistentValue dispatch_semantics_action_; tonic::DartPersistentValue begin_frame_; tonic::DartPersistentValue draw_frame_; @@ -474,10 +438,6 @@ class PlatformConfiguration final { int next_response_id_ = 1; std::unordered_map> pending_responses_; - - // ID starts at 1 because an ID of 0 indicates that no response is expected. - uint64_t next_key_response_id_ = 1; - std::unordered_map pending_key_responses_; }; } // namespace flutter diff --git a/engine/src/flutter/lib/ui/window/window.cc b/engine/src/flutter/lib/ui/window/window.cc index f02f76503d..f1ffe3ea0b 100644 --- a/engine/src/flutter/lib/ui/window/window.cc +++ b/engine/src/flutter/lib/ui/window/window.cc @@ -36,25 +36,6 @@ void Window::DispatchPointerDataPacket(const PointerDataPacket& packet) { library_.value(), "_dispatchPointerDataPacket", {data_handle})); } -void Window::DispatchKeyDataPacket(const KeyDataPacket& packet, - uint64_t response_id) { - std::shared_ptr dart_state = library_.dart_state().lock(); - if (!dart_state) { - return; - } - tonic::DartState::Scope scope(dart_state); - - const std::vector& buffer = packet.data(); - Dart_Handle data_handle = - tonic::DartByteData::Create(buffer.data(), buffer.size()); - if (Dart_IsError(data_handle)) { - return; - } - tonic::LogIfError( - tonic::DartInvokeField(library_.value(), "_dispatchKeyData", - {data_handle, tonic::ToDart(response_id)})); -} - void Window::UpdateWindowMetrics(const ViewportMetrics& metrics) { viewport_metrics_ = metrics; diff --git a/engine/src/flutter/lib/ui/window/window.h b/engine/src/flutter/lib/ui/window/window.h index 9a6847d3c4..aad79f9963 100644 --- a/engine/src/flutter/lib/ui/window/window.h +++ b/engine/src/flutter/lib/ui/window/window.h @@ -31,14 +31,6 @@ class Window final { // Dispatch a packet to the framework that indicates one or a few pointer // events. void DispatchPointerDataPacket(const PointerDataPacket& packet); - // Dispatch a packet to the framework that indicates a key event. - // - // The `response_id` is used to label the response of whether the key event - // is handled by the framework, typically the return value of - // PlatformConfiguration::RegisterKeyDataResponse. - // It should be used later in - // PlatformConfiguration::CompleteKeyDataResponse. - void DispatchKeyDataPacket(const KeyDataPacket& packet, uint64_t response_id); void UpdateWindowMetrics(const ViewportMetrics& metrics); private: diff --git a/engine/src/flutter/runtime/runtime_controller.cc b/engine/src/flutter/runtime/runtime_controller.cc index 8d2e3f878b..8976f14585 100644 --- a/engine/src/flutter/runtime/runtime_controller.cc +++ b/engine/src/flutter/runtime/runtime_controller.cc @@ -238,20 +238,6 @@ bool RuntimeController::DispatchPointerDataPacket( return false; } -bool RuntimeController::DispatchKeyDataPacket(const KeyDataPacket& packet, - KeyDataResponse callback) { - if (auto* platform_configuration = GetPlatformConfigurationIfAvailable()) { - TRACE_EVENT1("flutter", "RuntimeController::DispatchKeyDataPacket", "mode", - "basic"); - uint64_t response_id = - platform_configuration->RegisterKeyDataResponse(std::move(callback)); - platform_configuration->get_window(0)->DispatchKeyDataPacket(packet, - response_id); - return true; - } - return false; -} - bool RuntimeController::DispatchSemanticsAction(int32_t id, SemanticsAction action, fml::MallocMapping args) { diff --git a/engine/src/flutter/runtime/runtime_controller.h b/engine/src/flutter/runtime/runtime_controller.h index a8b5a84be4..3a8feeb1bd 100644 --- a/engine/src/flutter/runtime/runtime_controller.h +++ b/engine/src/flutter/runtime/runtime_controller.h @@ -385,20 +385,6 @@ class RuntimeController : public PlatformConfigurationClient { /// bool DispatchPointerDataPacket(const PointerDataPacket& packet); - //---------------------------------------------------------------------------- - /// @brief Dispatch the specified pointer data message to the running - /// root isolate. - /// - /// @param[in] packet The key data message to dispatch to the isolate. - /// @param[in] callback Called when the framework has decided whether - /// to handle this key data. - /// - /// @return If the key data message was dispatched. This may fail is - /// an isolate is not running. - /// - bool DispatchKeyDataPacket(const KeyDataPacket& packet, - KeyDataResponse callback); - //---------------------------------------------------------------------------- /// @brief Dispatch the semantics action to the specified accessibility /// node. diff --git a/engine/src/flutter/shell/common/engine.cc b/engine/src/flutter/shell/common/engine.cc index a3d1862fbf..7618a58e7b 100644 --- a/engine/src/flutter/shell/common/engine.cc +++ b/engine/src/flutter/shell/common/engine.cc @@ -434,14 +434,6 @@ void Engine::DispatchPointerDataPacket( pointer_data_dispatcher_->DispatchPacket(std::move(packet), trace_flow_id); } -void Engine::DispatchKeyDataPacket(std::unique_ptr packet, - KeyDataResponse callback) { - TRACE_EVENT0("flutter", "Engine::DispatchKeyDataPacket"); - if (runtime_controller_) { - runtime_controller_->DispatchKeyDataPacket(*packet, std::move(callback)); - } -} - void Engine::DispatchSemanticsAction(int id, SemanticsAction action, fml::MallocMapping args) { diff --git a/engine/src/flutter/shell/common/engine.h b/engine/src/flutter/shell/common/engine.h index 0b935ba703..0dca6dc0fc 100644 --- a/engine/src/flutter/shell/common/engine.h +++ b/engine/src/flutter/shell/common/engine.h @@ -735,21 +735,6 @@ class Engine final : public RuntimeDelegate, PointerDataDispatcher::Delegate { void DispatchPointerDataPacket(std::unique_ptr packet, uint64_t trace_flow_id); - //---------------------------------------------------------------------------- - /// @brief Notifies the engine that the embedder has sent it a key data - /// packet. A key data packet contains one key event. This call - /// originates in the platform view and the shell has forwarded - /// the same to the engine on the UI task runner here. The engine - /// will decide whether to handle this event, and send the - /// result using `callback`, which will be called exactly once. - /// - /// @param[in] packet The key data packet. - /// @param[in] callback Called when the framework has decided whether - /// to handle this key data. - /// - void DispatchKeyDataPacket(std::unique_ptr packet, - KeyDataResponse callback); - //---------------------------------------------------------------------------- /// @brief Notifies the engine that the embedder encountered an /// accessibility related action on the specified node. This call diff --git a/engine/src/flutter/shell/common/platform_view.cc b/engine/src/flutter/shell/common/platform_view.cc index dee2fea53f..7862d63b9a 100644 --- a/engine/src/flutter/shell/common/platform_view.cc +++ b/engine/src/flutter/shell/common/platform_view.cc @@ -39,12 +39,6 @@ void PlatformView::DispatchPointerDataPacket( pointer_data_packet_converter_.Convert(std::move(packet))); } -void PlatformView::DispatchKeyDataPacket(std::unique_ptr packet, - KeyDataResponse callback) { - delegate_.OnPlatformViewDispatchKeyDataPacket(std::move(packet), - std::move(callback)); -} - void PlatformView::DispatchSemanticsAction(int32_t id, SemanticsAction action, fml::MallocMapping args) { diff --git a/engine/src/flutter/shell/common/platform_view.h b/engine/src/flutter/shell/common/platform_view.h index 83c99a93c0..8921dfb777 100644 --- a/engine/src/flutter/shell/common/platform_view.h +++ b/engine/src/flutter/shell/common/platform_view.h @@ -127,20 +127,6 @@ class PlatformView { virtual void OnPlatformViewDispatchPointerDataPacket( std::unique_ptr packet) = 0; - //-------------------------------------------------------------------------- - /// @brief Notifies the delegate that the platform view has encountered - /// a key event. This key event and the callback needs to be - /// forwarded to the running root isolate hosted by the engine - /// on the UI thread. - /// - /// @param[in] packet The key data packet containing one key event. - /// @param[in] callback Called when the framework has decided whether - /// to handle this key data. - /// - virtual void OnPlatformViewDispatchKeyDataPacket( - std::unique_ptr packet, - std::function callback) = 0; - //-------------------------------------------------------------------------- /// @brief Notifies the delegate that the platform view has encountered /// an accessibility related action on the specified node. This @@ -591,17 +577,6 @@ class PlatformView { /// void DispatchPointerDataPacket(std::unique_ptr packet); - //---------------------------------------------------------------------------- - /// @brief Dispatches key events from the embedder to the framework. Each - /// key data packet contains one physical event and multiple - /// logical key events. Each call to this method wakes up the UI - /// thread. - /// - /// @param[in] packet The key data packet to dispatch to the framework. - /// - void DispatchKeyDataPacket(std::unique_ptr packet, - Delegate::KeyDataResponse callback); - //-------------------------------------------------------------------------- /// @brief Used by the embedder to specify a texture that it wants the /// rasterizer to composite within the Flutter layer tree. All diff --git a/engine/src/flutter/shell/common/shell.cc b/engine/src/flutter/shell/common/shell.cc index f6241bda79..08e8a45006 100644 --- a/engine/src/flutter/shell/common/shell.cc +++ b/engine/src/flutter/shell/common/shell.cc @@ -942,23 +942,6 @@ void Shell::OnPlatformViewDispatchPointerDataPacket( next_pointer_flow_id_++; } -// |PlatformView::Delegate| -void Shell::OnPlatformViewDispatchKeyDataPacket( - std::unique_ptr packet, - std::function callback) { - TRACE_EVENT0("flutter", "Shell::OnPlatformViewDispatchKeyDataPacket"); - FML_DCHECK(is_setup_); - FML_DCHECK(task_runners_.GetPlatformTaskRunner()->RunsTasksOnCurrentThread()); - - task_runners_.GetUITaskRunner()->PostTask( - fml::MakeCopyable([engine = weak_engine_, packet = std::move(packet), - callback = std::move(callback)]() mutable { - if (engine) { - engine->DispatchKeyDataPacket(std::move(packet), std::move(callback)); - } - })); -} - // |PlatformView::Delegate| void Shell::OnPlatformViewDispatchSemanticsAction(int32_t id, SemanticsAction action, diff --git a/engine/src/flutter/shell/common/shell.h b/engine/src/flutter/shell/common/shell.h index b0b1994fa8..87ba86cb83 100644 --- a/engine/src/flutter/shell/common/shell.h +++ b/engine/src/flutter/shell/common/shell.h @@ -525,11 +525,6 @@ class Shell final : public PlatformView::Delegate, void OnPlatformViewDispatchPointerDataPacket( std::unique_ptr packet) override; - // |PlatformView::Delegate| - void OnPlatformViewDispatchKeyDataPacket( - std::unique_ptr packet, - std::function callback) override; - // |PlatformView::Delegate| void OnPlatformViewDispatchSemanticsAction(int32_t id, SemanticsAction action, diff --git a/engine/src/flutter/shell/common/shell_unittests.cc b/engine/src/flutter/shell/common/shell_unittests.cc index 9a77142edb..5d11dd5fcb 100644 --- a/engine/src/flutter/shell/common/shell_unittests.cc +++ b/engine/src/flutter/shell/common/shell_unittests.cc @@ -67,10 +67,6 @@ class MockPlatformViewDelegate : public PlatformView::Delegate { MOCK_METHOD1(OnPlatformViewDispatchPointerDataPacket, void(std::unique_ptr packet)); - MOCK_METHOD2(OnPlatformViewDispatchKeyDataPacket, - void(std::unique_ptr packet, - KeyDataResponse callback)); - MOCK_METHOD3(OnPlatformViewDispatchSemanticsAction, void(int32_t id, SemanticsAction action, diff --git a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm index 337dca9fa6..c3eeb47ab8 100644 --- a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm +++ b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm @@ -36,6 +36,7 @@ NSString* const FlutterDefaultDartEntrypoint = nil; NSString* const FlutterDefaultInitialRoute = nil; NSString* const FlutterEngineWillDealloc = @"FlutterEngineWillDealloc"; +NSString* const FlutterKeyDataChannel = @"flutter/keydata"; static constexpr int kNumProfilerSamplesPerSec = 5; @interface FlutterEngineRegistrar : NSObject @@ -301,13 +302,20 @@ static constexpr int kNumProfilerSamplesPerSec = 5; key_data.synthesized = event.synthesized; auto packet = std::make_unique(key_data, character); + NSData* message = [NSData dataWithBytes:packet->data().data() length:packet->data().size()]; - auto response = [callback, userData](bool handled) { - if (callback != nullptr) { - callback(handled, userData); + auto response = ^(NSData* reply) { + if (callback == nullptr) { + return; } + BOOL handled = FALSE; + if (reply.length == 1 && *reinterpret_cast(reply.bytes) == 1) { + handled = TRUE; + } + callback(handled, userData); }; - self.platformView->DispatchKeyDataPacket(std::move(packet), std::move(response)); + + [self sendOnChannel:FlutterKeyDataChannel message:message binaryReply:response]; } - (void)ensureSemanticsEnabled { diff --git a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterEnginePlatformViewTest.mm b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterEnginePlatformViewTest.mm index 25bfa3ac5a..4a4a8877ce 100644 --- a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterEnginePlatformViewTest.mm +++ b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterEnginePlatformViewTest.mm @@ -26,8 +26,6 @@ class FakeDelegate : public PlatformView::Delegate { void OnPlatformViewDispatchPlatformMessage(std::unique_ptr message) override {} void OnPlatformViewDispatchPointerDataPacket(std::unique_ptr packet) override { } - void OnPlatformViewDispatchKeyDataPacket(std::unique_ptr packet, - std::function callback) override {} void OnPlatformViewDispatchSemanticsAction(int32_t id, SemanticsAction action, fml::MallocMapping args) override {} diff --git a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterPlatformViewsTest.mm b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterPlatformViewsTest.mm index 3ba48177da..a7d5a25a46 100644 --- a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterPlatformViewsTest.mm +++ b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterPlatformViewsTest.mm @@ -95,8 +95,6 @@ class FlutterPlatformViewsTestMockPlatformViewDelegate : public PlatformView::De void OnPlatformViewDispatchPlatformMessage(std::unique_ptr message) override {} void OnPlatformViewDispatchPointerDataPacket(std::unique_ptr packet) override { } - void OnPlatformViewDispatchKeyDataPacket(std::unique_ptr packet, - std::function callback) override {} void OnPlatformViewDispatchSemanticsAction(int32_t id, SemanticsAction action, fml::MallocMapping args) override {} diff --git a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/accessibility_bridge_test.mm b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/accessibility_bridge_test.mm index 731b1adb7b..1e281e250e 100644 --- a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/accessibility_bridge_test.mm +++ b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/accessibility_bridge_test.mm @@ -79,8 +79,6 @@ class MockDelegate : public PlatformView::Delegate { void OnPlatformViewDispatchPlatformMessage(std::unique_ptr message) override {} void OnPlatformViewDispatchPointerDataPacket(std::unique_ptr packet) override { } - void OnPlatformViewDispatchKeyDataPacket(std::unique_ptr packet, - std::function callback) override {} void OnPlatformViewDispatchSemanticsAction(int32_t id, SemanticsAction action, fml::MallocMapping args) override {} diff --git a/engine/src/flutter/shell/platform/darwin/ios/platform_view_ios.h b/engine/src/flutter/shell/platform/darwin/ios/platform_view_ios.h index 4010da254b..3af69b7b2b 100644 --- a/engine/src/flutter/shell/platform/darwin/ios/platform_view_ios.h +++ b/engine/src/flutter/shell/platform/darwin/ios/platform_view_ios.h @@ -90,9 +90,6 @@ class PlatformViewIOS final : public PlatformView { // |PlatformView| PointerDataDispatcherMaker GetDispatcherMaker() override; - void DispatchKeyDataPacket(std::unique_ptr packet, - std::function callback); - // |PlatformView| void SetSemanticsEnabled(bool enabled) override; diff --git a/engine/src/flutter/shell/platform/embedder/embedder.cc b/engine/src/flutter/shell/platform/embedder/embedder.cc index 4afe2f2e9b..5c00230ba6 100644 --- a/engine/src/flutter/shell/platform/embedder/embedder.cc +++ b/engine/src/flutter/shell/platform/embedder/embedder.cc @@ -69,6 +69,26 @@ extern const intptr_t kPlatformStrongDillSize; const int32_t kFlutterSemanticsNodeIdBatchEnd = -1; const int32_t kFlutterSemanticsCustomActionIdBatchEnd = -1; +// A message channel to send platform-independent FlutterKeyData to the +// framework. +// +// This should be kept in sync with the following variables: +// +// - lib/ui/platform_dispatcher.dart, _kFlutterKeyDataChannel +// - shell/platform/darwin/ios/framework/Source/FlutterEngine.mm, +// FlutterKeyDataChannel +// +// Not to be confused with "flutter/keyevent", which is used to send raw +// key event data in a platform-dependent format. +// +// ## Format +// +// Send: KeyDataPacket.data(). +// +// Expected reply: Whether the event is handled. Exactly 1 byte long, with value +// 1 for handled, and 0 for not. Malformed value is considered false. +const char* kFlutterKeyDataChannel = "flutter/keydata"; + static FlutterEngineResult LogEmbedderError(FlutterEngineResult code, const char* reason, const char* code_name, @@ -1593,6 +1613,45 @@ static inline flutter::KeyEventType MapKeyEventType( return flutter::KeyEventType::kUp; } +// Send a platform message to the framework. +// +// The `data_callback` will be invoked with `user_data`, and must not be empty. +static FlutterEngineResult InternalSendPlatformMessage( + FLUTTER_API_SYMBOL(FlutterEngine) engine, + const char* channel, + const uint8_t* data, + size_t size, + FlutterDataCallback data_callback, + void* user_data) { + FlutterEngineResult result; + + FlutterPlatformMessageResponseHandle* response_handle; + result = FlutterPlatformMessageCreateResponseHandle( + engine, data_callback, user_data, &response_handle); + if (result != kSuccess) { + return result; + } + + const FlutterPlatformMessage message = { + sizeof(FlutterPlatformMessage), // struct_size + channel, // channel + data, // message + size, // message_size + response_handle, // response_handle + }; + + result = FlutterEngineSendPlatformMessage(engine, &message); + // Whether `SendPlatformMessage` succeeds or not, the response handle must be + // released. + FlutterEngineResult release_result = + FlutterPlatformMessageReleaseResponseHandle(engine, response_handle); + if (result != kSuccess) { + return result; + } + + return release_result; +} + FlutterEngineResult FlutterEngineSendKeyEvent(FLUTTER_API_SYMBOL(FlutterEngine) engine, const FlutterKeyEvent* event, @@ -1619,18 +1678,27 @@ FlutterEngineResult FlutterEngineSendKeyEvent(FLUTTER_API_SYMBOL(FlutterEngine) auto packet = std::make_unique(key_data, character); - auto response = [callback, user_data](bool handled) { - if (callback != nullptr) { - callback(handled, user_data); - } + struct MessageData { + FlutterKeyEventCallback callback; + void* user_data; }; - return reinterpret_cast(engine) - ->DispatchKeyDataPacket(std::move(packet), response) - ? kSuccess - : LOG_EMBEDDER_ERROR(kInternalInconsistency, - "Could not dispatch the key event to the " - "running Flutter application."); + MessageData* message_data = + new MessageData{.callback = callback, .user_data = user_data}; + + return InternalSendPlatformMessage( + engine, kFlutterKeyDataChannel, packet->data().data(), + packet->data().size(), + [](const uint8_t* data, size_t size, void* user_data) { + auto message_data = std::unique_ptr( + reinterpret_cast(user_data)); + bool handled = false; + if (size == 1) { + handled = *data != 0; + } + message_data->callback(handled, message_data->user_data); + }, + message_data); } FlutterEngineResult FlutterEngineSendPlatformMessage( diff --git a/engine/src/flutter/shell/platform/embedder/embedder_engine.cc b/engine/src/flutter/shell/platform/embedder/embedder_engine.cc index d96fa0c81b..5d8d785d17 100644 --- a/engine/src/flutter/shell/platform/embedder/embedder_engine.cc +++ b/engine/src/flutter/shell/platform/embedder/embedder_engine.cc @@ -127,22 +127,6 @@ bool EmbedderEngine::DispatchPointerDataPacket( return true; } -bool EmbedderEngine::DispatchKeyDataPacket( - std::unique_ptr packet, - KeyDataResponse callback) { - if (!IsValid() || !packet) { - return false; - } - - auto platform_view = shell_->GetPlatformView(); - if (!platform_view) { - return false; - } - - platform_view->DispatchKeyDataPacket(std::move(packet), std::move(callback)); - return true; -} - bool EmbedderEngine::SendPlatformMessage( std::unique_ptr message) { if (!IsValid() || !message) { diff --git a/engine/src/flutter/shell/platform/embedder/embedder_engine.h b/engine/src/flutter/shell/platform/embedder/embedder_engine.h index 30b0dd87a4..d65bc81973 100644 --- a/engine/src/flutter/shell/platform/embedder/embedder_engine.h +++ b/engine/src/flutter/shell/platform/embedder/embedder_engine.h @@ -52,22 +52,6 @@ class EmbedderEngine { bool DispatchPointerDataPacket( std::unique_ptr packet); - //---------------------------------------------------------------------------- - /// @brief Notifies the platform view that the embedder has sent it a key - /// data packet. A key data packet contains one key event. This - /// call originates in the platform view and the shell has - /// forwarded the same to the engine on the UI task runner here. - /// The platform view will decide whether to handle this event, - /// and send the result using `callback`, which will be called - /// exactly once. - /// - /// @param[in] packet The key data packet. - /// @param[in] callback Called when the framework has decided whether - /// to handle this key data. - /// - bool DispatchKeyDataPacket(std::unique_ptr packet, - KeyDataResponse callback); - bool SendPlatformMessage(std::unique_ptr message); bool RegisterTexture(int64_t texture); diff --git a/engine/src/flutter/shell/platform/embedder/fixtures/main.dart b/engine/src/flutter/shell/platform/embedder/fixtures/main.dart index f1f8ca51a7..5196f37727 100644 --- a/engine/src/flutter/shell/platform/embedder/fixtures/main.dart +++ b/engine/src/flutter/shell/platform/embedder/fixtures/main.dart @@ -567,6 +567,27 @@ void key_data_echo() async { signalNativeTest(); } +// After platform channel 'test/starts_echo' receives a message, starts echoing +// the event data with `_echoKeyEvent`, and returns synthesized as handled. +@pragma('vm:entry-point') +void key_data_late_echo() async { + channelBuffers.setListener('test/starts_echo', (ByteData? data, PlatformMessageResponseCallback callback) { + PlatformDispatcher.instance.onKeyData = (KeyData data) { + _echoKeyEvent( + _serializeKeyEventType(data.type), + data.timeStamp.inMicroseconds, + data.physical, + data.logical, + data.character == null ? 0 : data.character!.codeUnitAt(0), + data.synthesized, + ); + return data.synthesized; + }; + callback(null); + }); + signalNativeTest(); +} + @pragma('vm:entry-point') void render_gradient() { PlatformDispatcher.instance.onBeginFrame = (Duration duration) { diff --git a/engine/src/flutter/shell/platform/embedder/tests/embedder_unittests.cc b/engine/src/flutter/shell/platform/embedder/tests/embedder_unittests.cc index f8b5d92d18..95d765a6cd 100644 --- a/engine/src/flutter/shell/platform/embedder/tests/embedder_unittests.cc +++ b/engine/src/flutter/shell/platform/embedder/tests/embedder_unittests.cc @@ -1355,7 +1355,7 @@ TEST_F(EmbedderTest, KeyDataIsCorrectlySerialized) { echoed_event.type = UnserializeKeyEventKind(tonic::DartConverter::FromDart( Dart_GetNativeArgument(args, 0))); - echoed_event.timestamp = tonic::DartConverter::FromDart( + echoed_event.timestamp = (double)tonic::DartConverter::FromDart( Dart_GetNativeArgument(args, 1)); echoed_event.physical = tonic::DartConverter::FromDart( Dart_GetNativeArgument(args, 2)); @@ -1440,6 +1440,99 @@ TEST_F(EmbedderTest, KeyDataIsCorrectlySerialized) { EXPECT_EQ(echoed_char, 0llu); } +TEST_F(EmbedderTest, KeyDataAreBuffered) { + auto message_latch = std::make_shared(); + std::vector echoed_events; + + auto native_echo_event = [&](Dart_NativeArguments args) { + echoed_events.push_back(FlutterKeyEvent{ + .timestamp = (double)tonic::DartConverter::FromDart( + Dart_GetNativeArgument(args, 1)), + .type = + UnserializeKeyEventKind(tonic::DartConverter::FromDart( + Dart_GetNativeArgument(args, 0))), + .physical = tonic::DartConverter::FromDart( + Dart_GetNativeArgument(args, 2)), + .logical = tonic::DartConverter::FromDart( + Dart_GetNativeArgument(args, 3)), + .synthesized = tonic::DartConverter::FromDart( + Dart_GetNativeArgument(args, 5)), + }); + + message_latch->Signal(); + }; + + auto& context = GetEmbedderContext(EmbedderTestContextType::kSoftwareContext); + EmbedderConfigBuilder builder(context); + builder.SetSoftwareRendererConfig(); + builder.SetDartEntrypoint("key_data_late_echo"); + fml::AutoResetWaitableEvent ready; + context.AddNativeCallback( + "SignalNativeTest", + CREATE_NATIVE_ENTRY( + [&ready](Dart_NativeArguments args) { ready.Signal(); })); + + context.AddNativeCallback("EchoKeyEvent", + CREATE_NATIVE_ENTRY(native_echo_event)); + + auto engine = builder.LaunchEngine(); + ASSERT_TRUE(engine.is_valid()); + ready.Wait(); + + FlutterKeyEvent sample_event{ + .struct_size = sizeof(FlutterKeyEvent), + .type = kFlutterKeyEventTypeDown, + .physical = 0x00070004, + .logical = 0x00000000061, + .character = "A", + .synthesized = false, + }; + + // Send an event. + sample_event.timestamp = 1.0l; + FlutterEngineSendKeyEvent( + engine.get(), &sample_event, [](bool handled, void* user_data) {}, + nullptr); + + // Should not receive echos because the callback is not set yet. + EXPECT_EQ(echoed_events.size(), 0u); + + // Send an empty message to 'test/starts_echo' to start echoing. + FlutterPlatformMessageResponseHandle* response_handle = nullptr; + FlutterPlatformMessageCreateResponseHandle( + engine.get(), [](const uint8_t* data, size_t size, void* user_data) {}, + nullptr, &response_handle); + + FlutterPlatformMessage message{ + .struct_size = sizeof(FlutterPlatformMessage), + .channel = "test/starts_echo", + .message = nullptr, + .message_size = 0, + .response_handle = response_handle, + }; + + FlutterEngineResult result = + FlutterEngineSendPlatformMessage(engine.get(), &message); + ASSERT_EQ(result, kSuccess); + + FlutterPlatformMessageReleaseResponseHandle(engine.get(), response_handle); + + // message_latch->Wait(); + message_latch->Wait(); + // All previous events should be received now. + EXPECT_EQ(echoed_events.size(), 1u); + + // Send a second event. + sample_event.timestamp = 10.0l; + FlutterEngineSendKeyEvent( + engine.get(), &sample_event, [](bool handled, void* user_data) {}, + nullptr); + message_latch->Wait(); + + // The event should be echoed, too. + EXPECT_EQ(echoed_events.size(), 2u); +} + TEST_F(EmbedderTest, KeyDataResponseIsCorrectlyInvoked) { UniqueEngine engine; fml::AutoResetWaitableEvent sync_latch; diff --git a/engine/src/flutter/shell/platform/fuchsia/flutter/platform_view_unittest.cc b/engine/src/flutter/shell/platform/fuchsia/flutter/platform_view_unittest.cc index bd76c79fc9..54d8941789 100644 --- a/engine/src/flutter/shell/platform/fuchsia/flutter/platform_view_unittest.cc +++ b/engine/src/flutter/shell/platform/fuchsia/flutter/platform_view_unittest.cc @@ -102,10 +102,6 @@ class MockPlatformViewDelegate : public flutter::PlatformView::Delegate { pointer_packets_.push_back(std::move(packet)); } // |flutter::PlatformView::Delegate| - void OnPlatformViewDispatchKeyDataPacket( - std::unique_ptr packet, - std::function callback) {} - // |flutter::PlatformView::Delegate| void OnPlatformViewDispatchSemanticsAction(int32_t id, flutter::SemanticsAction action, fml::MallocMapping args) {}