engine icon indicating copy to clipboard operation
engine copied to clipboard

Isolate the AccessibilityBridge from single views

Open dkwingsmt opened this issue 2 years ago • 1 comments

This PR changes the accessibility bridge and its delegate classes, so that they no longer assume that the embedding engine only has one view or one bridge, but instead receive their view and bridge through their constructors.

With this PR, all uses of FlutterWindowsEngine::accessibility_bridge() and FlutterEngine.accessibilityBridge in production code have been removed. They're now only referred in unit tests.

Why is this needed?

Part of the multiwindow project (design doc): In the multi-view world, FlutterEngine will not only have multiple view controllers, but also multiple accessiblity bridges, each corresponding to a view. This requires that we get rid of the GetView and GetBridge methods.

The decision of multiplying accessibility bridges is made because there's nothing in the bridge that keeps us from doing so, and the bridge works directly with the view it was given.

Design details

The hardest part of this change is a circular dependency between the bridge and the delegate. The bridge used to take the delegate in a constructor, while the delegate now also wants to refer to the bridge. To resolve this, either make the bridge take the delegate later, or make the delegate aware of the bridge later. I chose the first option, because the bridge already has an UpdateDelegate method.

I still kept AccessibilityBridge's one-argument constructor because many tests are still using it. Let me know if you'd like to all of the tests updated.

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.

dkwingsmt avatar Oct 03 '22 20:10 dkwingsmt

An alternative approach has been posted in https://github.com/flutter/engine/pull/36597.

dkwingsmt avatar Oct 04 '22 21:10 dkwingsmt

Closing this for now unless #36597 is denied.

dkwingsmt avatar Oct 21 '22 20:10 dkwingsmt