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

[Android] Add onUserLeaveHint support to ReactActivityDelegate #43488

Closed

Conversation

behenate
Copy link
Contributor

Summary:

This PR adds onUserLeaveHint support into the ReactActivityDelegate. It allows modules to receive an event every time user moves the app into the background. This is slightly different than onPause - it's called only when the user intentionally moves the app into the background, e.g. when receiving a call onPause should be called but onUserLeaveHint shouldn't.

This feature is especially useful for libraries implementing features like Picture in Picture (PiP), where using onUserLeaveHint is the recommended way of auto-entering PiP for android < 12.

This is a re-submission of #42741. The problematic assert has been changed to an if - it's not a problem if onUserLeaveHint is received from an activity different to the current one, but in that case we shouldn't emit the event.

Changelog:

[ANDROID] [ADDED] - Added onUserLeaveHint support into ReactActivityDelegate

Test Plan:

Tested in the rn-tester app - callbacks are correctly called on both old and new architecture.

@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. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. labels Mar 14, 2024
@analysis-bot
Copy link

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 18,073,866 -9
android hermes armeabi-v7a n/a --
android hermes x86 n/a --
android hermes x86_64 n/a --
android jsc arm64-v8a 21,440,610 -11
android jsc armeabi-v7a n/a --
android jsc x86 n/a --
android jsc x86_64 n/a --

Base commit: 6c50418
Branch: main

@facebook-github-bot
Copy link
Contributor

@cortinico has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label Mar 15, 2024
@facebook-github-bot
Copy link
Contributor

@cortinico merged this pull request in 3cf6c64.

@facebook-github-bot
Copy link
Contributor

This pull request has been reverted by f2f62cd.

@cortinico
Copy link
Contributor

@behenate sadly this had to be reverted once more due to a crash on one of our internal products. I'll get back to you on Monday with more details and we'll see what are the next steps

@behenate
Copy link
Contributor Author

behenate commented Mar 16, 2024

@cortinico Interesting, I'm not sure what could be causing a crash in this code as it's pretty straightforward. Waiting for more info!

@cortinico
Copy link
Contributor

@behenate so the app gets stuck on the loading screen with this stacktrace:

E  Handling exception for crash
java.lang.AssertionError
	at com.facebook.infer.annotation.Assertions.assertCondition(Assertions.java:82)
	at com.facebook.react.ReactInstanceManager.onUserLeaveHint(ReactInstanceManager.java:596)
	at com.facebook.react.ReactDelegate.onUserLeaveHint(ReactDelegate.java:105)
	at com.facebook.react.ReactActivityDelegate.onUserLeaveHint(ReactActivityDelegate.java:129)
	at com.facebook.react.ReactActivity.onUserLeaveHint(ReactActivity.java:111)
	at android.app.Activity.performUserLeaving(Activity.java:9167)
	at android.app.Instrumentation.callActivityOnUserLeaving(Instrumentation.java:1667)
	at android.app.ActivityThread.performUserLeavingActivity(ActivityThread.java:5693)
	at android.app.ActivityThread.handlePauseActivity(ActivityThread.java:5673)
	at android.app.servertransaction.PauseActivityItem.execute(PauseActivityItem.java:48)
	at android.app.servertransaction.ActivityTransactionItem.execute(ActivityTransactionItem.java:45)
	at android.app.servertransaction.TransactionExecutor.executeLifecycleState(TransactionExecutor.java:180)
	at android.app.servertransaction.TransactionExecutor.execute(TransactionExecutor.java:98)
	at android.app.ActivityThread$H.handleMessage(ActivityThread.java:2685)
	at android.os.Handler.dispatchMessage(Handler.java:106)
	at android.os.Looper.loopOnce(Looper.java:230)
	at android.os.Looper.loop(Looper.java:319)
	at android.app.ActivityThread.main(ActivityThread.java:8893)
	at java.lang.reflect.Method.invoke(Native Method)
	at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:608)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1103)

@cortinico
Copy link
Contributor

I'm unsure why onUserLeaveHint gets called when the app starts, but that's probably fine to don't assert around mRequireActivity and just go with:

      UiThreadUtil.assertOnUiThread();

      ReactContext currentContext = getCurrentReactContext();
      if (currentContext != null) {
        currentContext.onUserLeaveHint(activity);
      }

@behenate
Copy link
Contributor Author

@cortinico I'm also not sure why onUserLeaveHint is called this early in the app, but it should be safe to ignore it. I will make another PR tomorrow. Hopefully third time's the charm 🙃

@cortinico
Copy link
Contributor

@cortinico I'm also not sure why onUserLeaveHint is called this early in the app, but it should be safe to ignore it. I will make another PR tomorrow. Hopefully third time's the charm 🙃

