Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

[Impeller] its not safe to presentWithTransaction from a background thread. #45182

Merged
merged 1 commit into from
Aug 29, 2023
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions shell/platform/darwin/ios/ios_surface_metal_impeller.mm
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,11 @@

// When there are platform views in the scene, the drawable needs to be presented in the same
// transaction as the one created for platform views. When the drawable are being presented from
// the raster thread, participate with the implicit CA transaction.
layer.presentsWithTransaction = YES;
// the raster thread, we may not be able to use a transaction as it will dirty the UIViews being
// 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];
Copy link
Contributor

Choose a reason for hiding this comment

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

Wont' this re-introduce the bug we cherry picked a fix for into 3.10?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yup

Copy link
Contributor

Choose a reason for hiding this comment

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

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?

Copy link
Member Author

Choose a reason for hiding this comment

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

We can add tests in the framework repo. The assertion error cannot be triggered on Simulators IIRC


return layer;
}
Expand Down