[GH] [FragmentStrictMode] Detect wrong Fragment container usage

## Proposed Changes - Detect usage of `Fragment` containers other than `FragmentContainerView` ## Testing Test: See `FragmentStrictModeTest#detectWrongFragmentContainer` ## Issues Fixed Fixes: 181137036 This is an imported pull request from #146. Resolves #146 Github-Pr-Head-Sha: cd84de8 GitOrigin-RevId: 02bc194 Change-Id: I86d8f2797c4e76957b36c31e91fb474e42c9ee1d
androidx · Mar 29, 2021 · df2d4fd · df2d4fd
1 parent db80b67
commit df2d4fd
Show file tree

Hide file tree

Showing 6 changed files with 88 additions and 0 deletions.
diff --git a/fragment/fragment/api/public_plus_experimental_current.txt b/fragment/fragment/api/public_plus_experimental_current.txt
@@ -470,6 +470,7 @@ package androidx.fragment.app.strictmode {
     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);
     method @RestrictTo(androidx.annotation.RestrictTo.Scope.LIBRARY) public static void onTargetFragmentUsage(androidx.fragment.app.Fragment);
+    method @RestrictTo(androidx.annotation.RestrictTo.Scope.LIBRARY) public static void onWrongFragmentContainer(androidx.fragment.app.Fragment);
     method public static void setDefaultPolicy(androidx.fragment.app.strictmode.FragmentStrictMode.Policy);
   }
 
@@ -489,6 +490,7 @@ package androidx.fragment.app.strictmode {
     method public androidx.fragment.app.strictmode.FragmentStrictMode.Policy.Builder detectRetainInstanceUsage();
     method public androidx.fragment.app.strictmode.FragmentStrictMode.Policy.Builder detectSetUserVisibleHint();
     method public androidx.fragment.app.strictmode.FragmentStrictMode.Policy.Builder detectTargetFragmentUsage();
+    method public androidx.fragment.app.strictmode.FragmentStrictMode.Policy.Builder detectWrongFragmentContainer();
     method public androidx.fragment.app.strictmode.FragmentStrictMode.Policy.Builder penaltyDeath();
     method public androidx.fragment.app.strictmode.FragmentStrictMode.Policy.Builder penaltyListener(androidx.fragment.app.strictmode.FragmentStrictMode.OnViolationListener);
     method public androidx.fragment.app.strictmode.FragmentStrictMode.Policy.Builder penaltyLog();
@@ -514,5 +516,9 @@ package androidx.fragment.app.strictmode {
     ctor public Violation();
   }
 
+  @RestrictTo(androidx.annotation.RestrictTo.Scope.LIBRARY) public final class WrongFragmentContainerViolation extends androidx.fragment.app.strictmode.Violation {
+    ctor public WrongFragmentContainerViolation();
+  }
+
 }
 
diff --git a/fragment/fragment/api/restricted_current.txt b/fragment/fragment/api/restricted_current.txt
@@ -496,6 +496,7 @@ package androidx.fragment.app.strictmode {
     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);
     method @RestrictTo(androidx.annotation.RestrictTo.Scope.LIBRARY) public static void onTargetFragmentUsage(androidx.fragment.app.Fragment);
+    method @RestrictTo(androidx.annotation.RestrictTo.Scope.LIBRARY) public static void onWrongFragmentContainer(androidx.fragment.app.Fragment);
     method public static void setDefaultPolicy(androidx.fragment.app.strictmode.FragmentStrictMode.Policy);
   }
 
@@ -515,6 +516,7 @@ package androidx.fragment.app.strictmode {
     method public androidx.fragment.app.strictmode.FragmentStrictMode.Policy.Builder detectRetainInstanceUsage();
     method public androidx.fragment.app.strictmode.FragmentStrictMode.Policy.Builder detectSetUserVisibleHint();
     method public androidx.fragment.app.strictmode.FragmentStrictMode.Policy.Builder detectTargetFragmentUsage();
+    method public androidx.fragment.app.strictmode.FragmentStrictMode.Policy.Builder detectWrongFragmentContainer();
     method public androidx.fragment.app.strictmode.FragmentStrictMode.Policy.Builder penaltyDeath();
     method public androidx.fragment.app.strictmode.FragmentStrictMode.Policy.Builder penaltyListener(androidx.fragment.app.strictmode.FragmentStrictMode.OnViolationListener);
     method public androidx.fragment.app.strictmode.FragmentStrictMode.Policy.Builder penaltyLog();
@@ -540,5 +542,9 @@ package androidx.fragment.app.strictmode {
     ctor public Violation();
   }
 
+  @RestrictTo(androidx.annotation.RestrictTo.Scope.LIBRARY) public final class WrongFragmentContainerViolation extends androidx.fragment.app.strictmode.Violation {
+    ctor public WrongFragmentContainerViolation();
+  }
+
 }
 
