Prevent double expansion of transaction operations
How predictive back with fragments works is that
we pop the FragmentTransaction from the
FragmentManager, but keep a copy of that
transaction around in case the back operation
is cancelled (either via gesture or interruption
by changes to the FragmentManager).
In the cancellation case, we then re-apply the
same FragmentTransaction again, putting the
FragmentManager back into the state it was in
before the predictive back happened.
However, when using operations that are
'expanded' like setPrimaryNavigationFragment(),
we need to explicitly call collapseOps() before
re-executing the operation so that we ensure
that these transactions never have a direct
reference to fragments from other transactions.
By ensuring we only don't double expandOps(),
we can avoid cases where FragmentManager does not
allow references to previous fragments such as when
using saveBackStack().
Relnote: "Fixed an `IllegalStateException` triggered
by `saveBackStack` only after a Predictive Back gesture
was cancelled or interrupted."
Test: new PredictiveBackTest test cases
BUG: 342419080
(cherry picked from https://android-review.googlesource.com/q/commit:922bb8acc3661be78480dd40f3c582af8b87f935)
Merged-In: I3387d9d02495112f211448d7f3c9f862299da697
Change-Id: I3387d9d02495112f211448d7f3c9f862299da697
diff --git a/fragment/fragment/src/androidTest/java/androidx/fragment/app/PredictiveBackTest.kt b/fragment/fragment/src/androidTest/java/androidx/fragment/app/PredictiveBackTest.kt
index 1571f82..db4e848 100644
--- a/fragment/fragment/src/androidTest/java/androidx/fragment/app/PredictiveBackTest.kt
+++ b/fragment/fragment/src/androidTest/java/androidx/fragment/app/PredictiveBackTest.kt
@@ -20,6 +20,7 @@
import android.window.BackEvent
import androidx.activity.BackEventCompat
import androidx.fragment.test.R
+import androidx.lifecycle.Lifecycle
import androidx.test.core.app.ActivityScenario
import androidx.test.ext.junit.runners.AndroidJUnit4
import androidx.test.filters.LargeTest
@@ -150,4 +151,143 @@
assertThat(fm.backStackEntryCount).isEqualTo(0)
}
}
+
+ @Test
+ fun backSaveStateAfterInterruptedByExecutePendingTransactions() {
+ withUse(ActivityScenario.launch(SimpleContainerActivity::class.java)) {
+ val fm = withActivity { supportFragmentManager }
+
+ val fragment1 = StrictViewFragment()
+
+ fm.beginTransaction()
+ .setReorderingAllowed(true)
+ .replace(R.id.fragmentContainer, fragment1, "1")
+ .setPrimaryNavigationFragment(fragment1)
+ .commit()
+ executePendingTransactions()
+
+ val fragment2 = StrictViewFragment()
+ fm.beginTransaction()
+ .setReorderingAllowed(true)
+ .replace(R.id.fragmentContainer, fragment2, "2")
+ .setPrimaryNavigationFragment(fragment2)
+ .addToBackStack("replacement")
+ .commit()
+ executePendingTransactions()
+
+ assertThat(fm.backStackEntryCount).isEqualTo(1)
+
+ val dispatcher = withActivity { onBackPressedDispatcher }
+ withActivity {
+ dispatcher.dispatchOnBackStarted(
+ BackEventCompat(0.1F, 0.1F, 0.1F, BackEvent.EDGE_LEFT)
+ )
+ }
+ // Interrupt the back event by forcing all pending transactions to be executed
+ executePendingTransactions()
+
+ assertThat(fm.backStackEntryCount).isEqualTo(1)
+
+ // Now save the FragmentTransaction that was interrupted
+ fm.saveBackStack("replacement")
+ executePendingTransactions()
+
+ assertThat(fm.backStackEntryCount).isEqualTo(0)
+ }
+ }
+
+ @Test
+ fun backSaveStateAfterInterruptedByCommitNow() {
+ withUse(ActivityScenario.launch(SimpleContainerActivity::class.java)) {
+ val fm = withActivity { supportFragmentManager }
+
+ val fragment1 = StrictViewFragment()
+
+ fm.beginTransaction()
+ .setReorderingAllowed(true)
+ .replace(R.id.fragmentContainer, fragment1, "1")
+ .setPrimaryNavigationFragment(fragment1)
+ .commit()
+ executePendingTransactions()
+
+ val fragment2 = StrictViewFragment()
+ fm.beginTransaction()
+ .setReorderingAllowed(true)
+ .replace(R.id.fragmentContainer, fragment2, "2")
+ .setPrimaryNavigationFragment(fragment2)
+ .addToBackStack("replacement")
+ .commit()
+ executePendingTransactions()
+
+ assertThat(fm.backStackEntryCount).isEqualTo(1)
+
+ val dispatcher = withActivity { onBackPressedDispatcher }
+ withActivity {
+ dispatcher.dispatchOnBackStarted(
+ BackEventCompat(0.1F, 0.1F, 0.1F, BackEvent.EDGE_LEFT)
+ )
+ }
+ // Interrupt the back event by committing a single action
+ withActivity {
+ fm.beginTransaction()
+ .setReorderingAllowed(true)
+ .setMaxLifecycle(fragment1, Lifecycle.State.RESUMED)
+ .commitNow()
+ }
+
+ assertThat(fm.backStackEntryCount).isEqualTo(1)
+
+ // Now save the FragmentTransaction that was interrupted
+ fm.saveBackStack("replacement")
+ executePendingTransactions()
+
+ assertThat(fm.backStackEntryCount).isEqualTo(0)
+ }
+ }
+
+ @Test
+ fun backSaveStateAfterCancelled() {
+ withUse(ActivityScenario.launch(SimpleContainerActivity::class.java)) {
+ val fm = withActivity { supportFragmentManager }
+
+ val fragment1 = StrictViewFragment()
+
+ fm.beginTransaction()
+ .setReorderingAllowed(true)
+ .replace(R.id.fragmentContainer, fragment1, "1")
+ .setPrimaryNavigationFragment(fragment1)
+ .commit()
+ executePendingTransactions()
+
+ val fragment2 = StrictViewFragment()
+ fm.beginTransaction()
+ .setReorderingAllowed(true)
+ .replace(R.id.fragmentContainer, fragment2, "2")
+ .setPrimaryNavigationFragment(fragment2)
+ .addToBackStack("replacement")
+ .commit()
+ executePendingTransactions()
+
+ assertThat(fm.backStackEntryCount).isEqualTo(1)
+
+ val dispatcher = withActivity { onBackPressedDispatcher }
+ withActivity {
+ dispatcher.dispatchOnBackStarted(
+ BackEventCompat(0.1F, 0.1F, 0.1F, BackEvent.EDGE_LEFT)
+ )
+ }
+
+ // Cancel the back operation via the dispatcher
+ withActivity { dispatcher.dispatchOnBackCancelled() }
+ executePendingTransactions()
+
+ assertThat(fm.backStackEntryCount).isEqualTo(1)
+
+ // Now save the FragmentTransaction that was cancelled
+ fm.saveBackStack("replacement")
+ executePendingTransactions()
+
+ assertThat(fm.backStackEntryCount).isEqualTo(0)
+ }
+ }
}
diff --git a/fragment/fragment/src/main/java/androidx/fragment/app/FragmentManager.java b/fragment/fragment/src/main/java/androidx/fragment/app/FragmentManager.java
index 15ce216..67c645a 100644
--- a/fragment/fragment/src/main/java/androidx/fragment/app/FragmentManager.java
+++ b/fragment/fragment/src/main/java/androidx/fragment/app/FragmentManager.java
@@ -1061,6 +1061,7 @@
}
if (mTransitioningOp != null) {
mTransitioningOp.mCommitted = false;
+ mTransitioningOp.collapseOps();
mTransitioningOp.runOnCommitInternal(true, () -> {
for (OnBackStackChangedListener listener : mBackStackChangeListeners) {
listener.onBackStackChangeCancelled();
@@ -1980,6 +1981,7 @@
// to the records about to be executed.
if (mTransitioningOp != null) {
mTransitioningOp.mCommitted = false;
+ mTransitioningOp.collapseOps();
if (isLoggingEnabled(Log.DEBUG)) {
Log.d(TAG, "Reversing mTransitioningOp " + mTransitioningOp
+ " as part of execSingleAction for action " + action);
@@ -2030,6 +2032,7 @@
// as the first pending action.
if (!mHandlingTransitioningOp && mTransitioningOp != null) {
mTransitioningOp.mCommitted = false;
+ mTransitioningOp.collapseOps();
if (isLoggingEnabled(Log.DEBUG)) {
Log.d(TAG, "Reversing mTransitioningOp " + mTransitioningOp
+ " as part of execPendingActions for actions " + mPendingActions);
