Repository navigation
fix(navigation): stop wrap-content sheets spinning in measure - #1929
Merged
Merged
Conversation
The host Invite sheet froze the app (Bugsnag 6ae543e9; 10 of 11 ANR dumps in a host's bugreport). Every main thread was inside compose-unstyled's Sheet measure, in updateContentHeight and shouldUseContentHeightForLayout. The wrap-content detent returned the sheetHeight compose-unstyled passes in, which puts the sheet in its content-height mode: measure writes the content's height into the sheet state and reads it back in the same pass. Scenes sit in a SharedTransitionLayout, so the sheet is measured twice a frame. The Invite sheet's Approve crossfade uses a SizeTransform, so it reports its target height to the lookahead pass and the animated height to the main pass. Each write invalidates the other, and the frame never ends. WrapContentDetent never reads sheetHeight. A layout modifier measures the content at its natural height (up to 92.5% of the window) and records it after layout, and the sheet's detents are re-resolved when it changes. Modal wrap-content sheets and InWindowBottomSheet both use it. WrapContentSheetLayoutLoopTest flips an AnimatedContent with a SizeTransform while a wrap-content sheet opens inside a SharedTransitionLayout. Before this change it spun at full CPU in the same frames as the device dumps; now it passes.
github-actions
Bot
added
type: fix
…asures The loop spins inside the sheet's own measure, so the counter in the test's layout modifier never ran. A 10 minute test task timeout makes a regression fail. Also treat a zero content height as unmeasured.
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes the host Invite sheet ANR (Bugsnag 6ae543e9). Tapping Share on a private group's info page froze the app. A host's bugreport had 11 ANR dumps, and in 10 of them the main thread was in compose-unstyled's
Sheetmeasure (updateContentHeight,shouldUseContentHeightForLayout).Cause.
WrapContentreturned thesheetHeightthat compose-unstyled passes in. That puts the sheet in content-height mode, where measure writes the content's height into the sheet state and reads it back. Scenes sit in aSharedTransitionLayout, so the sheet is measured twice a frame. The Invite sheet's Approve crossfade (SizeTransform) reports its target height to the lookahead pass and the animated height to the main pass. The two writes keep invalidating each other.Change.
WrapContentDetentnever readssheetHeight:onSizeChanged.TrackcallsinvalidateDetents()when the height changes.Modal wrap-content sheets and
InWindowBottomSheet(swipe pay) both use it.WrapContentSheetLayoutLoopTestflips anAnimatedContentwith aSizeTransformwhile a wrap-content sheet opens inside aSharedTransitionLayout. Oncode/cashit spins at full CPU in the same frames as the device dumps. With this change it passes.