diff --git a/.../fragment/src/androidTest/java/androidx/fragment/app/strictmode/FragmentStrictModeTest.kt b/.../fragment/src/androidTest/java/androidx/fragment/app/strictmode/FragmentStrictModeTest.kt
@@ -266,4 +266,31 @@ public class FragmentStrictModeTest {
         StrictFragment().targetRequestCode
         assertThat(violation).isInstanceOf(TargetFragmentUsageViolation::class.java)
     }
+
+    @Test
+    public fun detectWrongFragmentContainer() {
+        var violation: Violation? = null
+        val policy = FragmentStrictMode.Policy.Builder()
+            .detectWrongFragmentContainer()
+            .penaltyListener { violation = it }
+            .build()
+        FragmentStrictMode.setDefaultPolicy(policy)
+
+        with(ActivityScenario.launch(FragmentTestActivity::class.java)) {
+            val fragmentManager = withActivity { supportFragmentManager }
+
+            fragmentManager.beginTransaction()
+                .add(R.id.content, StrictFragment())
+                .commit()
+            executePendingTransactions()
+            assertThat(violation).isInstanceOf(WrongFragmentContainerViolation::class.java)
+
+            violation = null
+            fragmentManager.beginTransaction()
+                .replace(R.id.content, StrictFragment())
+                .commit()
+            executePendingTransactions()
+            assertThat(violation).isInstanceOf(WrongFragmentContainerViolation::class.java)
+        }
+    }
 }
diff --git a/fragment/fragment/src/main/java/androidx/fragment/app/FragmentStateManager.java b/fragment/fragment/src/main/java/androidx/fragment/app/FragmentStateManager.java
@@ -31,6 +31,7 @@
 import androidx.annotation.Nullable;
 import androidx.core.view.ViewCompat;
 import androidx.fragment.R;
+import androidx.fragment.app.strictmode.FragmentStrictMode;
 import androidx.lifecycle.ViewModelStoreOwner;
 
 class FragmentStateManager {
@@ -484,6 +485,9 @@ void createView() {
             }
             FragmentContainer fragmentContainer = mFragment.mFragmentManager.getContainer();
             container = (ViewGroup) fragmentContainer.onFindViewById(mFragment.mContainerId);
+            if (!(container instanceof FragmentContainerView)) {
+                FragmentStrictMode.onWrongFragmentContainer(mFragment);
+            }
             if (container == null && !mFragment.mRestored) {
                 String resName;
                 try {

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
@@ -26,6 +26,7 @@
 import androidx.annotation.RestrictTo;
 import androidx.annotation.VisibleForTesting;
 import androidx.fragment.app.Fragment;
+import androidx.fragment.app.FragmentContainerView;
 import androidx.fragment.app.FragmentManager;
 
 import java.util.Collections;
@@ -56,6 +57,7 @@ private enum Flag {
         DETECT_RETAIN_INSTANCE_USAGE,
         DETECT_SET_USER_VISIBLE_HINT,
         DETECT_TARGET_FRAGMENT_USAGE,
+        DETECT_WRONG_FRAGMENT_CONTAINER,
     }
 
     private FragmentStrictMode() {}
@@ -195,6 +197,17 @@ public Builder detectTargetFragmentUsage() {
                 return this;
             }
 
+            /**
+             * Detects cases where a #{@link Fragment} is added to a container other than a
+             * #{@link FragmentContainerView}.
+             */
+            @NonNull
+            @SuppressLint("BuilderSetStyle")
+            public Builder detectWrongFragmentContainer() {
+                flags.add(Flag.DETECT_WRONG_FRAGMENT_CONTAINER);
+                return this;
+            }
+
             /**
              * Construct the Policy instance.
              *
@@ -280,6 +293,14 @@ public static void onTargetFragmentUsage(@NonNull Fragment fragment) {
         }
     }
 
+    @RestrictTo(RestrictTo.Scope.LIBRARY)
+    public static void onWrongFragmentContainer(@NonNull Fragment fragment) {
+        Policy policy = getNearestPolicy(fragment);
+        if (policy.flags.contains(Flag.DETECT_WRONG_FRAGMENT_CONTAINER)) {
+            handlePolicyViolation(fragment, policy, new WrongFragmentContainerViolation());
+        }
+    }
+
     @VisibleForTesting
     static void onPolicyViolation(@NonNull Fragment fragment, @NonNull Violation violation) {
         Policy policy = getNearestPolicy(fragment);

diff --git a/...gment/src/main/java/androidx/fragment/app/strictmode/WrongFragmentContainerViolation.java b/...gment/src/main/java/androidx/fragment/app/strictmode/WrongFragmentContainerViolation.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#detectWrongFragmentContainer()}. */
+@RestrictTo(RestrictTo.Scope.LIBRARY) // TODO: Make API public as soon as we have a few checks
+public final class WrongFragmentContainerViolation extends Violation {
+}