Ensure TM system has consistent view of interop layer constants #39086
Conversation
This file contains 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
…ebook#39001) Summary: Pull Request resolved: facebook#39001 Changelog: [Internal] Differential Revision: https://internalfb.com/D48076894 fbshipit-source-id: 668df594e658c619ff38cc7c62e34c1c8e4907eb
Summary: During React Native teardown, we should stop all React surfaces. Otherwise, the app could crash with this error: ``` 08-06 14:54:08.644 14843 14843 F DEBUG : Abort message: 'xplat/js/react-native-github/packages/react-native/ReactCommon/react/renderer/scheduler/Scheduler.cpp:171: function ~Scheduler: assertion failed (surfaceIds.empty() && "Scheduler was destroyed with outstanding Surfaces.")' ``` When can teardown occur? One case: an exception occurs on the NativeModules thread. NOTE: This diff impacts the **new** Bridgeless mode lifecycle methods. Changelog: [Internal] Differential Revision: https://internalfb.com/D47926966 fbshipit-source-id: 01cad22f56c8b9d6af848e634dd7686117d0a8a2
…ebook#38999) Summary: Pull Request resolved: facebook#38999 After React Native tears down, a RedBox can appear, prompting the user to reload. **Problem:** After React Native reloads, the React Native screen wouldn't show up. **Cause:** ReactContext.onHostResume() wasn't executed. Why: - React Native teardown moves the React manager into the **onHostDestroy()** state. - During initialization, React Native only calls ReactContext.onHostResume(), if the React manager was *already* in the **onHostResume()** state. https://www.internalfb.com/code/fbsource/[f82938c7cc9a0ee722c85c33d1027f326049d37c]/xplat/js/react-native-github/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridgeless/ReactHost.java?lines=924-925 **Question:** Why does React Native only call ReactContext.onHostResume(), **if the React manager was already in the onHostResume() state?** In short, we want ReactContext.onHostResume() to be delayed until the user navigates to the first React Native screen. Please read the comments in the code to understand why. ## The fix If we're initializing React Native during a reload, just always call ReactContext.onHostResume(). If React Native is reloading, it seems reasonable to assume that: 1. We must have navigated to a React Native screen in the past, or 2. We must be on a React Native screen. Changelog: [Internal] Differential Revision: https://internalfb.com/D48076895 fbshipit-source-id: 950b9270778d6f84f2eaa86aaba465276216ba8a
Summary: Pull Request resolved: facebook#38998 Instead of using the mActivity reference directly, let's just use getCurrentActivity() and setCurrentActivity(). Changelog: [Internal] Differential Revision: https://internalfb.com/D48076896 fbshipit-source-id: 4019aea9b1b0d11b55915a173cdf0f70ed5ba7b0
Summary: Pull Request resolved: facebook#38997 After React Native gets destroyed (e.g: via an exception), the ReactHost resets its current activity. ## Problem React Native can display RedBoxes after React Native destruction (e.g: in the case of an exception). Displaying RedBoxes requires the current activity, which gets nullified. So, the RedBox might not show up after destruction. ## Changes This diff makes ReactHost keep a track of its last non-null activity in a WeakRef. Then, the DevMenu just uses the last non-null activity to display RedBoxes (and everything else). Changelog: [Internal] Differential Revision: https://internalfb.com/D48076893 fbshipit-source-id: 741b230b8a6dcc1e4ab1e0aeeefa5e7f12119326
Summary: The new reload/create/destroy methods work by chaining tasks together. This task chain has the type Task<ReactInstance>. **The problem:** If any step in the chain fails, task.getResult() actually returns null - not the ReactInstance. Many steps in the existing reload() and destroy() task chains don't account for this case. So: - The reload() and destroy() task chains sometimes swallow errors. - Sometimes steps in the reload() and destroy() task chains don't execute: they use .successTask This diff makes two changes: 1. Ensure each step **always** executes (i.e: use .continueWith vs .success) 2. Ensure each step first checks if the Task<ReactInstance> isn't faulted/cancelled. If the task is faulted/cancelled, a soft exception gets reported, and the current ReactInstance gets returned. Changelog: [Internal Differential Revision: https://internalfb.com/D48080779 fbshipit-source-id: 24e5101eb5fe71e753f3f7d03895a9f20ffb1871
) Summary: Pull Request resolved: facebook#39000 Whenever React Native tears down (including on logout), we need to drop unconsumed Pre-rendering surfaces. D45012714 initially implemented this change, but this diff wasn't complete: it would only drop unconsumed pre-rendered surfaces when ***the Facebook infra* initiated** React Native to tear down. But, React Native could initiate tear down **by iteself** (e.g: via an uncaught exception on the Native Modules thread). ## Changes In this diff, make the React Manager support an onBeforeDestroy listener. Then, integrate these listeners into the teardown/reload algorithms. That way, no matter how React Native tears down, we **alwasy** drop unconsumed pre-rendered surfaces. Changelog: [Internal] Differential Revision: https://internalfb.com/D48323647 fbshipit-source-id: 156bf3db9cc94a885e25d36a0dbe6ddbded39ebc
Summary: When reloads fail, React Native currently just renders a blank screen. We should provde some sort of feedback to the developer. Hence, this diff makes the RedBox show up. Changelog: [Internal] Differential Revision: D48335851 fbshipit-source-id: b5361e80d373409103801906c13b1d4c422eb19e
Summary: In this diff, we're just using the global.ErrorUtils defined in error-guard.js polyfill. **Problem:** JS Errors thrown after the [ErrorUtils.setGlobalHandler()](https://fburl.com/code/vys2zomm) is called don't bubble up to C++: https://www.internalfb.com/code/fbsource/[2ac326e13b15]/xplat/js/react-native-github/packages/react-native/Libraries/Core/setUpErrorHandling.js?lines=32 Differential Revision: https://internalfb.com/D47885427 fbshipit-source-id: 7a8ba2ac74ee1c87ffb64d0b920ec9473b8b7cd7
Summary: In this diff, we're use a global.ErrorUtils defined in Fb4aBundle.js **This problem is resolved:** JS Errors thrown after the [ErrorUtils.setGlobalHandler()](https://fburl.com/code/vys2zomm) is called don't bubble up to C++: https://www.internalfb.com/code/fbsource/[2ac326e13b15]/xplat/js/react-native-github/packages/react-native/Libraries/Core/setUpErrorHandling.js?lines=32 Differential Revision: https://internalfb.com/D47885426 fbshipit-source-id: c52ce6ccb4b88a477692d676e7e52533242ecd61
Summary: Our largest blocker for the TurboModule interop layer is a "module not found" issue. **Hypothesis:** This is a gating-related bug. ## Changes This diff tries to simplify the gating of the TurboModule interop layer. Instead of reading the flags again and again from two different classes (the module manager and its delegate), just read the flags once when the module system is initialized. https://www.internalfb.com/code/fbsource/[ae79b760626ec81ceadbf2829e1593199d4df031]/xplat/js/react-native-github/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridgeless/ReactInstance.java?lines=106-113%2C210-215%2C217-223%2C251 This will ensure that the TurboModule system has one consistent view of the interop layer flags, throughout its lifetime. Changelog: [Internal] Differential Revision: D48489274 fbshipit-source-id: 52a0c2fdd3ef6e2fc5be3e0c64c4457255a62baa
facebook-github-bot
added
CLA Signed
|
This pull request was exported from Phabricator. Differential Revision: D48489274 |
Base commit: 84e5934 |
|
This pull request was successfully merged by @RSNara in f312d6e. When will my fix make it into a release? | Upcoming Releases |
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:
Our largest blocker for the TurboModule interop layer is a "module not found" issue.
Hypothesis: This is a gating-related bug.
Changes
This diff tries to simplify the gating of the TurboModule interop layer. Instead of reading the flags again and again from two different classes (the module manager and its delegate), just read the flags once when the module system is initialized.
https://www.internalfb.com/code/fbsource/[ae79b760626ec81ceadbf2829e1593199d4df031]/xplat/js/react-native-github/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridgeless/ReactInstance.java?lines=106-113%2C210-215%2C217-223%2C251
This will ensure that the TurboModule system has one consistent view of the interop layer flags, throughout its lifetime.
Changelog: [Internal]
Differential Revision: D48489274