Skip to content

Commit f43a33c

Browse files
author
Dianne Hackborn
committed
Work on issue #7232641: ISE crash when rotating phone in label list mode
This doesn't fix the problem; I think it is an app problem. It does improve a bunch of the debugging to help better identify what is going on, and introduces some checks when adding a fragment to fail immediately if we are getting into a state when a fragment is going to be in the added list multiple times (which is pretty much guaranteed to lead to a failure at some point in the future). Change-Id: If3a8700763facd61c4505c6ff872ae66875afc8d
1 parent 13987fb commit f43a33c

File tree

2 files changed

+137
-61
lines changed

2 files changed

+137
-61
lines changed

‎core/java/android/app/BackStackRecord.java‎

Lines changed: 105 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import android.os.Parcelable;
2121
import android.text.TextUtils;
2222
import android.util.Log;
23+
import android.util.LogWriter;
2324

2425
import java.io.FileDescriptor;
2526
import java.io.PrintWriter;
@@ -94,11 +95,12 @@ public BackStackState(Parcel in) {
9495
public BackStackRecord instantiate(FragmentManagerImpl fm) {
9596
BackStackRecord bse = new BackStackRecord(fm);
9697
int pos = 0;
98+
int num = 0;
9799
while (pos < mOps.length) {
98100
BackStackRecord.Op op = new BackStackRecord.Op();
99101
op.cmd = mOps[pos++];
100102
if (FragmentManagerImpl.DEBUG) Log.v(FragmentManagerImpl.TAG,
101-
"BSE " + bse + " set base fragment #" + mOps[pos]);
103+
"Instantiate " + bse + " op #" + num + " base fragment #" + mOps[pos]);
102104
int findex = mOps[pos++];
103105
if (findex >= 0) {
104106
Fragment f = fm.mActive.get(findex);
@@ -115,12 +117,13 @@ public BackStackRecord instantiate(FragmentManagerImpl fm) {
115117
op.removed = new ArrayList<Fragment>(N);
116118
for (int i=0; i<N; i++) {
117119
if (FragmentManagerImpl.DEBUG) Log.v(FragmentManagerImpl.TAG,
118-
"BSE " + bse + " set remove fragment #" + mOps[pos]);
120+
"Instantiate " + bse + " set remove fragment #" + mOps[pos]);
119121
Fragment r = fm.mActive.get(mOps[pos++]);
120122
op.removed.add(r);
121123
}
122124
}
123125
bse.addOp(op);
126+
num++;
124127
}
125128
bse.mTransition = mTransition;
126129
bse.mTransitionStyle = mTransitionStyle;
@@ -168,7 +171,7 @@ public BackStackState[] newArray(int size) {
168171
*/
169172
final class BackStackRecord extends FragmentTransaction implements
170173
FragmentManager.BackStackEntry, Runnable {
171-
static final String TAG = "BackStackEntry";
174+
static final String TAG = FragmentManagerImpl.TAG;
172175

173176
final FragmentManagerImpl mManager;
174177

@@ -206,46 +209,69 @@ static final class Op {
206209
boolean mAllowAddToBackStack = true;
207210
String mName;
208211
boolean mCommitted;
209-
int mIndex;
212+
int mIndex = -1;
210213

211214
int mBreadCrumbTitleRes;
212215
CharSequence mBreadCrumbTitleText;
213216
int mBreadCrumbShortTitleRes;
214217
CharSequence mBreadCrumbShortTitleText;
215218

216-
public void dump(String prefix, FileDescriptor fd, PrintWriter writer, String[] args) {
217-
writer.print(prefix); writer.print("mName="); writer.print(mName);
218-
writer.print(" mIndex="); writer.print(mIndex);
219-
writer.print(" mCommitted="); writer.println(mCommitted);
220-
if (mTransition != FragmentTransaction.TRANSIT_NONE) {
221-
writer.print(prefix); writer.print("mTransition=#");
222-
writer.print(Integer.toHexString(mTransition));
223-
writer.print(" mTransitionStyle=#");
224-
writer.println(Integer.toHexString(mTransitionStyle));
225-
}
226-
if (mEnterAnim != 0 || mExitAnim !=0) {
227-
writer.print(prefix); writer.print("mEnterAnim=#");
228-
writer.print(Integer.toHexString(mEnterAnim));
229-
writer.print(" mExitAnim=#");
230-
writer.println(Integer.toHexString(mExitAnim));
231-
}
232-
if (mPopEnterAnim != 0 || mPopExitAnim !=0) {
233-
writer.print(prefix); writer.print("mPopEnterAnim=#");
234-
writer.print(Integer.toHexString(mPopEnterAnim));
235-
writer.print(" mPopExitAnim=#");
236-
writer.println(Integer.toHexString(mPopExitAnim));
219+
@Override
220+
public String toString() {
221+
StringBuilder sb = new StringBuilder(128);
222+
sb.append("BackStackEntry{");
223+
sb.append(Integer.toHexString(System.identityHashCode(this)));
224+
if (mIndex >= 0) {
225+
sb.append(" #");
226+
sb.append(mIndex);
237227
}
238-
if (mBreadCrumbTitleRes != 0 || mBreadCrumbTitleText != null) {
239-
writer.print(prefix); writer.print("mBreadCrumbTitleRes=#");
240-
writer.print(Integer.toHexString(mBreadCrumbTitleRes));
241-
writer.print(" mBreadCrumbTitleText=");
242-
writer.println(mBreadCrumbTitleText);
228+
if (mName != null) {
229+
sb.append(" ");
230+
sb.append(mName);
243231
}
244-
if (mBreadCrumbShortTitleRes != 0 || mBreadCrumbShortTitleText != null) {
245-
writer.print(prefix); writer.print("mBreadCrumbShortTitleRes=#");
246-
writer.print(Integer.toHexString(mBreadCrumbShortTitleRes));
247-
writer.print(" mBreadCrumbShortTitleText=");
248-
writer.println(mBreadCrumbShortTitleText);
232+
sb.append("}");
233+
return sb.toString();
234+
}
235+
236+
public void dump(String prefix, FileDescriptor fd, PrintWriter writer, String[] args) {
237+
dump(prefix, writer, true);
238+
}
239+
240+
void dump(String prefix, PrintWriter writer, boolean full) {
241+
if (full) {
242+
writer.print(prefix); writer.print("mName="); writer.print(mName);
243+
writer.print(" mIndex="); writer.print(mIndex);
244+
writer.print(" mCommitted="); writer.println(mCommitted);
245+
if (mTransition != FragmentTransaction.TRANSIT_NONE) {
246+
writer.print(prefix); writer.print("mTransition=#");
247+
writer.print(Integer.toHexString(mTransition));
248+
writer.print(" mTransitionStyle=#");
249+
writer.println(Integer.toHexString(mTransitionStyle));
250+
}
251+
if (mEnterAnim != 0 || mExitAnim !=0) {
252+
writer.print(prefix); writer.print("mEnterAnim=#");
253+
writer.print(Integer.toHexString(mEnterAnim));
254+
writer.print(" mExitAnim=#");
255+
writer.println(Integer.toHexString(mExitAnim));
256+
}
257+
if (mPopEnterAnim != 0 || mPopExitAnim !=0) {
258+
writer.print(prefix); writer.print("mPopEnterAnim=#");
259+
writer.print(Integer.toHexString(mPopEnterAnim));
260+
writer.print(" mPopExitAnim=#");
261+
writer.println(Integer.toHexString(mPopExitAnim));
262+
}
263+
if (mBreadCrumbTitleRes != 0 || mBreadCrumbTitleText != null) {
264+
writer.print(prefix); writer.print("mBreadCrumbTitleRes=#");
265+
writer.print(Integer.toHexString(mBreadCrumbTitleRes));
266+
writer.print(" mBreadCrumbTitleText=");
267+
writer.println(mBreadCrumbTitleText);
268+
}
269+
if (mBreadCrumbShortTitleRes != 0 || mBreadCrumbShortTitleText != null) {
270+
writer.print(prefix); writer.print("mBreadCrumbShortTitleRes=#");
271+
writer.print(Integer.toHexString(mBreadCrumbShortTitleRes));
272+
writer.print(" mBreadCrumbShortTitleText=");
273+
writer.println(mBreadCrumbShortTitleText);
274+
}
249275
}
250276

251277
if (mHead != null) {
@@ -254,36 +280,52 @@ public void dump(String prefix, FileDescriptor fd, PrintWriter writer, String[]
254280
Op op = mHead;
255281
int num = 0;
256282
while (op != null) {
257-
writer.print(prefix); writer.print(" Op #"); writer.print(num);
258-
writer.println(":");
259-
writer.print(innerPrefix); writer.print("cmd="); writer.print(op.cmd);
260-
writer.print(" fragment="); writer.println(op.fragment);
261-
if (op.enterAnim != 0 || op.exitAnim != 0) {
262-
writer.print(prefix); writer.print("enterAnim=#");
263-
writer.print(Integer.toHexString(op.enterAnim));
264-
writer.print(" exitAnim=#");
265-
writer.println(Integer.toHexString(op.exitAnim));
283+
String cmdStr;
284+
switch (op.cmd) {
285+
case OP_NULL: cmdStr="NULL"; break;
286+
case OP_ADD: cmdStr="ADD"; break;
287+
case OP_REPLACE: cmdStr="REPLACE"; break;
288+
case OP_REMOVE: cmdStr="REMOVE"; break;
289+
case OP_HIDE: cmdStr="HIDE"; break;
290+
case OP_SHOW: cmdStr="SHOW"; break;
291+
case OP_DETACH: cmdStr="DETACH"; break;
292+
case OP_ATTACH: cmdStr="ATTACH"; break;
293+
default: cmdStr="cmd=" + op.cmd; break;
266294
}
267-
if (op.popEnterAnim != 0 || op.popExitAnim != 0) {
268-
writer.print(prefix); writer.print("popEnterAnim=#");
269-
writer.print(Integer.toHexString(op.popEnterAnim));
270-
writer.print(" popExitAnim=#");
271-
writer.println(Integer.toHexString(op.popExitAnim));
295+
writer.print(prefix); writer.print(" Op #"); writer.print(num);
296+
writer.print(": "); writer.print(cmdStr);
297+
writer.print(" "); writer.println(op.fragment);
298+
if (full) {
299+
if (op.enterAnim != 0 || op.exitAnim != 0) {
300+
writer.print(innerPrefix); writer.print("enterAnim=#");
301+
writer.print(Integer.toHexString(op.enterAnim));
302+
writer.print(" exitAnim=#");
303+
writer.println(Integer.toHexString(op.exitAnim));
304+
}
305+
if (op.popEnterAnim != 0 || op.popExitAnim != 0) {
306+
writer.print(innerPrefix); writer.print("popEnterAnim=#");
307+
writer.print(Integer.toHexString(op.popEnterAnim));
308+
writer.print(" popExitAnim=#");
309+
writer.println(Integer.toHexString(op.popExitAnim));
310+
}
272311
}
273312
if (op.removed != null && op.removed.size() > 0) {
274313
for (int i=0; i<op.removed.size(); i++) {
275314
writer.print(innerPrefix);
276315
if (op.removed.size() == 1) {
277316
writer.print("Removed: ");
278317
} else {
279-
writer.println("Removed:");
280-
writer.print(innerPrefix); writer.print(" #"); writer.print(num);
318+
if (i == 0) {
319+
writer.println("Removed:");
320+
}
321+
writer.print(innerPrefix); writer.print(" #"); writer.print(i);
281322
writer.print(": ");
282323
}
283324
writer.println(op.removed.get(i));
284325
}
285326
}
286327
op = op.next;
328+
num++;
287329
}
288330
}
289331
}
@@ -538,7 +580,12 @@ public int commitAllowingStateLoss() {
538580

539581
int commitInternal(boolean allowStateLoss) {
540582
if (mCommitted) throw new IllegalStateException("commit already called");
541-
if (FragmentManagerImpl.DEBUG) Log.v(TAG, "Commit: " + this);
583+
if (FragmentManagerImpl.DEBUG) {
584+
Log.v(TAG, "Commit: " + this);
585+
LogWriter logw = new LogWriter(Log.VERBOSE, TAG);
586+
PrintWriter pw = new PrintWriter(logw);
587+
dump(" ", null, pw, null);
588+
}
542589
mCommitted = true;
543590
if (mAddToBackStack) {
544591
mIndex = mManager.allocBackStackIndex(this);
@@ -641,7 +688,12 @@ public void run() {
641688
}
642689

643690
public void popFromBackStack(boolean doStateMove) {
644-
if (FragmentManagerImpl.DEBUG) Log.v(TAG, "popFromBackStack: " + this);
691+
if (FragmentManagerImpl.DEBUG) {
692+
Log.v(TAG, "popFromBackStack: " + this);
693+
LogWriter logw = new LogWriter(Log.VERBOSE, TAG);
694+
PrintWriter pw = new PrintWriter(logw);
695+
dump(" ", null, pw, null);
696+
}
645697

646698
bumpBackStackNesting(-1);
647699

‎core/java/android/app/FragmentManager.java‎

Lines changed: 32 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import android.content.res.Configuration;
2323
import android.content.res.TypedArray;
2424
import android.os.Bundle;
25+
import android.os.Debug;
2526
import android.os.Handler;
2627
import android.os.Looper;
2728
import android.os.Parcel;
@@ -771,9 +772,9 @@ public void performPendingDeferredStart(Fragment f) {
771772

772773
void moveToState(Fragment f, int newState, int transit, int transitionStyle,
773774
boolean keepActive) {
774-
//if (DEBUG) Log.v(TAG, "moveToState: " + f
775-
// + " oldState=" + f.mState + " newState=" + newState
776-
// + " mRemoving=" + f.mRemoving + " Callers=" + Debug.getCallers(5));
775+
if (DEBUG && false) Log.v(TAG, "moveToState: " + f
776+
+ " oldState=" + f.mState + " newState=" + newState
777+
+ " mRemoving=" + f.mRemoving + " Callers=" + Debug.getCallers(5));
777778

778779
// Fragments that are not currently added will sit in the onCreate() state.
779780
if ((!f.mAdded || f.mDetached) && newState > Fragment.CREATED) {
@@ -1112,6 +1113,9 @@ public void addFragment(Fragment fragment, boolean moveToStateNow) {
11121113
if (DEBUG) Log.v(TAG, "add: " + fragment);
11131114
makeActive(fragment);
11141115
if (!fragment.mDetached) {
1116+
if (mAdded.contains(fragment)) {
1117+
throw new IllegalStateException("Fragment already added: " + fragment);
1118+
}
11151119
mAdded.add(fragment);
11161120
fragment.mAdded = true;
11171121
fragment.mRemoving = false;
@@ -1128,6 +1132,14 @@ public void removeFragment(Fragment fragment, int transition, int transitionStyl
11281132
if (DEBUG) Log.v(TAG, "remove: " + fragment + " nesting=" + fragment.mBackStackNesting);
11291133
final boolean inactive = !fragment.isInBackStack();
11301134
if (!fragment.mDetached || inactive) {
1135+
if (false) {
1136+
// Would be nice to catch a bad remove here, but we need
1137+
// time to test this to make sure we aren't crashes cases
1138+
// where it is not a problem.
1139+
if (!mAdded.contains(fragment)) {
1140+
throw new IllegalStateException("Fragment not added: " + fragment);
1141+
}
1142+
}
11311143
if (mAdded != null) {
11321144
mAdded.remove(fragment);
11331145
}
@@ -1200,6 +1212,7 @@ public void detachFragment(Fragment fragment, int transition, int transitionStyl
12001212
if (fragment.mAdded) {
12011213
// We are not already in back stack, so need to remove the fragment.
12021214
if (mAdded != null) {
1215+
if (DEBUG) Log.v(TAG, "remove from detach: " + fragment);
12031216
mAdded.remove(fragment);
12041217
}
12051218
if (fragment.mHasMenu && fragment.mMenuVisible) {
@@ -1219,6 +1232,10 @@ public void attachFragment(Fragment fragment, int transition, int transitionStyl
12191232
if (mAdded == null) {
12201233
mAdded = new ArrayList<Fragment>();
12211234
}
1235+
if (mAdded.contains(fragment)) {
1236+
throw new IllegalStateException("Fragment already added: " + fragment);
1237+
}
1238+
if (DEBUG) Log.v(TAG, "add from attach: " + fragment);
12221239
mAdded.add(fragment);
12231240
fragment.mAdded = true;
12241241
if (fragment.mHasMenu && fragment.mMenuVisible) {
@@ -1717,19 +1734,18 @@ void restoreAllState(Parcelable state, ArrayList<Fragment> nonConfig) {
17171734
FragmentState fs = fms.mActive[i];
17181735
if (fs != null) {
17191736
Fragment f = fs.instantiate(mActivity, mParent);
1720-
if (DEBUG) Log.v(TAG, "restoreAllState: adding #" + i + ": " + f);
1737+
if (DEBUG) Log.v(TAG, "restoreAllState: active #" + i + ": " + f);
17211738
mActive.add(f);
17221739
// Now that the fragment is instantiated (or came from being
17231740
// retained above), clear mInstance in case we end up re-restoring
17241741
// from this FragmentState again.
17251742
fs.mInstance = null;
17261743
} else {
1727-
if (DEBUG) Log.v(TAG, "restoreAllState: adding #" + i + ": (null)");
17281744
mActive.add(null);
17291745
if (mAvailIndices == null) {
17301746
mAvailIndices = new ArrayList<Integer>();
17311747
}
1732-
if (DEBUG) Log.v(TAG, "restoreAllState: adding avail #" + i);
1748+
if (DEBUG) Log.v(TAG, "restoreAllState: avail #" + i);
17331749
mAvailIndices.add(i);
17341750
}
17351751
}
@@ -1760,7 +1776,10 @@ void restoreAllState(Parcelable state, ArrayList<Fragment> nonConfig) {
17601776
"No instantiated fragment for index #" + fms.mAdded[i]));
17611777
}
17621778
f.mAdded = true;
1763-
if (DEBUG) Log.v(TAG, "restoreAllState: making added #" + i + ": " + f);
1779+
if (DEBUG) Log.v(TAG, "restoreAllState: added #" + i + ": " + f);
1780+
if (mAdded.contains(f)) {
1781+
throw new IllegalStateException("Already added!");
1782+
}
17641783
mAdded.add(f);
17651784
}
17661785
} else {
@@ -1772,8 +1791,13 @@ void restoreAllState(Parcelable state, ArrayList<Fragment> nonConfig) {
17721791
mBackStack = new ArrayList<BackStackRecord>(fms.mBackStack.length);
17731792
for (int i=0; i<fms.mBackStack.length; i++) {
17741793
BackStackRecord bse = fms.mBackStack[i].instantiate(this);
1775-
if (DEBUG) Log.v(TAG, "restoreAllState: adding bse #" + i
1794+
if (DEBUG) {
1795+
Log.v(TAG, "restoreAllState: back stack #" + i
17761796
+ " (index " + bse.mIndex + "): " + bse);
1797+
LogWriter logw = new LogWriter(Log.VERBOSE, TAG);
1798+
PrintWriter pw = new PrintWriter(logw);
1799+
bse.dump(" ", pw, false);
1800+
}
17771801
mBackStack.add(bse);
17781802
if (bse.mIndex >= 0) {
17791803
setBackStackIndex(bse.mIndex, bse);

0 commit comments

Comments
 (0)