Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(lowMemory): align the app.lowMemory field for JVM and NDK #1342

Merged
merged 1 commit into from
Aug 17, 2021

Conversation

lemnik
Copy link
Contributor

@lemnik lemnik commented Aug 13, 2021

Goal

The app.lowMemory attribute should be reported in the same way for all errors.

Design

Cache the most recent value sent to ComponentCallbacks.onLowMemory and ComponentCallbacks2.onTrimMemory in the AppDataCollector and report that value instead of looking-up the value at crash-time.

When we receive a trim memory event that indicates any state less than COMPLETE we set the cached lowMemory state to false.

Testing

Manual testing for both native and JVM crashes, and a new unit test to cover the mapping between memory-trim values and the lowMemory flag.

@lemnik lemnik requested a review from kattrali August 13, 2021 09:19
@lemnik lemnik force-pushed the PLAT-7107/consistent-lowMemory-reporting branch 4 times, most recently from 782d404 to 6184f98 Compare August 13, 2021 09:44
@@ -340,6 +340,7 @@ public Unit invoke(String oldOrientation, String newOrientation) {
}, new Function1<Boolean, Unit>() {
@Override
public Unit invoke(Boolean isLowMemory) {
appDataCollector.setLowMemory(isLowMemory == Boolean.TRUE);
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be checking the value of the instance instead of equality with the static constant? I highly doubt this would end up with a constructed Boolean() instance but still.

Suggested change
appDataCollector.setLowMemory(isLowMemory == Boolean.TRUE);
appDataCollector.setLowMemory(isLowMemory.booleanValue());

@lemnik lemnik force-pushed the PLAT-7107/consistent-lowMemory-reporting branch from 6184f98 to 31d74f8 Compare August 16, 2021 16:37
@lemnik lemnik merged commit e2f08e5 into next Aug 17, 2021
@lemnik lemnik deleted the PLAT-7107/consistent-lowMemory-reporting branch August 17, 2021 07:42
@lemnik lemnik mentioned this pull request Aug 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants