[Android] Add onUserLeaveHint support to ReactActivityDelegate
#42741
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
|
Hi @behenate! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks! |
Base commit: ae42e02 |
facebook-github-bot
added
CLA Signed
cortinico
requested changes
Jan 31, 2024
packages/react-native/ReactAndroid/src/main/java/com/facebook/react/ReactHost.kt
Outdated
Show resolved
Hide resolved
|
@cortinico has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@cortinico merged this pull request in 3b6c522. |
|
This needs to be reverted as it's introducing a crash internally. I'll follow up on it tomorrow @behenate |
|
This pull request has been reverted by 78f5002. |
cortinico
reviewed
Feb 2, 2024
Comment on lines
+601
to
+609
| if (mCurrentActivity != null) { | ||
| Assertions.assertCondition( | ||
| activity == mCurrentActivity, | ||
| "Called onUserLeaveHint on an activity that is not the current activity, this is incorrect! " | ||
| + "Current activity: " | ||
| + mCurrentActivity.getClass().getSimpleName() | ||
| + " " | ||
| + "Leaving activity: " | ||
| + activity.getClass().getSimpleName()); |
There was a problem hiding this comment.
@behenate Ok so one of our product was crashing on this assert
I haven't had the time to investigate what's going on, but practically we should understand if this assert is actually necessary.
That's the stacktrace:
02-01 14:38:52.320 4500 4522 V fb-android-monitor: java.lang.AssertionError
02-01 14:38:52.320 4500 4522 V fb-android-monitor: at com.facebook.infer.annotation.Assertions.assertCondition(Assertions.java:82)
02-01 14:38:52.320 4500 4522 V fb-android-monitor: at com.facebook.react.ReactInstanceManager.onUserLeaveHint(ReactInstanceManager.java:598)
02-01 14:38:52.320 4500 4522 V fb-android-monitor: at com.facebook.react.ReactDelegate.onUserLeaveHint(ReactDelegate.java:107)
02-01 14:38:52.320 4500 4522 V fb-android-monitor: at com.facebook.react.ReactActivityDelegate.onUserLeaveHint(ReactActivityDelegate.java:127)
02-01 14:38:52.320 4500 4522 V fb-android-monitor: at com.facebook.react.ReactActivity.onUserLeaveHint(ReactActivity.java:111)
02-01 14:38:52.320 4500 4522 V fb-android-monitor: at android.app.Activity.performUserLeaving(Activity.java:8189)
02-01 14:38:52.320 4500 4522 V fb-android-monitor: at android.app.Instrumentation.callActivityOnUserLeaving(Instrumentation.java:1520)
02-01 14:38:52.320 4500 4522 V fb-android-monitor: at android.app.ActivityThread.performUserLeavingActivity(ActivityThread.java:4659)
02-01 14:38:52.320 4500 4522 V fb-android-monitor: at android.app.ActivityThread.handlePauseActivity(ActivityThread.java:4643)
02-01 14:38:52.320 4500 4522 V fb-android-monitor: at android.app.servertransaction.PauseActivityItem.execute(PauseActivityItem.java:46)
02-01 14:38:52.320 4500 4522 V fb-android-monitor: at android.app.servertransaction.TransactionExecutor.executeLifecycleState(TransactionExecutor.java:176)
02-01 14:38:52.320 4500 4522 V fb-android-monitor: at android.app.servertransaction.TransactionExecutor.execute(TransactionExecutor.java:97)
02-01 14:38:52.320 4500 4522 V fb-android-monitor: at android.app.ActivityThread$H.handleMessage(ActivityThread.java:2066)
02-01 14:38:52.320 4500 4522 V fb-android-monitor: at android.os.Handler.dispatchMessage(Handler.java:106)
02-01 14:38:52.320 4500 4522 V fb-android-monitor: at android.os.Looper.loop(Looper.java:223)
02-01 14:38:52.320 4500 4522 V fb-android-monitor: at android.app.ActivityThread.main(ActivityThread.java:7656)
02-01 14:38:52.320 4500 4522 V fb-android-monitor: at java.lang.reflect.Method.invoke(Native Method)
02-01 14:38:52.320 4500 4522 V fb-android-monitor: at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:592)
02-01 14:38:52.320 4500 4522 V fb-android-monitor: at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:947)
02-01 14:38:52.320 4500 4522 V fb-android-monitor: at Z.init(unknown)
Could you resubmit 3b6c522 without this assert?
There was a problem hiding this comment.
@behenate are you able to follow-up here?
There was a problem hiding this comment.
@cortinico Sorry for no reply, I'm currently out of office until February 14th, I will follow up when I'm back :)
facebook-github-bot
pushed a commit
that referenced
this pull request
Mar 15, 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. 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` Pull Request resolved: #43488 Test Plan: Tested in the `rn-tester` app - callbacks are correctly called on both old and new architecture. Reviewed By: javache Differential Revision: D54905564 Pulled By: cortinico fbshipit-source-id: 3f24271405f66bf3a9450320a484e522224eefc1
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
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:
This PR adds
onUserLeaveHintsupport into theReactActivityDelegate. It allows modules to receive an event every time user moves the app into the background. This is slightly different thanonPause- it's called only when the user intentionally moves the app into the background, e.g. when receiving a callonPauseshould be called butonUserLeaveHintshouldn't.This feature is especially useful for libraries implementing features like Picture in Picture (PiP), where using
onUserLeaveHintis the recommended way of auto-entering PiP for android < 12.Changelog:
[ANDROID] [ADDED] - Added
onUserLeaveHintsupport intoReactActivityDelegateTest Plan:
Tested in the
rn-testerapp - callbacks are correctly called on both old and new architecture.