Skip to content

Commit 03fcc33

Browse files
author
Dianne Hackborn
committed
Fix issue #6284404: ArrayIndexOutOfBoundsException in...
...FragmentManagerImpl.restoreAllState This was a bug related to the difference between the pre- and post-HC behavior of onSaveInstanceState(). Prior to HC, state was saved before calling onPause(). Starting with HC, it is saved between onPause() and onStop(). To maintain compatibility with existing applications, there is a check in ActivityThread for pre-HC to in that case emulate the behavior of old applications, still calling onSaveInstanceState() before onPause() but using the state later. One of the special cases we had to deal with in the old model of saving state before pausing was restarting an activity that is already paused. Consider, for example: you have two activities on screen, the one on top not fullscreen so you can see the one behind. The top activity is resumed, the behind activity is paused. In the pre-HC world, the behind activity would have already had its state saved. Now you rotate the screen, and we need to restart the activities. We need to destroy the behind activity and create a new instance, but the new instance has to end up in the paused state. To accompish this, we restart it with a flag saying that it should end up paused. For the pre-HC world, since it ends up paused, we need to make sure we still have its instance state kept around in case we need it because we can't regenerate it (since it is already paused). So that is what the changed code here is doing. It goes through the normal create/start/resume steps, but holds on to the current saved state so that it isn't lost when resume clears it, and then puts the activity back to paused and stuffs that old saved state back in to it. The problem is that this code was doing it for every application, even HC apps. So we end up in a bad state, when a HC app has its saved state sitting there as if it had been saved, even though it is only paused. Now if we go to restart the activity again, instead of asking it for a new saved state (as we should for a HC app as part of stopping it), we just re-use the existing saved state again. Now this wouldn't generally be a huge problem. Worst case, when we restart the activity yet again we are just instantiating it from the same saved state as we used last time, dropping whatever changes may have happened in-between. Who cares? All it has been doing is sitting there in the background, visible to the user, but not something they can interact with. If the activity made changes to its fragments, those changes will be lost, and we will restore it from the older state. However... if one of those fragements is a retained fragment, this will *not* appear in the saved state, but actually be retained across each activity instance. And now we have a problem: if the retained fragments are changed during this time, the next activity instance will be created from the most recent state for the retained fragments, but the older state for everyting else. If these are inconsistent... wham, dead app. To fix this, just don't keep the saved state for HC apps. Also includes a small optimization to ActivityStack to not push the home screen to the front redundantly. Change-Id: Ic3900b12940de25cdd7c5fb9a2a28fb1f4c6cd1a
1 parent d9016b2 commit 03fcc33

File tree

4 files changed

+21
-5
lines changed

4 files changed

+21
-5
lines changed

core/java/android/app/ActivityThread.java

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2090,8 +2090,15 @@ private void handleLaunchActivity(ActivityClientRecord r, Intent customIntent) {
20902090
r.activity.mCalled = false;
20912091
mInstrumentation.callActivityOnPause(r.activity);
20922092
// We need to keep around the original state, in case
2093-
// we need to be created again.
2094-
r.state = oldState;
2093+
// we need to be created again. But we only do this
2094+
// for pre-Honeycomb apps, which always save their state
2095+
// when pausing, so we can not have them save their state
2096+
// when restarting from a paused state. For HC and later,
2097+
// we want to (and can) let the state be saved as the normal
2098+
// part of stopping the activity.
2099+
if (r.isPreHoneycomb()) {
2100+
r.state = oldState;
2101+
}
20952102
if (!r.activity.mCalled) {
20962103
throw new SuperNotCalledException(
20972104
"Activity " + r.intent.getComponent().toShortString() +

core/java/android/app/Fragment.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import android.util.AndroidRuntimeException;
2929
import android.util.AttributeSet;
3030
import android.util.DebugUtils;
31+
import android.util.Log;
3132
import android.util.SparseArray;
3233
import android.view.ContextMenu;
3334
import android.view.ContextMenu.ContextMenuInfo;
@@ -108,7 +109,9 @@ public Fragment instantiate(Activity activity) {
108109
mInstance.mRetainInstance = mRetainInstance;
109110
mInstance.mDetached = mDetached;
110111
mInstance.mFragmentManager = activity.mFragments;
111-
112+
if (FragmentManagerImpl.DEBUG) Log.v(FragmentManagerImpl.TAG,
113+
"Instantiated fragment " + mInstance);
114+
112115
return mInstance;
113116
}
114117

core/java/android/app/FragmentManager.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1074,14 +1074,15 @@ void makeActive(Fragment f) {
10741074
f.setIndex(mAvailIndices.remove(mAvailIndices.size()-1));
10751075
mActive.set(f.mIndex, f);
10761076
}
1077+
if (DEBUG) Log.v(TAG, "Allocated fragment index " + f);
10771078
}
10781079

10791080
void makeInactive(Fragment f) {
10801081
if (f.mIndex < 0) {
10811082
return;
10821083
}
10831084

1084-
if (DEBUG) Log.v(TAG, "Freeing fragment index " + f.mIndex);
1085+
if (DEBUG) Log.v(TAG, "Freeing fragment index " + f);
10851086
mActive.set(f.mIndex, null);
10861087
if (mAvailIndices == null) {
10871088
mAvailIndices = new ArrayList<Integer>();
@@ -1493,6 +1494,7 @@ ArrayList<Fragment> retainNonConfig() {
14931494
fragments.add(f);
14941495
f.mRetaining = true;
14951496
f.mTargetIndex = f.mTarget != null ? f.mTarget.mIndex : -1;
1497+
if (DEBUG) Log.v(TAG, "retainNonConfig: keeping retained " + f);
14961498
}
14971499
}
14981500
}

services/java/com/android/server/am/ActivityStack.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2619,6 +2619,7 @@ final int startActivityUncheckedLocked(ActivityRecord r,
26192619
}
26202620

26212621
boolean addingToTask = false;
2622+
boolean movedHome = false;
26222623
TaskRecord reuseTask = null;
26232624
if (((launchFlags&Intent.FLAG_ACTIVITY_NEW_TASK) != 0 &&
26242625
(launchFlags&Intent.FLAG_ACTIVITY_MULTIPLE_TASK) == 0)
@@ -2657,6 +2658,7 @@ final int startActivityUncheckedLocked(ActivityRecord r,
26572658
if (callerAtFront) {
26582659
// We really do want to push this one into the
26592660
// user's face, right now.
2661+
movedHome = true;
26602662
moveHomeToFrontFromLaunchLocked(launchFlags);
26612663
moveTaskToFrontLocked(taskTop.task, r, options);
26622664
}
@@ -2835,7 +2837,9 @@ final int startActivityUncheckedLocked(ActivityRecord r,
28352837
r.setTask(reuseTask, reuseTask, true);
28362838
}
28372839
newTask = true;
2838-
moveHomeToFrontFromLaunchLocked(launchFlags);
2840+
if (!movedHome) {
2841+
moveHomeToFrontFromLaunchLocked(launchFlags);
2842+
}
28392843

28402844
} else if (sourceRecord != null) {
28412845
if (!addingToTask &&

0 commit comments

Comments
 (0)