[GH] [FragmentStrictMode] Detect Fragment reuse

## Proposed Changes - Detect reuse of `Fragment` instances ## Testing Test: See `FragmentStrictModeTest#detectFragmentReuse` and `FragmentStrictModeTest#detectFragmentReuseInFlightTransaction` ## Issues Fixed Fixes: 153738653 This is an imported pull request from #142. Resolves #142 Github-Pr-Head-Sha: d9bcea2 GitOrigin-RevId: c54931f Change-Id: Ie6bc7618c3dde8df027be1940cfb95970d9578a4
androidx · Mar 23, 2021 · 9719885 · 9719885
1 parent 35756d0
commit 9719885
Show file tree

Hide file tree

Showing 8 changed files with 134 additions and 0 deletions.
diff --git 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
@@ -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/src/androidTest/java/androidx/fragment/app/strictmode/FragmentStrictModeTest.kt b/.../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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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 &lt;fragment&gt; 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);