Process synchronous event beats in the frame that requested them - #58530
Open
janicduplessis wants to merge 1 commit into
Open
janicduplessis wants to merge 1 commit into
janicduplessis wants to merge 1 commit into
Conversation
janicduplessis
force-pushed
the
safe-area/1-alt-view-tag-flusher
branch
from
September 15, 2026 03:39
aa3d9fa to
7e69b03
Compare
janicduplessis
force-pushed
the
safe-area/1-alt-view-tag-flusher
branch
from
September 15, 2026 15:12
7e69b03 to
110bae8
Compare
janicduplessis
marked this pull request as ready for review
September 15, 2026 15:17
Contributor
Author
|
@Abbondanzo This one is ready for review, the 2nd PR of the safe area insets stack. I changed the approach a little bit from what you reviewed initially, I think it is a lot cleaner this way, just a little bit more plumbing to pass around the view tag, but allows to avoid looping through every windows which I really didn't like. |
|
@Abbondanzo has imported this pull request. If you are a Meta employee, you can view this in D120200496. |
meta-codesync Bot
pushed a commit
that referenced
this pull request
Sep 16, 2026
Summary: React tags are positive; `-1` marks the absence of one in several places with a bare literal. This names the convention — `kNoTag` in `ReactPrimitives.h`, next to the `Tag` alias and mirroring `ViewUtil.NO_SURFACE_ID` on the Android side — and replaces the literals across the renderer: `ShadowViewMutation`'s `parentTag` default, factories and comparison, the stub view tree (folding its duplicate `NO_VIEW_TAG` constant into `kNoTag`), the `CppMountItem` mirrors on Android, `UIManagerViewTransitionDelegate`'s `nativeTag` and the view transition fallback that feeds it, and the pointer-events no-override comparisons. Split out of the safe area insets stack: #58530 threads a `Tag` with no-tag semantics through `experimental_flushSync` and wants the named sentinel, but the cleanup stands on its own and can land regardless of that PR's fate. ## Changelog: [INTERNAL] - Name the no-tag sentinel (`kNoTag`) and use it for the hardcoded `-1` tags Pull Request resolved: #58531 Test Plan: Compiles via the Fantom tester build; `yarn fantom` mounting-heavy suite passes 224/224 (exercises the `StubViewTree` asserts). C++ API snapshots regenerated — the only delta is the new `kNoTag` constant. Reviewed By: javache Differential Revision: D120199347 Pulled By: Abbondanzo fbshipit-source-id: 4198a33fa601edff53c54bce935814b3414c5820
janicduplessis
force-pushed
the
safe-area/1-alt-view-tag-flusher
branch
3 times, most recently
from
September 16, 2026 15:49
55337fc to
8290f12
Compare
Contributor
Author
|
@Abbondanzo Rebased on top of the tag PR that landed. |
EventEmitter::experimental_flushSync only requests a beat, processed at the next EventBeat::induce. On iOS the run loop observer that induces the beat runs before Core Animation's commit observer, so a request made from layoutSubviews — inside CA's commit cycle — is only processed one frame later. AppleEventBeat now additionally schedules an induce in the display phase of the current commit cycle. Core Animation runs a commit as layout → display → commit, so a zero-sized layer marked as needing display during layout has its display called after the whole layout pass and before the transaction is committed. The layer is attached to the window of the requesting view: experimental_flushSync carries the tag of the emitting view — cached from its ShadowNodeFamily when the family is attached at creation, before the emitter is published, so reading it takes no lock — through EventDispatcher and EventQueue to EventBeat::requestSynchronous, with kNoTag meaning no view attribution; a no-argument overload keeps unattributed requesters unchanged. AppleEventBeat resolves the tag to the view's window layer through a resolver injected by RCTSurfacePresenter (findComponentViewWithTag: on the mounting registry, a nullable, non-creating, main-thread lookup). The requesting view's window is by definition the root of the layer tree whose layout emitted the request, so the flusher is guaranteed a display phase in the current commit cycle, including for content UIKit mounts in a window of its own, like a full screen modal or LogBox. Requests from several windows in one cycle each dirty their own layer; the first display to fire drains the queue and the rest no-op on the request flag. VirtualView's synchronous flushes get the same targeting through their own emitter. A related fix in EventBeat itself: a synchronous request is no longer stranded behind an already-scheduled asynchronous beat (it would silently lose its this-frame guarantee, and the leftover flag would make an unrelated later beat blocking). AppleEventBeat.cpp becomes .mm for the Objective-C. Covered by new unit tests in EventBeatTest.cpp, which drive the protected induce through a subclass standing in for the platform. The C++ API snapshots are regenerated; the deltas are the requestSynchronous overload pair, the resolver type, and the AppleEventBeat constructor and destructor.
janicduplessis
force-pushed
the
safe-area/1-alt-view-tag-flusher
branch
from
September 16, 2026 16:09
8290f12 to
d804f43
Compare
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:
EventEmitter::experimental_flushSynconly requests an event beat; the beat is processed at the nextEventBeat::induce. On iOS the run loop observer that induces the beat runs before Core Animation's commit observer, so a request made fromlayoutSubviews— inside CA's commit cycle — is only processed one frame later. Anything that reports layout-driven state to JS synchronously (VirtualViewmode changes, and safe area insets in the PRs that build on this) renders a frame late in exactly the cases that matter.AppleEventBeatnow also schedules an induce in the display phase of the current commit cycle. Core Animation runs a commit as layout → display → commit, so a zero-sized layer marked as needing display during layout gets itsdisplaycall after the whole layout pass and before the transaction is committed. That layer needs to live in the tree being committed, so the beat has to know which tree that is — and the emitter tells it:experimental_flushSynccarries the tag of the emitting view throughEventDispatcherandEventQueuetoEventBeat::requestSynchronous(Tag), withkNoTag(Name the no-tag sentinel and use it for the hardcoded -1 tags #58531) meaning no view attribution; a no-argument overload keeps unattributed requesters and the existing tests unchanged. The emitter caches the tag from itsShadowNodeFamilywhen the family is attached at creation, before the emitter is published, so reading it takes no lock — in particular notDispatchMutex, which the JS thread holds across whole-tree mount-flag walks during commits.AppleEventBeatresolves the tag to the layer of the view's window through a resolver injected byRCTSurfacePresenter(findComponentViewWithTag:on the mounting registry — nullable, non-creating, main thread) and attaches its flusher layer there. The requesting view's window is by definition the root of the layer tree whose layout emitted the request, so the flusher is guaranteed a display phase in the current commit cycle — including for content UIKit mounts in a window of its own, like a full screen modal or LogBox. Requests within one cycle coalesce into a single induce.One related fix in
EventBeatitself: a synchronous request is no longer stranded behind an already-scheduled asynchronous beat (it would silently lose its this-frame guarantee, and the leftover flag would make an unrelated later beat blocking).AppleEventBeat.cppbecomes.mmfor the Objective-C.Risk: this changes when queued events are flushed on iOS for every
experimental_flushSynccaller — todayVirtualView, and safe area insets with the PRs on top. The worst case is a beat processed a frame earlier than before, inside a Core Animation commit; the run loop observer path is untouched and still catches anything the display phase misses (an emitter with no tag, an unmounted view, a request off the main thread). Android ignores the tag. Revert is self-contained.Design Q&A:
What happens when two views in different windows update at once?
Each requesting window gets its own dirty flusher layer (the map is keyed by host layer), and the first
displayto fire induces the beat, which drains the whole event queue — every window's updates mount before that commit presents. The remaining flushers hit theisEventBeatRequested_guard and no-op, so it is one beat total, not one per window. If windows ever commit in separate transactions, each request still resolves within its own window's cycle, since its layer sits in the tree that emitted it. Only requesting windows carry a dirty layer.The tag is cached when the emitter is created — can the cache go stale?
The value cannot: an emitter is created once per
ShadowNodeFamily(which hands it the tag before publication), and a family keeps one tag for its whole life, across clones and state updates. What changes over a view's life — its window — is deliberately not cached: the tag resolves to a view at flush time andview.window.layeris read live, so a view that moved between windows targets its current tree. Stale-tag scenarios (flush mid-unmount, recycled views) resolve to nil — the registry erases the entry and recycled views get tag0— and degrade to run-loop-observer timing. The tag only ever influences where the induce is scheduled, never what is delivered or to whom, so the blast radius of any staleness is one frame of timing, not correctness.Does
VirtualViewneed changes to benefit?No — its sync mode-change flush goes through its own emitter, so the tag attribution is automatic. The case this improves is a mode change emitted during Core Animation layout (a resize pulling a virtualized item into view): on
mainthat renders one frame late; here the induce lands in the display phase of the VirtualView's own window, including inside a full screen modal.Changelog:
[INTERNAL] - Process synchronous event beats in the frame that requested them on iOS, scheduling the induce on the requesting view's window
Test Plan:
New unit tests in
EventBeatTest.cppcover the beat semantics: a synchronous request during an already-scheduled asynchronous beat, coalescing, and induce ordering. They drive the protectedinducethrough a subclass standing in for the platform.On device, with the safe area insets prop from the PRs above merged on top: an RNTester example renders a loud marker (yellow background) while a view observes the safe area but has not received an inset event yet, so any presented marker frame means the dispatch was not synchronous. The full apply → landscape → portrait sequence inside a full screen modal on an iPhone 17 Pro simulator, decomposed with ffmpeg into 982 frames and every frame scanned for the marker color — zero marker frames, and mid-rotation frames already carry the incoming orientation's insets, so the padding animates with the rotation. Scoped honestly: the first inset event after setting the prop is processed at the call site, so the marker primarily proves no regression; the same-frame path for layout-driven changes rests on the by-construction argument above plus the rotation frames.
tag-capture.mp4
yarn fantom .../ViewSafeAreaInsets-itest.jspasses 4/4 with the prop merged on top. C++ API snapshots regenerated (scripts/cxx-api/parser, Doxygen 1.16.1): the deltas are therequestSynchronousoverload pair,EventEmitter::getTag, the resolver type, and theAppleEventBeatconstructor and destructor.Stack — split out of #57967, which stays open as the prototype and design discussion. GitHub will not take a fork branch as a pull request base, so each of these targets
mainand its diff contains the ones below it until they merge. Each PR is one commit on top of the previous one.This is the bottom of the stack, so its diff is already just this change.
👉 1. #58530 — Process synchronous event beats in the frame that requested them
2. #58109 — Add an
experimental_onSafeAreaInsetsChangeview prop3. #58110 — Report the window safe area insets through Dimensions
4. #58112 — Render the internal SafeAreaView from the safe area insets prop
5. #58113 — Remove the native SafeAreaView and the deprecated public export
An earlier variant that targeted the surface's root view instead of the view's window was closed in #58528; its review thread carries the analysis behind the window-based resolution. #58108 was the per-window predecessor this supersedes.