From 0b5c61250b290e78b2c37c7c22b8146056cd30d8 Mon Sep 17 00:00:00 2001 From: Felix Oghina Date: Mon, 8 Aug 2016 09:01:16 -0700 Subject: [PATCH] check lifecycle event is coming from current activity Summary: If a paused activity is destroyed (e.g. because of resource contention), we send onHostDestroyed to all modules even if there's an on-screen, resumed activity using the current react instance. This diff adds a check to make sure lifecycle events come from the current activity, and ignores ones that don't. Reviewed By: astreet Differential Revision: D3655422 fbshipit-source-id: 0f95fda124df3732447853b9bc34c40836a4b1da --- .../facebook/react/ReactInstanceManager.java | 27 +++++++++++++++++++ .../react/XReactInstanceManagerImpl.java | 18 +++++++++++++ .../react/bridge/LifecycleEventListener.java | 26 ++++++++++++++---- 3 files changed, 66 insertions(+), 5 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManager.java b/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManager.java index de75fab6e..ad23ebf56 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManager.java @@ -97,8 +97,22 @@ public abstract class ReactInstanceManager { /** * Call this from {@link Activity#onPause()}. This notifies any listening modules so they can do * any necessary cleanup. + * + * @deprecated Use {@link #onHostPause(Activity)} instead. */ + @Deprecated public abstract void onHostPause(); + + /** + * Call this from {@link Activity#onPause()}. This notifies any listening modules so they can do + * any necessary cleanup. The passed Activity is the current Activity being paused. This will + * always be the foreground activity that would be returned by + * {@link ReactContext#getCurrentActivity()}. + * + * @param activity the activity being paused + */ + public abstract void onHostPause(Activity activity); + /** * Use this method when the activity resumes to enable invoking the back button directly from JS. * @@ -117,8 +131,21 @@ public abstract class ReactInstanceManager { /** * Call this from {@link Activity#onDestroy()}. This notifies any listening modules so they can do * any necessary cleanup. + * + * @deprecated use {@link #onHostDestroy(Activity)} instead */ + @Deprecated public abstract void onHostDestroy(); + + /** + * Call this from {@link Activity#onDestroy()}. This notifies any listening modules so they can do + * any necessary cleanup. If the activity being destroyed is not the current activity, no modules + * are notified. + * + * @param activity the activity being destroyed + */ + public abstract void onHostDestroy(Activity activity); + public abstract void onActivityResult(int requestCode, int resultCode, Intent data); public abstract void showDevOptionsDialog(); diff --git a/ReactAndroid/src/main/java/com/facebook/react/XReactInstanceManagerImpl.java b/ReactAndroid/src/main/java/com/facebook/react/XReactInstanceManagerImpl.java index 400af02e7..6be093312 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/XReactInstanceManagerImpl.java +++ b/ReactAndroid/src/main/java/com/facebook/react/XReactInstanceManagerImpl.java @@ -489,6 +489,17 @@ import static com.facebook.systrace.Systrace.TRACE_TAG_REACT_JAVA_BRIDGE; moveToBeforeResumeLifecycleState(); } + @Override + public void onHostPause(Activity activity) { + Assertions.assertNotNull(mCurrentActivity); + Assertions.assertCondition( + activity == mCurrentActivity, + "Pausing an activity that is not the current activity, this is incorrect! " + + "Current activity: " + mCurrentActivity.getClass().getSimpleName() + " " + + "Paused activity: " + activity.getClass().getSimpleName()); + onHostPause(); + } + /** * Use this method when the activity resumes to enable invoking the back button directly from JS. * @@ -525,6 +536,13 @@ import static com.facebook.systrace.Systrace.TRACE_TAG_REACT_JAVA_BRIDGE; mCurrentActivity = null; } + @Override + public void onHostDestroy(Activity activity) { + if (activity == mCurrentActivity) { + onHostDestroy(); + } + } + @Override public void destroy() { UiThreadUtil.assertOnUiThread(); diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/LifecycleEventListener.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/LifecycleEventListener.java index faecb9730..c78d947a0 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/LifecycleEventListener.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/LifecycleEventListener.java @@ -10,23 +10,39 @@ package com.facebook.react.bridge; /** - * Listener for receiving activity/service lifecycle events. + * Listener for receiving activity lifecycle events. + * + * When multiple activities share a react instance, only the most recent one's lifecycle events get + * forwarded to listeners. Consider the following scenarios: + * + * 1. Navigating from Activity A to B will trigger two events: A#onHostPause and B#onHostResume. Any + * subsequent lifecycle events coming from Activity A, such as onHostDestroy, will be ignored. + * 2. Navigating back from Activity B to Activity A will trigger the same events: B#onHostPause and + * A#onHostResume. Any subsequent events coming from Activity B, such as onHostDestroy, are + * ignored. + * 3. Navigating back from Activity A to a non-React Activity or to the home screen will trigger two + * events: onHostPause and onHostDestroy. + * 4. Navigating from Activity A to a non-React Activity B will trigger one event: onHostPause. + * Later, if Activity A is destroyed (e.g. because of resource contention), onHostDestroy is + * triggered. */ public interface LifecycleEventListener { /** - * Called when host (activity/service) receives resume event (e.g. {@link Activity#onResume} + * Called when host activity receives resume event (e.g. {@link Activity#onResume}. Always called + * for the most current activity. */ void onHostResume(); /** - * Called when host (activity/service) receives pause event (e.g. {@link Activity#onPause} + * Called when host activity receives pause event (e.g. {@link Activity#onPause}. Always called + * for the most current activity. */ void onHostPause(); /** - * Called when host (activity/service) receives destroy event (e.g. {@link Activity#onDestroy} + * Called when host activity receives destroy event (e.g. {@link Activity#onDestroy}. Only called + * for the last React activity to be destroyed. */ void onHostDestroy(); - }