RUM-10703: Fix Session Replay NPE when getting TextView padding#2784
Conversation
d539443 to
8cb6c7c
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #2784 +/- ##
===========================================
- Coverage 69.97% 69.83% -0.14%
===========================================
Files 824 824
Lines 30881 30887 +6
Branches 5195 5196 +1
===========================================
- Hits 21606 21568 -38
- Misses 7828 7848 +20
- Partials 1447 1471 +24
🚀 New features to boost your workflow:
|
8cb6c7c to
366bbbf
Compare
| left = textView.totalPaddingStart.densityNormalized(pixelsDensity).toLong(), | ||
| right = textView.totalPaddingEnd.densityNormalized(pixelsDensity).toLong() | ||
| ) | ||
| return if (textView.layout != null) { |
There was a problem hiding this comment.
are we sure it is a fix that will be enough in general when fetching properties of TextView, given we cannot reproduce it?
To me the issue is that layout is null is related to the fact that we either try to create wireframe too early or too late in the TextView lifecycle and there is no guarantee that we fix this call site, but then we can have a crash somewhere later if layout is still null.
Is there a way to check that a view is in a good and reliable state in general before we start pulling the properties for the wireframe generation?
There was a problem hiding this comment.
The client is not able to reproduce the issue systemically neither on their side, so they can't tell which TextView exact is causing the issue. Even though they gave us the code snippet, we only know that they are using viewbinding, and programmatically assign the text content in their activity, otherwise nothing irregular.
According to the source code, null layout can also happen between TextView cleaning layout and making new layout, and in this case we are not able to prevent it by checking if the view is well loaded or not, the view can update at anytime. so in my point of view, checking layout is the best thing that we can do to make sure it.
366bbbf to
5984380
Compare
5 participants
What does this PR do?
Here is the crash stack trace about this issue:
From the documentation of
TextView.getLayout()we know that it's probably caused by the text update:Based on that, we just need to make sure that the
layoutis present before reaching thegetTotalPaddingBottomfunction.Motivation
RUM-10703
Additional Notes
The whole function is called from MainThread, so it's unlikely that between checking
textView.layoutand usingtextView.totalPaddingBottom.thelayoutis set to null again due to the concurrencyReview checklist (to be filled by reviewers)