Skip to content

Commit

Permalink
Clarify semantics action dispatch id parameter (flutter#38356)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
cbracken authored Dec 16, 2022
1 parent d91e208 commit 1fe1ec4
Show file tree
Hide file tree
Showing 20 changed files with 54 additions and 48 deletions.
4 changes: 2 additions & 2 deletions lib/ui/hooks.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down
10 changes: 5 additions & 5 deletions lib/ui/platform_dispatcher.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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.
///
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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<int, SemanticsAction, ByteData?>(
onSemanticsAction,
_onSemanticsActionZone,
id,
nodeId,
SemanticsAction.values[action]!,
args,
);
Expand Down
4 changes: 2 additions & 2 deletions lib/ui/window/platform_configuration.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<tonic::DartState> dart_state =
Expand All @@ -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<int32_t>(action)),
{tonic::ToDart(node_id), tonic::ToDart(static_cast<int32_t>(action)),
args_handle}));
}

Expand Down
4 changes: 2 additions & 2 deletions lib/ui/window/platform_configuration.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
2 changes: 1 addition & 1 deletion lib/web_ui/lib/platform_dispatcher.dart
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ typedef FrameCallback = void Function(Duration duration);
typedef TimingsCallback = void Function(List<FrameTiming> 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);
Expand Down
4 changes: 2 additions & 2 deletions lib/web_ui/lib/src/engine/platform_dispatcher.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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<int, ui.SemanticsAction, ByteData?>(
_onSemanticsAction, _onSemanticsActionZone, id, action, args);
_onSemanticsAction, _onSemanticsActionZone, nodeId, action, args);
}

// TODO(dnfield): make this work on web.
Expand Down
4 changes: 2 additions & 2 deletions runtime/runtime_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
4 changes: 2 additions & 2 deletions runtime/runtime_controller.h
Original file line number Diff line number Diff line change
Expand Up @@ -407,15 +407,15 @@ 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.
///
/// @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);

Expand Down
5 changes: 3 additions & 2 deletions shell/common/engine.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
4 changes: 2 additions & 2 deletions shell/common/engine.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
5 changes: 3 additions & 2 deletions shell/common/platform_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
8 changes: 4 additions & 4 deletions shell/common/platform_view.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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);

Expand Down
6 changes: 3 additions & 3 deletions shell/common/shell.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
}));
}
Expand Down
2 changes: 1 addition & 1 deletion shell/common/shell.h
Original file line number Diff line number Diff line change
Expand Up @@ -545,7 +545,7 @@ class Shell final : public PlatformView::Delegate,
std::unique_ptr<PointerDataPacket> packet) override;

// |PlatformView::Delegate|
void OnPlatformViewDispatchSemanticsAction(int32_t id,
void OnPlatformViewDispatchSemanticsAction(int32_t node_id,
SemanticsAction action,
fml::MallocMapping args) override;

Expand Down
18 changes: 11 additions & 7 deletions shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
}

/**
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

/**
Expand Down
4 changes: 2 additions & 2 deletions shell/platform/embedder/embedder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -2461,7 +2461,7 @@ FlutterEngineResult FlutterEngineDispatchSemanticsAction(
auto engine_action = static_cast<flutter::SemanticsAction>(action);
if (!reinterpret_cast<flutter::EmbedderEngine*>(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.");
Expand Down
4 changes: 2 additions & 2 deletions shell/platform/embedder/embedder.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions shell/platform/embedder/embedder_engine.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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()) {
Expand All @@ -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;
}

Expand Down
2 changes: 1 addition & 1 deletion shell/platform/embedder/embedder_engine.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down

0 comments on commit 1fe1ec4

Please sign in to comment.