Skip to content

fix crash in ReactGroupView when subview clipping is enabled#42811

Closed
sammy-SC wants to merge 2 commits into
facebook:mainfrom
sammy-SC:export-D53348831
Closed

fix crash in ReactGroupView when subview clipping is enabled#42811
sammy-SC wants to merge 2 commits into
facebook:mainfrom
sammy-SC:export-D53348831

Conversation

@sammy-SC

@sammy-SC sammy-SC commented Feb 2, 2024

Copy link
Copy Markdown
Contributor

Summary:
changelog: [internal]

Attempt to fixing a crash in ReactViewGroup when removeClippedSubviews is enabled.
The implementation of removeClippedSubviews in ReactViewGroup is stateful and must be in sync between children of view and member variables in ReactViewGroup that keep reference to all children. When it gets out of sync, it manifests itself as java.lang.IndexOutOfBoundsException crash.

The mounting layer counts on the fact that view hierarchy will never be directly mutated and all mutations will go through ReactClippingViewManager. ReactClippingViewManager, if clipping is enabled, calls appropriate methods on ReactViewGroup to make sure member variables to manage clipping are in sync with view hierarchy.

This is true, except for a retry mechanism in SurfaceMountingManager. The retry mechanism tries to manually reconciliation the state with android view hierarchy. It bypasses ReactClippingViewManager in the process.

Reviewed By: javache

Differential Revision: D53348831

Summary:

changelog: [internal]

Outdated comment, let's remove it.

Reviewed By: fabriziocucci

Differential Revision: D53348035
Summary:
changelog: [internal]

Attempt to fixing a crash in ReactViewGroup when removeClippedSubviews is enabled.
The implementation of removeClippedSubviews in ReactViewGroup is stateful and must be in sync between children of view and [member variables in ReactViewGroup](https://fburl.com/code/l22wewzc) that keep reference to all children. When it gets out of sync, it manifests itself as `java.lang.IndexOutOfBoundsException` crash.

The mounting layer counts on the fact that view hierarchy will never be directly mutated and all mutations will go through [ReactClippingViewManager](https://fburl.com/code/esl3vqhh). ReactClippingViewManager, if clipping is enabled, calls appropriate methods on ReactViewGroup to make sure member variables to manage clipping are in sync with view hierarchy. 

This is true, except for a retry mechanism in SurfaceMountingManager. The retry mechanism tries to manually reconciliation the state with android view hierarchy. It bypasses ReactClippingViewManager in the process.

Reviewed By: javache

Differential Revision: D53348831
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Feb 2, 2024
@facebook-github-bot

Copy link
Copy Markdown
Contributor

This pull request was exported from Phabricator. Differential Revision: D53348831

@analysis-bot

Copy link
Copy Markdown
Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 17,201,488 +4,139
android hermes armeabi-v7a n/a --
android hermes x86 n/a --
android hermes x86_64 n/a --
android jsc arm64-v8a 20,580,365 +4,138
android jsc armeabi-v7a n/a --
android jsc x86 n/a --
android jsc x86_64 n/a --

Base commit: 184b295
Branch: main

@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label Feb 3, 2024
@facebook-github-bot

Copy link
Copy Markdown
Contributor

This pull request has been merged in 4473fe8.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported Merged This PR has been merged. p: Facebook Partner: Facebook Partner

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants