fix: blurView integration with ScreenStack #1406
Merged
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.
Description
BlurView
sometimes callsdraw
method of the app's rootView, causing unexpected behavior in our library. SinceBlurView
is a child ofScreenStack
in the crashing example, callingop.draw()
callsBlurView
onDraw
method, which then sometimes callsdraw
method in app'srootView
. It results in visiting thedrawAndRelease
method again. The important thing isdrawingOps.indices
makesIntRange
under the hood, which is then used in next loop calls. At the same time, after the most nestedfor
loop ends, it clears thedrawingOps
, but this change is not visible in the formerfor
since it just loops overIntRange
created before. This results inindexOutOfBounds
crash when we try to obtain anop
from emptydrawingOps
. As an addition, If we didn't useindices
, butfor (op in drawingOps)
, it would result inConcurrentModificationException
.Current fix makes a copy of
drawingOps
which is then used infor
loop, and clearing thedrawingOps
so the nested calls cannot change the original one.Test code and steps to reproduce
https://github.com/mjm918/RNScreensBlurViewCrash-Repro/blob/master/AppCrash.tsx
Checklist