Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Ensure TM system has consistent view of interop layer constants #39086

Closed
wants to merge 11 commits into from
Closed

Ensure TM system has consistent view of interop layer constants #39086

wants to merge 11 commits into from

Commits on Aug 20, 2023

  1. Easy: Rename adb logs: Rename old/newPreload() -> old/newStart() (fac… 

    …ebook#39001)

Summary:
Pull Request resolved: facebook#39001

Changelog: [Internal]

Differential Revision: https://internalfb.com/D48076894

fbshipit-source-id: 668df594e658c619ff38cc7c62e34c1c8e4907eb
    @RSNara @facebook-github-bot
    RSNara authored and facebook-github-bot committed Aug 20, 2023
    Configuration menu
    Copy the full SHA
    bf017b0 View commit details
    Browse the repository at this point in the history

  2. Fix: React Native teardown crashes app 

    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
    @RSNara @facebook-github-bot
    RSNara authored and facebook-github-bot committed Aug 20, 2023
    Configuration menu
    Copy the full SHA
    775123b View commit details
    Browse the repository at this point in the history

  3. Fix: React Native reloads after teardown render a "blank screen" (fac… 

    …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
    @RSNara @facebook-github-bot
    RSNara authored and facebook-github-bot committed Aug 20, 2023
    Configuration menu
    Copy the full SHA
    7f4e67f View commit details
    Browse the repository at this point in the history

  4. Refactor: Use get/setCurrentActivity in ReactHost (facebook#38998) 

    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
    @RSNara @facebook-github-bot
    RSNara authored and facebook-github-bot committed Aug 20, 2023
    Configuration menu
    Copy the full SHA
    bb15ef6 View commit details
    Browse the repository at this point in the history

  5. Fix: RedBoxes don't show up after teardowns (facebook#38997) 

    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
    @RSNara @facebook-github-bot
    RSNara authored and facebook-github-bot committed Aug 20, 2023
    Configuration menu
    Copy the full SHA
    8abb3b1 View commit details
    Browse the repository at this point in the history

  6. Enhance/fix error reporting in reload and destroy 

    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
    @RSNara @facebook-github-bot
    RSNara authored and facebook-github-bot committed Aug 20, 2023
    Configuration menu
    Copy the full SHA
    8331f03 View commit details
    Browse the repository at this point in the history

  7. Clean up pre-rendered surfaces properly during teardowns (facebook#39000 

    )

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
    @RSNara @facebook-github-bot
    RSNara authored and facebook-github-bot committed Aug 20, 2023
    Configuration menu
    Copy the full SHA
    dc7d8d3 View commit details
    Browse the repository at this point in the history

  8. Show RedBox when reloads fail 

    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
    @RSNara @facebook-github-bot
    RSNara authored and facebook-github-bot committed Aug 20, 2023
    Configuration menu
    Copy the full SHA
    a144e22 View commit details
    Browse the repository at this point in the history

  9. @nocommit: Polyfill ErrorUtils: JS Errors don't bubble up to C++ 

    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
    @RSNara @facebook-github-bot
    RSNara authored and facebook-github-bot committed Aug 20, 2023
    Configuration menu
    Copy the full SHA
    034a099 View commit details
    Browse the repository at this point in the history

  10. @nocommit: Fb4aBundle ErrorUtils: JS Errors do bubble up to C++ 

    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
    @RSNara @facebook-github-bot
    RSNara authored and facebook-github-bot committed Aug 20, 2023
    Configuration menu
    Copy the full SHA
    44ca900 View commit details
    Browse the repository at this point in the history

  11. Ensure TM system has consistent view of interop layer constants 

    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
    @RSNara @facebook-github-bot
    RSNara authored and facebook-github-bot committed Aug 20, 2023
    Configuration menu
    Copy the full SHA
    e1902f3 View commit details
    Browse the repository at this point in the history