[Impeller] its not safe to presentWithTransaction from a background thread. #45182
Conversation
|
There are a number of alternative solutions to this, all of which are fairly risky, so instead we should start with the safe option. |
| // presented. If there is a non-Flutter UIView active, such as in add2app or a | ||
| // presentViewController page transition, then this will cause CoreAnimation assertion errors and | ||
| // exit the app. | ||
| layer.presentsWithTransaction = [[NSThread currentThread] isMainThread]; |
There was a problem hiding this comment.
Wont' this re-introduce the bug we cherry picked a fix for into 3.10?
There was a problem hiding this comment.
Is there any way we could add some tests in this? Explicitly start a transaction on the main thread, then try to commit one on the background?
There was a problem hiding this comment.
We can add tests in the framework repo. The assertion error cannot be triggered on Simulators IIRC
|
The actual error is:
If you trigger animations on the main thread like pushing a new view controller with an animation (what the video player does). when we set presentsWithTransaction, that implicitly creates a CATransaction for the background thread (though we can see the same effect by manually creating the transaction). When we [present drawable], that marks the UIViews up tot the root dirty, which triggers a layout, and then triggers the layout off main thread error. |
|
I think this LGTM. Also, found some old context: |
|
Going to land this now while we work on figuring out a better fix going forward. |
jonahwilliams
added
the
autosubmit
…133580) flutter/engine@d1e6eb0...73cc3fb 2023-08-29 skia-flutter-autoroll@skia.org Roll Skia from b7e826d5324f to 15de7f9c3b3b (3 revisions) (flutter/engine#45231) 2023-08-29 jonahwilliams@google.com [Impeller] its not safe to presentWithTransaction from a background thread. (flutter/engine#45182) 2023-08-29 skia-flutter-autoroll@skia.org Roll Skia from b0c1b2868129 to b7e826d5324f (4 revisions) (flutter/engine#45229) 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 aaclarke@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
…hread. (flutter#45182) Sad Trombone. Because Flutter runs from a background thread, if we touch a CATransaction while there is a main thread transaction occuring then we'll trigger a framework error. Fixes flutter/flutter#133147
Sad Trombone. Because Flutter runs from a background thread, if we touch a CATransaction while there is a main thread transaction occuring then we'll trigger a framework error.
Fixes flutter/flutter#133147