-
Notifications
You must be signed in to change notification settings - Fork 6k
Only attempt surface creation in viewDidLayoutSubviews if the application is active. #19592
Conversation
…tion is active. UIKit does not guarantee that the application is in the active state when it lays out it subviews. However, in `viewDidLayoutSubviews`, Flutter attempts to create the render surface and wait for the first frame. These operations require GPU access which is forbidden when in the background on iOS. This layout while in the background only seems to happen under very specific circumstances as described in the reproducible test case. Notably, this behavior cannot be reproduced when the application is just launched with the "Background Fetch" XCode run scheme. The other instance of invocations to create the rendering surface have been audited and I can confirm that those are already in the correct lifecycle callbacks. This was the only instance where the surface creation was in a non-lifecycle callback. Fixes flutter/flutter#55969 (This issue has a clear reproducible test case and was used as the basis for verification.) Fixes flutter/flutter#57676 (This is significantly harder to reproduce but the issue looks to be the same. I tried this patch on multiple devices overnight and could not reproduce this.) This underlying cause here was identified by the enhanced GPU error reporting introduced in iOS 14. I am not able to submit those patches because the Beta builders are not available on infra. I will submit those when available so that issues like this have more actionable error messages in the future.
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat. Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
gaaclarke
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You might want to try making a unit test for this. If you partial mock the view controller, you can rip out surfaceUpdated and show that it doesn't get called. You'll have to find a way to make it think it isn't active though =T
I am not able to figure this out. In the meantime, I have verified this with all the devices I have access to. To your point about how the shell is launched when it is initially skipped, I seem to be getting both a viewDidLayoutSubviews and a viewWillAppear: then the application is eventually foregrounded. In both cases, the viewport metrics are non-zero and the launch continues. So I think this patch is safe. I am concerned that we are recreating the surface unnecessarily on launch but I don't think that is a concern for this patch. I filed flutter/flutter#61050 so we can go chase this down. |
… application is active. (flutter/engine#19592)
* a53782f Roll Skia from 6130d5079d55 to 0c0d8dd6d637 (3 revisions) (flutter/engine#19570) * 0541502 kick build (flutter/engine#19575) * 07d5090 Roll Skia from 0c0d8dd6d637 to cf5e35f72130 (13 revisions) (flutter/engine#19573) * 35b5aa5 switch const finder to package_config (flutter/engine#19576) * de0932b Manual roll of Dart 06cb010247...69aba23371 (flutter/engine#19577) * b919945 include list_libraries.dart as a snapshot for fuchsia (flutter/engine#19567) * 3fe5edf Roll Skia from cf5e35f72130 to b4d60f807dbd (5 revisions) (flutter/engine#19587) * a15bc1b [CanvasKit] Dispose the overlay surface when a platform view is disposed (flutter/engine#19546) * 0dc86cd Only attempt surface creation in viewDidLayoutSubviews if the application is active. (flutter/engine#19592)
…tion is active. (flutter#19592) UIKit does not guarantee that the application is in the active state when it lays out it subviews. However, in `viewDidLayoutSubviews`, Flutter attempts to create the render surface and wait for the first frame. These operations require GPU access which is forbidden when in the background on iOS. This layout while in the background only seems to happen under very specific circumstances as described in the reproducible test case. Notably, this behavior cannot be reproduced when the application is just launched with the "Background Fetch" XCode run scheme. The other instance of invocations to create the rendering surface have been audited and I can confirm that those are already in the correct lifecycle callbacks. This was the only instance where the surface creation was in a non-lifecycle callback. Fixes flutter/flutter#55969 (This issue has a clear reproducible test case and was used as the basis for verification.) Fixes flutter/flutter#57676 (This is significantly harder to reproduce but the issue looks to be the same. I tried this patch on multiple devices overnight and could not reproduce this.) This underlying cause here was identified by the enhanced GPU error reporting introduced in iOS 14. I am not able to submit those patches because the Beta builders are not available on infra. I will submit those when available so that issues like this have more actionable error messages in the future.
Oh cool, do you have a link?
Umm, @jmagman might be another motivation to move the infra before GM. |
In this talk, it's the first thing they talk about. Unfortunately, this years videos don't seem to have transcripts yet so you'll have to to seek past the intros. |
I would love to test betas on our infra, so I'm not the party who needs additional motivation. 😄 |
|
This is the cause of flutter/flutter#61584 |
* a53782f Roll Skia from 6130d5079d55 to 0c0d8dd6d637 (3 revisions) (flutter/engine#19570) * 0541502 kick build (flutter/engine#19575) * 07d5090 Roll Skia from 0c0d8dd6d637 to cf5e35f72130 (13 revisions) (flutter/engine#19573) * 35b5aa5 switch const finder to package_config (flutter/engine#19576) * de0932b Manual roll of Dart 06cb010247...69aba23371 (flutter/engine#19577) * b919945 include list_libraries.dart as a snapshot for fuchsia (flutter/engine#19567) * 3fe5edf Roll Skia from cf5e35f72130 to b4d60f807dbd (5 revisions) (flutter/engine#19587) * a15bc1b [CanvasKit] Dispose the overlay surface when a platform view is disposed (flutter/engine#19546) * 0dc86cd Only attempt surface creation in viewDidLayoutSubviews if the application is active. (flutter/engine#19592)
UIKit does not guarantee that the application is in the active state when it
lays out it subviews. However, in
viewDidLayoutSubviews, Flutter attempts tocreate the render surface and wait for the first frame. These operations require
GPU access which is forbidden when in the background on iOS. This layout while
in the background only seems to happen under very specific circumstances as
described in the reproducible test case. Notably, this behavior cannot be
reproduced when the application is just launched with the "Background Fetch"
XCode run scheme. The other instance of invocations to create the rendering
surface have been audited and I can confirm that those are already in the
correct lifecycle callbacks. This was the only instance where the surface
creation was in a non-lifecycle callback.
reproducible test case and was used as the basis for verification.
to reproduce but the issue looks to be the same. I tried this patch on multiple
devices overnight and could not reproduce this.
This underlying cause here was identified by the enhanced GPU error reporting
introduced in iOS 14. I am not able to submit those patches because the Beta
builders are not available on infra. I will submit those when available so that
issues like this have more actionable error messages in the future.