From b58f13a456e5ad0b87412d0d40962747c8028d5c Mon Sep 17 00:00:00 2001 From: Kacper Kafara Date: Thu, 21 Nov 2024 12:12:23 +0100 Subject: [PATCH] fix(Android): prevent crash with InsetsObserverProxy already registered (#2526) ## Description Activity restarts do not invalidate JVM state entirely (at least it seems so), therefore InsetsObserverProxy kept being registered on some decorview between activity restarts (context recreations) leading to crash (we guard against multiple registrations). Fixes #2519 Possibly fixes #2507 ## Changes `InsetsOberverProxy` row registers for context lifecycle, when it's destroyed it cleans its state. ## Test code and steps to reproduce Haven't managed to reproduce this myself, but got confirmation in #2507 that this patch works. ## Checklist - [ ] Included code example that can be used to test this change - [ ] Updated TS types - [ ] Updated documentation: - [ ] https://github.com/software-mansion/react-native-screens/blob/main/guides/GUIDE_FOR_LIBRARY_AUTHORS.md - [ ] https://github.com/software-mansion/react-native-screens/blob/main/native-stack/README.md - [ ] https://github.com/software-mansion/react-native-screens/blob/main/src/types.tsx - [ ] https://github.com/software-mansion/react-native-screens/blob/main/src/native-stack/types.tsx - [ ] Ensured that CI passes --- .../rnscreens/InsetsObserverProxy.kt | 54 ++++++++++++++++--- .../swmansion/rnscreens/RNScreensPackage.kt | 4 ++ .../rnscreens/bottomsheet/DimmingFragment.kt | 4 +- 3 files changed, 53 insertions(+), 9 deletions(-) diff --git a/android/src/main/java/com/swmansion/rnscreens/InsetsObserverProxy.kt b/android/src/main/java/com/swmansion/rnscreens/InsetsObserverProxy.kt index 8da1f52ed1..6fb4fc3fbd 100644 --- a/android/src/main/java/com/swmansion/rnscreens/InsetsObserverProxy.kt +++ b/android/src/main/java/com/swmansion/rnscreens/InsetsObserverProxy.kt @@ -1,12 +1,15 @@ package com.swmansion.rnscreens +import android.util.Log import android.view.View import androidx.core.view.OnApplyWindowInsetsListener import androidx.core.view.ViewCompat import androidx.core.view.WindowInsetsCompat +import com.facebook.react.bridge.LifecycleEventListener +import com.facebook.react.bridge.ReactApplicationContext import java.lang.ref.WeakReference -object InsetsObserverProxy : OnApplyWindowInsetsListener { +object InsetsObserverProxy : OnApplyWindowInsetsListener, LifecycleEventListener { private val listeners: ArrayList = arrayListOf() private var eventSourceView: WeakReference = WeakReference(null) @@ -15,8 +18,16 @@ object InsetsObserverProxy : OnApplyWindowInsetsListener { // whether this observer has been initially registered. private var hasBeenRegistered: Boolean = false + // Mainly debug variable to log warnings in case we missed some code path regarding + // context lifetime handling. + private var isObservingContextLifetime: Boolean = false + private var shouldForwardInsetsToView = true + // Allow only when we have not been registered yet or the view we're observing has been + // invalidated due to some lifecycle we have not observed. + private val allowRegistration get() = !hasBeenRegistered || eventSourceView.get() == null + override fun onApplyWindowInsets( v: View, insets: WindowInsetsCompat, @@ -34,6 +45,34 @@ object InsetsObserverProxy : OnApplyWindowInsetsListener { return rollingInsets } + // Call this method to ensure that the observer proxy is + // unregistered when apps is destroyed or we change activity. + fun registerWithContext(context: ReactApplicationContext) { + if (isObservingContextLifetime) { + Log.w( + "[RNScreens]", + "InsetObserverProxy registers on new context while it has not been invalidated on the old one. Please report this as issue at https://github.com/software-mansion/react-native-screens/issues", + ) + } + + isObservingContextLifetime = true + context.addLifecycleEventListener(this) + } + + override fun onHostResume() = Unit + + override fun onHostPause() = Unit + + override fun onHostDestroy() { + val observedView = getObservedView() + if (hasBeenRegistered && observedView != null) { + ViewCompat.setOnApplyWindowInsetsListener(observedView, null) + hasBeenRegistered = false + eventSourceView.clear() + } + isObservingContextLifetime = false + } + fun addOnApplyWindowInsetsListener(listener: OnApplyWindowInsetsListener) { listeners.add(listener) } @@ -42,16 +81,17 @@ object InsetsObserverProxy : OnApplyWindowInsetsListener { listeners.remove(listener) } - fun registerOnView(view: View) { - if (!hasBeenRegistered) { + /** + * @return boolean whether the proxy registered as a listener on a view + */ + fun registerOnView(view: View): Boolean { + if (allowRegistration) { ViewCompat.setOnApplyWindowInsetsListener(view, this) eventSourceView = WeakReference(view) hasBeenRegistered = true - } else if (getObservedView() != view) { - throw IllegalStateException( - "[RNScreens] Attempt to register InsetsObserverProxy on $view while it has been already registered on ${getObservedView()}", - ) + return true } + return false } fun unregister() { diff --git a/android/src/main/java/com/swmansion/rnscreens/RNScreensPackage.kt b/android/src/main/java/com/swmansion/rnscreens/RNScreensPackage.kt index 84124dd8ed..e0a0f01eb0 100644 --- a/android/src/main/java/com/swmansion/rnscreens/RNScreensPackage.kt +++ b/android/src/main/java/com/swmansion/rnscreens/RNScreensPackage.kt @@ -29,6 +29,10 @@ class RNScreensPackage : TurboReactPackage() { screenDummyLayoutHelper = ScreenDummyLayoutHelper(reactContext) } + // Proxy needs to register for lifecycle events in order to unregister itself + // on activity restarts. + InsetsObserverProxy.registerWithContext(reactContext) + return listOf>( ScreenContainerViewManager(), ScreenViewManager(), diff --git a/android/src/main/java/com/swmansion/rnscreens/bottomsheet/DimmingFragment.kt b/android/src/main/java/com/swmansion/rnscreens/bottomsheet/DimmingFragment.kt index b719be8861..6ed054f8a4 100644 --- a/android/src/main/java/com/swmansion/rnscreens/bottomsheet/DimmingFragment.kt +++ b/android/src/main/java/com/swmansion/rnscreens/bottomsheet/DimmingFragment.kt @@ -208,7 +208,7 @@ class DimmingFragment( override fun onStart() { // This is the earliest we can access child fragment manager & present another fragment super.onStart() - insetsProxy.registerOnView(requireRootView()) + insetsProxy.registerOnView(requireDecorView()) presentNestedFragment() } @@ -307,7 +307,7 @@ class DimmingFragment( } } - private fun requireRootView(): View = + private fun requireDecorView(): View = checkNotNull(screen.reactContext.currentActivity) { "[RNScreens] Attempt to access activity on detached context" } .window.decorView