From fb3cffeb35368da22f99b85d45039c4e6e471c06 Mon Sep 17 00:00:00 2001 From: Dianne Hackborn Date: Mon, 25 Oct 2010 17:08:56 -0700 Subject: [PATCH] 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 --- core/java/android/app/Activity.java | 41 ++++++++------ core/java/android/app/ActivityThread.java | 5 ++ core/java/android/app/Fragment.java | 32 ++++++----- core/java/android/app/FragmentManager.java | 18 +++++++ core/java/android/app/LoaderManager.java | 62 ++++++++++++++++++---- 5 files changed, 119 insertions(+), 39 deletions(-) diff --git a/core/java/android/app/Activity.java b/core/java/android/app/Activity.java index f08d88de6ba39..378a8bdb1ca22 100644 --- a/core/java/android/app/Activity.java +++ b/core/java/android/app/Activity.java @@ -645,11 +645,13 @@ public class Activity extends ContextThemeWrapper 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 @@ public class Activity extends ContextThemeWrapper return mLoaderManager; } mCheckedForLoaderManager = true; - mLoaderManager = getLoaderManager(-1, mStarted, true); + mLoaderManager = getLoaderManager(-1, mLoadersStarted, true); return mLoaderManager; } @@ -777,9 +779,13 @@ public class Activity extends ContextThemeWrapper mAllLoaderManagers = new SparseArray(); } 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 @@ public class Activity extends ContextThemeWrapper */ 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 @@ public class Activity extends ContextThemeWrapper } final void performStart() { - mFragments.mStateSaved = false; + mFragments.noteStateNotSaved(); mCalled = false; mFragments.execPendingActions(); mInstrumentation.callActivityOnStart(this); @@ -4267,7 +4276,7 @@ public class Activity extends ContextThemeWrapper } final void performRestart() { - mFragments.mStateSaved = false; + mFragments.noteStateNotSaved(); synchronized (mManagedCursors) { final int N = mManagedCursors.size(); @@ -4347,8 +4356,8 @@ public class Activity extends ContextThemeWrapper } final void performStop() { - if (mStarted) { - mStarted = false; + if (mLoadersStarted) { + mLoadersStarted = false; if (mLoaderManager != null) { if (!mChangingConfigurations) { mLoaderManager.doStop(); @@ -4407,7 +4416,7 @@ public class Activity extends ContextThemeWrapper 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 f3f7ee7a0618c..2abe822c10146 100644 --- a/core/java/android/app/ActivityThread.java +++ b/core/java/android/app/ActivityThread.java @@ -1757,6 +1757,7 @@ public final class ActivityThread { for (int i=0; i(); @@ -1271,6 +1285,10 @@ final class FragmentManagerImpl implements FragmentManager { 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 28abcaa7735f1..4d4ea9aee482c 100644 --- a/core/java/android/app/LoaderManager.java +++ b/core/java/android/app/LoaderManager.java @@ -40,7 +40,12 @@ public interface LoaderManager { public Loader 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 not 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 @@ class LoaderManagerImpl implements LoaderManager { // previously run loader until the new loader's data is available. final SparseArray mInactiveLoaders = new SparseArray(); + Activity mActivity; boolean mStarted; boolean mRetaining; boolean mRetainingStarted; @@ -172,12 +178,12 @@ class LoaderManagerImpl implements LoaderManager { 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 @@ class LoaderManagerImpl implements LoaderManager { // 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 @@ class LoaderManagerImpl implements LoaderManager { } } + void callOnLoadFinished(Loader 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 @@ class LoaderManagerImpl implements LoaderManager { } } - 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 callback) { LoaderInfo info = new LoaderInfo(id, args, (LoaderManager.LoaderCallbacks)callback); @@ -286,7 +312,7 @@ class LoaderManagerImpl implements LoaderManager { 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)info.mLoader; @@ -348,7 +374,13 @@ class LoaderManagerImpl implements LoaderManager { 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 @@ class LoaderManagerImpl implements LoaderManager { 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 @@ class LoaderManagerImpl implements LoaderManager { 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;