diff --git a/fragment/fragment/api/public_plus_experimental_current.txt b/fragment/fragment/api/public_plus_experimental_current.txt index 388322b01bf4f..d9798068972c7 100644 --- a/fragment/fragment/api/public_plus_experimental_current.txt +++ b/fragment/fragment/api/public_plus_experimental_current.txt @@ -459,8 +459,13 @@ package androidx.fragment.app { package androidx.fragment.app.strictmode { + @RestrictTo(androidx.annotation.RestrictTo.Scope.LIBRARY) public final class FragmentReuseViolation extends androidx.fragment.app.strictmode.Violation { + ctor public FragmentReuseViolation(); + } + @RestrictTo(androidx.annotation.RestrictTo.Scope.LIBRARY) public final class FragmentStrictMode { method public static androidx.fragment.app.strictmode.FragmentStrictMode.Policy getDefaultPolicy(); + method @RestrictTo(androidx.annotation.RestrictTo.Scope.LIBRARY) public static void onFragmentReuse(androidx.fragment.app.Fragment); method @RestrictTo(androidx.annotation.RestrictTo.Scope.LIBRARY) public static void onFragmentTagUsage(androidx.fragment.app.Fragment); method @RestrictTo(androidx.annotation.RestrictTo.Scope.LIBRARY) public static void onRetainInstanceUsage(androidx.fragment.app.Fragment); method @RestrictTo(androidx.annotation.RestrictTo.Scope.LIBRARY) public static void onSetUserVisibleHint(androidx.fragment.app.Fragment); @@ -479,6 +484,7 @@ package androidx.fragment.app.strictmode { @RestrictTo(androidx.annotation.RestrictTo.Scope.LIBRARY) public static final class FragmentStrictMode.Policy.Builder { ctor public FragmentStrictMode.Policy.Builder(); method public androidx.fragment.app.strictmode.FragmentStrictMode.Policy build(); + method public androidx.fragment.app.strictmode.FragmentStrictMode.Policy.Builder detectFragmentReuse(); method public androidx.fragment.app.strictmode.FragmentStrictMode.Policy.Builder detectFragmentTagUsage(); method public androidx.fragment.app.strictmode.FragmentStrictMode.Policy.Builder detectRetainInstanceUsage(); method public androidx.fragment.app.strictmode.FragmentStrictMode.Policy.Builder detectSetUserVisibleHint(); diff --git a/fragment/fragment/api/restricted_current.txt b/fragment/fragment/api/restricted_current.txt index 442b1f18f2bd5..da9959381d415 100644 --- a/fragment/fragment/api/restricted_current.txt +++ b/fragment/fragment/api/restricted_current.txt @@ -485,8 +485,13 @@ package androidx.fragment.app { package androidx.fragment.app.strictmode { + @RestrictTo(androidx.annotation.RestrictTo.Scope.LIBRARY) public final class FragmentReuseViolation extends androidx.fragment.app.strictmode.Violation { + ctor public FragmentReuseViolation(); + } + @RestrictTo(androidx.annotation.RestrictTo.Scope.LIBRARY) public final class FragmentStrictMode { method public static androidx.fragment.app.strictmode.FragmentStrictMode.Policy getDefaultPolicy(); + method @RestrictTo(androidx.annotation.RestrictTo.Scope.LIBRARY) public static void onFragmentReuse(androidx.fragment.app.Fragment); method @RestrictTo(androidx.annotation.RestrictTo.Scope.LIBRARY) public static void onFragmentTagUsage(androidx.fragment.app.Fragment); method @RestrictTo(androidx.annotation.RestrictTo.Scope.LIBRARY) public static void onRetainInstanceUsage(androidx.fragment.app.Fragment); method @RestrictTo(androidx.annotation.RestrictTo.Scope.LIBRARY) public static void onSetUserVisibleHint(androidx.fragment.app.Fragment); @@ -505,6 +510,7 @@ package androidx.fragment.app.strictmode { @RestrictTo(androidx.annotation.RestrictTo.Scope.LIBRARY) public static final class FragmentStrictMode.Policy.Builder { ctor public FragmentStrictMode.Policy.Builder(); method public androidx.fragment.app.strictmode.FragmentStrictMode.Policy build(); + method public androidx.fragment.app.strictmode.FragmentStrictMode.Policy.Builder detectFragmentReuse(); method public androidx.fragment.app.strictmode.FragmentStrictMode.Policy.Builder detectFragmentTagUsage(); method public androidx.fragment.app.strictmode.FragmentStrictMode.Policy.Builder detectRetainInstanceUsage(); method public androidx.fragment.app.strictmode.FragmentStrictMode.Policy.Builder detectSetUserVisibleHint(); diff --git a/fragment/fragment/src/androidTest/java/androidx/fragment/app/strictmode/FragmentStrictModeTest.kt b/fragment/fragment/src/androidTest/java/androidx/fragment/app/strictmode/FragmentStrictModeTest.kt index 097ea46d9b133..669e9f8d01547 100644 --- a/fragment/fragment/src/androidTest/java/androidx/fragment/app/strictmode/FragmentStrictModeTest.kt +++ b/fragment/fragment/src/androidTest/java/androidx/fragment/app/strictmode/FragmentStrictModeTest.kt @@ -129,6 +129,72 @@ public class FragmentStrictModeTest { } } + @Test + public fun detectFragmentReuse() { + var violation: Violation? = null + val policy = FragmentStrictMode.Policy.Builder() + .detectFragmentReuse() + .penaltyListener { violation = it } + .build() + FragmentStrictMode.setDefaultPolicy(policy) + + with(ActivityScenario.launch(FragmentTestActivity::class.java)) { + val fragmentManager = withActivity { supportFragmentManager } + val fragment = StrictFragment() + + fragmentManager.beginTransaction() + .add(fragment, null) + .commit() + executePendingTransactions() + + fragmentManager.beginTransaction() + .remove(fragment) + .commit() + executePendingTransactions() + + fragmentManager.beginTransaction() + .add(fragment, null) + .commit() + executePendingTransactions() + + InstrumentationRegistry.getInstrumentation().waitForIdleSync() + assertThat(violation).isInstanceOf(FragmentReuseViolation::class.java) + } + } + + @Test + public fun detectFragmentReuseInFlightTransaction() { + var violation: Violation? = null + val policy = FragmentStrictMode.Policy.Builder() + .detectFragmentReuse() + .penaltyListener { violation = it } + .build() + FragmentStrictMode.setDefaultPolicy(policy) + + with(ActivityScenario.launch(FragmentTestActivity::class.java)) { + val fragmentManager = withActivity { supportFragmentManager } + val fragment = StrictFragment() + + fragmentManager.beginTransaction() + .add(fragment, null) + .commit() + executePendingTransactions() + + fragmentManager.beginTransaction() + .remove(fragment) + .commit() + // Don't execute transaction here, keep it in-flight + + fragmentManager.beginTransaction() + .add(fragment, null) + .commit() + executePendingTransactions() + + InstrumentationRegistry.getInstrumentation().waitForIdleSync() + assertThat(violation).isInstanceOf(FragmentReuseViolation::class.java) + } + } + @Test public fun detectFragmentTagUsage() { var violation: Violation? = null diff --git a/fragment/fragment/src/main/java/androidx/fragment/app/Fragment.java b/fragment/fragment/src/main/java/androidx/fragment/app/Fragment.java index 7ca761774f5de..1dace46dd86f5 100644 --- a/fragment/fragment/src/main/java/androidx/fragment/app/Fragment.java +++ b/fragment/fragment/src/main/java/androidx/fragment/app/Fragment.java @@ -283,6 +283,10 @@ public void run() { // track it separately. boolean mIsCreated; + // True if the fragment was already added to a FragmentManager, but has since been removed + // again. + boolean mRemoved; + // Max Lifecycle state this Fragment can achieve. Lifecycle.State mMaxState = Lifecycle.State.RESUMED; @@ -2184,6 +2188,7 @@ void initState() { mTag = null; mHidden = false; mDetached = false; + mRemoved = true; } /** diff --git a/fragment/fragment/src/main/java/androidx/fragment/app/FragmentManager.java b/fragment/fragment/src/main/java/androidx/fragment/app/FragmentManager.java index 6484fae01df30..2bd831ca1e545 100644 --- a/fragment/fragment/src/main/java/androidx/fragment/app/FragmentManager.java +++ b/fragment/fragment/src/main/java/androidx/fragment/app/FragmentManager.java @@ -1728,6 +1728,9 @@ FragmentStateManager createOrGetFragmentStateManager(@NonNull Fragment f) { } FragmentStateManager addFragment(@NonNull Fragment fragment) { + if (fragment.mRemoved) { + FragmentStrictMode.onFragmentReuse(fragment); + } if (isLoggingEnabled(Log.VERBOSE)) Log.v(TAG, "add: " + fragment); FragmentStateManager fragmentStateManager = createOrGetFragmentStateManager(fragment); fragment.mFragmentManager = this; diff --git a/fragment/fragment/src/main/java/androidx/fragment/app/FragmentTransaction.java b/fragment/fragment/src/main/java/androidx/fragment/app/FragmentTransaction.java index d6a26366f37ae..f8a11fb834985 100644 --- a/fragment/fragment/src/main/java/androidx/fragment/app/FragmentTransaction.java +++ b/fragment/fragment/src/main/java/androidx/fragment/app/FragmentTransaction.java @@ -32,6 +32,7 @@ import androidx.annotation.StringRes; import androidx.annotation.StyleRes; import androidx.core.view.ViewCompat; +import androidx.fragment.app.strictmode.FragmentStrictMode; import androidx.lifecycle.Lifecycle; import java.lang.annotation.Retention; @@ -253,6 +254,9 @@ FragmentTransaction add(@NonNull ViewGroup container, @NonNull Fragment fragment } void doAddOp(int containerViewId, Fragment fragment, @Nullable String tag, int opcmd) { + if (fragment.mRemoved) { + FragmentStrictMode.onFragmentReuse(fragment); + } final Class fragmentClass = fragment.getClass(); final int modifiers = fragmentClass.getModifiers(); if (fragmentClass.isAnonymousClass() || !Modifier.isPublic(modifiers) diff --git a/fragment/fragment/src/main/java/androidx/fragment/app/strictmode/FragmentReuseViolation.java b/fragment/fragment/src/main/java/androidx/fragment/app/strictmode/FragmentReuseViolation.java new file mode 100644 index 0000000000000..f209c61293ca6 --- /dev/null +++ b/fragment/fragment/src/main/java/androidx/fragment/app/strictmode/FragmentReuseViolation.java @@ -0,0 +1,24 @@ +/* + * Copyright 2021 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package androidx.fragment.app.strictmode; + +import androidx.annotation.RestrictTo; + +/** See #{@link FragmentStrictMode.Policy.Builder#detectFragmentReuse()}. */ +@RestrictTo(RestrictTo.Scope.LIBRARY) // TODO: Make API public as soon as we have a few checks +public final class FragmentReuseViolation extends Violation { +} diff --git a/fragment/fragment/src/main/java/androidx/fragment/app/strictmode/FragmentStrictMode.java b/fragment/fragment/src/main/java/androidx/fragment/app/strictmode/FragmentStrictMode.java index f49e8e934a13e..1a554d70ac566 100644 --- a/fragment/fragment/src/main/java/androidx/fragment/app/strictmode/FragmentStrictMode.java +++ b/fragment/fragment/src/main/java/androidx/fragment/app/strictmode/FragmentStrictMode.java @@ -51,6 +51,7 @@ private enum Flag { PENALTY_LOG, PENALTY_DEATH, + DETECT_FRAGMENT_REUSE, DETECT_FRAGMENT_TAG_USAGE, DETECT_RETAIN_INSTANCE_USAGE, DETECT_SET_USER_VISIBLE_HINT, @@ -145,6 +146,17 @@ public Builder penaltyListener(@NonNull OnViolationListener listener) { return this; } + /** + * Detects cases, where a #{@link Fragment} instance is reused, after it was previously + * removed from a #{@link FragmentManager}. + */ + @NonNull + @SuppressLint("BuilderSetStyle") + public Builder detectFragmentReuse() { + flags.add(Flag.DETECT_FRAGMENT_REUSE); + return this; + } + /** Detects usage of the <fragment> tag inside XML layouts. */ @NonNull @SuppressLint("BuilderSetStyle") @@ -228,6 +240,14 @@ private static Policy getNearestPolicy(@Nullable Fragment fragment) { return defaultPolicy; } + @RestrictTo(RestrictTo.Scope.LIBRARY) + public static void onFragmentReuse(@NonNull Fragment fragment) { + Policy policy = getNearestPolicy(fragment); + if (policy.flags.contains(Flag.DETECT_FRAGMENT_REUSE)) { + handlePolicyViolation(fragment, policy, new FragmentReuseViolation()); + } + } + @RestrictTo(RestrictTo.Scope.LIBRARY) public static void onFragmentTagUsage(@NonNull Fragment fragment) { Policy policy = getNearestPolicy(fragment);