Fix issue #3130426: Finsky crash in switching from window carousel
Need to note that we no longer have saved state before delivering
results or new intents to an activity.
Also do some work on loaders to prevent apps from making fragment
changes as a result of receiving loader data. This makes apps
consistent crash in a case that they would previously sometimes
crash (if they got the loader data after onPause).
Change-Id: I46e9e46d0aa05d9d7d6a275a2a488a18a20a5747
diff --git a/core/java/android/app/Activity.java b/core/java/android/app/Activity.java
index f08d88d..378a8bd 100644
--- a/core/java/android/app/Activity.java
+++ b/core/java/android/app/Activity.java
@@ -645,11 +645,13 @@
Activity mParent;
boolean mCalled;
boolean mCheckedForLoaderManager;
- boolean mStarted;
+ boolean mLoadersStarted;
private boolean mResumed;
private boolean mStopped;
boolean mFinished;
boolean mStartedActivity;
+ /** true if the activity is going through a transient pause */
+ /*package*/ boolean mTemporaryPause = false;
/** true if the activity is being destroyed in order to recreate it with a new configuration */
/*package*/ boolean mChangingConfigurations = false;
/*package*/ int mConfigChangeFlags;
@@ -768,7 +770,7 @@
return mLoaderManager;
}
mCheckedForLoaderManager = true;
- mLoaderManager = getLoaderManager(-1, mStarted, true);
+ mLoaderManager = getLoaderManager(-1, mLoadersStarted, true);
return mLoaderManager;
}
@@ -777,9 +779,13 @@
mAllLoaderManagers = new SparseArray<LoaderManagerImpl>();
}
LoaderManagerImpl lm = mAllLoaderManagers.get(index);
- if (lm == null && create) {
- lm = new LoaderManagerImpl(started);
- mAllLoaderManagers.put(index, lm);
+ if (lm == null) {
+ if (create) {
+ lm = new LoaderManagerImpl(this, started);
+ mAllLoaderManagers.put(index, lm);
+ }
+ } else {
+ lm.updateActivity(this);
}
return lm;
}
@@ -979,13 +985,16 @@
*/
protected void onStart() {
mCalled = true;
- mStarted = true;
- if (mLoaderManager != null) {
- mLoaderManager.doStart();
- } else if (!mCheckedForLoaderManager) {
- mLoaderManager = getLoaderManager(-1, mStarted, false);
+
+ if (!mLoadersStarted) {
+ mLoadersStarted = true;
+ if (mLoaderManager != null) {
+ mLoaderManager.doStart();
+ } else if (!mCheckedForLoaderManager) {
+ mLoaderManager = getLoaderManager(-1, mLoadersStarted, false);
+ }
+ mCheckedForLoaderManager = true;
}
- mCheckedForLoaderManager = true;
}
/**
@@ -4249,7 +4258,7 @@
}
final void performStart() {
- mFragments.mStateSaved = false;
+ mFragments.noteStateNotSaved();
mCalled = false;
mFragments.execPendingActions();
mInstrumentation.callActivityOnStart(this);
@@ -4267,7 +4276,7 @@
}
final void performRestart() {
- mFragments.mStateSaved = false;
+ mFragments.noteStateNotSaved();
synchronized (mManagedCursors) {
final int N = mManagedCursors.size();
@@ -4347,8 +4356,8 @@
}
final void performStop() {
- if (mStarted) {
- mStarted = false;
+ if (mLoadersStarted) {
+ mLoadersStarted = false;
if (mLoaderManager != null) {
if (!mChangingConfigurations) {
mLoaderManager.doStop();
@@ -4407,7 +4416,7 @@
if (Config.LOGV) Log.v(
TAG, "Dispatching result: who=" + who + ", reqCode=" + requestCode
+ ", resCode=" + resultCode + ", data=" + data);
- mFragments.mStateSaved = false;
+ mFragments.noteStateNotSaved();
if (who == null) {
onActivityResult(requestCode, resultCode, data);
} else {
diff --git a/core/java/android/app/ActivityThread.java b/core/java/android/app/ActivityThread.java
index f3f7ee7..2abe822 100644
--- a/core/java/android/app/ActivityThread.java
+++ b/core/java/android/app/ActivityThread.java
@@ -1757,6 +1757,7 @@
for (int i=0; i<N; i++) {
Intent intent = intents.get(i);
intent.setExtrasClassLoader(r.activity.getClassLoader());
+ r.activity.mFragments.noteStateNotSaved();
mInstrumentation.callActivityOnNewIntent(r.activity, intent);
}
}
@@ -1767,11 +1768,13 @@
if (r != null) {
final boolean resumed = !r.paused;
if (resumed) {
+ r.activity.mTemporaryPause = true;
mInstrumentation.callActivityOnPause(r.activity);
}
deliverNewIntents(r, intents);
if (resumed) {
mInstrumentation.callActivityOnResume(r.activity);
+ r.activity.mTemporaryPause = false;
}
}
}
@@ -2594,6 +2597,7 @@
try {
// Now we are idle.
r.activity.mCalled = false;
+ r.activity.mTemporaryPause = true;
mInstrumentation.callActivityOnPause(r.activity);
if (!r.activity.mCalled) {
throw new SuperNotCalledException(
@@ -2614,6 +2618,7 @@
deliverResults(r, res.results);
if (resumed) {
mInstrumentation.callActivityOnResume(r.activity);
+ r.activity.mTemporaryPause = false;
}
}
}
diff --git a/core/java/android/app/Fragment.java b/core/java/android/app/Fragment.java
index 12bf7e5..3ec0912 100644
--- a/core/java/android/app/Fragment.java
+++ b/core/java/android/app/Fragment.java
@@ -403,7 +403,7 @@
View mView;
LoaderManagerImpl mLoaderManager;
- boolean mStarted;
+ boolean mLoadersStarted;
boolean mCheckedForLoaderManager;
/**
@@ -728,7 +728,7 @@
return mLoaderManager;
}
mCheckedForLoaderManager = true;
- mLoaderManager = mActivity.getLoaderManager(mIndex, mStarted, true);
+ mLoaderManager = mActivity.getLoaderManager(mIndex, mLoadersStarted, true);
return mLoaderManager;
}
@@ -880,13 +880,16 @@
*/
public void onStart() {
mCalled = true;
- mStarted = true;
- if (!mCheckedForLoaderManager) {
- mCheckedForLoaderManager = true;
- mLoaderManager = mActivity.getLoaderManager(mIndex, mStarted, false);
- }
- if (mLoaderManager != null) {
- mLoaderManager.doStart();
+
+ if (!mLoadersStarted) {
+ mLoadersStarted = true;
+ if (!mCheckedForLoaderManager) {
+ mCheckedForLoaderManager = true;
+ mLoaderManager = mActivity.getLoaderManager(mIndex, mLoadersStarted, false);
+ }
+ if (mLoaderManager != null) {
+ mLoaderManager.doStart();
+ }
}
}
@@ -971,7 +974,7 @@
// + " mLoaderManager=" + mLoaderManager);
if (!mCheckedForLoaderManager) {
mCheckedForLoaderManager = true;
- mLoaderManager = mActivity.getLoaderManager(mIndex, mStarted, false);
+ mLoaderManager = mActivity.getLoaderManager(mIndex, mLoadersStarted, false);
}
if (mLoaderManager != null) {
mLoaderManager.doDestroy();
@@ -1182,7 +1185,7 @@
}
if (mLoaderManager != null) {
writer.print(prefix); writer.print("mLoaderManager="); writer.print(mLoaderManager);
- writer.print(" mStarted="); writer.print(mStarted);
+ writer.print(" mLoadersStarted="); writer.print(mLoadersStarted);
writer.print(" mCheckedForLoaderManager=");
writer.println(mCheckedForLoaderManager);
}
@@ -1190,11 +1193,12 @@
void performStop() {
onStop();
- if (mStarted) {
- mStarted = false;
+
+ if (mLoadersStarted) {
+ mLoadersStarted = false;
if (!mCheckedForLoaderManager) {
mCheckedForLoaderManager = true;
- mLoaderManager = mActivity.getLoaderManager(mIndex, mStarted, false);
+ mLoaderManager = mActivity.getLoaderManager(mIndex, mLoadersStarted, false);
}
if (mLoaderManager != null) {
if (mActivity == null || !mActivity.mChangingConfigurations) {
diff --git a/core/java/android/app/FragmentManager.java b/core/java/android/app/FragmentManager.java
index 37e7253..d9a6171 100644
--- a/core/java/android/app/FragmentManager.java
+++ b/core/java/android/app/FragmentManager.java
@@ -86,6 +86,15 @@
/**
* Start a series of edit operations on the Fragments associated with
* this FragmentManager.
+ *
+ * <p>Note: A fragment transaction can only be created/committed prior
+ * to an activity saving its state. If you try to commit a transaction
+ * after {@link Activity#onSaveInstanceState Activity.onSaveInstanceState()}
+ * (and prior to a following {@link Activity#onStart Activity.onStart}
+ * or {@link Activity#onResume Activity.onResume()}, you will get an error.
+ * This is because the framework takes care of saving your current fragments
+ * in the state, and if changes are made after the state is saved then they
+ * will be lost.</p>
*/
public FragmentTransaction openTransaction();
@@ -271,6 +280,7 @@
boolean mNeedMenuInvalidate;
boolean mStateSaved;
+ String mNoTransactionsBecause;
// Temporary vars for state save and restore.
Bundle mStateBundle = null;
@@ -843,6 +853,10 @@
throw new IllegalStateException(
"Can not perform this action after onSaveInstanceState");
}
+ if (mNoTransactionsBecause != null) {
+ throw new IllegalStateException(
+ "Can not perform this action inside of " + mNoTransactionsBecause);
+ }
synchronized (this) {
if (mPendingActions == null) {
mPendingActions = new ArrayList<Runnable>();
@@ -1271,6 +1285,10 @@
mActivity = activity;
}
+ public void noteStateNotSaved() {
+ mStateSaved = false;
+ }
+
public void dispatchCreate() {
mStateSaved = false;
moveToState(Fragment.CREATED, false);
diff --git a/core/java/android/app/LoaderManager.java b/core/java/android/app/LoaderManager.java
index 28abcaa..4d4ea9a 100644
--- a/core/java/android/app/LoaderManager.java
+++ b/core/java/android/app/LoaderManager.java
@@ -40,7 +40,12 @@
public Loader<D> onCreateLoader(int id, Bundle args);
/**
- * Called when a previously created loader has finished its load.
+ * Called when a previously created loader has finished its load. Note
+ * that normally an application is <em>not</em> allowed to commit fragment
+ * transactions while in this call, since it can happen after an
+ * activity's state is saved. See {@link FragmentManager#openTransaction()
+ * FragmentManager.openTransaction()} for further discussion on this.
+ *
* @param loader The Loader that has finished.
* @param data The data generated by the Loader.
*/
@@ -102,6 +107,7 @@
// previously run loader until the new loader's data is available.
final SparseArray<LoaderInfo> mInactiveLoaders = new SparseArray<LoaderInfo>();
+ Activity mActivity;
boolean mStarted;
boolean mRetaining;
boolean mRetainingStarted;
@@ -172,12 +178,12 @@
stop();
}
}
- if (mStarted && mData != null && mCallbacks != null) {
+ if (mStarted && mData != null) {
// This loader was retained, and now at the point of
// finishing the retain we find we remain started, have
// our data, and the owner has a new callback... so
// let's deliver the data now.
- mCallbacks.onLoadFinished(mLoader, mData);
+ callOnLoadFinished(mLoader, mData);
}
}
}
@@ -219,9 +225,7 @@
// Notify of the new data so the app can switch out the old data before
// we try to destroy it.
mData = data;
- if (mCallbacks != null) {
- mCallbacks.onLoadFinished(loader, data);
- }
+ callOnLoadFinished(loader, data);
if (DEBUG) Log.v(TAG, "onLoadFinished returned: " + this);
@@ -236,6 +240,23 @@
}
}
+ void callOnLoadFinished(Loader<Object> loader, Object data) {
+ if (mCallbacks != null) {
+ String lastBecause = null;
+ if (mActivity != null) {
+ lastBecause = mActivity.mFragments.mNoTransactionsBecause;
+ mActivity.mFragments.mNoTransactionsBecause = "onLoadFinished";
+ }
+ try {
+ mCallbacks.onLoadFinished(loader, data);
+ } finally {
+ if (mActivity != null) {
+ mActivity.mFragments.mNoTransactionsBecause = lastBecause;
+ }
+ }
+ }
+ }
+
@Override
public String toString() {
StringBuilder sb = new StringBuilder(64);
@@ -252,10 +273,15 @@
}
}
- LoaderManagerImpl(boolean started) {
+ LoaderManagerImpl(Activity activity, boolean started) {
+ mActivity = activity;
mStarted = started;
}
+ void updateActivity(Activity activity) {
+ mActivity = activity;
+ }
+
private LoaderInfo createLoader(int id, Bundle args,
LoaderManager.LoaderCallbacks<Object> callback) {
LoaderInfo info = new LoaderInfo(id, args, (LoaderManager.LoaderCallbacks<Object>)callback);
@@ -286,7 +312,7 @@
if (info.mData != null && mStarted) {
// If the loader has already generated its data, report it now.
- info.mCallbacks.onLoadFinished(info.mLoader, info.mData);
+ info.callOnLoadFinished(info.mLoader, info.mData);
}
return (Loader<D>)info.mLoader;
@@ -348,7 +374,13 @@
void doStart() {
if (DEBUG) Log.v(TAG, "Starting: " + this);
-
+ if (mStarted) {
+ RuntimeException e = new RuntimeException("here");
+ e.fillInStackTrace();
+ Log.w(TAG, "Called doStart when already started: " + this, e);
+ return;
+ }
+
// Call out to sub classes so they can start their loaders
// Let the existing loaders know that we want to be notified when a load is complete
for (int i = mLoaders.size()-1; i >= 0; i--) {
@@ -359,6 +391,12 @@
void doStop() {
if (DEBUG) Log.v(TAG, "Stopping: " + this);
+ if (!mStarted) {
+ RuntimeException e = new RuntimeException("here");
+ e.fillInStackTrace();
+ Log.w(TAG, "Called doStop when not started: " + this, e);
+ return;
+ }
for (int i = mLoaders.size()-1; i >= 0; i--) {
mLoaders.valueAt(i).stop();
@@ -368,6 +406,12 @@
void doRetain() {
if (DEBUG) Log.v(TAG, "Retaining: " + this);
+ if (!mStarted) {
+ RuntimeException e = new RuntimeException("here");
+ e.fillInStackTrace();
+ Log.w(TAG, "Called doRetain when not started: " + this, e);
+ return;
+ }
mRetaining = true;
mStarted = false;