fix(android): onHostPause crashes with multiple active activities

[hannojg]

Summary:

This PR addresses the problem outlined here:

• [android] java.lang.AssertionError: Pausing an activity that is not the current activity, this is incorrect! #54121

Problems tl;dr: ReactHost doesn't account for the fact that there can be multiple activities at the same time foregrounded.

There are scenarios where we can have multiple activities can be in resume, and we don't want to pause the ReactHost while the app is still in foreground (thats my assumption, maybe I am wrong here):

• ReactHost is held by the MainApplication (in the past it was the ReactActivity). It wants to be notified when the whole app gets paused to:
  • disable the hardware back btn handler to JS
  • call lifecycle methods for other classes subscribing to it

react-native/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/runtime/ReactHostImpl.kt

Lines 258 to 281 in e926ad0

	override fun onHostPause(activity: Activity?) {
	val method = "onHostPause(activity)"
	stateTracker.enterState(method)
	val currentActivity = this.currentActivity
	if (currentActivity != null) {
	val isSameActivity = activity === currentActivity
	if (!isSameActivity) {
	val currentActivityClass = currentActivity.javaClass.simpleName
	val activityClass = if (activity == null) "null" else activity.javaClass.simpleName
	val isNotSameActivityMessage =
	"Pausing an activity that is not the current activity, this is incorrect! Current activity: $currentActivityClass Paused activity: $activityClass"
	if (ReactNativeFeatureFlags.skipActivityIdentityAssertionOnHostPause()) {
	FLog.w(TAG, method, isNotSameActivityMessage)
	} else {
	Assertions.assertCondition(isSameActivity, isNotSameActivityMessage)
	}
	}
	}
	maybeEnableDevSupport(false)
	defaultHardwareBackBtnHandler = null
	reactLifecycleStateManager.moveToOnHostPause(currentReactContext, currentActivity)
	}

However, when the app is still in foreground by means of another ReactActivity, neither do we want to disable the back btn handling, nor notify that the app is full on pause.

Hence, the fix is to:

• Keep track of active activities
• Only pause when there is no active activity any longer

Note

Currently I removed all the checking code in onHostPause, as i believe its not needed, and potentially inherited from times where the ReactInstance was bound to a specific activity on the old arch.
I can ofc add it back, or guard my new approach around a feature flag if thats preferred.

Changelog:

[ANDROID] [FIXED] - Android can crash when multiple activities are in foreground and one pauses (fixes "Pausing an activity that is not the current activity, this is incorrect!")

Test Plan:

I tested these changes on top of the reproduction changes mentioned in the issue, and confirm that the app no longer crashes:

output_fixed.mp4

[hannojg]

After thoughts

Right now this would break the documentation on LifecycleEventListener a bit, however, multiple active activities is a case it didn't consider earlier:

react-native/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/LifecycleEventListener.kt

Lines 12 to 28 in e926ad0

	/**
	* 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 {

So I think either we should

• Update the documentation here as well to mention this case, or
• Maybe think of another fix that tracks the current activity by which activity was last interacted with, or
• We maybe want to revisit the design choices for LifecycleEventListener and how it lives on the ReactContext (MainApplication), while it tries to notify about lifecycle events of activities

[javache]

You'll see there's a featureflag in this code skipActivityIdentityAssertionOnHostPause, we're actively working on this!

Thanks for the PR, @fabriziocucci: does this approach make sense?

[fabriziocucci]

Thanks a lot for the PR @hannojg! 🙌

I'm gonna be looking at this (hopefully) next week, will let you know as soon as I've evaluated this and potentially other options.