updateRuntimeShadowNodeReferencesOnCommit Feature Flag + Enable#82
Closed
Flewp wants to merge 4 commits into0.78.0-discordfrom
Closed
updateRuntimeShadowNodeReferencesOnCommit Feature Flag + Enable#82Flewp wants to merge 4 commits into0.78.0-discordfrom
updateRuntimeShadowNodeReferencesOnCommit Feature Flag + Enable#82Flewp wants to merge 4 commits into0.78.0-discordfrom
Conversation
Summary: Pull Request resolved: facebook#50753 Runtime Shadow Node Reference Updates (RSNRU) is currently implemented through the clone method which on each internal clone updates the runtime reference to point to the new clone. This guarantees that the runtime reference always points at the latest revision of the shadow node. This came with the constraint that RSNRU could only run from one thread at all times, otherwise the React renderer state (current fiber tree) would end up being corrupted by receiving reference updates from multiple threads cloning shadow nodes. This change moves the reference update step to the locked scope of the commit phase. Since the runtime is blocking on the commit and the scope is locked, it is safe and correct to update the runtime references with the latest revision of the shadow node after running state progression and layout. By moving the reference update to the commit, we can support shadow node syncing from any thread since the actual runtime references are now executing at a safe time and the renderer state will stay valid at all times. This change is gated behind the `updateRuntimeShadowNodeReferencesOnCommit` feature flag, which enabled shadow node syncing from any thread and reference updates only during the commit. Changelog: [Internal] Reviewed By: rubennorte Differential Revision: D73038439 fbshipit-source-id: d90308498f3c0625dc87158f15311d1088aad8b0
Summary: Pull Request resolved: facebook#51843 Changelog: [Internal] To avoid corrupting the React fiber tree when committing from multiple threads this diff only updates shadow node references within the fiber tree for commits originating from React. This guarantees that during the update of the references no React render will start or is running, making the update of the shadow node reference safe to execute. S527994 Reviewed By: sammy-SC Differential Revision: D76043845 fbshipit-source-id: bfcbeaae7fc8b976a1c2db6682330cef9ca25ab8
…facebook#49981) Summary: Pull Request resolved: facebook#49981 Changelog: [internal] (because MutationObserver isn't a public API yet) ## Context `MutationObserver` is a [JavaScript API](https://developer.mozilla.org/en-US/docs/Web/API/MutationObserver) used to report mutations in the DOM tree. The mutations available on Web are: changing children, changing attributes and changing text. In React Native, only changing children is supported. Mutation detection needs to happen synchronously when mutations happen (in ShadowTree commits) and the notification is dispatched as a microtask in JS, which means we can only report mutations happening in JavaScript. There's an assumption that only React (in JS) can change the structure of the tree in Fabric, so we implemented `MutationObserver` considering this assummption. Unfortunately, while the assumption is correct (we can only mutate children from React) the implementation didn't take into account that commit hooks were triggered from multiple threads, even if the structure doesn't change (e.g.: with Fabric state updates). In this case, we do the checks but we never dispatch the notifications. This can cause crashes (see T217617393) if we try to check for mutations from the main thread while we add new observers in the JS thread (because that logic wasn't thread safe). This fixes that crash by, in MutationObserver, not only preventing notifications from commits not coming from React, but also preventing the determination altogether. In order to do this, this modifies the signature of the commit hooks to also pass the commit options, and adds a new field in the commit options with the source of the commit (for now, just "React" or "Unknown"). In `MutationObserver`, before accessing the data structures of the observer, we check if the commit is coming from React, and return early otherwise. Reviewed By: javache Differential Revision: D71036705 fbshipit-source-id: 985c8f903375cbf876dce5174e04563f74d7621a
Author
|
Closing in favor of just cherry-picking facebook#49981 straight to the .78 branch. |
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
2 participants
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.
This moves shadow node reference updates to the commit phase with the goal of thread safety for syncing shadow nodes and renderer state correctness. Potential fix for some state issues involving nodes/views that don't exist.
Combination of a munging of three commits:
ShadowTreeCommitSourceused in the followup