Skip to content

macOS: Record test accessibility notifications in a dictionary - #191854

Merged
cbracken merged 1 commit into
flutter:masterfrom
cbracken:eliminate-more-pointless-conversions
Aug 28, 2026
Merged

cbracken merged 1 commit into
flutter:masterfrom
cbracken:eliminate-more-pointless-conversions

Conversation

@cbracken

Copy link
Copy Markdown
Member

AccessibilityBridgeMacSpy was recording the notifications it saw in a std::unordered_map<std::string, gfx::NativeViewAccessible>, converting the NSAccessibilityNotificationName key to a std::string on the way in and back again on the way out.

Both the key and value are Obj-C objects, so we can use an NSMutableDictionary and avoid all the conversions. Similar to #191853, std::string conversions use strlen to determine the length, which truncates at any embedded NUL chars, potentially hiding bugs.

Spotted while working on: #191616

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

If this change needs to override an active code freeze, provide a comment explaining why. The code freeze workflow can be overridden by code reviewers. See pinned issues for any active code freezes with guidance.

Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the gemini-code-assist bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.

`AccessibilityBridgeMacSpy` was recording the notifications it saw in a
`std::unordered_map<std::string, gfx::NativeViewAccessible>`, converting
the `NSAccessibilityNotificationName` key to a `std::string` on the way
in and back again on the way out.

Both the key and value are Obj-C objects, so we can use an
`NSMutableDictionary` and avoid all the conversions. Similar to flutter#191853,
`std::string` conversions use `strlen` to determine the length, which
truncates at any embedded NUL chars, potentially hiding bugs.

Spotted while working on: flutter#191616
@cbracken
cbracken requested a review from a team as a code owner August 27, 2026 03:55
@flutter-dashboard flutter-dashboard Bot added the CICD Run CI/CD label Aug 27, 2026
@github-actions github-actions Bot added engine flutter/engine related. See also e: labels. platform-macos Building on or for macOS specifically a: desktop Running on desktop team-macos Owned by the macOS platform team labels Aug 27, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the AccessibilityBridgeMacSpy test helper in AccessibilityBridgeMacTest.mm to use an Objective-C NSMutableDictionary instead of a C++ std::unordered_map for tracking notifications. This simplifies the notification dispatching and lookup by avoiding conversions between NSAccessibilityNotificationName and std::string. The associated test assertions have been updated accordingly to use Objective-C dictionary APIs. There are no review comments, and I have no additional feedback to provide.

@cbracken cbracken changed the title macOS: Record spied accessibility notifications in a dictionary macOS: Record accessibility notifications in a dictionary Aug 27, 2026
@cbracken cbracken changed the title macOS: Record accessibility notifications in a dictionary macOS: Record test accessibility notifications in a dictionary Aug 27, 2026
pull Bot pushed a commit to Mu-L/flutter that referenced this pull request Aug 28, 2026
…1855)

`FlutterPlatformViewsController` was holding its view factories and
their gesture blocking policies in `std::unordered_map`s keyed by
`std::string`, so both the registration path and the create path
converted an `NSString` to a `std::string` first. But all the mapped
values are Obj-C types so we can migrate to na `NSMutableDictionary`.
This matches how we do it in the macOS embedder.

This removes the last of the `UTF8String` conversions in this file. As
noted in flutter#191853 and flutter#191854 `std::string`'s `const char*` constructor
determines the length using `strlen`, so it truncates at an embedded NUL
and dereferences null if `UTF8String` ever returns NULL, which it does
for a nil receiver.

Spotted while working on:
flutter#191616

No new tests since this just changes the implementation without changing
any semantics.

<!--
Thanks for filing a pull request!
Reviewers are typically assigned within a week of filing a request.
To learn more about code review, see our documentation on Tree Hygiene:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md
-->


## Pre-launch Checklist

- [X] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [X] I read the [AI contribution guidelines] and understand my
responsibilities, or I am not using AI tools.
- [X] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [X] I read and followed the [Flutter Style Guide], including [Features
we expect every widget to implement].
- [X] I signed the [CLA].
- [X] I listed at least one issue that this PR fixes in the description
above.
- [X] I updated/added relevant documentation (doc comments with `///`).
- [X] I added new tests to check the change I am making, or this PR is
[test-exempt].
- [X] I followed the [breaking change policy] and added [Data Driven
Fixes] where supported.
- [X] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel
on [Discord].

