From 2a7ecc50afeeea9a6c69c8c3926028cd1a4143dd Mon Sep 17 00:00:00 2001 From: Kacper Kafara Date: Wed, 20 Nov 2024 14:14:03 +0100 Subject: [PATCH 1/3] Resister InsetObserverProxy with context when package is created --- .../rnscreens/InsetsObserverProxy.kt | 23 ++++++++++++++++++- .../swmansion/rnscreens/RNScreensPackage.kt | 4 ++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/android/src/main/java/com/swmansion/rnscreens/InsetsObserverProxy.kt b/android/src/main/java/com/swmansion/rnscreens/InsetsObserverProxy.kt index 8da1f52ed1..ac9214ad7f 100644 --- a/android/src/main/java/com/swmansion/rnscreens/InsetsObserverProxy.kt +++ b/android/src/main/java/com/swmansion/rnscreens/InsetsObserverProxy.kt @@ -4,9 +4,11 @@ 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) @@ -34,6 +36,25 @@ 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) { + 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() + } + } + fun addOnApplyWindowInsetsListener(listener: OnApplyWindowInsetsListener) { listeners.add(listener) } 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(), From 21bbf5664520ac93a3f936e1856942abc149a4ad Mon Sep 17 00:00:00 2001 From: Kacper Kafara Date: Thu, 21 Nov 2024 11:41:06 +0100 Subject: [PATCH 2/3] Return result boolean instead of crashing hard --- .../swmansion/rnscreens/InsetsObserverProxy.kt | 17 +++++++++++------ .../rnscreens/bottomsheet/DimmingFragment.kt | 4 ++-- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/android/src/main/java/com/swmansion/rnscreens/InsetsObserverProxy.kt b/android/src/main/java/com/swmansion/rnscreens/InsetsObserverProxy.kt index ac9214ad7f..76c9e3f9e7 100644 --- a/android/src/main/java/com/swmansion/rnscreens/InsetsObserverProxy.kt +++ b/android/src/main/java/com/swmansion/rnscreens/InsetsObserverProxy.kt @@ -19,6 +19,10 @@ object InsetsObserverProxy : OnApplyWindowInsetsListener, LifecycleEventListener 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, @@ -63,16 +67,17 @@ object InsetsObserverProxy : OnApplyWindowInsetsListener, LifecycleEventListener 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/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 From 8023b6e6668c972986163d3777217957f3cb6124 Mon Sep 17 00:00:00 2001 From: Kacper Kafara Date: Thu, 21 Nov 2024 11:52:47 +0100 Subject: [PATCH 3/3] Add some additional logging logic --- .../com/swmansion/rnscreens/InsetsObserverProxy.kt | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/android/src/main/java/com/swmansion/rnscreens/InsetsObserverProxy.kt b/android/src/main/java/com/swmansion/rnscreens/InsetsObserverProxy.kt index 76c9e3f9e7..6fb4fc3fbd 100644 --- a/android/src/main/java/com/swmansion/rnscreens/InsetsObserverProxy.kt +++ b/android/src/main/java/com/swmansion/rnscreens/InsetsObserverProxy.kt @@ -1,5 +1,6 @@ package com.swmansion.rnscreens +import android.util.Log import android.view.View import androidx.core.view.OnApplyWindowInsetsListener import androidx.core.view.ViewCompat @@ -17,6 +18,10 @@ object InsetsObserverProxy : OnApplyWindowInsetsListener, LifecycleEventListener // 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 @@ -43,6 +48,14 @@ object InsetsObserverProxy : OnApplyWindowInsetsListener, LifecycleEventListener // 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) } @@ -57,6 +70,7 @@ object InsetsObserverProxy : OnApplyWindowInsetsListener, LifecycleEventListener hasBeenRegistered = false eventSourceView.clear() } + isObservingContextLifetime = false } fun addOnApplyWindowInsetsListener(listener: OnApplyWindowInsetsListener) {