From 1fe1ec414f53a30485750f802e212f322263d0ae Mon Sep 17 00:00:00 2001 From: Chris Bracken Date: Fri, 16 Dec 2022 13:40:48 -0800 Subject: [PATCH] Clarify semantics action dispatch id parameter (#38356) Previously the embedder API documented this as an action ID, but it's actually the semantics node ID. This fixes the docs and renames the parameter to clarify its purpose. This callback is registered in the framework render bindings: https://github.com/flutter/flutter/blob/9102f2fe0bd26db6074ac4a17785296cd341ecb9/packages/flutter/lib/src/rendering/binding.dart#L43 Handled by `_handleSemanticsAction`: https://github.com/flutter/flutter/blob/9102f2fe0bd26db6074ac4a17785296cd341ecb9/packages/flutter/lib/src/rendering/binding.dart#L360-L366 Which invokes `SemanticsOwner.performAction`, where the node is looked up by ID: https://github.com/flutter/flutter/blob/9102f2fe0bd26db6074ac4a17785296cd341ecb9/packages/flutter/lib/src/semantics/semantics.dart#L3258-L3277 --- lib/ui/hooks.dart | 4 ++-- lib/ui/platform_dispatcher.dart | 10 +++++----- lib/ui/window/platform_configuration.cc | 4 ++-- lib/ui/window/platform_configuration.h | 4 ++-- lib/web_ui/lib/platform_dispatcher.dart | 2 +- .../lib/src/engine/platform_dispatcher.dart | 4 ++-- runtime/runtime_controller.cc | 4 ++-- runtime/runtime_controller.h | 4 ++-- shell/common/engine.cc | 5 +++-- shell/common/engine.h | 4 ++-- shell/common/platform_view.cc | 5 +++-- shell/common/platform_view.h | 8 ++++---- shell/common/shell.cc | 6 +++--- shell/common/shell.h | 2 +- .../flutter/embedding/engine/FlutterJNI.java | 18 +++++++++++------- .../engine/renderer/FlutterRenderer.java | 4 ++-- shell/platform/embedder/embedder.cc | 4 ++-- shell/platform/embedder/embedder.h | 4 ++-- shell/platform/embedder/embedder_engine.cc | 4 ++-- shell/platform/embedder/embedder_engine.h | 2 +- 20 files changed, 54 insertions(+), 48 deletions(-) diff --git a/lib/ui/hooks.dart b/lib/ui/hooks.dart index 1cdbf90baeebf..38375989e96a1 100644 --- a/lib/ui/hooks.dart +++ b/lib/ui/hooks.dart @@ -92,8 +92,8 @@ void _dispatchPointerDataPacket(ByteData packet) { } @pragma('vm:entry-point') -void _dispatchSemanticsAction(int id, int action, ByteData? args) { - PlatformDispatcher.instance._dispatchSemanticsAction(id, action, args); +void _dispatchSemanticsAction(int nodeId, int action, ByteData? args) { + PlatformDispatcher.instance._dispatchSemanticsAction(nodeId, action, args); } @pragma('vm:entry-point') diff --git a/lib/ui/platform_dispatcher.dart b/lib/ui/platform_dispatcher.dart index 6ffc0c5959f8e..f348a4f8db00e 100644 --- a/lib/ui/platform_dispatcher.dart +++ b/lib/ui/platform_dispatcher.dart @@ -33,7 +33,7 @@ typedef PointerDataPacketCallback = void Function(PointerDataPacket packet); typedef KeyDataCallback = bool Function(KeyData data); /// Signature for [PlatformDispatcher.onSemanticsAction]. -typedef SemanticsActionCallback = void Function(int id, SemanticsAction action, ByteData? args); +typedef SemanticsActionCallback = void Function(int nodeId, SemanticsAction action, ByteData? args); /// Signature for responses to platform messages. /// @@ -1089,10 +1089,10 @@ class PlatformDispatcher { } /// A callback that is invoked whenever the user requests an action to be - /// performed. + /// performed on a semantics node. /// /// This callback is used when the user expresses the action they wish to - /// perform based on the semantics supplied by updateSemantics. + /// perform based on the semantics node supplied by updateSemantics. /// /// The framework invokes this callback in the same zone in which the /// callback was set. @@ -1128,11 +1128,11 @@ class PlatformDispatcher { } // Called from the engine, via hooks.dart - void _dispatchSemanticsAction(int id, int action, ByteData? args) { + void _dispatchSemanticsAction(int nodeId, int action, ByteData? args) { _invoke3( onSemanticsAction, _onSemanticsActionZone, - id, + nodeId, SemanticsAction.values[action]!, args, ); diff --git a/lib/ui/window/platform_configuration.cc b/lib/ui/window/platform_configuration.cc index d3d56c13f1029..22f6c77962e7a 100644 --- a/lib/ui/window/platform_configuration.cc +++ b/lib/ui/window/platform_configuration.cc @@ -171,7 +171,7 @@ void PlatformConfiguration::DispatchPlatformMessage( tonic::ToDart(response_id)})); } -void PlatformConfiguration::DispatchSemanticsAction(int32_t id, +void PlatformConfiguration::DispatchSemanticsAction(int32_t node_id, SemanticsAction action, fml::MallocMapping args) { std::shared_ptr dart_state = @@ -190,7 +190,7 @@ void PlatformConfiguration::DispatchSemanticsAction(int32_t id, tonic::CheckAndHandleError(tonic::DartInvoke( dispatch_semantics_action_.Get(), - {tonic::ToDart(id), tonic::ToDart(static_cast(action)), + {tonic::ToDart(node_id), tonic::ToDart(static_cast(action)), args_handle})); } diff --git a/lib/ui/window/platform_configuration.h b/lib/ui/window/platform_configuration.h index c5019962867cc..2f86727c59c30 100644 --- a/lib/ui/window/platform_configuration.h +++ b/lib/ui/window/platform_configuration.h @@ -325,12 +325,12 @@ class PlatformConfiguration final { /// originates on the platform view and has been forwarded to the /// platform configuration here by the engine. /// - /// @param[in] id The identifier of the accessibility node. + /// @param[in] node_id The identifier of the accessibility node. /// @param[in] action The accessibility related action performed on the /// node of the specified ID. /// @param[in] args Optional data that applies to the specified action. /// - void DispatchSemanticsAction(int32_t id, + void DispatchSemanticsAction(int32_t node_id, SemanticsAction action, fml::MallocMapping args); diff --git a/lib/web_ui/lib/platform_dispatcher.dart b/lib/web_ui/lib/platform_dispatcher.dart index 6d508576df1cc..82d991915d8ff 100644 --- a/lib/web_ui/lib/platform_dispatcher.dart +++ b/lib/web_ui/lib/platform_dispatcher.dart @@ -9,7 +9,7 @@ typedef FrameCallback = void Function(Duration duration); typedef TimingsCallback = void Function(List timings); typedef PointerDataPacketCallback = void Function(PointerDataPacket packet); typedef KeyDataCallback = bool Function(KeyData data); -typedef SemanticsActionCallback = void Function(int id, SemanticsAction action, ByteData? args); +typedef SemanticsActionCallback = void Function(int nodeId, SemanticsAction action, ByteData? args); typedef PlatformMessageResponseCallback = void Function(ByteData? data); typedef PlatformMessageCallback = void Function( String name, ByteData? data, PlatformMessageResponseCallback? callback); diff --git a/lib/web_ui/lib/src/engine/platform_dispatcher.dart b/lib/web_ui/lib/src/engine/platform_dispatcher.dart index e7d6f80b7f355..e003513953583 100644 --- a/lib/web_ui/lib/src/engine/platform_dispatcher.dart +++ b/lib/web_ui/lib/src/engine/platform_dispatcher.dart @@ -1084,9 +1084,9 @@ class EnginePlatformDispatcher extends ui.PlatformDispatcher { /// Engine code should use this method instead of the callback directly. /// Otherwise zones won't work properly. void invokeOnSemanticsAction( - int id, ui.SemanticsAction action, ByteData? args) { + int nodeId, ui.SemanticsAction action, ByteData? args) { invoke3( - _onSemanticsAction, _onSemanticsActionZone, id, action, args); + _onSemanticsAction, _onSemanticsActionZone, nodeId, action, args); } // TODO(dnfield): make this work on web. diff --git a/runtime/runtime_controller.cc b/runtime/runtime_controller.cc index 9870d5e7804ca..b874ae698d55d 100644 --- a/runtime/runtime_controller.cc +++ b/runtime/runtime_controller.cc @@ -278,13 +278,13 @@ bool RuntimeController::DispatchPointerDataPacket( return false; } -bool RuntimeController::DispatchSemanticsAction(int32_t id, +bool RuntimeController::DispatchSemanticsAction(int32_t node_id, SemanticsAction action, fml::MallocMapping args) { TRACE_EVENT1("flutter", "RuntimeController::DispatchSemanticsAction", "mode", "basic"); if (auto* platform_configuration = GetPlatformConfigurationIfAvailable()) { - platform_configuration->DispatchSemanticsAction(id, action, + platform_configuration->DispatchSemanticsAction(node_id, action, std::move(args)); return true; } diff --git a/runtime/runtime_controller.h b/runtime/runtime_controller.h index 3f8fa2dcb3ab4..7bfe9254b3e79 100644 --- a/runtime/runtime_controller.h +++ b/runtime/runtime_controller.h @@ -407,7 +407,7 @@ class RuntimeController : public PlatformConfigurationClient { /// @brief Dispatch the semantics action to the specified accessibility /// node. /// - /// @param[in] id The identified of the accessibility node. + /// @param[in] node_id The identified of the accessibility node. /// @param[in] action The semantics action to perform on the specified /// accessibility node. /// @param[in] args Optional data that applies to the specified action. @@ -415,7 +415,7 @@ class RuntimeController : public PlatformConfigurationClient { /// @return If the semantics action was dispatched. This may fail if an /// isolate is not running. /// - bool DispatchSemanticsAction(int32_t id, + bool DispatchSemanticsAction(int32_t node_id, SemanticsAction action, fml::MallocMapping args); diff --git a/shell/common/engine.cc b/shell/common/engine.cc index 7d2a63d57cad2..34acee5fa77a9 100644 --- a/shell/common/engine.cc +++ b/shell/common/engine.cc @@ -416,10 +416,11 @@ void Engine::DispatchPointerDataPacket( pointer_data_dispatcher_->DispatchPacket(std::move(packet), trace_flow_id); } -void Engine::DispatchSemanticsAction(int id, +void Engine::DispatchSemanticsAction(int node_id, SemanticsAction action, fml::MallocMapping args) { - runtime_controller_->DispatchSemanticsAction(id, action, std::move(args)); + runtime_controller_->DispatchSemanticsAction(node_id, action, + std::move(args)); } void Engine::SetSemanticsEnabled(bool enabled) { diff --git a/shell/common/engine.h b/shell/common/engine.h index 70402c24a7876..2e89710825384 100644 --- a/shell/common/engine.h +++ b/shell/common/engine.h @@ -722,12 +722,12 @@ class Engine final : public RuntimeDelegate, PointerDataDispatcher::Delegate { /// originates on the platform view and has been forwarded to the /// engine here on the UI task runner by the shell. /// - /// @param[in] id The identifier of the accessibility node. + /// @param[in] node_id The identifier of the accessibility node. /// @param[in] action The accessibility related action performed on the /// node of the specified ID. /// @param[in] args Optional data that applies to the specified action. /// - void DispatchSemanticsAction(int id, + void DispatchSemanticsAction(int node_id, SemanticsAction action, fml::MallocMapping args); diff --git a/shell/common/platform_view.cc b/shell/common/platform_view.cc index ce2b22009f821..7cb0b091d5bcd 100644 --- a/shell/common/platform_view.cc +++ b/shell/common/platform_view.cc @@ -36,10 +36,11 @@ void PlatformView::DispatchPointerDataPacket( pointer_data_packet_converter_.Convert(std::move(packet))); } -void PlatformView::DispatchSemanticsAction(int32_t id, +void PlatformView::DispatchSemanticsAction(int32_t node_id, SemanticsAction action, fml::MallocMapping args) { - delegate_.OnPlatformViewDispatchSemanticsAction(id, action, std::move(args)); + delegate_.OnPlatformViewDispatchSemanticsAction(node_id, action, + std::move(args)); } void PlatformView::SetSemanticsEnabled(bool enabled) { diff --git a/shell/common/platform_view.h b/shell/common/platform_view.h index d46b8b1326ff6..f9e9bcb58dbf3 100644 --- a/shell/common/platform_view.h +++ b/shell/common/platform_view.h @@ -144,14 +144,14 @@ class PlatformView { /// event must be forwarded to the running root isolate hosted /// by the engine on the UI thread. /// - /// @param[in] id The identifier of the accessibility node. + /// @param[in] node_id The identifier of the accessibility node. /// @param[in] action The accessibility related action performed on the /// node of the specified ID. /// @param[in] args An optional list of argument that apply to the /// specified action. /// virtual void OnPlatformViewDispatchSemanticsAction( - int32_t id, + int32_t node_id, SemanticsAction action, fml::MallocMapping args) = 0; @@ -406,12 +406,12 @@ class PlatformView { /// @brief Used by embedders to dispatch an accessibility action to a /// running isolate hosted by the engine. /// - /// @param[in] id The identifier of the accessibility node on which to + /// @param[in] node_id The identifier of the accessibility node on which to /// perform the action. /// @param[in] action The action /// @param[in] args The arguments /// - void DispatchSemanticsAction(int32_t id, + void DispatchSemanticsAction(int32_t node_id, SemanticsAction action, fml::MallocMapping args); diff --git a/shell/common/shell.cc b/shell/common/shell.cc index 693d36a143366..a4cf64505b8ee 100644 --- a/shell/common/shell.cc +++ b/shell/common/shell.cc @@ -995,17 +995,17 @@ void Shell::OnPlatformViewDispatchPointerDataPacket( } // |PlatformView::Delegate| -void Shell::OnPlatformViewDispatchSemanticsAction(int32_t id, +void Shell::OnPlatformViewDispatchSemanticsAction(int32_t node_id, SemanticsAction action, fml::MallocMapping args) { FML_DCHECK(is_setup_); FML_DCHECK(task_runners_.GetPlatformTaskRunner()->RunsTasksOnCurrentThread()); task_runners_.GetUITaskRunner()->PostTask( - fml::MakeCopyable([engine = engine_->GetWeakPtr(), id, action, + fml::MakeCopyable([engine = engine_->GetWeakPtr(), node_id, action, args = std::move(args)]() mutable { if (engine) { - engine->DispatchSemanticsAction(id, action, std::move(args)); + engine->DispatchSemanticsAction(node_id, action, std::move(args)); } })); } diff --git a/shell/common/shell.h b/shell/common/shell.h index 71ee082c0a4ac..f51edc9f9e343 100644 --- a/shell/common/shell.h +++ b/shell/common/shell.h @@ -545,7 +545,7 @@ class Shell final : public PlatformView::Delegate, std::unique_ptr packet) override; // |PlatformView::Delegate| - void OnPlatformViewDispatchSemanticsAction(int32_t id, + void OnPlatformViewDispatchSemanticsAction(int32_t node_id, SemanticsAction action, fml::MallocMapping args) override; diff --git a/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java b/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java index 799ad214b3601..aba6f671deb5d 100644 --- a/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java +++ b/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java @@ -787,13 +787,13 @@ private void updateCustomAccessibilityActions( } /** Sends a semantics action to Flutter's engine, without any additional arguments. */ - public void dispatchSemanticsAction(int id, @NonNull AccessibilityBridge.Action action) { - dispatchSemanticsAction(id, action, null); + public void dispatchSemanticsAction(int nodeId, @NonNull AccessibilityBridge.Action action) { + dispatchSemanticsAction(nodeId, action, null); } /** Sends a semantics action to Flutter's engine, with additional arguments. */ public void dispatchSemanticsAction( - int id, @NonNull AccessibilityBridge.Action action, @Nullable Object args) { + int nodeId, @NonNull AccessibilityBridge.Action action, @Nullable Object args) { ensureAttachedToNative(); ByteBuffer encodedArgs = null; @@ -802,7 +802,7 @@ public void dispatchSemanticsAction( encodedArgs = StandardMessageCodec.INSTANCE.encodeMessage(args); position = encodedArgs.position(); } - dispatchSemanticsAction(id, action.value, encodedArgs, position); + dispatchSemanticsAction(nodeId, action.value, encodedArgs, position); } /** @@ -815,14 +815,18 @@ public void dispatchSemanticsAction( */ @UiThread public void dispatchSemanticsAction( - int id, int action, @Nullable ByteBuffer args, int argsPosition) { + int nodeId, int action, @Nullable ByteBuffer args, int argsPosition) { ensureRunningOnMainThread(); ensureAttachedToNative(); - nativeDispatchSemanticsAction(nativeShellHolderId, id, action, args, argsPosition); + nativeDispatchSemanticsAction(nativeShellHolderId, nodeId, action, args, argsPosition); } private native void nativeDispatchSemanticsAction( - long nativeShellHolderId, int id, int action, @Nullable ByteBuffer args, int argsPosition); + long nativeShellHolderId, + int nodeId, + int action, + @Nullable ByteBuffer args, + int argsPosition); /** * Instructs Flutter to enable/disable its semantics tree, which is used by Flutter to support diff --git a/shell/platform/android/io/flutter/embedding/engine/renderer/FlutterRenderer.java b/shell/platform/android/io/flutter/embedding/engine/renderer/FlutterRenderer.java index 0d09e03f91619..e9559b4383b08 100644 --- a/shell/platform/android/io/flutter/embedding/engine/renderer/FlutterRenderer.java +++ b/shell/platform/android/io/flutter/embedding/engine/renderer/FlutterRenderer.java @@ -515,8 +515,8 @@ public void setSemanticsEnabled(boolean enabled) { // TODO(mattcarroll): describe the native behavior that this invokes public void dispatchSemanticsAction( - int id, int action, @Nullable ByteBuffer args, int argsPosition) { - flutterJNI.dispatchSemanticsAction(id, action, args, argsPosition); + int nodeId, int action, @Nullable ByteBuffer args, int argsPosition) { + flutterJNI.dispatchSemanticsAction(nodeId, action, args, argsPosition); } /** diff --git a/shell/platform/embedder/embedder.cc b/shell/platform/embedder/embedder.cc index 64b856971cb10..9888ad6676899 100644 --- a/shell/platform/embedder/embedder.cc +++ b/shell/platform/embedder/embedder.cc @@ -2451,7 +2451,7 @@ FlutterEngineResult FlutterEngineUpdateAccessibilityFeatures( FlutterEngineResult FlutterEngineDispatchSemanticsAction( FLUTTER_API_SYMBOL(FlutterEngine) engine, - uint64_t id, + uint64_t node_id, FlutterSemanticsAction action, const uint8_t* data, size_t data_length) { @@ -2461,7 +2461,7 @@ FlutterEngineResult FlutterEngineDispatchSemanticsAction( auto engine_action = static_cast(action); if (!reinterpret_cast(engine) ->DispatchSemanticsAction( - id, engine_action, + node_id, engine_action, fml::MallocMapping::Copy(data, data_length))) { return LOG_EMBEDDER_ERROR(kInternalInconsistency, "Could not dispatch semantics action."); diff --git a/shell/platform/embedder/embedder.h b/shell/platform/embedder/embedder.h index e4fd8cc217490..bff1be0df26c3 100644 --- a/shell/platform/embedder/embedder.h +++ b/shell/platform/embedder/embedder.h @@ -2305,7 +2305,7 @@ FlutterEngineResult FlutterEngineUpdateAccessibilityFeatures( /// @brief Dispatch a semantics action to the specified semantics node. /// /// @param[in] engine A running engine instance. -/// @param[in] identifier The semantics action identifier. +/// @param[in] node_id The semantics node identifier. /// @param[in] action The semantics action. /// @param[in] data Data associated with the action. /// @param[in] data_length The data length. @@ -2315,7 +2315,7 @@ FlutterEngineResult FlutterEngineUpdateAccessibilityFeatures( FLUTTER_EXPORT FlutterEngineResult FlutterEngineDispatchSemanticsAction( FLUTTER_API_SYMBOL(FlutterEngine) engine, - uint64_t id, + uint64_t node_id, FlutterSemanticsAction action, const uint8_t* data, size_t data_length); diff --git a/shell/platform/embedder/embedder_engine.cc b/shell/platform/embedder/embedder_engine.cc index 498546c089220..cc4da738f72fc 100644 --- a/shell/platform/embedder/embedder_engine.cc +++ b/shell/platform/embedder/embedder_engine.cc @@ -193,7 +193,7 @@ bool EmbedderEngine::SetAccessibilityFeatures(int32_t flags) { return true; } -bool EmbedderEngine::DispatchSemanticsAction(int id, +bool EmbedderEngine::DispatchSemanticsAction(int node_id, flutter::SemanticsAction action, fml::MallocMapping args) { if (!IsValid()) { @@ -203,7 +203,7 @@ bool EmbedderEngine::DispatchSemanticsAction(int id, if (!platform_view) { return false; } - platform_view->DispatchSemanticsAction(id, action, std::move(args)); + platform_view->DispatchSemanticsAction(node_id, action, std::move(args)); return true; } diff --git a/shell/platform/embedder/embedder_engine.h b/shell/platform/embedder/embedder_engine.h index 80d2cbeffa8d0..60d33ba76e652 100644 --- a/shell/platform/embedder/embedder_engine.h +++ b/shell/platform/embedder/embedder_engine.h @@ -65,7 +65,7 @@ class EmbedderEngine { bool SetAccessibilityFeatures(int32_t flags); - bool DispatchSemanticsAction(int id, + bool DispatchSemanticsAction(int node_id, flutter::SemanticsAction action, fml::MallocMapping args);