Don't destroy Fragments without calling initState()
The only place where keepActive was used was
when a Fragment was animating to the destroyed
state and then it was re-added before the
animation completes resulting in a Fragment that
had its Lifecycle destroyed and, due to keepActive,
did not have its Lifecycle/other state reset
via initState(). This can leave the Fragment in
an inconsistent state and breaks the invariant
around destroyed being a terminal state for a
Lifecycle object.
Instead, we can cancel the animation where it
is and proceed from that state (since that's a
known valid state). This also allows us to
remove the keepActive flag entirely, simplifying
the logic.
Test: all tests still pass
Change-Id: I3bd86e8c99936835c905a0241d90daf1941159bd
diff --git a/fragment/fragment/src/main/java/androidx/fragment/app/FragmentLayoutInflaterFactory.java b/fragment/fragment/src/main/java/androidx/fragment/app/FragmentLayoutInflaterFactory.java
index 469e1ed..91842df 100644
--- a/fragment/fragment/src/main/java/androidx/fragment/app/FragmentLayoutInflaterFactory.java
+++ b/fragment/fragment/src/main/java/androidx/fragment/app/FragmentLayoutInflaterFactory.java
@@ -125,7 +125,7 @@
// at the right phase of the lifecycle so that we will have mView populated
// for compliant fragments below.
if (mFragmentManager.mCurState < Fragment.CREATED && fragment.mFromLayout) {
- mFragmentManager.moveToState(fragment, Fragment.CREATED, false);
+ mFragmentManager.moveToState(fragment, Fragment.CREATED);
} else {
mFragmentManager.moveToState(fragment);
}
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 2dce59b..8c06e17 100644
--- a/fragment/fragment/src/main/java/androidx/fragment/app/FragmentManager.java
+++ b/fragment/fragment/src/main/java/androidx/fragment/app/FragmentManager.java
@@ -1156,7 +1156,7 @@
return;
}
f.mDeferStart = false;
- moveToState(f, mCurState, false);
+ moveToState(f, mCurState);
}
}
@@ -1165,7 +1165,7 @@
}
@SuppressWarnings("ReferenceEquality")
- void moveToState(Fragment f, int newState, boolean keepActive) {
+ void moveToState(Fragment f, int newState) {
// Fragments that are not currently added will sit in the onCreate() state.
if ((!f.mAdded || f.mDetached) && newState > Fragment.CREATED) {
newState = Fragment.CREATED;
@@ -1201,11 +1201,9 @@
if (f.mState < newState && (f.getAnimatingAway() != null || f.getAnimator() != null)) {
// The fragment is currently being animated... but! Now we
// want to move our state back up. Give up on waiting for the
- // animation, move to whatever the final state should be once
- // the animation is done, and then we can proceed from there.
+ // animation and proceed from where we are.
f.setAnimatingAway(null);
f.setAnimator(null);
- moveToState(f, f.getStateAfterAnimating(), true);
}
switch (f.mState) {
case Fragment.INITIALIZING:
@@ -1252,7 +1250,7 @@
+ " that does not belong to this FragmentManager!");
}
if (f.mTarget.mState < Fragment.CREATED) {
- moveToState(f.mTarget, Fragment.CREATED, true);
+ moveToState(f.mTarget, Fragment.CREATED);
}
f.mTargetWho = f.mTarget.mWho;
f.mTarget = null;
@@ -1265,7 +1263,7 @@
+ " that does not belong to this FragmentManager!");
}
if (target.mState < Fragment.CREATED) {
- moveToState(target, Fragment.CREATED, true);
+ moveToState(target, Fragment.CREATED);
}
}
@@ -1472,21 +1470,19 @@
f.performDetach();
dispatchOnFragmentDetached(f, false);
- if (!keepActive) {
- if (beingRemoved || mNonConfig.shouldDestroy(f)) {
- makeInactive(f);
- } else {
- f.mHost = null;
- f.mParentFragment = null;
- f.mFragmentManager = null;
- if (f.mTargetWho != null) {
- Fragment target = mActive.get(f.mTargetWho);
- if (target != null && target.getRetainInstance()) {
- // Only keep references to other retained Fragments
- // to avoid developers accessing Fragments that
- // are never coming back
- f.mTarget = target;
- }
+ if (beingRemoved || mNonConfig.shouldDestroy(f)) {
+ makeInactive(f);
+ } else {
+ f.mHost = null;
+ f.mParentFragment = null;
+ f.mFragmentManager = null;
+ if (f.mTargetWho != null) {
+ Fragment target = mActive.get(f.mTargetWho);
+ if (target != null && target.getRetainInstance()) {
+ // Only keep references to other retained Fragments
+ // to avoid developers accessing Fragments that
+ // are never coming back
+ f.mTarget = target;
}
}
}
@@ -1536,7 +1532,7 @@
if (fragment.getAnimatingAway() != null) {
fragment.setAnimatingAway(null);
destroyFragmentView(fragment);
- moveToState(fragment, fragment.getStateAfterAnimating(), false);
+ moveToState(fragment, fragment.getStateAfterAnimating());
}
}
});
@@ -1560,7 +1556,7 @@
fragment.setAnimator(null);
if (animator != null && container.indexOfChild(viewToAnimate) < 0) {
destroyFragmentView(fragment);
- moveToState(fragment, fragment.getStateAfterAnimating(), false);
+ moveToState(fragment, fragment.getStateAfterAnimating());
}
}
});
@@ -1583,7 +1579,7 @@
}
void moveToState(Fragment f) {
- moveToState(f, mCurState, false);
+ moveToState(f, mCurState);
}
private void ensureInflatedFragmentView(Fragment f) {
@@ -1687,7 +1683,7 @@
nextState = Math.min(nextState, Fragment.INITIALIZING);
}
}
- moveToState(f, nextState, false);
+ moveToState(f, nextState);
if (f.mView != null) {
// Move the view if it is out of order
@@ -2516,7 +2512,7 @@
for (int i = 0; i < numAdded; i++) {
Fragment fragment = mAdded.get(i);
if (fragment.mState < state) {
- moveToState(fragment, state, false);
+ moveToState(fragment, state);
if (fragment.mView != null && !fragment.mHidden && fragment.mIsNewlyAdded) {
added.add(fragment);
}
@@ -2555,7 +2551,7 @@
}
fragment.setAnimatingAway(null);
destroyFragmentView(fragment);
- moveToState(fragment, stateAfterAnimating, false);
+ moveToState(fragment, stateAfterAnimating);
} else if (fragment.getAnimator() != null) {
fragment.getAnimator().end();
}
@@ -2882,9 +2878,9 @@
// We need to ensure that onDestroy and any other clean up is done
// so move the Fragment up to CREATED, then mark it as being removed, then
// destroy it.
- moveToState(f, Fragment.CREATED, false);
+ moveToState(f, Fragment.CREATED);
f.mRemoving = true;
- moveToState(f, Fragment.INITIALIZING, false);
+ moveToState(f, Fragment.INITIALIZING);
continue;
}
fs.mInstance = f;
diff --git a/fragment/fragment/src/main/java/androidx/fragment/app/FragmentTransition.java b/fragment/fragment/src/main/java/androidx/fragment/app/FragmentTransition.java
index 80fdf8e..ddd481d 100644
--- a/fragment/fragment/src/main/java/androidx/fragment/app/FragmentTransition.java
+++ b/fragment/fragment/src/main/java/androidx/fragment/app/FragmentTransition.java
@@ -1196,7 +1196,7 @@
if (fragment.mState < Fragment.CREATED && manager.mCurState >= Fragment.CREATED
&& !transaction.mReorderingAllowed) {
manager.makeActive(fragment);
- manager.moveToState(fragment, Fragment.CREATED, false);
+ manager.moveToState(fragment, Fragment.CREATED);
}
}
if (setFirstOut && (containerTransition == null || containerTransition.firstOut == null)) {
