This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Guard against orphaned semantic objects from referencing dead accessibility bridge on iOS#13857
Merged
darrenaustin merged 3 commits intoflutter:masterfrom Nov 14, 2019
Merged
Conversation
darrenaustin
requested review from
dnfield,
goderbauer,
gspencergoog and
jonahwilliams
Contributor
chinmaygarde
left a comment
chinmaygarde
left a comment
There was a problem hiding this comment.
There is a pretty strong preference against preprocessor macros in the style guide. The pattern looks fine but I am in favor of just moving this to a method (say -[SemnticsObject isAccessibilityBridgeAlive]) with the same documentation and have the methods return the appropriate zeroed return value in case of the negative response.
Contributor
Author
|
Thanks for the feedback. Yeah, I am usually not a fan of macros either, but in this case I was trying to minimize the boilerplate. My first pass was exactly what you are suggesting. I will switch it back. |
chinmaygarde
approved these changes
Nov 14, 2019
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Nov 14, 2019
… accessibility bridge on iOS (flutter/engine#13857)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Nov 14, 2019
… accessibility bridge on iOS (flutter/engine#13857)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Nov 15, 2019
… accessibility bridge on iOS (flutter/engine#13857)
This was referenced Nov 15, 2019
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Nov 15, 2019
… accessibility bridge on iOS (flutter/engine#13857)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Nov 15, 2019
… accessibility bridge on iOS (flutter/engine#13857)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Nov 15, 2019
… accessibility bridge on iOS (flutter/engine#13857)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Nov 15, 2019
… accessibility bridge on iOS (flutter/engine#13857)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Nov 15, 2019
… accessibility bridge on iOS (flutter/engine#13857)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Nov 15, 2019
… accessibility bridge on iOS (flutter/engine#13857)
flar
pushed a commit
to flutter/flutter
that referenced
this pull request
Nov 15, 2019
* 174e0e9 Roll src/third_party/dart dc35290111..dc808f3fcb (5 commits) (flutter/engine#13859) * 33d997c Roll fuchsia/sdk/core/mac-amd64 from 7XOyl... to VMTIz... (flutter/engine#13861) * b4899d9 Roll src/third_party/skia d860a78fd60c..e57ca4931952 (44 commits) (flutter/engine#13862) * f456423 RendererContextSwitch guard flutter's gl context rework. (flutter/engine#13812) * 6bab64e Fix test to account for pixel ratio transformations being framework responsibility. (flutter/engine#13850) * 5b10fa3 Guard against orphaned semantic objects from referencing dead accessibility bridge on iOS (flutter/engine#13857) * 97df087 [fuchsia] Package flutter_frontend_server snapshot for fuchsia (flutter/engine#13865) * 0832dfd [flow][fuchsia] Add more tracing to layers and Fuchsia surface pool (flutter/engine#13864) * 141dc785d Roll src/third_party/skia e57ca4931952..c1c4634dcb07 (15 commits) (flutter/engine#13866) * 90a6054 Revert "Roll src/third_party/dart dc35290111..dc808f3fcb (5 commits) (#13859)" (flutter/engine#13867)
This was referenced Dec 16, 2019
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The core issue causing flutter/flutter#43795 is that when VoiceOver is turned off, it will still hang onto our
SemanticObjectsfor the focused node. In response to VoiceOver shutting down (via theUIAccessibilityVoiceOverStatusChangedmessage handler), we tear down theAccessibilityBridge. When this happens we release all of our SemanticsObjects and delete the bridge. However, because iOS will hold onto (at least in this case) the focused semantics object, when VoiceOver is turned back on it will notify the the semantic object that it no longer has focus. If the node tries to do anything that involves the bridge, it will try to dereference a dead weak pointer, so it crashes.I attempted several approaches to see if there was a way to get iOS to release the node (reseting the semantics tree to empty, sending various
UIAccessibilityPostNotificationmessages), but nothing would cause it to release the now defunct SemanticsObject.I looked into wrapping each of the
SemanticsObjectswith a safe guarding object that would only delegate to the real semantics object if the bridge were still active. This would be way more boilerplate than it would be worth (in addition to adding to taking up more memory for each node).So the solution I came up with is this. I added a new macro
RETURN_IF_ORPHANEDthat can be used at the top of any accessibility message handler in aSemanticsObject. It will check if the node's bridge is still active and if not, then just return immediately. This will guard against any references to a dead bridge. Not the most elegant solution, and I am open to better suggestions. Are there any similar patterns in use in the engine?Fixes: flutter/flutter#43795