Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FragmentStrictMode] Detect Fragment reuse #142

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions fragment/fragment/api/public_plus_experimental_current.txt
Expand Up @@ -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);
Expand All @@ -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();
Expand Down
6 changes: 6 additions & 0 deletions fragment/fragment/api/restricted_current.txt
Expand Up @@ -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);
Expand All @@ -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();
Expand Down
Expand Up @@ -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
Expand Down
Expand Up @@ -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;

Expand Down Expand Up @@ -2184,6 +2188,7 @@ void initState() {
mTag = null;
mHidden = false;
mDetached = false;
mRemoved = true;
}

/**
Expand Down
Expand Up @@ -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;
Expand Down
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Expand Down
@@ -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 {
}
Expand Up @@ -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,
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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);
Expand Down