Move ownership of `AccessibilityBridgeWindows` to `FlutterWindowsView` #41308

yaakovschectman · 2023-04-18T19:51:44Z

Move ownership of the pointer to AccessibilityBridgeWindows from FlutterWindowsEngine to FlutterWindowsView in preparation for multi-window.

flutter/flutter#124995

Pre-launch Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
I listed at least one issue that this PR fixes in the description above.
I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See testing the engine for instructions on writing and running engine tests.
I updated/added relevant documentation (doc comments with ///).
I signed the CLA.
All existing and new tests are passing.

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

yaakovschectman · 2023-04-18T19:53:06Z

shell/platform/windows/flutter_windows_view.h

@@ -200,10 +201,18 @@ class FlutterWindowsView : public WindowBindingHandlerDelegate,
  // |WindowBindingHandlerDelegate|
  virtual ui::AXFragmentRootDelegateWin* GetAxFragmentRootDelegate() override;

+  // Called to re/set the accessibility bridge pointer.
+  virtual void UpdateSemanticsEnabled(bool enabled);


The content of this new method could have instead been placed in OnUpdateSemanticsEnabled, which is what calls FlutterWindowsEngine::UpdateSemanticsEnabled. However, once we have multiple views, a call to OnUpdateSemanticsEnabled on a view would not necessarily call it for other views.

flutter-dashboard · 2023-04-18T22:27:51Z

Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change).

If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review.

Changes reported for pull request #41308 at sha 847dd86

shell/platform/windows/flutter_windows_view.h

shell/platform/windows/accessibility_bridge_windows_unittests.cc

shell/platform/windows/flutter_windows_engine.cc

loic-sharma

Looks good!

…indowsView` (flutter/engine#41308)

…125271) flutter/engine@2db85cb...122c3b3 2023-04-21 737941+loic-sharma@users.noreply.github.com [Windows] Don't block raster thread until v-blank (flutter/engine#41231) 2023-04-21 chillers@google.com Manual roll skia to d5b4acfb4 (flutter/engine#41378) 2023-04-21 magder@google.com Run mac unopt arm builds with arm toolchain (flutter/engine#41353) 2023-04-20 zanderso@users.noreply.github.com Revert "re-land "Migrate mac_host_engine to engine v2 builds." (#41233)"" (flutter/engine#41380) 2023-04-20 skia-flutter-autoroll@skia.org Roll Dart SDK from df05e451b79a to 50b96abe9f6f (1 revision) (flutter/engine#41379) 2023-04-20 109111084+yaakovschectman@users.noreply.github.com Move ownership of `AccessibilityBridgeWindows` to `FlutterWindowsView` (flutter/engine#41308) 2023-04-20 skia-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from AoPEjX8Xfq1v0h4kx... to PqBDstaESE_l77k1e... (flutter/engine#41373) 2023-04-20 godofredoc@google.com Revert "Upload windows arm artifacts to production bucket." (flutter/engine#41372) 2023-04-20 godofredoc@google.com re-land "Migrate mac_host_engine to engine v2 builds." (#41233)" (flutter/engine#41323) 2023-04-20 godofredoc@google.com Upload windows arm artifacts to production bucket. (flutter/engine#41324) 2023-04-20 jason-simmons@users.noreply.github.com [Impeller] Change the default color format for the GLES backend to RGBA (flutter/engine#41342) 2023-04-20 maRci002@users.noreply.github.com [web] change status bar color based on SystemUiOverlayStyle (flutter/engine#40599) 2023-04-20 skia-flutter-autoroll@skia.org Roll Fuchsia Mac SDK from OcPCdaE17MAihaCrD... to 4OrPF9lzqCKGwBLRh... (flutter/engine#41367) 2023-04-20 skia-flutter-autoroll@skia.org Roll Skia from fc09f9b2fb27 to f4609aa2eaba (1 revision) (flutter/engine#41366) 2023-04-20 skia-flutter-autoroll@skia.org Roll Dart SDK from 7d165bd0bb5e to df05e451b79a (2 revisions) (flutter/engine#41365) 2023-04-20 skia-flutter-autoroll@skia.org Roll Skia from 80c38970791e to fc09f9b2fb27 (1 revision) (flutter/engine#41362) 2023-04-20 skia-flutter-autoroll@skia.org Roll Skia from c50081c62219 to 80c38970791e (2 revisions) (flutter/engine#41360) 2023-04-20 skia-flutter-autoroll@skia.org Roll Skia from c21e7df194c3 to c50081c62219 (11 revisions) (flutter/engine#41358) 2023-04-20 skia-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from Tun7i4VLz6ncx8JJJ... to AoPEjX8Xfq1v0h4kx... (flutter/engine#41357) 2023-04-20 skia-flutter-autoroll@skia.org Roll Dart SDK from 88a3b66b50d6 to 7d165bd0bb5e (1 revision) (flutter/engine#41356) 2023-04-20 jason-simmons@users.noreply.github.com Manual Skia roll from ad90b6bd4760 to c21e7df194c3 (flutter/engine#41341) 2023-04-20 jonahwilliams@google.com [impeller] convert src over to src for solid color (flutter/engine#41351) Also rolling transitive DEPS: fuchsia/sdk/core/linux-amd64 from Tun7i4VLz6nc to PqBDstaESE_l fuchsia/sdk/core/mac-amd64 from OcPCdaE17MAi to 4OrPF9lzqCKG If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC chinmaygarde@google.com,rmistry@google.com,zra@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

The ownership of the accessibility bridge was moved from `FlutterWindowsEngine` to `FlutterWindowsView`. This change moves leftover accessibility bridge helpers/logic from the engine to the view. Addresses: flutter/flutter#124995 Follow-up to: #41308 [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style

…r#43710) The ownership of the accessibility bridge was moved from `FlutterWindowsEngine` to `FlutterWindowsView`. This change moves leftover accessibility bridge helpers/logic from the engine to the view. Addresses: flutter/flutter#124995 Follow-up to: flutter#41308 [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style

yaakovschectman added 3 commits April 18, 2023 10:39

A11ybridge ctor

f153826

Move bridge

73b1f36

Comments

b9043ee

yaakovschectman added accessibility affects: desktop platform-windows labels Apr 18, 2023

yaakovschectman requested review from cbracken, loic-sharma and dkwingsmt April 18, 2023 19:51

yaakovschectman self-assigned this Apr 18, 2023

yaakovschectman commented Apr 18, 2023

View reviewed changes

Format

f0d39ee

yaakovschectman marked this pull request as ready for review April 18, 2023 20:22

Merge branch 'main' into win_multi_bridge

847dd86

flutter-dashboard bot added the will affect goldens label Apr 18, 2023

Merge branch 'main' into win_multi_bridge

d560d20

yaakovschectman requested a review from chunhtai April 19, 2023 19:57

loic-sharma reviewed Apr 19, 2023

View reviewed changes

shell/platform/windows/flutter_windows_view.h Outdated Show resolved Hide resolved

dkwingsmt reviewed Apr 19, 2023

View reviewed changes

shell/platform/windows/accessibility_bridge_windows_unittests.cc Outdated Show resolved Hide resolved

shell/platform/windows/flutter_windows_engine.cc Outdated Show resolved Hide resolved

loic-sharma approved these changes Apr 19, 2023

View reviewed changes

yaakovschectman and others added 3 commits April 20, 2023 10:12

PR Feedback

ffceb1a

Formatting

a55be1a

Merge branch 'main' into win_multi_bridge

818a072

yaakovschectman merged commit 7574a0c into flutter:main Apr 20, 2023

yaakovschectman deleted the win_multi_bridge branch April 20, 2023 20:28

engine-flutter-autoroll mentioned this pull request Apr 20, 2023

Roll Flutter Engine from 2db85cbe6a01 to 7574a0cb864b (17 revisions) flutter/flutter#125249

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 20, 2023

7574a0cb8 Move ownership of AccessibilityBridgeWindows to `FlutterW…

