Fragment removal fixes
* Prevents a crash when item collection changes while
mFragmentManager.isStateSaved() == true. E.g. while the app is
minimised.
* Removes zombie Fragments that are no longer bound to any
items (issue after rotation).
* Removes saved state of removed items.
* Provides a grace period of 10s for previously bound Fragments
(pre-config change) to get rebound. This is to avoid Fragment churn.
To faciliate those, we need a Lifecycle, which is now a part of the
contructor. Two additional utility constructors were added to simplify
using the adapter in the context of a Fragment / FragmentActivity being
the ViewPager2 hosts.
Bug: 122669030
Test: ./gradlew viewpager2:connectedCheck
Change-Id: I3bdeca06780e4907f6765e86e47041219e4ab8d3
diff --git a/viewpager2/api/1.0.0-alpha03.txt b/viewpager2/api/1.0.0-alpha03.txt
index 4b1a6aa..1b64963 100644
--- a/viewpager2/api/1.0.0-alpha03.txt
+++ b/viewpager2/api/1.0.0-alpha03.txt
@@ -2,7 +2,9 @@
package androidx.viewpager2.adapter {
public abstract class FragmentStateAdapter extends androidx.recyclerview.widget.RecyclerView.Adapter<androidx.viewpager2.adapter.FragmentViewHolder> implements androidx.viewpager2.adapter.StatefulAdapter {
- ctor public FragmentStateAdapter(androidx.fragment.app.FragmentManager);
+ ctor public FragmentStateAdapter(androidx.fragment.app.FragmentActivity);
+ ctor public FragmentStateAdapter(androidx.fragment.app.Fragment);
+ ctor public FragmentStateAdapter(androidx.fragment.app.FragmentManager, androidx.lifecycle.Lifecycle);
method public boolean containsItem(long);
method public abstract androidx.fragment.app.Fragment getItem(int);
method public final void onBindViewHolder(androidx.viewpager2.adapter.FragmentViewHolder, int);
diff --git a/viewpager2/api/current.txt b/viewpager2/api/current.txt
index 4b1a6aa..1b64963 100644
--- a/viewpager2/api/current.txt
+++ b/viewpager2/api/current.txt
@@ -2,7 +2,9 @@
package androidx.viewpager2.adapter {
public abstract class FragmentStateAdapter extends androidx.recyclerview.widget.RecyclerView.Adapter<androidx.viewpager2.adapter.FragmentViewHolder> implements androidx.viewpager2.adapter.StatefulAdapter {
- ctor public FragmentStateAdapter(androidx.fragment.app.FragmentManager);
+ ctor public FragmentStateAdapter(androidx.fragment.app.FragmentActivity);
+ ctor public FragmentStateAdapter(androidx.fragment.app.Fragment);
+ ctor public FragmentStateAdapter(androidx.fragment.app.FragmentManager, androidx.lifecycle.Lifecycle);
method public boolean containsItem(long);
method public abstract androidx.fragment.app.Fragment getItem(int);
method public final void onBindViewHolder(androidx.viewpager2.adapter.FragmentViewHolder, int);
diff --git a/viewpager2/integration-tests/testapp/src/main/java/com/example/androidx/viewpager2/CardFragmentActivity.kt b/viewpager2/integration-tests/testapp/src/main/java/com/example/androidx/viewpager2/CardFragmentActivity.kt
index 794104d..55cf5b2 100644
--- a/viewpager2/integration-tests/testapp/src/main/java/com/example/androidx/viewpager2/CardFragmentActivity.kt
+++ b/viewpager2/integration-tests/testapp/src/main/java/com/example/androidx/viewpager2/CardFragmentActivity.kt
@@ -38,7 +38,7 @@
override fun onCreate(savedInstanceState: Bundle?) {
super.onCreate(savedInstanceState)
- viewPager.adapter = object : FragmentStateAdapter(supportFragmentManager) {
+ viewPager.adapter = object : FragmentStateAdapter(this) {
override fun getItem(position: Int): Fragment {
return CardFragment.create(Card.DECK[position])
}
diff --git a/viewpager2/integration-tests/testapp/src/main/java/com/example/androidx/viewpager2/MutableCollectionFragmentActivity.kt b/viewpager2/integration-tests/testapp/src/main/java/com/example/androidx/viewpager2/MutableCollectionFragmentActivity.kt
index 746c1ad..21c49dd 100644
--- a/viewpager2/integration-tests/testapp/src/main/java/com/example/androidx/viewpager2/MutableCollectionFragmentActivity.kt
+++ b/viewpager2/integration-tests/testapp/src/main/java/com/example/androidx/viewpager2/MutableCollectionFragmentActivity.kt
@@ -37,7 +37,7 @@
class MutableCollectionFragmentActivity : MutableCollectionBaseActivity() {
override fun createViewPagerAdapter(): RecyclerView.Adapter<*> {
val items = items // avoids resolving the ViewModel multiple times
- return object : FragmentStateAdapter(supportFragmentManager) {
+ return object : FragmentStateAdapter(this) {
override fun getItem(position: Int): PageFragment {
val itemId = items.itemId(position)
val itemText = items.getItemById(itemId)
diff --git a/viewpager2/src/androidTest/java/androidx/viewpager2/widget/BaseTest.kt b/viewpager2/src/androidTest/java/androidx/viewpager2/widget/BaseTest.kt
index 54f92c2..2c42ef2 100644
--- a/viewpager2/src/androidTest/java/androidx/viewpager2/widget/BaseTest.kt
+++ b/viewpager2/src/androidTest/java/androidx/viewpager2/widget/BaseTest.kt
@@ -428,7 +428,7 @@
typealias AdapterProviderForItems = (items: List<String>) -> AdapterProvider
val fragmentAdapterProvider: AdapterProviderForItems = { items ->
- { activity: TestActivity -> FragmentAdapter(activity.supportFragmentManager, items) }
+ { activity: TestActivity -> FragmentAdapter(activity, items) }
}
/**
diff --git a/viewpager2/src/androidTest/java/androidx/viewpager2/widget/swipe/FragmentAdapter.kt b/viewpager2/src/androidTest/java/androidx/viewpager2/widget/swipe/FragmentAdapter.kt
index aafcea4..8eedc0f 100644
--- a/viewpager2/src/androidTest/java/androidx/viewpager2/widget/swipe/FragmentAdapter.kt
+++ b/viewpager2/src/androidTest/java/androidx/viewpager2/widget/swipe/FragmentAdapter.kt
@@ -22,7 +22,7 @@
import android.view.View
import android.view.ViewGroup
import androidx.fragment.app.Fragment
-import androidx.fragment.app.FragmentManager
+import androidx.fragment.app.FragmentActivity
import androidx.viewpager2.adapter.FragmentStateAdapter
import org.hamcrest.Matchers.allOf
import org.hamcrest.Matchers.greaterThanOrEqualTo
@@ -33,9 +33,9 @@
private const val ARG_KEY = "key"
class FragmentAdapter(
- fragmentManager: FragmentManager,
+ fragmentActivity: FragmentActivity,
private val items: List<String>
-) : FragmentStateAdapter(fragmentManager), SelfChecking {
+) : FragmentStateAdapter(fragmentActivity), SelfChecking {
private val attachCount = AtomicInteger(0)
private val destroyCount = AtomicInteger(0)
diff --git a/viewpager2/src/main/java/androidx/viewpager2/adapter/FragmentStateAdapter.java b/viewpager2/src/main/java/androidx/viewpager2/adapter/FragmentStateAdapter.java
index 101a933..9ab45dd 100644
--- a/viewpager2/src/main/java/androidx/viewpager2/adapter/FragmentStateAdapter.java
+++ b/viewpager2/src/main/java/androidx/viewpager2/adapter/FragmentStateAdapter.java
@@ -17,6 +17,8 @@
package androidx.viewpager2.adapter;
import android.os.Bundle;
+import android.os.Handler;
+import android.os.Looper;
import android.os.Parcelable;
import android.view.View;
import android.view.ViewGroup;
@@ -25,12 +27,20 @@
import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
+import androidx.collection.ArraySet;
import androidx.collection.LongSparseArray;
import androidx.core.view.ViewCompat;
import androidx.fragment.app.Fragment;
+import androidx.fragment.app.FragmentActivity;
import androidx.fragment.app.FragmentManager;
import androidx.fragment.app.FragmentStatePagerAdapter;
+import androidx.lifecycle.Lifecycle;
+import androidx.lifecycle.LifecycleEventObserver;
+import androidx.lifecycle.LifecycleOwner;
import androidx.recyclerview.widget.RecyclerView;
+import androidx.viewpager2.widget.ViewPager2;
+
+import java.util.Set;
/**
* Similar in behavior to {@link FragmentStatePagerAdapter}
@@ -50,19 +60,58 @@
*/
public abstract class FragmentStateAdapter extends
RecyclerView.Adapter<FragmentViewHolder> implements StatefulAdapter {
+ // State saving config
private static final String KEY_PREFIX_FRAGMENT = "f#";
private static final String KEY_PREFIX_STATE = "s#";
- private final LongSparseArray<Fragment> mFragments = new LongSparseArray<>();
- private final LongSparseArray<Fragment.SavedState> mSavedStates = new LongSparseArray<>();
-
- // Keeps track what ViewHolder was last bound to an item.
- private final LongSparseArray<Integer> mItemViewHolderMap = new LongSparseArray<>();
+ // Fragment GC config
+ private static final long GRACE_WINDOW_TIME_MS = 10_000; // 10 seconds
private final FragmentManager mFragmentManager;
+ private final Lifecycle mLifecycle;
- public FragmentStateAdapter(@NonNull FragmentManager fragmentManager) {
+ // Fragment bookkeeping
+ private final LongSparseArray<Fragment> mFragments = new LongSparseArray<>();
+ private final LongSparseArray<Fragment.SavedState> mSavedStates = new LongSparseArray<>();
+ private final LongSparseArray<Integer> mItemIdToViewHolder = new LongSparseArray<>();
+
+ // Fragment GC
+ @SuppressWarnings("WeakerAccess") // to avoid creation of a synthetic accessor
+ boolean mIsInGracePeriod = false;
+ private boolean mHasStaleFragments = false;
+
+ /**
+ * @param fragmentActivity if the {@link ViewPager2} lives directly in a
+ * {@link FragmentActivity} subclass.
+ *
+ * @see FragmentStateAdapter#FragmentStateAdapter(Fragment)
+ * @see FragmentStateAdapter#FragmentStateAdapter(FragmentManager, Lifecycle)
+ */
+ public FragmentStateAdapter(@NonNull FragmentActivity fragmentActivity) {
+ this(fragmentActivity.getSupportFragmentManager(), fragmentActivity.getLifecycle());
+ }
+
+ /**
+ * @param fragment if the {@link ViewPager2} lives directly in a {@link Fragment} subclass.
+ *
+ * @see FragmentStateAdapter#FragmentStateAdapter(FragmentActivity)
+ * @see FragmentStateAdapter#FragmentStateAdapter(FragmentManager, Lifecycle)
+ */
+ public FragmentStateAdapter(@NonNull Fragment fragment) {
+ this(fragment.getChildFragmentManager(), fragment.getLifecycle());
+ }
+
+ /**
+ * @param fragmentManager of {@link ViewPager2}'s host
+ * @param lifecycle of {@link ViewPager2}'s host
+ *
+ * @see FragmentStateAdapter#FragmentStateAdapter(FragmentActivity)
+ * @see FragmentStateAdapter#FragmentStateAdapter(Fragment)
+ */
+ public FragmentStateAdapter(@NonNull FragmentManager fragmentManager,
+ @NonNull Lifecycle lifecycle) {
mFragmentManager = fragmentManager;
+ mLifecycle = lifecycle;
super.setHasStableIds(true);
}
@@ -84,10 +133,10 @@
final Long boundItemId = itemForViewHolder(viewHolderId); // item currently bound to the VH
if (boundItemId != null && boundItemId != itemId) {
removeFragment(boundItemId);
- mItemViewHolderMap.remove(boundItemId);
+ mItemIdToViewHolder.remove(boundItemId);
}
- mItemViewHolderMap.put(itemId, viewHolderId); // this might overwrite an existing entry
+ mItemIdToViewHolder.put(itemId, viewHolderId); // this might overwrite an existing entry
ensureFragment(position);
/** Special case when {@link RecyclerView} decides to keep the {@link container}
@@ -108,17 +157,52 @@
}
});
}
+
+ gcFragments();
+ }
+
+ @SuppressWarnings("WeakerAccess") // to avoid creation of a synthetic accessor
+ void gcFragments() {
+ if (!mHasStaleFragments || shouldDelayFragmentTransactions()) {
+ return;
+ }
+
+ // Remove Fragments for items that are no longer part of the data-set
+ Set<Long> toRemove = new ArraySet<>();
+ for (int ix = 0; ix < mFragments.size(); ix++) {
+ long itemId = mFragments.keyAt(ix);
+ if (!containsItem(itemId)) {
+ toRemove.add(itemId);
+ mItemIdToViewHolder.remove(itemId); // in case they're still bound
+ }
+ }
+
+ // Remove Fragments that are not bound anywhere -- pending a grace period
+ if (!mIsInGracePeriod) {
+ mHasStaleFragments = false; // we've executed all GC checks
+
+ for (int ix = 0; ix < mFragments.size(); ix++) {
+ long itemId = mFragments.keyAt(ix);
+ if (!mItemIdToViewHolder.containsKey(itemId)) {
+ toRemove.add(itemId);
+ }
+ }
+ }
+
+ for (Long itemId : toRemove) {
+ removeFragment(itemId);
+ }
}
private Long itemForViewHolder(int viewHolderId) {
Long boundItemId = null;
- for (int ix = 0; ix < mItemViewHolderMap.size(); ix++) {
- if (mItemViewHolderMap.valueAt(ix) == viewHolderId) {
+ for (int ix = 0; ix < mItemIdToViewHolder.size(); ix++) {
+ if (mItemIdToViewHolder.valueAt(ix) == viewHolderId) {
if (boundItemId != null) {
throw new IllegalStateException("Design assumption violated: "
+ "a ViewHolder can only be bound to one item at a time.");
}
- boundItemId = mItemViewHolderMap.keyAt(ix);
+ boundItemId = mItemIdToViewHolder.keyAt(ix);
}
}
return boundItemId;
@@ -189,6 +273,8 @@
scheduleViewAttach(fragment, container);
// TODO(b/122669030): this call might fail, so address with recovery steps
mFragmentManager.beginTransaction().add(fragment, "f" + holder.getItemId()).commitNow();
+
+ gcFragments();
}
private void scheduleViewAttach(final Fragment fragment, final FrameLayout container) {
@@ -237,7 +323,7 @@
final Long boundItemId = itemForViewHolder(viewHolderId); // item currently bound to the VH
if (boundItemId != null) {
removeFragment(boundItemId);
- mItemViewHolderMap.remove(boundItemId);
+ mItemIdToViewHolder.remove(boundItemId);
}
}
@@ -265,12 +351,29 @@
}
}
+ if (!containsItem(itemId)) {
+ mSavedStates.remove(itemId);
+ }
+
+ if (!fragment.isAdded()) {
+ mFragments.remove(itemId);
+ return;
+ }
+
+ if (shouldDelayFragmentTransactions()) {
+ mHasStaleFragments = true;
+ return;
+ }
+
if (fragment.isAdded() && containsItem(itemId)) {
mSavedStates.put(itemId, mFragmentManager.saveFragmentInstanceState(fragment));
}
-
- mFragments.remove(itemId);
mFragmentManager.beginTransaction().remove(fragment).commitNow();
+ mFragments.remove(itemId);
+ }
+
+ private boolean shouldDelayFragmentTransactions() {
+ return mFragmentManager.isStateSaved();
}
/**
@@ -353,12 +456,45 @@
if (isValidKey(key, KEY_PREFIX_STATE)) {
long itemId = parseIdFromKey(key, KEY_PREFIX_STATE);
Fragment.SavedState state = bundle.getParcelable(key);
- mSavedStates.put(itemId, state);
+ if (containsItem(itemId)) {
+ mSavedStates.put(itemId, state);
+ }
continue;
}
throw new IllegalArgumentException("Unexpected key in savedState: " + key);
}
+
+ if (!mFragments.isEmpty()) {
+ mHasStaleFragments = true;
+ mIsInGracePeriod = true;
+ gcFragments();
+ scheduleGracePeriodEnd();
+ }
+ }
+
+ private void scheduleGracePeriodEnd() {
+ final Handler handler = new Handler(Looper.getMainLooper());
+ final Runnable runnable = new Runnable() {
+ @Override
+ public void run() {
+ mIsInGracePeriod = false;
+ gcFragments(); // good opportunity to GC
+ }
+ };
+
+ mLifecycle.addObserver(new LifecycleEventObserver() {
+ @Override
+ public void onStateChanged(@NonNull LifecycleOwner source,
+ @NonNull Lifecycle.Event event) {
+ if (event == Lifecycle.Event.ON_DESTROY) {
+ handler.removeCallbacks(runnable);
+ source.getLifecycle().removeObserver(this);
+ }
+ }
+ });
+
+ handler.postDelayed(runnable, GRACE_WINDOW_TIME_MS);
}
// Helper function for dealing with save / restore state
