Stop crashing on inconsistent clipping bookkeeping in ReactViewGroup.isViewClipped#57365
Closed
rozele wants to merge 1 commit into
Closed
Stop crashing on inconsistent clipping bookkeeping in ReactViewGroup.isViewClipped#57365rozele wants to merge 1 commit into
rozele wants to merge 1 commit into
Conversation
…isViewClipped Summary: WARNING: Generated by Autopilot (alpha) - review carefully, verify the underlying claim before accepting. Agent: React Native Agent | Trajectory: https://www.internalfb.com/intern/devai/devmate/inspector/0ea7c6e3-35be-490c-a17f-66677f75cb75/ | SC job: https://www.internalfb.com/intern/sandcastle/instance/18014401274782879/ --- ## What `ReactViewGroup.isViewClipped` (the `removeClippedSubviews` clipping path) crashes with `java.lang.IllegalStateException: Check failed.` at the hard `check(parent === this)` in its missing-tag fallback. The crash surfaces in CrashBot via `system_vros_crashes` with the title frame `com.facebook.react.internal.tracing.PerformanceTracer.trace` (an incidental Choreographer `doFrame` wrapper); the real throw site is `ReactViewGroup.isViewClipped` ← `updateSubviewClipStatus` ← `ChildrenLayoutChangeListener.onLayoutChange`. Tracked as T277554117 (MID `system_vros_crashes/5593272fc690b1c4d240d949710c49e9`). ## Why When a child view in `allChildren` has no `view_clipped` tag, the method falls back to inferring clip state from the view's parent. The existing code already logs a `ReactNoCrashSoftException` (`RVG_IS_VIEW_CLIPPED`) for this inconsistency, signaling that the intent is graceful degradation — but it then hits `check(parent === this)`, which hard-crashes whenever the tag-less child is parented to some *other* `ViewGroup` (a reparented/stale-bookkeeping state) rather than to `this` or to no parent. This is a long-standing invariant (the bookkeeping dates to D66383241 / D66539065, Dec 2024; the `check` was carried through the Kotlin conversion in D75797215), not a new code regression — it was first observed in crash data on 2026-04-10 and only crossed the top-N FSA detector threshold on 2026-06-28 (`%SAD ≈ 0`, ~17 occurrences / 7 days). ## Fix Replace the hard `check(parent === this)` with a non-fatal classification consistent with the surrounding fallback: a view that is transitioning, has no parent, or is parented elsewhere is no longer a live child of `this`, so it is reported as clipped (`true`); only a view still parented to `this` and not transitioning is reported as not clipped (`false`). This preserves the existing behavior for every previously non-crashing path (`parent == null` / `transitioning` → `true`; `parent === this` → `false`) and converts only the previously-crashing reparented case into the same "treat as removed" outcome the soft exception already anticipates. Differential Revision: D109987865
|
@rozele has exported this pull request. If you are a Meta employee, you can view the originating Diff in D109987865. |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary:
WARNING: Generated by Autopilot (alpha) - review carefully, verify the underlying claim before accepting.
Agent: React Native Agent | Trajectory: https://www.internalfb.com/intern/devai/devmate/inspector/0ea7c6e3-35be-490c-a17f-66677f75cb75/ | SC job: https://www.internalfb.com/intern/sandcastle/instance/18014401274782879/
What
ReactViewGroup.isViewClipped(theremoveClippedSubviewsclipping path) crashes withjava.lang.IllegalStateException: Check failed.at the hardcheck(parent === this)in its missing-tag fallback. The crash surfaces in CrashBot viasystem_vros_crasheswith the title framecom.facebook.react.internal.tracing.PerformanceTracer.trace(an incidental ChoreographerdoFramewrapper); the real throw site isReactViewGroup.isViewClipped←updateSubviewClipStatus←ChildrenLayoutChangeListener.onLayoutChange. Tracked as T277554117 (MIDsystem_vros_crashes/5593272fc690b1c4d240d949710c49e9).Why
When a child view in
allChildrenhas noview_clippedtag, the method falls back to inferring clip state from the view's parent. The existing code already logs aReactNoCrashSoftException(RVG_IS_VIEW_CLIPPED) for this inconsistency, signaling that the intent is graceful degradation — but it then hitscheck(parent === this), which hard-crashes whenever the tag-less child is parented to some otherViewGroup(a reparented/stale-bookkeeping state) rather than tothisor to no parent. This is a long-standing invariant (the bookkeeping dates to D66383241 / D66539065, Dec 2024; thecheckwas carried through the Kotlin conversion in D75797215), not a new code regression — it was first observed in crash data on 2026-04-10 and only crossed the top-N FSA detector threshold on 2026-06-28 (%SAD ≈ 0, ~17 occurrences / 7 days).Fix
Replace the hard
check(parent === this)with a non-fatal classification consistent with the surrounding fallback: a view that is transitioning, has no parent, or is parented elsewhere is no longer a live child ofthis, so it is reported as clipped (true); only a view still parented tothisand not transitioning is reported as not clipped (false). This preserves the existing behavior for every previously non-crashing path (parent == null/transitioning→true;parent === this→false) and converts only the previously-crashing reparented case into the same "treat as removed" outcome the soft exception already anticipates.Differential Revision: D109987865