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.

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

Cleanup: Use get/setCurrentActivity in ReactHost #38998

Closed
wants to merge 4 commits into from

Conversation

RSNara
Copy link
Contributor

@RSNara RSNara commented Aug 14, 2023

Summary:
Instead of using the mActivity reference directly, let's just use getCurrentActivity() and setCurrentActivity().

Changelog: [Internal]

Differential Revision: D48076896

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. p: Facebook Partner: Facebook Partner labels Aug 14, 2023
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D48076896

@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D48076896

RSNara added a commit to RSNara/react-native that referenced this pull request Aug 14, 2023
Summary:
Pull Request resolved: facebook#38998

Instead of using the mActivity reference directly, let's just use getCurrentActivity() and setCurrentActivity().

Changelog: [Internal]

Differential Revision: D48076896

fbshipit-source-id: 60fef8b3d8756439e4d0a910f4d1e502a2c59293
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D48076896

RSNara added a commit to RSNara/react-native that referenced this pull request Aug 14, 2023
Summary:
Pull Request resolved: facebook#38998

Instead of using the mActivity reference directly, let's just use getCurrentActivity() and setCurrentActivity().

Changelog: [Internal]

Differential Revision: D48076896

fbshipit-source-id: c29e4db3dd02183b4ce873bccdef5776fb5c4e72
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D48076896

RSNara added a commit to RSNara/react-native that referenced this pull request Aug 14, 2023
Summary:
Pull Request resolved: facebook#38998

Instead of using the mActivity reference directly, let's just use getCurrentActivity() and setCurrentActivity().

Changelog: [Internal]

Differential Revision: D48076896

fbshipit-source-id: cabcd550ec6fec955a38834b6a80e8b0b33a2e47
@analysis-bot
Copy link

analysis-bot commented Aug 14, 2023

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 8,949,212 +301
android hermes armeabi-v7a n/a --
android hermes x86 n/a --
android hermes x86_64 n/a --
android jsc arm64-v8a 9,542,923 +708
android jsc armeabi-v7a n/a --
android jsc x86 n/a --
android jsc x86_64 n/a --

Base commit: f4f1894
Branch: main

@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D48076896

RSNara added a commit to RSNara/react-native that referenced this pull request Aug 14, 2023
Summary:
Pull Request resolved: facebook#38998

Instead of using the mActivity reference directly, let's just use getCurrentActivity() and setCurrentActivity().

Changelog: [Internal]

Differential Revision: D48076896

fbshipit-source-id: d35b6b1d811b8b2386a6cc1f0e1c69a83521c70b
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D48076896

RSNara added a commit to RSNara/react-native that referenced this pull request Aug 14, 2023
Summary:
Pull Request resolved: facebook#38998

Instead of using the mActivity reference directly, let's just use getCurrentActivity() and setCurrentActivity().

Changelog: [Internal]

Differential Revision: D48076896

fbshipit-source-id: 64734d93339ac3b92bd3a4d18878d5b6ac9279f7
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D48076896

@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D48076896

RSNara added a commit to RSNara/react-native that referenced this pull request Aug 15, 2023
Summary:
Pull Request resolved: facebook#38998

Instead of using the mActivity reference directly, let's just use getCurrentActivity() and setCurrentActivity().

Changelog: [Internal]

Differential Revision: D48076896

fbshipit-source-id: 65dba86bfecf935ae24797c92269538fd2ef3aa6
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D48076896

RSNara added a commit to RSNara/react-native that referenced this pull request Aug 15, 2023
Summary:
Pull Request resolved: facebook#38998

Instead of using the mActivity reference directly, let's just use getCurrentActivity() and setCurrentActivity().

Changelog: [Internal]

Differential Revision: D48076896

