From ab209a63a286ffb0ee57e884986839fa25d583ca Mon Sep 17 00:00:00 2001 From: Adam Powell Date: Thu, 26 Jan 2017 14:14:34 -0800 Subject: [PATCH] Lifecycle guarantees for target fragments Ported from frameworks/support change id I824eb312fbdfd6b548fe94aa2cd5b03afbaa97c7 When a target fragment was set using Fragment#setTargetFragment, all bets were off, especially when restoring from instance state. Order of lifecyle initialization was undefined meaning that two bugs could occur, both of which manifested as the target fragment was not initialized by the time the referring fragment's onCreate ran. One could happen if the target fragment is on the back stack, the other could happen if the target fragment was ordered unfortunately in FragmentManager implementation details. (!) Fix both by guaranteeing that any target fragment gets pushed forward to at least the CREATED state before we dispatch any lifecycle events for the referring fragment. Add some sanity checks that try to keep developers from setting target fragments across different FragmentManagers or from creating target cycles, both of which are bad ideas. Bug: 33653458 Test: CTS FragmentLifecycleTest Change-Id: If624d4665d29e205d37b9b384322e64d6d6d3615 --- core/java/android/app/Fragment.java | 18 ++++++++++++++++++ core/java/android/app/FragmentManager.java | 15 +++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/core/java/android/app/Fragment.java b/core/java/android/app/Fragment.java index 73b96f1ba1694..612998dd51583 100644 --- a/core/java/android/app/Fragment.java +++ b/core/java/android/app/Fragment.java @@ -762,6 +762,24 @@ public class Fragment implements ComponentCallbacks2, OnCreateContextMenuListene * are going to call back with {@link #onActivityResult(int, int, Intent)}. */ public void setTargetFragment(Fragment fragment, int requestCode) { + // Don't allow a caller to set a target fragment in another FragmentManager, + // but there's a snag: people do set target fragments before fragments get added. + // We'll have the FragmentManager check that for validity when we move + // the fragments to a valid state. + final FragmentManager mine = getFragmentManager(); + final FragmentManager theirs = fragment.getFragmentManager(); + if (mine != null && theirs != null && mine != theirs) { + throw new IllegalArgumentException("Fragment " + fragment + + " must share the same FragmentManager to be set as a target fragment"); + } + + // Don't let someone create a cycle. + for (Fragment check = fragment; check != null; check = check.getTargetFragment()) { + if (check == this) { + throw new IllegalArgumentException("Setting " + fragment + " as the target of " + + this + " would create a target cycle"); + } + } mTarget = fragment; mTargetRequestCode = requestCode; } diff --git a/core/java/android/app/FragmentManager.java b/core/java/android/app/FragmentManager.java index 44f1322f4b40c..32cf1c341b4c4 100644 --- a/core/java/android/app/FragmentManager.java +++ b/core/java/android/app/FragmentManager.java @@ -1110,10 +1110,25 @@ final class FragmentManagerImpl extends FragmentManager implements LayoutInflate } } } + f.mHost = mHost; f.mParentFragment = mParent; f.mFragmentManager = mParent != null ? mParent.mChildFragmentManager : mHost.getFragmentManagerImpl(); + + // If we have a target fragment, push it along to at least CREATED + // so that this one can rely on it as an initialized dependency. + if (f.mTarget != null) { + if (!mActive.contains(f.mTarget)) { + throw new IllegalStateException("Fragment " + f + + " declared target fragment " + f.mTarget + + " that does not belong to this FragmentManager!"); + } + if (f.mTarget.mState < Fragment.CREATED) { + moveToState(f.mTarget, Fragment.CREATED, 0, 0, true); + } + } + dispatchOnFragmentPreAttached(f, mHost.getContext(), false); f.mCalled = false; f.onAttach(mHost.getContext());