1994074

…indowsView` (flutter/engine#41308)

engine-flutter-autoroll mentioned this pull request Apr 20, 2023

Roll Flutter Engine from 2db85cbe6a01 to 7574a0cb864b (17 revisions) flutter/flutter#125260

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 20, 2023

7574a0cb8 Move ownership of AccessibilityBridgeWindows to `FlutterW…

461912c

…indowsView` (flutter/engine#41308)

engine-flutter-autoroll mentioned this pull request Apr 21, 2023

Roll Flutter Engine from 2db85cbe6a01 to 351037901f3a (18 revisions) flutter/flutter#125264

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 21, 2023

7574a0cb8 Move ownership of AccessibilityBridgeWindows to `FlutterW…

546823b

…indowsView` (flutter/engine#41308)

engine-flutter-autoroll mentioned this pull request Apr 21, 2023

Roll Flutter Engine from 2db85cbe6a01 to ff0ff3cbd1db (19 revisions) flutter/flutter#125267

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 21, 2023

7574a0cb8 Move ownership of AccessibilityBridgeWindows to `FlutterW…

b386dff

…indowsView` (flutter/engine#41308)

engine-flutter-autoroll mentioned this pull request Apr 21, 2023

Roll Flutter Engine from 2db85cbe6a01 to a2b02b8b95d5 (21 revisions) flutter/flutter#125269

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 21, 2023

7574a0cb8 Move ownership of AccessibilityBridgeWindows to `FlutterW…

aede6c9

…indowsView` (flutter/engine#41308)

engine-flutter-autoroll mentioned this pull request Apr 21, 2023

Roll Flutter Engine from 2db85cbe6a01 to 122c3b382012 (22 revisions) flutter/flutter#125271

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 21, 2023

7574a0cb8 Move ownership of AccessibilityBridgeWindows to `FlutterW…

71fd4c1

…indowsView` (flutter/engine#41308)

loic-sharma mentioned this pull request Jul 14, 2023

[Windows] Remove accessibility bridge helpers from the engine #43710

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Move ownership of `AccessibilityBridgeWindows` to `FlutterWindowsView` #41308

Move ownership of `AccessibilityBridgeWindows` to `FlutterWindowsView` #41308

Uh oh!

yaakovschectman commented Apr 18, 2023

Uh oh!

yaakovschectman Apr 18, 2023

Uh oh!

flutter-dashboard bot commented Apr 18, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

loic-sharma left a comment

Uh oh!

Uh oh!

Move ownership of AccessibilityBridgeWindows to FlutterWindowsView #41308

Move ownership of AccessibilityBridgeWindows to FlutterWindowsView #41308

Uh oh!

Conversation

yaakovschectman commented Apr 18, 2023

Pre-launch Checklist

Uh oh!

yaakovschectman Apr 18, 2023

Choose a reason for hiding this comment

Uh oh!

flutter-dashboard bot commented Apr 18, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

loic-sharma left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Move ownership of `AccessibilityBridgeWindows` to `FlutterWindowsView` #41308

Move ownership of `AccessibilityBridgeWindows` to `FlutterWindowsView` #41308