From 67806cbbb5324705dbe864adee31c117bd7bd7f1 Mon Sep 17 00:00:00 2001 From: Krzysztof Magiera Date: Sat, 15 Feb 2020 01:12:59 +0100 Subject: [PATCH] Fix several crashes in native stack related to fragment library (#331) This change fixes two crashes related to fragment library. The first issue was a crash caused by return transaction started while the previous transaction was ongoing (e.g., quickly adding new screen on top of the stack and immediately dismissing it). The main fix was applied in the fragment library and therefore as a part of this change we update the dependency to fragment:1.2.1 which is the current latest stable version. As a result of the fragment library change we started observing other issue. The second issue was caused by the fact that under certain circumstances the view associated with a fragment couldn't been added despite it still being attached to a parent. This was resulting in a crash. This change adds a cleanup code that properly detaches the view: we do it in onCreateView but also when the fragment destroys its view (in onViewDestroy callback). The latter is necessary because when fragments are restored the order of onCreateView calls is reversed which causes inner views to attach despite their fragment managers not being initialized. --- android/build.gradle | 5 +++-- .../swmansion/rnscreens/ScreenContainer.java | 7 ------- .../swmansion/rnscreens/ScreenFragment.java | 20 ++++++++++++++++++- .../rnscreens/ScreenStackFragment.java | 2 +- .../rnscreens/ScreenStackHeaderConfig.java | 6 +++++- 5 files changed, 28 insertions(+), 12 deletions(-) diff --git a/android/build.gradle b/android/build.gradle index 7a5aa23b..ea3e2fd2 100644 --- a/android/build.gradle +++ b/android/build.gradle @@ -49,8 +49,9 @@ repositories { dependencies { implementation 'com.facebook.react:react-native:+' implementation 'androidx.appcompat:appcompat:1.1.0' - implementation 'androidx.coordinatorlayout:coordinatorlayout:1.0.0' - implementation 'com.google.android.material:material:1.0.0' + implementation 'androidx.fragment:fragment:1.2.1' + implementation 'androidx.coordinatorlayout:coordinatorlayout:1.1.0' + implementation 'com.google.android.material:material:1.1.0' } def configureReactNativePom(def pom) { diff --git a/android/src/main/java/com/swmansion/rnscreens/ScreenContainer.java b/android/src/main/java/com/swmansion/rnscreens/ScreenContainer.java index 9caad54c..ecc7804c 100644 --- a/android/src/main/java/com/swmansion/rnscreens/ScreenContainer.java +++ b/android/src/main/java/com/swmansion/rnscreens/ScreenContainer.java @@ -220,13 +220,6 @@ public class ScreenContainer extends ViewGroup { protected void onDetachedFromWindow() { super.onDetachedFromWindow(); mIsAttached = false; - - // fragment manager is destroyed so we can't do anything with it anymore - mFragmentManager = null; - // so we don't add the same screen twice after re-attach - removeAllViews(); - // after re-attach we'll update the screen and add views again - markUpdated(); } @Override diff --git a/android/src/main/java/com/swmansion/rnscreens/ScreenFragment.java b/android/src/main/java/com/swmansion/rnscreens/ScreenFragment.java index 4edac11f..569280af 100644 --- a/android/src/main/java/com/swmansion/rnscreens/ScreenFragment.java +++ b/android/src/main/java/com/swmansion/rnscreens/ScreenFragment.java @@ -5,6 +5,7 @@ import android.os.Bundle; import android.view.LayoutInflater; import android.view.View; import android.view.ViewGroup; +import android.view.ViewParent; import androidx.annotation.Nullable; import androidx.fragment.app.Fragment; @@ -14,6 +15,17 @@ import com.facebook.react.uimanager.UIManagerModule; public class ScreenFragment extends Fragment { + protected static View recycleView(View view) { + // screen fragments reuse view instances instead of creating new ones. In order to reuse a given + // view it needs to be detached from the view hierarchy to allow the fragment to attach it back. + ViewParent parent = view.getParent(); + if (parent != null) { + ((ViewGroup) parent).endViewTransition(view); + ((ViewGroup) parent).removeView(view); + } + return view; + } + protected Screen mScreenView; public ScreenFragment() { @@ -30,7 +42,7 @@ public class ScreenFragment extends Fragment { public View onCreateView(LayoutInflater inflater, @Nullable ViewGroup container, @Nullable Bundle savedInstanceState) { - return mScreenView; + return recycleView(mScreenView); } public Screen getScreen() { @@ -56,6 +68,12 @@ public class ScreenFragment extends Fragment { dispatchOnAppear(); } + @Override + public void onDestroyView() { + super.onDestroyView(); + recycleView(getView()); + } + @Override public void onDestroy() { super.onDestroy(); diff --git a/android/src/main/java/com/swmansion/rnscreens/ScreenStackFragment.java b/android/src/main/java/com/swmansion/rnscreens/ScreenStackFragment.java index 3b4e9e1b..c688afbc 100644 --- a/android/src/main/java/com/swmansion/rnscreens/ScreenStackFragment.java +++ b/android/src/main/java/com/swmansion/rnscreens/ScreenStackFragment.java @@ -143,7 +143,7 @@ public class ScreenStackFragment extends ScreenFragment { mScreenRootView = configureView(); } - return mScreenRootView; + return recycleView(mScreenRootView); } public boolean isDismissable() { diff --git a/android/src/main/java/com/swmansion/rnscreens/ScreenStackHeaderConfig.java b/android/src/main/java/com/swmansion/rnscreens/ScreenStackHeaderConfig.java index d03ca296..9e93000d 100644 --- a/android/src/main/java/com/swmansion/rnscreens/ScreenStackHeaderConfig.java +++ b/android/src/main/java/com/swmansion/rnscreens/ScreenStackHeaderConfig.java @@ -132,6 +132,11 @@ public class ScreenStackHeaderConfig extends ViewGroup { return; } + AppCompatActivity activity = (AppCompatActivity) getScreenFragment().getActivity(); + if (activity == null) { + return; + } + if (mIsHidden) { if (mToolbar.getParent() != null) { getScreenFragment().removeToolbar(); @@ -143,7 +148,6 @@ public class ScreenStackHeaderConfig extends ViewGroup { getScreenFragment().setToolbar(mToolbar); } - AppCompatActivity activity = (AppCompatActivity) getScreenFragment().getActivity(); activity.setSupportActionBar(mToolbar); ActionBar actionBar = activity.getSupportActionBar();