Yup I also have repros instruction to verify if the app still crashes so we can see if all is green

facebook-github-bot pushed a commit that referenced this pull request Mar 20, 2024
Summary:
This PR adds `onUserLeaveHint` support into the `ReactActivityDelegate`. It allows modules to receive an event every time user moves the app into the background. This is slightly different than `onPause` - it's called only when the user intentionally moves the app into the background, e.g. when receiving a call `onPause` should be called but `onUserLeaveHint` shouldn't.

This feature is especially useful for libraries implementing features like Picture in Picture (PiP), where using `onUserLeaveHint` is the [recommended way of auto-entering PiP](https://developer.android.com/develop/ui/views/picture-in-picture#:~:text=You%20might%20want%20to%20include%20logic%20that%20switches%20an%20activity%20into%20PiP%20mode%20instead%20of%20going%20into%20the%20background.%20For%20example%2C%20Google%20Maps%20switches%20to%20PiP%20mode%20if%20the%20user%20presses%20the%20home%20or%20recents%20button%20while%20the%20app%20is%20navigating.%20You%20can%20catch%20this%20case%20by%20overriding%20onUserLeaveHint()%3A) for android < 12.

This is a re-submission of #42741 and #43488 without problematic asserts, which were unnecessary (`onUserLeaveHint` is not critical to the lifecycle of the app), but were causing problems in some apps.

## Changelog:

[ANDROID] [ADDED] - Added `onUserLeaveHint` support into `ReactActivityDelegate`

Pull Request resolved: #43567

Test Plan: Tested in the `rn-tester` app - callbacks are correctly called on both old and new architecture.

Reviewed By: javache

Differential Revision: D55123632

Pulled By: cortinico

fbshipit-source-id: 144a1d84b691af9cf3c0cffad446e674b4b68927
RSNara added a commit to RSNara/react-native that referenced this pull request Mar 22, 2024
…ok#43488"

Summary:
Original commit changeset: 144a1d84b691

Original Phabricator Diff: D55123632

Differential Revision: D55218594
RSNara added a commit to RSNara/react-native that referenced this pull request Mar 22, 2024
…ok#43488"

Summary:
Original commit changeset: 144a1d84b691

Original Phabricator Diff: D55123632

Differential Revision: D55218594
RSNara added a commit to RSNara/react-native that referenced this pull request Mar 22, 2024
…ok#43488"

Summary:
Original commit changeset: 144a1d84b691

Original Phabricator Diff: D55123632

Differential Revision: D55218594
RSNara added a commit to RSNara/react-native that referenced this pull request Mar 22, 2024
…ok#43488" (facebook#43623)

Summary:
Pull Request resolved: facebook#43623

Original commit changeset: 144a1d84b691

Original Phabricator Diff: D55123632

Differential Revision: D55218594
RSNara added a commit to RSNara/react-native that referenced this pull request Mar 22, 2024
…ok#43488"

Summary:
Original commit changeset: 144a1d84b691

Original Phabricator Diff: D55123632

Differential Revision: D55218594
RSNara added a commit to RSNara/react-native that referenced this pull request Mar 22, 2024
…ok#43488"

Summary:
Original commit changeset: 144a1d84b691

Original Phabricator Diff: D55123632

Differential Revision: D55218594
RSNara added a commit to RSNara/react-native that referenced this pull request Mar 22, 2024
…ok#43488"

Summary:
Original commit changeset: 144a1d84b691

Original Phabricator Diff: D55123632

Differential Revision: D55218594
RSNara added a commit to RSNara/react-native that referenced this pull request Mar 22, 2024
…ok#43488"

Summary:
Original commit changeset: 144a1d84b691

Original Phabricator Diff: D55123632

Differential Revision: D55218594
RSNara added a commit to RSNara/react-native that referenced this pull request Mar 22, 2024
…ok#43488"

Summary:
Original commit changeset: 144a1d84b691

Original Phabricator Diff: D55123632

Differential Revision: D55218594
RSNara added a commit to RSNara/react-native that referenced this pull request Mar 22, 2024
…ok#43488"

Summary:
Original commit changeset: 144a1d84b691

Original Phabricator Diff: D55123632

Differential Revision: D55218594
RSNara added a commit to RSNara/react-native that referenced this pull request Mar 22, 2024
…ok#43488"

Summary:
Original commit changeset: 144a1d84b691

Original Phabricator Diff: D55123632

Differential Revision: D55218594
RSNara added a commit to RSNara/react-native that referenced this pull request Mar 22, 2024
…ok#43488"

Summary:
Original commit changeset: 144a1d84b691

Original Phabricator Diff: D55123632

Differential Revision: D55218594
RSNara added a commit to RSNara/react-native that referenced this pull request Mar 22, 2024
…ok#43488"

Summary:
Original commit changeset: 144a1d84b691

Original Phabricator Diff: D55123632

Differential Revision: D55218594
RSNara added a commit to RSNara/react-native that referenced this pull request Mar 22, 2024
…ok#43488"

Summary:
Original commit changeset: 144a1d84b691

Original Phabricator Diff: D55123632

Differential Revision: D55218594
RSNara added a commit to RSNara/react-native that referenced this pull request Mar 22, 2024
…ok#43488"

Summary:
Original commit changeset: 144a1d84b691

Original Phabricator Diff: D55123632

Differential Revision: D55218594
RSNara added a commit to RSNara/react-native that referenced this pull request Mar 22, 2024
…ok#43488"

Summary:
Original commit changeset: 144a1d84b691

Original Phabricator Diff: D55123632

Differential Revision: D55218594
RSNara added a commit to RSNara/react-native that referenced this pull request Mar 22, 2024
…ok#43488"

Summary:
Original commit changeset: 144a1d84b691

Original Phabricator Diff: D55123632

Differential Revision: D55218594
RSNara added a commit to RSNara/react-native that referenced this pull request Mar 22, 2024
…ok#43488"

Summary:
Original commit changeset: 144a1d84b691

Original Phabricator Diff: D55123632

Differential Revision: D55218594
RSNara added a commit to RSNara/react-native that referenced this pull request Mar 22, 2024
…ok#43488"

Summary:
Original commit changeset: 144a1d84b691

Original Phabricator Diff: D55123632

Differential Revision: D55218594
RSNara added a commit to RSNara/react-native that referenced this pull request Mar 22, 2024
…ok#43488"

Summary:
Original commit changeset: 144a1d84b691

Original Phabricator Diff: D55123632

Differential Revision: D55218594
RSNara added a commit to RSNara/react-native that referenced this pull request Mar 22, 2024
…ok#43488"

Summary:
Original commit changeset: 144a1d84b691

Original Phabricator Diff: D55123632

Differential Revision: D55218594
RSNara added a commit to RSNara/react-native that referenced this pull request Mar 22, 2024
…ok#43488"

Summary:
Original commit changeset: 144a1d84b691

Original Phabricator Diff: D55123632

Differential Revision: D55218594
RSNara added a commit to RSNara/react-native that referenced this pull request Mar 22, 2024
…ok#43488"

Summary:
Original commit changeset: 144a1d84b691

Original Phabricator Diff: D55123632

Differential Revision: D55218594
RSNara added a commit to RSNara/react-native that referenced this pull request Mar 22, 2024
…ok#43488"

Summary:
Original commit changeset: 144a1d84b691

Original Phabricator Diff: D55123632

Differential Revision: D55218594
RSNara added a commit to RSNara/react-native that referenced this pull request Mar 22, 2024
…ok#43488"

Summary:
Original commit changeset: 144a1d84b691

Original Phabricator Diff: D55123632

Differential Revision: D55218594
RSNara added a commit to RSNara/react-native that referenced this pull request Mar 22, 2024
…ok#43488"

Summary:
Original commit changeset: 144a1d84b691

Original Phabricator Diff: D55123632

Differential Revision: D55218594
RSNara added a commit to RSNara/react-native that referenced this pull request Mar 22, 2024
…ok#43488"

Summary:
Original commit changeset: 144a1d84b691

Original Phabricator Diff: D55123632

Differential Revision: D55218594
RSNara added a commit to RSNara/react-native that referenced this pull request Mar 22, 2024
…ok#43488"

Summary:
Original commit changeset: 144a1d84b691

Original Phabricator Diff: D55123632

Differential Revision: D55218594
rubennorte added a commit to rubennorte/react-native that referenced this pull request Mar 25, 2024
…ok#43488"

Summary: This change broke some apps at Meta. Reverting until we figure out a safe way to land this.

Differential Revision: D55324557
rubennorte added a commit to rubennorte/react-native that referenced this pull request Mar 25, 2024
…ok#43488"

Summary:
Changelog: [internal]

This change broke some apps at Meta. Reverting until we figure out a safe way to land this.

Differential Revision: D55324557
@cortinico
Copy link
Contributor

Sadly we have to revert this once more as it's causing a couple of internal apps to fail. Here the stacktrace:

#43642

facebook-github-bot pushed a commit that referenced this pull request Mar 25, 2024
#43642)

Summary:
Pull Request resolved: #43642

Changelog: [internal]

This change broke some apps at Meta. Reverting until we figure out a safe way to land this.

Reviewed By: rshest

Differential Revision: D55324557

fbshipit-source-id: 684d3dc3780fc41e2e91f367d97fa5cd392e6638
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. Merged This PR has been merged. Reverted Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants