8000 [GH] [FragmentStrictMode] Detect Fragment reuse · androidx/androidx@9719885 · GitHub
[go: up one dir, main page]
Skip to content

Commit 9719885

Browse files
simonschillercopybara-github
simonschiller
authored and
copybara-github
committed
[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
1 parent 35756d0 commit 9719885

File tree

8 files changed

+134
-0
lines changed

8 files changed

+134
-0
lines changed

‎fragment/fragment/api/public_plus_experimental_current.txt‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -459,8 +459,13 @@ package androidx.fragment.app {
459459

460460
package androidx.fragment.app.strictmode {
461461

462+
@RestrictTo(androidx.annotation.RestrictTo.Scope.LIBRARY) public final class FragmentReuseViolation extends androidx.fragment.app.strictmode.Violation {
463+
ctor public FragmentReuseViolation();
464+
}
465+
462466
@RestrictTo(androidx.annotation.RestrictTo.Scope.LIBRARY) public final class FragmentStrictMode {
463467
method public static androidx.fragment.app.strictmode.FragmentStrictMode.Policy getDefaultPolicy();
468+
method @RestrictTo(androidx.annotation.RestrictTo.Scope.LIBRARY) public static void onFragmentReuse(androidx.fragment.app.Fragment);
464469
method @RestrictTo(androidx.annotation.RestrictTo.Scope.LIBRARY) public static void onFragmentTagUsage(androidx.fragment.app.Fragment);
465470
method @RestrictTo(androidx.annotation.RestrictTo.Scope.LIBRARY) public static void onRetainInstanceUsage(androidx.fragment.app.Fragment);
466471
method @RestrictTo(androidx.annotation.RestrictTo.Scope.LIBRARY) public static void onSetUserVisibleHint(androidx.fragment.app.Fragment);
@@ -479,6 +484,7 @@ package androidx.fragment.app.strictmode {
479484
@RestrictTo(androidx.annotation.RestrictTo.Scope.LIBRARY) public static final class FragmentStrictMode.Policy.Builder {
480485
ctor public FragmentStrictMode.Policy.Builder();
481486
method public androidx.fragment.app.strictmode.FragmentStrictMode.Policy build();
487+
method public androidx.fragment.app.strictmode.FragmentStrictMode.Policy.Builder detectFragmentReuse();
482488
method public androidx.fragment.app.strictmode.FragmentStrictMode.Policy.Builder detectFragmentTagUsage();
483489
method public androidx.fragment.app.strictmode.FragmentStrictMode.Policy.Builder detectRetainInstanceUsage();
484490
method public androidx.fragment.app.strictmode.FragmentStrictMode.Policy.Builder detectSetUserVisibleHint();

‎fragment/fragment/api/restricted_current.txt‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -485,8 +485,13 @@ package androidx.fragment.app {
485485

486486
package androidx.fragment.app.strictmode {
487487

488+
@RestrictTo(androidx.annotation.RestrictTo.Scope.LIBRARY) public final class FragmentReuseViolation extends androidx.fragment.app.strictmode.Violation {
489+
ctor public FragmentReuseViolation();
490+
}
491+
488492
@RestrictTo(androidx.annotation.RestrictTo.Scope.LIBRARY) public final class FragmentStrictMode {
489493
method public static androidx.fragment.app.strictmode.FragmentStrictMode.Policy getDefaultPolicy();
494+
method @RestrictTo(androidx.annotation.RestrictTo.Scope.LIBRARY) public static void onFragmentReuse(androidx.fragment.app.Fragment);
490495
method @RestrictTo(androidx.annotation.RestrictTo.Scope.LIBRARY) public static void onFragmentTagUsage(androidx.fragment.app.Fragment);
491496
method @RestrictTo(androidx.annotation.RestrictTo.Scope.LIBRARY) public static void onRetainInstanceUsage(androidx.fragment.app.Fragment);
492497
method @RestrictTo(androidx.annotation.RestrictTo.Scope.LIBRARY) public static void onSetUserVisibleHint(androidx.fragment.app.Fragment);
@@ -505,6 +510,7 @@ package androidx.fragment.app.strictmode {
505510
@RestrictTo(androidx.annotation.RestrictTo.Scope.LIBRARY) public static final class FragmentStrictMode.Policy.Builder {
506511
ctor public FragmentStrictMode.Policy.Builder();
507512
method public androidx.fragment.app.strictmode.FragmentStrictMode.Policy build();
513+
method public androidx.fragment.app.strictmode.FragmentStrictMode.Policy.Builder detectFragmentReuse();
508514
method public androidx.fragment.app.strictmode.FragmentStrictMode.Policy.Builder detectFragmentTagUsage();
509515
method public androidx.fragment.app.strictmode.FragmentStrictMode.Policy.Builder detectRetainInstanceUsage();
510516
method public androidx.fragment.app.strictmode.FragmentStrictMode.Policy.Builder detectSetUserVisibleHint();

‎fragment/fragment/src/androidTest/java/androidx/fragment/app/strictmode/FragmentStrictModeTest.kt‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,72 @@ public class FragmentStrictModeTest {
129129
}
130130
}
131131

132+
@Test
133+
public fun detectFragmentReuse() {
134+
var violation: Violation? = null
135+
val policy = FragmentStrictMode.Policy.Builder()
136+
.detectFragmentReuse()
137+
.penaltyListener { violation = it }
138+
.build()
139+
FragmentStrictMode.setDefaultPolicy(policy)
140+
141+
with(ActivityScenario.launch(FragmentTestActivity::class.java)) {
142+
val fragmentManager = withActivity { supportFragmentManager }
143+
val fragment = StrictFragment()
144+
145+
fragmentManager.beginTransaction()
146+
.add(fragment, null)
147+
.commit()
148+
executePendingTransactions()
149+
150+
fragmentManager.beginTransaction()
151+
.remove(fragment)
152+
.commit()
153+
executePendingTransactions()
154+
155+
fragmentManager.beginTransaction()
156+
.add(fragment, null)
157+
.commit()
158+
executePendingTransactions()
159+
160+
InstrumentationRegistry.getInstrumentation().waitForIdleSync()
161+
assertThat(violation).isInstanceOf(FragmentReuseViolation::class.java)
162+
}
163+
}
164+
165+
@Test
166+
public fun detectFragmentReuseInFlightTransaction() {
167+
var violation: Violation? = null
168+
val policy = FragmentStrictMode.Policy.Builder()
169+
.detectFragmentReuse()
170+
.penaltyListener { violation = it }
171+
.build()
172+
FragmentStrictMode.setDefaultPolicy(policy)
173+
174+
with(ActivityScenario.launch(FragmentTestActivity::class.java)) {
175+
val fragmentManager = withActivity { supportFragmentManager }
176+
val fragment = StrictFragment()
177+
178+
fragmentManager.beginTransaction()
179+
.add(fragment, null)
180+
.commit()
181+
executePendingTransactions()
182+
183+
fragmentManager.beginTransaction()
184+
.remove(fragment)
185+
.commit()
186+
// Don't execute transaction here, keep it in-flight
187+
188+
fragmentManager.beginTransaction()
189+
.add(fragment, null)
190+
.commit()
191+
executePendingTransactions()
192+
193+
InstrumentationRegistry.getInstrumentation().waitForIdleSync()
194+
assertThat(violation).isInstanceOf(FragmentReuseViolation::class.java)
195+
}
196+
}
197+
132198
@Test
133199
public fun detectFragmentTagUsage() {
134200
var violation: Violation? = null

‎fragment/fragment/src/main/java/androidx/fragment/app/Fragment.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -283,6 +283,10 @@ public void run() {
283283
// track it separately.
284284
boolean mIsCreated;
285285

286+
// True if the fragment was already added to a FragmentManager, but has since been removed
287+
// again.
288+
boolean mRemoved;
289+
286290
// Max Lifecycle state this Fragment can achieve.
287291
Lifecycle.State mMaxState = Lifecycle.State.RESUMED;
288292

@@ -2184,6 +2188,7 @@ void initState() {
21842188
mTag = null;
21852189
mHidden = false;
21862190
mDetached = false;
2191+
mRemoved = true;
21872192
}
21882193

21892194
/**

‎fragment/fragment/src/main/java/androidx/fragment/app/FragmentManager.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1728,6 +1728,9 @@ FragmentStateManager createOrGetFragmentStateManager(@NonNull Fragment f) {
17281728
}
17291729

17301730
FragmentStateManager addFragment(@NonNull Fragment fragment) {
1731+
if (fragment.mRemoved) {
1732+
FragmentStrictMode.onFragmentReuse(fragment);
1733+
}
17311734
if (isLoggingEnabled(Log.VERBOSE)) Log.v(TAG, "add: " + fragment);
17321735
FragmentStateManager fragmentStateManager = createOrGetFragmentStateManager(fragment);
17331736
fragment.mFragmentManager = this;

‎fragment/fragment/src/main/java/androidx/fragment/app/FragmentTransaction.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import androidx.annotation.StringRes;
3333
import androidx.annotation.StyleRes;
3434
import androidx.core.view.ViewCompat;
35+
import androidx.fragment.app.strictmode.FragmentStrictMode;
3536
import androidx.lifecycle.Lifecycle;
3637

3738
import java.lang.annotation.Retention;
@@ -253,6 +254,9 @@ FragmentTransaction add(@NonNull ViewGroup container, @NonNull Fragment fragment
253254
}
254255

255256
void doAddOp(int containerViewId, Fragment fragment, @Nullable String tag, int opcmd) {
257+
if (fragment.mRemoved) {
258+
FragmentStrictMode.onFragmentReuse(fragment);
259+
}
256260
final Class<?> fragmentClass = fragment.getClass();
257261
final int modifiers = fragmentClass.getModifiers();
258262
if (fragmentClass.isAnonymousClass() || !Modifier.isPublic(modifiers)

‎fragment/fragment/src/main/java/androidx/fragment/app/strictmode/FragmentReuseViolation.java‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
/*
2+
* Copyright 2021 The Android Open Source Project
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package androidx.fragment.app.strictmode;
18+
19+
import androidx.annotation.RestrictTo;
20+
21+
/** See #{@link FragmentStrictMode.Policy.Builder#detectFragmentReuse()}. */
22+
@RestrictTo(RestrictTo.Scope.LIBRARY) // TODO: Make API public as soon as we have a few checks
23+
public final class FragmentReuseViolation extends Violation {
24+
}

‎fragment/fragment/src/main/java/androidx/fragment/app/strictmode/FragmentStrictMode.java‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ private enum Flag {
5151
PENALTY_LOG,
5252
PENALTY_DEATH,
5353

54+
DETECT_FRAGMENT_REUSE,
5455
DETECT_FRAGMENT_TAG_USAGE,
5556
DETECT_RETAIN_INSTANCE_USAGE,
5657
DETECT_SET_USER_VISIBLE_HINT,
@@ -145,6 +146,17 @@ public Builder penaltyListener(@NonNull OnViolationListener listener) {
145146
return this;
146147
}
147148

149+
/**
150+
* Detects cases, where a #{@link Fragment} instance is reused, after it was previously
151+
* removed from a #{@link FragmentManager}.
152+
*/
153+
@NonNull
154+
@SuppressLint("BuilderSetStyle")
155+
public Builder detectFragmentReuse() {
156+
flags.add(Flag.DETECT_FRAGMENT_REUSE);
157+
return this;
158+
}
159+
148160
/** Detects usage of the &lt;fragment&gt; tag inside XML layouts. */
149161
@NonNull
150162
@SuppressLint("BuilderSetStyle")
@@ -228,6 +240,14 @@ private static Policy getNearestPolicy(@Nullable Fragment fragment) {
228240
return defaultPolicy;
229241
}
230242

243+
@RestrictTo(RestrictTo.Scope.LIBRARY)
244+
public static void onFragmentReuse(@NonNull Fragment fragment) {
245+
Policy policy = getNearestPolicy(fragment);
246+
if (policy.flags.contains(Flag.DETECT_FRAGMENT_REUSE)) {
247+
handlePolicyViolation(fragment, policy, new FragmentReuseViolation());
248+
}
249+
}
250+
231251
@RestrictTo(RestrictTo.Scope.LIBRARY)
232252
public static void onFragmentTagUsage(@NonNull Fragment fragment) {
233253
Policy policy = getNearestPolicy(fragment);

0 commit comments

Comments
 (0)
0