If this change needs to override an active code freeze, provide a
comment explaining why. The code freeze workflow can be overridden by
code reviewers. See pinned issues for any active code freezes with
guidance.

**Note**: The Flutter team is currently trialing the use of [Gemini Code
Assist for
GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code).
Comments from the `gemini-code-assist` bot should not be taken as
authoritative feedback from the Flutter team. If you find its comments
useful you can update your code accordingly, but if you are unsure or
disagree with the feedback, please feel free to wait for a Flutter team
member's review for guidance on which automated comments should be
addressed.

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview
[AI contribution guidelines]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#ai-contribution-guidelines
[Tree Hygiene]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md
[test-exempt]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes
[Discord]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md
[Data Driven Fixes]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
@cbracken
cbracken added this pull request to the merge queue Aug 28, 2026
Merged via the queue into flutter:master with commit 48da26a Aug 28, 2026
24 of 25 checks passed
@cbracken
cbracken deleted the eliminate-more-pointless-conversions branch August 28, 2026 02:41
auto-submit Bot pushed a commit to flutter/packages that referenced this pull request Aug 28, 2026
flutter/flutter@e8dca90...8d8c1aa

2026-08-28 [email protected] Roll Packages from bd3cbc1 to cd4cdd0 (15 revisions) (flutter/flutter#191965)
2026-08-28 [email protected] Bump customer testing version to pick up forui tests (flutter/flutter#191931)
2026-08-28 [email protected] Use leading scroll offset instead of infinite offset to reveal pinned slivers (flutter/flutter#191329)
2026-08-28 [email protected] Roll Skia from 21f71051e749 to fe0b9a5261f9 (1 revision) (flutter/flutter#191960)
2026-08-28 [email protected] Share the common Linux window controller code in mixins (flutter/flutter#191930)
2026-08-28 [email protected] Roll Dart SDK from 2cf2f69fd9f0 to ae63a6eca3f1 (1 revision) (flutter/flutter#191953)
2026-08-28 [email protected] Roll Skia from 11e3a7cb0fd0 to 21f71051e749 (3 revisions) (flutter/flutter#191942)
2026-08-28 [email protected] Roll Dart SDK from e030e77d5413 to 2cf2f69fd9f0 (2 revisions) (flutter/flutter#191934)
2026-08-28 [email protected] Remove no-shuffle in flutter tools overall experience test (flutter/flutter#191861)
2026-08-28 [email protected] [flutter_tools] Gracefully handle Windows Application Control and security policy execution blocks (flutter/flutter#191912)
2026-08-28 [email protected] [tool] Migrate ConfigCommand to modular dependency injection (flutter/flutter#190743)
2026-08-28 [email protected] Roll Skia from 37459da0a755 to 11e3a7cb0fd0 (9 revisions) (flutter/flutter#191929)
2026-08-28 [email protected] Connect to moved-to-rect after the window is realized (flutter/flutter#191773)
2026-08-28 [email protected] macOS: Compare accessibility strings as NSStrings in tests (flutter/flutter#191853)
2026-08-28 [email protected] macOS: Record test accessibility notifications in a dictionary (flutter/flutter#191854)
2026-08-28 [email protected] Roll Fuchsia Test Scripts from 6SXRz6wsA2Zm_rAqP... to u_fSMsPU22VvUsvSo... (flutter/flutter#191927)
2026-08-28 [email protected] Roll Dart SDK from cb4c39d067f3 to e030e77d5413 (3 revisions) (flutter/flutter#191923)
2026-08-27 [email protected] Migrate ios driver tests batch 2 to simulators (flutter/flutter#190930)
2026-08-27 [email protected] [flutter_test] Ensure LiveTestWidgetsFlutterBinding executes frame and pointer callbacks in the test zone (flutter/flutter#191595)
2026-08-27 [email protected] Deprecate legacy gen_defaults tool (flutter/flutter#191911)
2026-08-27 [email protected] Fix black window in the subsurface view renderer (flutter/flutter#191850)
2026-08-27 [email protected] Roll Skia from 82e972ea84b1 to 37459da0a755 (10 revisions) (flutter/flutter#191906)
2026-08-27 [email protected] iOS: Key the platform view factory registries by NSString (flutter/flutter#191855)
2026-08-27 [email protected] Sync CHANGELOG.md from stable (flutter/flutter#191895)
2026-08-27 [email protected] [flutter_tools] Honor configured build-dir across flutter project and build targets (flutter/flutter#191592)
2026-08-27 [email protected] Revert: Handle linear and radial gradients in UberSDF (flutter/flutter#191913)
2026-08-27 [email protected] Explicitly disable HCPP in hybrid_android_views integration test (flutter/flutter#191894)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC [email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
victorsanni pushed a commit to victorsanni/packages that referenced this pull request Sep 9, 2026
…r#12679)

flutter/flutter@e8dca90...8d8c1aa

2026-08-28 [email protected] Roll Packages from bd3cbc1 to cd4cdd0 (15 revisions) (flutter/flutter#191965)
2026-08-28 [email protected] Bump customer testing version to pick up forui tests (flutter/flutter#191931)
2026-08-28 [email protected] Use leading scroll offset instead of infinite offset to reveal pinned slivers (flutter/flutter#191329)
2026-08-28 [email protected] Roll Skia from 21f71051e749 to fe0b9a5261f9 (1 revision) (flutter/flutter#191960)
2026-08-28 [email protected] Share the common Linux window controller code in mixins (flutter/flutter#191930)
2026-08-28 [email protected] Roll Dart SDK from 2cf2f69fd9f0 to ae63a6eca3f1 (1 revision) (flutter/flutter#191953)
2026-08-28 [email protected] Roll Skia from 11e3a7cb0fd0 to 21f71051e749 (3 revisions) (flutter/flutter#191942)
2026-08-28 [email protected] Roll Dart SDK from e030e77d5413 to 2cf2f69fd9f0 (2 revisions) (flutter/flutter#191934)
2026-08-28 [email protected] Remove no-shuffle in flutter tools overall experience test (flutter/flutter#191861)
2026-08-28 [email protected] [flutter_tools] Gracefully handle Windows Application Control and security policy execution blocks (flutter/flutter#191912)
2026-08-28 [email protected] [tool] Migrate ConfigCommand to modular dependency injection (flutter/flutter#190743)
2026-08-28 [email protected] Roll Skia from 37459da0a755 to 11e3a7cb0fd0 (9 revisions) (flutter/flutter#191929)
2026-08-28 [email protected] Connect to moved-to-rect after the window is realized (flutter/flutter#191773)
2026-08-28 [email protected] macOS: Compare accessibility strings as NSStrings in tests (flutter/flutter#191853)
2026-08-28 [email protected] macOS: Record test accessibility notifications in a dictionary (flutter/flutter#191854)
2026-08-28 [email protected] Roll Fuchsia Test Scripts from 6SXRz6wsA2Zm_rAqP... to u_fSMsPU22VvUsvSo... (flutter/flutter#191927)
2026-08-28 [email protected] Roll Dart SDK from cb4c39d067f3 to e030e77d5413 (3 revisions) (flutter/flutter#191923)
2026-08-27 [email protected] Migrate ios driver tests batch 2 to simulators (flutter/flutter#190930)
2026-08-27 [email protected] [flutter_test] Ensure LiveTestWidgetsFlutterBinding executes frame and pointer callbacks in the test zone (flutter/flutter#191595)
2026-08-27 [email protected] Deprecate legacy gen_defaults tool (flutter/flutter#191911)
2026-08-27 [email protected] Fix black window in the subsurface view renderer (flutter/flutter#191850)
2026-08-27 [email protected] Roll Skia from 82e972ea84b1 to 37459da0a755 (10 revisions) (flutter/flutter#191906)
2026-08-27 [email protected] iOS: Key the platform view factory registries by NSString (flutter/flutter#191855)
2026-08-27 [email protected] Sync CHANGELOG.md from stable (flutter/flutter#191895)
2026-08-27 [email protected] [flutter_tools] Honor configured build-dir across flutter project and build targets (flutter/flutter#191592)
2026-08-27 [email protected] Revert: Handle linear and radial gradients in UberSDF (flutter/flutter#191913)
2026-08-27 [email protected] Explicitly disable HCPP in hybrid_android_views integration test (flutter/flutter#191894)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC [email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
victorsanni pushed a commit to victorsanni/packages that referenced this pull request Sep 9, 2026
…r#12679)

flutter/flutter@e8dca90...8d8c1aa

2026-08-28 [email protected] Roll Packages from bd3cbc1 to cd4cdd0 (15 revisions) (flutter/flutter#191965)
2026-08-28 [email protected] Bump customer testing version to pick up forui tests (flutter/flutter#191931)
2026-08-28 [email protected] Use leading scroll offset instead of infinite offset to reveal pinned slivers (flutter/flutter#191329)
2026-08-28 [email protected] Roll Skia from 21f71051e749 to fe0b9a5261f9 (1 revision) (flutter/flutter#191960)
2026-08-28 [email protected] Share the common Linux window controller code in mixins (flutter/flutter#191930)
2026-08-28 [email protected] Roll Dart SDK from 2cf2f69fd9f0 to ae63a6eca3f1 (1 revision) (flutter/flutter#191953)
2026-08-28 [email protected] Roll Skia from 11e3a7cb0fd0 to 21f71051e749 (3 revisions) (flutter/flutter#191942)
2026-08-28 [email protected] Roll Dart SDK from e030e77d5413 to 2cf2f69fd9f0 (2 revisions) (flutter/flutter#191934)
2026-08-28 [email protected] Remove no-shuffle in flutter tools overall experience test (flutter/flutter#191861)
2026-08-28 [email protected] [flutter_tools] Gracefully handle Windows Application Control and security policy execution blocks (flutter/flutter#191912)
2026-08-28 [email protected] [tool] Migrate ConfigCommand to modular dependency injection (flutter/flutter#190743)
2026-08-28 [email protected] Roll Skia from 37459da0a755 to 11e3a7cb0fd0 (9 revisions) (flutter/flutter#191929)
2026-08-28 [email protected] Connect to moved-to-rect after the window is realized (flutter/flutter#191773)
2026-08-28 [email protected] macOS: Compare accessibility strings as NSStrings in tests (flutter/flutter#191853)
2026-08-28 [email protected] macOS: Record test accessibility notifications in a dictionary (flutter/flutter#191854)
2026-08-28 [email protected] Roll Fuchsia Test Scripts from 6SXRz6wsA2Zm_rAqP... to u_fSMsPU22VvUsvSo... (flutter/flutter#191927)
2026-08-28 [email protected] Roll Dart SDK from cb4c39d067f3 to e030e77d5413 (3 revisions) (flutter/flutter#191923)
2026-08-27 [email protected] Migrate ios driver tests batch 2 to simulators (flutter/flutter#190930)
2026-08-27 [email protected] [flutter_test] Ensure LiveTestWidgetsFlutterBinding executes frame and pointer callbacks in the test zone (flutter/flutter#191595)
2026-08-27 [email protected] Deprecate legacy gen_defaults tool (flutter/flutter#191911)
2026-08-27 [email protected] Fix black window in the subsurface view renderer (flutter/flutter#191850)
2026-08-27 [email protected] Roll Skia from 82e972ea84b1 to 37459da0a755 (10 revisions) (flutter/flutter#191906)
2026-08-27 [email protected] iOS: Key the platform view factory registries by NSString (flutter/flutter#191855)
2026-08-27 [email protected] Sync CHANGELOG.md from stable (flutter/flutter#191895)
2026-08-27 [email protected] [flutter_tools] Honor configured build-dir across flutter project and build targets (flutter/flutter#191592)
2026-08-27 [email protected] Revert: Handle linear and radial gradients in UberSDF (flutter/flutter#191913)
2026-08-27 [email protected] Explicitly disable HCPP in hybrid_android_views integration test (flutter/flutter#191894)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC [email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a: desktop Running on desktop CICD Run CI/CD engine flutter/engine related. See also e: labels. platform-macos Building on or for macOS specifically team-macos Owned by the macOS platform team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants