Fix FrameMetrics NPE
#4284
Merged
Fix FrameMetrics NPE
#4284
Conversation
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
Coverage Report 1Affected Products
Test Logs |
Size Report 1Affected Products
Test Logs |
visumickey
reviewed
Nov 4, 2022
| if (ex instanceof NullPointerException && Build.VERSION.SDK_INT > Build.VERSION_CODES.P) { | ||
| // Re-throw above API 28, since the NPE is fixed in API 29: | ||
| // https://android.googlesource.com/platform/frameworks/base/+/140ff5ea8e2d99edc3fbe63a43239e459334c76b | ||
| throw ex; |
There was a problem hiding this comment.
Is the intent to notify the developers of the exception during development? Interesting to see a pattern where we throw and exception in a catch module.
raymondlam
approved these changes
Nov 7, 2022
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Copy of #4184 due to CLA issues (Jeremy leaving).
Investigation
Let's take a look at Android's source code, in android.view.View.
The exception was thrown from
findFrameMetricsObserver, insideremoveFrameMetricsListenerSince this null check didn't exists, this triggers NPE when
mFrameMetricsObserveris uninitialized. Jeremy found that this was fixed in later Android versions. Why then, ismFrameMetricsObserveruninitialized? It's becauseaddFrameMetricsListenerfails silently!Conclusion
So overall, this is the same issue caused by the view not being hardware-accelerated. The correct error message is hidden away because
addFrameMetricsListenerfails silently, that's why we should catch them both and use the same error message. The developer's provided crash versions also lines up with when Android fixed it: the above reference fix was made in 2019, and Android 7, 8, 9 are all before 2019, with Android 10 releasing in 2019 thus fixing the issue. There is no crash before Android 7 because as Rosario pointed out, FrameMetrics was added in Android 7 (API 24).