Skip to content

Commit 836737d

Browse files
committed
Fix memory leak related to ActionBar
1 parent c231ff0 commit 836737d

4 files changed

Lines changed: 135 additions & 0 deletions

File tree

android/src/main/java/com/swmansion/rnscreens/ScreenStackFragment.kt

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ import android.view.View
1414
import android.view.ViewGroup
1515
import android.view.animation.Animation
1616
import android.widget.LinearLayout
17+
import androidx.appcompat.app.ActionBar
18+
import androidx.appcompat.app.AppCompatActivity
1719
import androidx.appcompat.widget.Toolbar
1820
import androidx.coordinatorlayout.widget.CoordinatorLayout
1921
import androidx.core.view.ViewCompat
@@ -57,6 +59,8 @@ class ScreenStackFragment :
5759
private var isToolbarShadowHidden = false
5860
private var isToolbarTranslucent = false
5961

62+
private var ownedActionBar: ActionBar? = null
63+
6064
private lateinit var sheetTransitionCoordinator: BottomSheetTransitionCoordinator
6165

6266
private var lastFocusedChild: View? = null
@@ -301,6 +305,37 @@ class ScreenStackFragment :
301305
super.onViewCreated(view, savedInstanceState)
302306
}
303307

308+
internal fun onActionBarSet(actionBar: ActionBar) {
309+
ownedActionBar = actionBar
310+
}
311+
312+
override fun onDestroyView() {
313+
// ScreenStackHeaderConfig.onUpdate() calls activity.setSupportActionBar(toolbar) each time
314+
// the top screen updates. AppCompatDelegateImpl stores the resulting ToolbarActionBar in
315+
// its mActionBar field for the lifetime of the activity. When a screen is popped and the new
316+
// top screen does not install a replacement action bar (e.g. headerShown: false),
317+
// the stale ToolbarActionBar — and the entire object graph hanging off the toolbar is never released.
318+
// When this fragment is being removed, we're clearing the activity's support action bar if it
319+
// still belongs to us. This will break the retention chain:
320+
// - AppCompatDelegateImpl.mActionBar
321+
// - ToolbarActionBar.mDecorToolbar
322+
// - ToolbarWidgetWrapper.mToolbar
323+
// - DebugMenuToolbar.config
324+
// - ScreenStackHeaderConfig.mParent
325+
// - Screen.fragment
326+
if (isRemoving) {
327+
ownedActionBar?.let { owned ->
328+
(activity as? AppCompatActivity)?.let { appCompat ->
329+
if (appCompat.supportActionBar === owned) {
330+
appCompat.setSupportActionBar(null)
331+
}
332+
}
333+
}
334+
ownedActionBar = null
335+
}
336+
super.onDestroyView()
337+
}
338+
304339
override fun onCreateAnimation(
305340
transit: Int,
306341
enter: Boolean,

android/src/main/java/com/swmansion/rnscreens/ScreenStackHeaderConfig.kt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -244,6 +244,8 @@ class ScreenStackHeaderConfig(
244244
activity.setSupportActionBar(toolbar)
245245
// non-null toolbar is set in the line above and it is used here
246246
val actionBar = requireNotNull(activity.supportActionBar)
247+
// notify the fragment so it can clear this action bar reference when being removed.
248+
screenFragment?.onActionBarSet(actionBar)
247249

248250
// hide back button
249251
actionBar.setDisplayHomeAsUpEnabled(
Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
import React from 'react';
2+
import { View, Text, Button, StyleSheet } from 'react-native';
3+
import { NavigationContainer, ParamListBase } from '@react-navigation/native';
4+
import {
5+
createNativeStackNavigator,
6+
NativeStackNavigationProp,
7+
} from '@react-navigation/native-stack';
8+
9+
type RouteParamList = {
10+
Home: undefined;
11+
Details: undefined;
12+
Settings: undefined;
13+
};
14+
15+
type NavigationProp<ParamList extends ParamListBase> = {
16+
navigation: NativeStackNavigationProp<ParamList>;
17+
};
18+
19+
type StackNavigationProp = NavigationProp<RouteParamList>;
20+
21+
const Stack = createNativeStackNavigator<RouteParamList>();
22+
23+
function HomeScreen({ navigation }: StackNavigationProp) {
24+
return (
25+
<View style={styles.container}>
26+
<Text style={styles.title}>Home Screen</Text>
27+
<Button
28+
title="Push Details"
29+
onPress={() => navigation.navigate('Details')}
30+
/>
31+
</View>
32+
);
33+
}
34+
35+
function DetailScreen({ navigation }: StackNavigationProp) {
36+
return (
37+
<View style={styles.container}>
38+
<Text style={styles.title}>Details Screen</Text>
39+
<Button
40+
title="Push Settings"
41+
onPress={() => navigation.navigate('Settings')}
42+
/>
43+
<View style={styles.spacer} />
44+
<Button
45+
title="Pop (Go Back)"
46+
onPress={() => navigation.goBack()}
47+
color="red"
48+
/>
49+
</View>
50+
);
51+
}
52+
53+
function DeepDetailScreen({ navigation }: StackNavigationProp) {
54+
return (
55+
<View style={styles.container}>
56+
<Text style={styles.title}>Settings Screen</Text>
57+
<View style={styles.spacer} />
58+
<Button
59+
title="Pop to Top"
60+
onPress={() => navigation.popToTop()}
61+
color="red"
62+
/>
63+
</View>
64+
);
65+
}
66+
67+
export default function App() {
68+
return (
69+
<NavigationContainer>
70+
<Stack.Navigator>
71+
<Stack.Screen
72+
name="Home"
73+
component={HomeScreen}
74+
options={{ headerShown: false }}
75+
/>
76+
<Stack.Screen name="Details" component={DetailScreen} />
77+
<Stack.Screen name="Settings" component={DeepDetailScreen} />
78+
</Stack.Navigator>
79+
</NavigationContainer>
80+
);
81+
}
82+
83+
const styles = StyleSheet.create({
84+
container: {
85+
flex: 1,
86+
alignItems: 'center',
87+
justifyContent: 'center',
88+
},
89+
title: {
90+
fontSize: 20,
91+
fontWeight: 'bold',
92+
marginBottom: 10,
93+
},
94+
spacer: {
95+
height: 20,
96+
},
97+
});

apps/src/tests/issue-tests/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,7 @@ export { default as Test3770 } from './Test3770';
187187
export { default as Test3793 } from './Test3793';
188188
export { default as Test3816 } from './Test3816';
189189
export { default as Test3833 } from './Test3833';
190+
export { default as TestXXXX } from './TestXXXX';
190191
export { default as TestScreenAnimation } from './TestScreenAnimation';
191192
// The following test was meant to demo the "go back" gesture using Reanimated
192193
// but the associated PR in react-navigation is currently put on hold

0 commit comments

Comments
 (0)