Measure used heap, not allocated heap, before capturing a screenshot - #209
Open
rjewell808 wants to merge 1 commit into
Open
Measure used heap, not allocated heap, before capturing a screenshot#209rjewell808 wants to merge 1 commit into
rjewell808 wants to merge 1 commit into
Conversation
hasEnoughMemoryForScreenshot() compared totalMemory() against maxMemory(). totalMemory() is the size of the heap the runtime has claimed from the OS, not the amount of it in use, so that ratio measures how much room the heap has left to grow rather than how much memory is available. Once the heap has grown to maxMemory() the ratio is pinned at 0 and every screenshot is refused, however much of that heap is free. Devices vary in when they reach that point and some report totalMemory() == maxMemory() from process start, in which case no screenshot is ever captured for the life of the process. The visible effect is that destinations presenting over another screen lose their backdrop: HotwireView.addScreenshot() returns early on a null bitmap, leaving the bare view behind the modal instead of the page. Use totalMemory() - freeMemory() for the memory in use, and extract the comparison so it can be tested.
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.
Symptoms
Destinations presented over another screen (bottom sheets, modals) render with no backdrop — the underlying screen shows as a flat background colour instead of the page it was displaying. On affected devices this happens on every such navigation, from the first one, and never recovers within the process.
Root cause
HotwireViewScreenshotHolder.copyViewToBitmap()refuses to capture whenhasEnoughMemoryForScreenshot()returns false, and that check measures the wrong thing:totalMemory()is the size of the heap the runtime has currently claimed from the OS, not the amount of it in use —freeMemory()is the unused portion of that heap, so memory in use istotalMemory() - freeMemory(). As written,remainingmeasures how much room the heap has left to grow, not how much memory is available.Once the heap has grown to
maxMemory()the expression is pinned at exactly0.0and every screenshot is refused for the rest of the process, however much of that heap is free. The ART heap does not shrink back, so this is one-way. Devices differ in when they reach it; some reporttotalMemory() == maxMemory()from process start, in which case no screenshot is ever captured at all.The capture then returns null on the silent early-return branch — no
viewScreenshotFailed, since that is only logged forPixelCopyfailures — andHotwireView.addScreenshot()returns early on a null bitmap, leaving the bareHotwireViewbehind the modal.Evidence
Instrumented in an app on a device that reaches the fully-grown state a couple of navigations in. Each row is one navigation to a bottom sheet; the first three values are logged from the app,
viewScreenshotCreatedis the library's own line:1 - total/max(current check)1 - (total-free)/max(actual)The heap grows to its maximum between the first and second navigation. The current check flips to
0.000and stays there — while free memory goes up, from 26 MB to 240 MB, against a screenshot that needs ~6 MB. Every subsequent modal in that process loses its backdrop.Fix
Use
totalMemory() - freeMemory()for the memory in use. The comparison is extracted to an internal function so it can be tested directly.Testing
HotwireViewScreenshotHolderTestcovering the regression (heap fully grown but mostly free) and the threshold either side of 20%../gradlew testReleasepasses forcoreandnavigation-fragments.viewScreenshotCreatedis logged on every navigation including those where the current check reports0.000, and modals render the underlying page as their backdrop.