From f1d88e6e48c510f8c73019890fa4cebb6bb12d69 Mon Sep 17 00:00:00 2001 From: George Mount Date: Mon, 13 Mar 2017 10:47:57 -0700 Subject: [PATCH] Allow dangerous reentrant behavior for older apps Bug 36140114 Older apps may not be disciplined and could be taking advantage of the previous behavior that allowed executePendingTransactions() during FragmentManager state changes. Test: ran fragment cts tests Change-Id: I22e8979615f4cf78e1594d34fbfa72c9c49cf607 --- core/java/android/app/BackStackRecord.java | 22 +----- core/java/android/app/FragmentManager.java | 79 +++++++++++++++------- 2 files changed, 56 insertions(+), 45 deletions(-) diff --git a/core/java/android/app/BackStackRecord.java b/core/java/android/app/BackStackRecord.java index c88448aa2ee79..f564e8d08754c 100644 --- a/core/java/android/app/BackStackRecord.java +++ b/core/java/android/app/BackStackRecord.java @@ -370,7 +370,7 @@ final class BackStackRecord extends FragmentTransaction implements public BackStackRecord(FragmentManagerImpl manager) { mManager = manager; - mAllowOptimization = getTargetSdk() > Build.VERSION_CODES.N_MR1; + mAllowOptimization = mManager.getTargetSdk() > Build.VERSION_CODES.N_MR1; } public int getId() { @@ -423,7 +423,7 @@ final class BackStackRecord extends FragmentTransaction implements } private void doAddOp(int containerViewId, Fragment fragment, String tag, int opcmd) { - if (getTargetSdk() > Build.VERSION_CODES.N_MR1) { + if (mManager.getTargetSdk() > Build.VERSION_CODES.N_MR1) { final Class fragmentClass = fragment.getClass(); final int modifiers = fragmentClass.getModifiers(); if ((fragmentClass.isAnonymousClass() || !Modifier.isPublic(modifiers) @@ -1022,22 +1022,4 @@ final class BackStackRecord extends FragmentTransaction implements public boolean isEmpty() { return mOps.isEmpty(); } - - /** - * @return the target SDK of the FragmentManager's application info. If the - * FragmentManager has been torn down, then 0 is returned. - */ - private int getTargetSdk() { - FragmentHostCallback host = mManager.mHost; - if (host != null) { - Context context = host.getContext(); - if (context != null) { - ApplicationInfo info = context.getApplicationInfo(); - if (info != null) { - return info.targetSdkVersion; - } - } - } - return 0; - } } diff --git a/core/java/android/app/FragmentManager.java b/core/java/android/app/FragmentManager.java index 0672e3bc13a85..0d859a11e39f8 100644 --- a/core/java/android/app/FragmentManager.java +++ b/core/java/android/app/FragmentManager.java @@ -23,9 +23,11 @@ import android.animation.AnimatorSet; import android.animation.PropertyValuesHolder; import android.animation.ValueAnimator; import android.content.Context; +import android.content.pm.ApplicationInfo; import android.content.res.Configuration; import android.content.res.Resources.NotFoundException; import android.content.res.TypedArray; +import android.os.Build; import android.os.Bundle; import android.os.Debug; import android.os.Looper; @@ -674,6 +676,10 @@ final class FragmentManagerImpl extends FragmentManager implements LayoutInflate // Postponed transactions. ArrayList mPostponedTransactions; + // Prior to O, we allowed executing transactions during fragment manager state changes. + // This is dangerous, but we want to keep from breaking old applications. + boolean mAllowOldReentrantBehavior; + Runnable mExecCommit = new Runnable() { @Override public void run() { @@ -2815,69 +2821,92 @@ final class FragmentManagerImpl extends FragmentManager implements LayoutInflate mHost = host; mContainer = container; mParent = parent; + mAllowOldReentrantBehavior = getTargetSdk() <= Build.VERSION_CODES.N_MR1; } - + + /** + * @return the target SDK of the FragmentManager's application info. If the + * FragmentManager has been torn down, then 0 is returned. + */ + int getTargetSdk() { + if (mHost != null) { + Context context = mHost.getContext(); + if (context != null) { + ApplicationInfo info = context.getApplicationInfo(); + if (info != null) { + return info.targetSdkVersion; + } + } + } + return 0; + } + public void noteStateNotSaved() { mStateSaved = false; } public void dispatchCreate() { mStateSaved = false; - mExecutingActions = true; - moveToState(Fragment.CREATED, false); - mExecutingActions = false; + dispatchMoveToState(Fragment.CREATED); } public void dispatchActivityCreated() { mStateSaved = false; - mExecutingActions = true; - moveToState(Fragment.ACTIVITY_CREATED, false); - mExecutingActions = false; + dispatchMoveToState(Fragment.ACTIVITY_CREATED); } public void dispatchStart() { mStateSaved = false; - mExecutingActions = true; - moveToState(Fragment.STARTED, false); - mExecutingActions = false; + dispatchMoveToState(Fragment.STARTED); } public void dispatchResume() { mStateSaved = false; - mExecutingActions = true; - moveToState(Fragment.RESUMED, false); - mExecutingActions = false; + dispatchMoveToState(Fragment.RESUMED); } public void dispatchPause() { - mExecutingActions = true; - moveToState(Fragment.STARTED, false); - mExecutingActions = false; + dispatchMoveToState(Fragment.STARTED); } public void dispatchStop() { - mExecutingActions = true; - moveToState(Fragment.STOPPED, false); - mExecutingActions = false; + dispatchMoveToState(Fragment.STOPPED); } public void dispatchDestroyView() { - mExecutingActions = true; - moveToState(Fragment.CREATED, false); - mExecutingActions = false; + dispatchMoveToState(Fragment.CREATED); } public void dispatchDestroy() { mDestroyed = true; execPendingActions(); - mExecutingActions = true; - moveToState(Fragment.INITIALIZING, false); - mExecutingActions = false; + dispatchMoveToState(Fragment.INITIALIZING); mHost = null; mContainer = null; mParent = null; } + /** + * This method is called by dispatch* methods to change the FragmentManager's state. + * It calls moveToState directly if the target SDK is older than O. Otherwise, it sets and + * clears mExecutingActions to ensure that there is no reentrancy while the + * FragmentManager is changing state. + * + * @param state The new state of the FragmentManager. + */ + private void dispatchMoveToState(int state) { + if (mAllowOldReentrantBehavior) { + moveToState(state, false); + } else { + try { + mExecutingActions = true; + moveToState(state, false); + } finally { + mExecutingActions = false; + } + } + } + public void dispatchMultiWindowModeChanged(boolean isInMultiWindowMode) { if (mAdded == null) { return;