fbshipit-source-id: c4d41b167d333f6943c36c38c5ab72d11e9ee62d
RSNara added a commit to RSNara/react-native that referenced this pull request Aug 15, 2023
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: 84956c754e185841c92a57eee7c73c333d728e77
RSNara added a commit to RSNara/react-native that referenced this pull request Aug 21, 2023
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: 5963025fada9030c57682711070ec9c6dc7a97fa
RSNara added a commit to RSNara/react-native that referenced this pull request Aug 21, 2023
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: 42b095972bd15bc3f6a21192935eadc1b65d0e34
RSNara added a commit to RSNara/react-native that referenced this pull request Aug 21, 2023
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: 8ae562ecd67ea42ae0c8eebe60e3806ce2a2a0b4
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D48076896

RSNara added a commit to RSNara/react-native that referenced this pull request Aug 21, 2023
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: b0132a0277f548edd1733396530fec6a74dbb3ce
RSNara added a commit to RSNara/react-native that referenced this pull request Aug 21, 2023
Summary:
Pull Request resolved: facebook#38998

Instead of using the mActivity reference directly, let's just use getCurrentActivity() and setCurrentActivity().

Changelog: [Internal]

Reviewed By: mdvacca

Differential Revision: D48076896

fbshipit-source-id: 44c2a0f922584a06991b3eb90d3e19407f9b29cc
RSNara added a commit to RSNara/react-native that referenced this pull request Aug 21, 2023
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: 2e5e025fd044316b85f321e946ce5a8821a67b48
RSNara added a commit to RSNara/react-native that referenced this pull request Aug 21, 2023
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: b43491dfcb3607bd8dd6c0e7dd72dc22469195d9
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D48076896

RSNara added a commit to RSNara/react-native that referenced this pull request Aug 21, 2023
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: 058c56a2a4a6f3b04e83ba2ff05293be0ae60e8c
RSNara added a commit to RSNara/react-native that referenced this pull request Aug 21, 2023
Summary:
Pull Request resolved: facebook#38998

Instead of using the mActivity reference directly, let's just use getCurrentActivity() and setCurrentActivity().

Changelog: [Internal]

Reviewed By: mdvacca

Differential Revision: D48076896

fbshipit-source-id: 6c317d3ad822a535f5ce0d0b486c85c93257807a
RSNara added a commit to RSNara/react-native that referenced this pull request Aug 21, 2023
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: 1649f1ef112c6d45d6ea7463d2f42396b45685df
RSNara added 4 commits August 21, 2023 11:32
…ebook#39001)

Summary:
Pull Request resolved: facebook#39001

Changelog: [Internal]

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

fbshipit-source-id: 977ede87d2a568899cbf61f983dc4761dfced9af
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: d9b0ca8f3a255919f8fe68c1ec3834e3a0e4f9e3
…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: f6529487f5953d9bae4431a290047fbab43d453c
Summary:
Pull Request resolved: facebook#38998

Instead of using the mActivity reference directly, let's just use getCurrentActivity() and setCurrentActivity().

Changelog: [Internal]

Reviewed By: mdvacca

Differential Revision: D48076896

fbshipit-source-id: 84fbacb3e05d3155b1b2eef05a39cc560a961a33
RSNara added a commit to RSNara/react-native that referenced this pull request Aug 21, 2023
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: c1f8dc3952ce4cc5967939b0eda642e158d64f06
RSNara added a commit to RSNara/react-native that referenced this pull request Aug 21, 2023
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: 7a8ab56037429c9221d23cc603d89f0dd883b099
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D48076896

RSNara added a commit to RSNara/react-native that referenced this pull request Aug 21, 2023
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: b35e00e1fece5b20830b4fb725e1d356bf539317
RSNara added a commit to RSNara/react-native that referenced this pull request Aug 21, 2023
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: f5abf64baef31c9a0e21fe27128e6c41a3655655
RSNara added a commit to RSNara/react-native that referenced this pull request Aug 21, 2023
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: 6e5616d8c43a1039861e052d82307b62aa09cb78
@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label Aug 21, 2023
@facebook-github-bot
Copy link
Contributor

This pull request has been merged in aec2257.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported Merged This PR has been merged. p: Facebook Partner: Facebook Partner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants