Skip to content

Commit

Permalink
fix(Android): prevent crash with InsetsObserverProxy already register…
Browse files Browse the repository at this point in the history
…ed (#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: <!-- For adding new props to native-stack
-->
- [ ]
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
  • Loading branch information
kkafar authored Nov 21, 2024
1 parent 1362858 commit b58f13a
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 9 deletions.
Original file line number Diff line number Diff line change
@@ -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<OnApplyWindowInsetsListener> = arrayListOf()
private var eventSourceView: WeakReference<View> = WeakReference(null)

Expand All @@ -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,
Expand All @@ -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)
}
Expand All @@ -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() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<ViewManager<*, *>>(
ScreenContainerViewManager(),
ScreenViewManager(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}

Expand Down Expand Up @@ -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

Expand Down

0 comments on commit b58f13a

Please sign in to comment.