Fix issue #6584942 IllegalStateException: Failure saving state...

...active SuggestFragment{419494f0} has cleared index: -1

There were issues when the same fragment was removed and then
added again before completely finishing the remove (such as due
to a running animation).

Two fixes:

- Now when you call FragmentTransaction.replace() and are replacing
  a fragment with the same fragment, this becomes a no-op, to avoid
  visual artifacts in the transition and bad states.
- When we are moving the fragment state up and it is currently
  animating away to the INITIALIZED state, we could end up making
  the fragment inactive as part of the cleanup.  In this case it
  shouldn't be made inactive; we just need to initialize it but
  keep it active since we are going to continue to use it.

Bug: 6584942
Change-Id: I8bfd73f2d8ef8f67b541b3e2525dfa5db6c3bfa5

3 files changed, 56 insertions, 38 deletions

diff --git a/core/java/android/app/BackStackRecord.java b/core/java/android/app/BackStackRecord.java
index e9e8e16..96814b7 100644
--- a/core/java/android/app/BackStackRecord.java
+++ b/core/java/android/app/BackStackRecord.java
@@ -53,7 +53,7 @@ final class BackStackState implements Parcelable {
         int pos = 0;
         while (op != null) {
             mOps[pos++] = op.cmd;
-            mOps[pos++] = op.fragment.mIndex;
+            mOps[pos++] = op.fragment != null ? op.fragment.mIndex : -1;
             mOps[pos++] = op.enterAnim;
             mOps[pos++] = op.exitAnim;
             mOps[pos++] = op.popEnterAnim;
@@ -99,8 +99,13 @@ final class BackStackState implements Parcelable {
             op.cmd = mOps[pos++];
             if (FragmentManagerImpl.DEBUG) Log.v(FragmentManagerImpl.TAG,
                     "BSE " + bse + " set base fragment #" + mOps[pos]);
-            Fragment f = fm.mActive.get(mOps[pos++]);
-            op.fragment = f;
+            int findex = mOps[pos++];
+            if (findex >= 0) {
+                Fragment f = fm.mActive.get(findex);
+                op.fragment = f;
+            } else {
+                op.fragment = null;
+            }
             op.enterAnim = mOps[pos++];
             op.exitAnim = mOps[pos++];
             op.popEnterAnim = mOps[pos++];
@@ -506,9 +511,11 @@ final class BackStackRecord extends FragmentTransaction implements
                 + " by " + amt);
         Op op = mHead;
         while (op != null) {
-            op.fragment.mBackStackNesting += amt;
-            if (FragmentManagerImpl.DEBUG) Log.v(TAG, "Bump nesting of "
-                    + op.fragment + " to " + op.fragment.mBackStackNesting);
+            if (op.fragment != null) {
+                op.fragment.mBackStackNesting += amt;
+                if (FragmentManagerImpl.DEBUG) Log.v(TAG, "Bump nesting of "
+                        + op.fragment + " to " + op.fragment.mBackStackNesting);
+            }
             if (op.removed != null) {
                 for (int i=op.removed.size()-1; i>=0; i--) {
                     Fragment r = op.removed.get(i);
@@ -568,23 +575,29 @@ final class BackStackRecord extends FragmentTransaction implements
                             Fragment old = mManager.mAdded.get(i);
                             if (FragmentManagerImpl.DEBUG) Log.v(TAG,
                                     "OP_REPLACE: adding=" + f + " old=" + old);
-                            if (old.mContainerId == f.mContainerId) {
-                                if (op.removed == null) {
-                                    op.removed = new ArrayList<Fragment>();
+                            if (f == null || old.mContainerId == f.mContainerId) {
+                                if (old == f) {
+                                    op.fragment = f = null;
+                                } else {
+                                    if (op.removed == null) {
+                                        op.removed = new ArrayList<Fragment>();
+                                    }
+                                    op.removed.add(old);
+                                    old.mNextAnim = op.exitAnim;
+                                    if (mAddToBackStack) {
+                                        old.mBackStackNesting += 1;
+                                        if (FragmentManagerImpl.DEBUG) Log.v(TAG, "Bump nesting of "
+                                                + old + " to " + old.mBackStackNesting);
+                                    }
+                                    mManager.removeFragment(old, mTransition, mTransitionStyle);
                                 }
-                                op.removed.add(old);
-                                old.mNextAnim = op.exitAnim;
-                                if (mAddToBackStack) {
-                                    old.mBackStackNesting += 1;
-                                    if (FragmentManagerImpl.DEBUG) Log.v(TAG, "Bump nesting of "
-                                            + old + " to " + old.mBackStackNesting);
-                                }
-                                mManager.removeFragment(old, mTransition, mTransitionStyle);
                             }
                         }
                     }
-                    f.mNextAnim = op.enterAnim;
-                    mManager.addFragment(f, false);
+                    if (f != null) {
+                        f.mNextAnim = op.enterAnim;
+                        mManager.addFragment(f, false);
+                    }
                 } break;
                 case OP_REMOVE: {
                     Fragment f = op.fragment;
@@ -644,10 +657,12 @@ final class BackStackRecord extends FragmentTransaction implements
                 } break;
                 case OP_REPLACE: {
                     Fragment f = op.fragment;
-                    f.mNextAnim = op.popExitAnim;
-                    mManager.removeFragment(f,
-                            FragmentManagerImpl.reverseTransit(mTransition),
-                            mTransitionStyle);
+                    if (f != null) {
+                        f.mNextAnim = op.popExitAnim;
+                        mManager.removeFragment(f,
+                                FragmentManagerImpl.reverseTransit(mTransition),
+                                mTransitionStyle);
+                    }
                     if (op.removed != null) {
                         for (int i=0; i<op.removed.size(); i++) {
                             Fragment old = op.removed.get(i);
diff --git a/core/java/android/app/Fragment.java b/core/java/android/app/Fragment.java
index 5e6b090..28876d3 100644
--- a/core/java/android/app/Fragment.java
+++ b/core/java/android/app/Fragment.java
@@ -1503,7 +1503,7 @@ public class Fragment implements ComponentCallbacks2, OnCreateContextMenuListene
     public void dump(String prefix, FileDescriptor fd, PrintWriter writer, String[] args) {
         writer.print(prefix); writer.print("mFragmentId=#");
                 writer.print(Integer.toHexString(mFragmentId));
-                writer.print(" mContainerId#=");
+                writer.print(" mContainerId=#");
                 writer.print(Integer.toHexString(mContainerId));
                 writer.print(" mTag="); writer.println(mTag);
         writer.print(prefix); writer.print("mState="); writer.print(mState);
diff --git a/core/java/android/app/FragmentManager.java b/core/java/android/app/FragmentManager.java
index 03ee419..9ba5305 100644
--- a/core/java/android/app/FragmentManager.java
+++ b/core/java/android/app/FragmentManager.java
@@ -726,11 +726,12 @@ final class FragmentManagerImpl extends FragmentManager {
                 return;
             }
             f.mDeferStart = false;
-            moveToState(f, mCurState, 0, 0);
+            moveToState(f, mCurState, 0, 0, false);
         }
     }
 
-    void moveToState(Fragment f, int newState, int transit, int transitionStyle) {
+    void moveToState(Fragment f, int newState, int transit, int transitionStyle,
+            boolean keepActive) {
         // Fragments that are not currently added will sit in the onCreate() state.
         if (!f.mAdded && newState > Fragment.CREATED) {
             newState = Fragment.CREATED;
@@ -757,7 +758,7 @@ final class FragmentManagerImpl extends FragmentManager {
                 // animation, move to whatever the final state should be once
                 // the animation is done, and then we can proceed from there.
                 f.mAnimatingAway = null;
-                moveToState(f, f.mStateAfterAnimating, 0, 0);
+                moveToState(f, f.mStateAfterAnimating, 0, 0, true);
             }
             switch (f.mState) {
                 case Fragment.INITIALIZING:
@@ -940,7 +941,7 @@ final class FragmentManagerImpl extends FragmentManager {
                                         if (fragment.mAnimatingAway != null) {
                                             fragment.mAnimatingAway = null;
                                             moveToState(fragment, fragment.mStateAfterAnimating,
-                                                    0, 0);
+                                                    0, 0, false);
                                         }
                                     }
                                 });
@@ -992,11 +993,13 @@ final class FragmentManagerImpl extends FragmentManager {
                                 throw new SuperNotCalledException("Fragment " + f
                                         + " did not call through to super.onDetach()");
                             }
-                            if (!f.mRetaining) {
-                                makeInactive(f);
-                            } else {
-                                f.mActivity = null;
-                                f.mFragmentManager = null;
+                            if (!keepActive) {
+                                if (!f.mRetaining) {
+                                    makeInactive(f);
+                                } else {
+                                    f.mActivity = null;
+                                    f.mFragmentManager = null;
+                                }
                             }
                         }
                     }
@@ -1007,7 +1010,7 @@ final class FragmentManagerImpl extends FragmentManager {
     }
     
     void moveToState(Fragment f) {
-        moveToState(f, mCurState, 0, 0);
+        moveToState(f, mCurState, 0, 0, false);
     }
 
     void moveToState(int newState, boolean always) {
@@ -1029,7 +1032,7 @@ final class FragmentManagerImpl extends FragmentManager {
             for (int i=0; i<mActive.size(); i++) {
                 Fragment f = mActive.get(i);
                 if (f != null) {
-                    moveToState(f, newState, transit, transitStyle);
+                    moveToState(f, newState, transit, transitStyle, false);
                     if (f.mLoaderManager != null) {
                         loadersRunning |= f.mLoaderManager.hasRunningLoaders();
                     }
@@ -1122,7 +1125,7 @@ final class FragmentManagerImpl extends FragmentManager {
             fragment.mAdded = false;
             fragment.mRemoving = true;
             moveToState(fragment, inactive ? Fragment.INITIALIZING : Fragment.CREATED,
-                    transition, transitionStyle);
+                    transition, transitionStyle, false);
         }
     }
     
@@ -1189,7 +1192,7 @@ final class FragmentManagerImpl extends FragmentManager {
                     mNeedMenuInvalidate = true;
                 }
                 fragment.mAdded = false;
-                moveToState(fragment, Fragment.CREATED, transition, transitionStyle);
+                moveToState(fragment, Fragment.CREATED, transition, transitionStyle, false);
             }
         }
     }
@@ -1204,7 +1207,7 @@ final class FragmentManagerImpl extends FragmentManager {
                 if (fragment.mHasMenu && fragment.mMenuVisible) {
                     mNeedMenuInvalidate = true;
                 }
-                moveToState(fragment, mCurState, transition, transitionStyle);
+                moveToState(fragment, mCurState, transition, transitionStyle, false);
             }
         }
     }