Skip to content

Commit 39b483d

Browse files
simonschillercopybara-github
simonschiller
authored and
copybara-github
committed
[GH] Call FragmentStrictMode listeners on main thread of host
## Proposed Changes - Don't call listeners on the violating thread anymore, instead use the main thread of the host. - Follow-up to #123, see [here](#123 (comment)) and [here](#123 (comment)) ## Testing Test: `FragmentStrictModeTest#listenerCalledOnCorrectThread` ## Issues Fixed Fixes: 153737341 This is an imported pull request from #131. Resolves #131 Github-Pr-Head-Sha: 4c0f698 GitOrigin-RevId: 77da4ec Change-Id: Ica37a16e5f258b765c105224cf0dcab61b29c52e
1 parent e8c7033 commit 39b483d

File tree

4 files changed

+68
-9
lines changed

4 files changed

+68
-9
lines changed

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

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,14 @@
1616

1717
package androidx.fragment.app.strictmode
1818

19+
import android.os.Looper
1920
import androidx.fragment.app.StrictFragment
2021
import androidx.fragment.app.executePendingTransactions
2122
import androidx.fragment.app.test.FragmentTestActivity
2223
import androidx.test.core.app.ActivityScenario
2324
import androidx.test.ext.junit.runners.AndroidJUnit4
2425
import androidx.test.filters.MediumTest
26+
import androidx.test.platform.app.InstrumentationRegistry
2527
import androidx.testutils.withActivity
2628
import com.google.common.truth.Truth.assertThat
2729
import com.google.common.truth.Truth.assertWithMessage
@@ -87,15 +89,42 @@ public class FragmentStrictModeTest {
8789

8890
FragmentStrictMode.setDefaultPolicy(policy("Default policy"))
8991
FragmentStrictMode.onPolicyViolation(childFragment, violation)
92+
InstrumentationRegistry.getInstrumentation().waitForIdleSync()
9093
assertThat(lastTriggeredPolicy).isEqualTo("Default policy")
9194

9295
fragmentManager.strictModePolicy = policy("Parent policy")
9396
FragmentStrictMode.onPolicyViolation(childFragment, violation)
97+
InstrumentationRegistry.getInstrumentation().waitForIdleSync()
9498
assertThat(lastTriggeredPolicy).isEqualTo("Parent policy")
9599

96100
parentFragment.childFragmentManager.strictModePolicy = policy("Child policy")
97101
FragmentStrictMode.onPolicyViolation(childFragment, violation)
102+
InstrumentationRegistry.getInstrumentation().waitForIdleSync()
98103
assertThat(lastTriggeredPolicy).isEqualTo("Child policy")
99104
}
100105
}
106+
107+
@Test
108+
public fun listenerCalledOnCorrectThread() {
109+
var thread: Thread? = null
110+
111+
val policy = FragmentStrictMode.Policy.Builder()
112+
.penaltyListener { thread = Thread.currentThread() }
113+
.build()
114+
FragmentStrictMode.setDefaultPolicy(policy)
115+
116+
with(ActivityScenario.launch(FragmentTestActivity::class.java)) {
117+
val fragmentManager = withActivity { supportFragmentManager }
118+
119+
val fragment = StrictFragment()
120+
fragmentManager.beginTransaction()
121+
.add(fragment, null)
122+
.commit()
123+
executePendingTransactions()
124+
125+
FragmentStrictMode.onPolicyViolation(fragment, object : Violation() {})
126+
InstrumentationRegistry.getInstrumentation().waitForIdleSync()
127+
assertThat(thread).isEqualTo(Looper.getMainLooper().thread)
128+
}
129+
}
101130
}

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
import androidx.activity.result.contract.ActivityResultContract;
3636
import androidx.annotation.NonNull;
3737
import androidx.annotation.Nullable;
38+
import androidx.annotation.RestrictTo;
3839
import androidx.core.app.ActivityCompat;
3940
import androidx.core.content.ContextCompat;
4041
import androidx.core.util.Preconditions;
@@ -247,8 +248,10 @@ Context getContext() {
247248
return mContext;
248249
}
249250

251+
/** @hide */
252+
@RestrictTo(RestrictTo.Scope.LIBRARY)
250253
@NonNull
251-
Handler getHandler() {
254+
public Handler getHandler() {
252255
return mHandler;
253256
}
254257
}

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2941,8 +2941,10 @@ void restoreSaveState(@Nullable Parcelable state) {
29412941
mLaunchedFragments = new ArrayDeque<>(fms.mLaunchedFragments);
29422942
}
29432943

2944+
/** @hide */
2945+
@RestrictTo(RestrictTo.Scope.LIBRARY)
29442946
@NonNull
2945-
FragmentHostCallback<?> getHost() {
2947+
public FragmentHostCallback<?> getHost() {
29462948
return mHost;
29472949
}
29482950

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

Lines changed: 32 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717
package androidx.fragment.app.strictmode;
1818

1919
import android.annotation.SuppressLint;
20+
import android.os.Handler;
21+
import android.os.Looper;
2022
import android.util.Log;
2123

2224
import androidx.annotation.NonNull;
@@ -129,7 +131,7 @@ public Builder penaltyDeath() {
129131

130132
/**
131133
* Call #{@link OnViolationListener#onViolation} for every violation. The listener will
132-
* be called on the thread which caused the violation.
134+
* be called on the main thread of the fragment host.
133135
*/
134136
@NonNull
135137
@SuppressLint("BuilderSetStyle")
@@ -184,21 +186,44 @@ private static Policy getNearestPolicy(@Nullable Fragment fragment) {
184186
}
185187

186188
@VisibleForTesting
187-
static void onPolicyViolation(@NonNull Fragment fragment, @NonNull Violation violation) {
188-
Policy policy = getNearestPolicy(fragment);
189-
String fragmentName = fragment.getClass().getName();
189+
static void onPolicyViolation(@NonNull Fragment fragment, @NonNull final Violation violation) {
190+
final Policy policy = getNearestPolicy(fragment);
191+
final String fragmentName = fragment.getClass().getName();
190192

191193
if (policy.flags.contains(Flag.PENALTY_LOG)) {
192194
Log.d(TAG, "Policy violation in " + fragmentName, violation);
193195
}
194196

195197
if (policy.listener != null) {
196-
policy.listener.onViolation(violation);
198+
runOnHostThread(fragment, new Runnable() {
199+
@Override
200+
public void run() {
201+
policy.listener.onViolation(violation);
202+
}
203+
});
197204
}
198205

199206
if (policy.flags.contains(Flag.PENALTY_DEATH)) {
200-
Log.e(TAG, "Policy violation with PENALTY_DEATH in " + fragmentName, violation);
201-
throw violation;
207+
runOnHostThread(fragment, new Runnable() {
208+
@Override
209+
public void run() {
210+
Log.e(TAG, "Policy violation with PENALTY_DEATH in " + fragmentName, violation);
211+
throw violation;
212+
}
213+
});
214+
}
215+
}
216+
217+
private static void runOnHostThread(@NonNull Fragment fragment, @NonNull Runnable runnable) {
218+
if (fragment.isAdded()) {
219+
Handler handler = fragment.getParentFragmentManager().getHost().getHandler();
220+
if (handler.getLooper() == Looper.myLooper()) {
221+
runnable.run(); // Already on correct thread -> run synchronously
222+
} else {
223+
handler.post(runnable); // Switch to correct thread
224+
}
225+
} else {
226+
runnable.run(); // Fragment is not attached to any host -> run synchronously
202227
}
203228
}
204229
}

0 commit comments

Comments
 (0)