fix: defer focused input layout sync out of synchronous onStart dispatch - #1576
Conversation
📊 Package size report
|
kirillzyusko
left a comment
There was a problem hiding this comment.
Looks good and e2e tests passed, so it should be all good. I left few minor comment, if you address them we can merge this PR.
I have one question - did you experience new crash yourself or in Sentry/crashlytics? Did crash gone after these changes?
We saw it in our production crash reporting — Pixel 8, Android 16, predictive back gesture on a screen using We're shipping this patch in our next release, so I'll follow up on #1575 if it ever shows up again in the wild. |
Do you have an ETA when you ship next release with patch and when you can say whether the issue is reproducible or not? 👀 |
LGTM |
|
Our next native build goes out this Wednesday (Aug 5) — we're applying this change as a local patch on 1.22.2 so it doesn't have to wait for an upstream release. Store rollout usually takes about a week to reach most of our users after that. On reproducibility: it's a very low-frequency crash for us (single occurrences over roughly a month of production traffic), so I'd expect a meaningful read in about 3–4 weeks of the patched build being live. I'll report back on #1575 either way — whether it's gone or it resurfaces. |
|
Okay, let's merge this PR then 👍 I think it has a minor impact (in terms of dispatching order, because we already defer |
📜 Description
Moves the
layoutObserver?.syncUpLayout()call out of the synchronousonStartdispatch and flushes it influshPendingStartEvent()instead — right before the deferredStartevent, so the originallayout → startevent order is unchanged. The resize branch (isResizeHandledInCallbackMethods) keeps its synchronous sync, so its behavior is untouched.💡 Motivation and Context
Fixes #1575.
#1461 deferred the
Starttransition event out of thedispatchWindowInsetsAnimationStartwindow because Reanimated processes it synchronously on the UI thread, and a synchronous Fabric mutation there can cancel a pending IME insets controller before AOSP callslistener.onReady— AOSP checksisCancelledonly before dispatchingonStartand never re-checks it afterwards (InsetsController.startAnimation, android16-release ~L2151).syncUpLayout()still dispatchesFocusedInputLayoutChangedEventthrough that same window and the same Reanimated-synchronous channel (useReanimatedFocusedInput/KeyboardAwareScrollView). On Android 16+ the victim can beImeBackAnimationController(predictive back IME dismiss), which starts its post-commitValueAnimatorinsideonReadyand crashes withIllegalStateException: Can't change insets on an animation that is cancelledon the first animator frame — observed in production on 1.21.14, which already includes #1461. Full stack and AOSP analysis in #1575.📢 Changelog
Android
FocusedInputLayoutChangedEventdispatch fromonStartto the pending-start-event flush (firstonProgress/onEnd), preventing reentrant cancellation of a pending IME insets controller during predictive back dismissal🤔 How Has This Been Tested?
ktlint1.3.1 (CI version) anddetekt(repo config) pass locally;yarn lint,yarn typescript,yarn testall green (existingKeyboardAwareScrollViewsuites, which consumeFocusedInputLayoutChangedEvent, still pass)IllegalStateException: Can't change insets on an animation that is cancelledviaImeBackAnimationController(predictive back IME dismiss) — still occurring on 1.21.14 with #1461 fix #1575 / Exception java.lang.IllegalStateException: Can't change insets on an animation that is cancelled. #1456 history); the mechanism is verified against AOSP source and matches the production stack frame-by-frameonStartevent #1461 established, and preserves relative event ordering in both flush paths (onProgressandonEnd)📝 Checklist