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

RUM-3992 Prevent crash in JankStats listener #1981

Merged
merged 1 commit into from
Apr 11, 2024

Conversation

xgouchet
Copy link
Collaborator

@xgouchet xgouchet commented Apr 10, 2024

What does this PR do?

Fixes #1976

In Android N (until Android P included), the internal View. findFrameMetricsObserver() method did not check the nullability of the mFrameMetricsObservers field, leading to possible NPE when trying to remove a listener that wasn't yet added.

Motivation

Some customer have seen a crash with an NPE thrown from the Android View when removing the FrameMetricsListener via the JankStats library.
Even though all versions of the official Android View are written in a way that NPE should never happen, those crash still occurred.

@xgouchet xgouchet requested review from a team as code owners April 10, 2024 13:19
@xgouchet xgouchet force-pushed the xgouchet/RUM-3992/jank_stats_crash branch from e6bedcb to 942d559 Compare April 10, 2024 14:26
@xgouchet xgouchet force-pushed the xgouchet/RUM-3992/jank_stats_crash branch from 942d559 to eca3a98 Compare April 11, 2024 07:04
@codecov-commenter
Copy link

codecov-commenter commented Apr 11, 2024

Codecov Report

Merging #1981 (eca3a98) into develop (768fa8a) will decrease coverage by 0.10%.
Report is 1 commits behind head on develop.
The diff coverage is 100.00%.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1981      +/-   ##
===========================================
- Coverage    83.71%   83.61%   -0.10%     
===========================================
  Files          487      487              
  Lines        17737    17695      -42     
  Branches      2663     2656       -7     
===========================================
- Hits         14848    14795      -53     
  Misses        2170     2170              
- Partials       719      730      +11     
Files Coverage Δ
...ernal/vitals/JankStatsActivityLifecycleListener.kt 69.17% <100.00%> (+0.75%) ⬆️

... and 24 files with indirect coverage changes

@xgouchet xgouchet merged commit ebf7542 into develop Apr 11, 2024
23 checks passed
@xgouchet xgouchet deleted the xgouchet/RUM-3992/jank_stats_crash branch April 11, 2024 07:50
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.

JankStatsActivityLifecycleListener.onActivityStopped -
3 participants