Skip to content

Commit f9a2748

Browse files
romtsnromtsn
authored
Merge 792d200 into 740e4bf 
2 parents 740e4bf + 792d200 commit f9a2748

4 files changed

Lines changed: 188 additions & 24 deletions

File tree

‎sentry-android-core/src/main/java/io/sentry/android/core/UserInteractionIntegration.java‎

Lines changed: 60 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,9 @@
1818
import io.sentry.util.Objects;
1919
import java.io.Closeable;
2020
import java.io.IOException;
21+
import java.lang.ref.WeakReference;
22+
import java.util.ArrayList;
23+
import java.util.WeakHashMap;
2124
import org.jetbrains.annotations.NotNull;
2225
import org.jetbrains.annotations.Nullable;
2326

@@ -30,6 +33,16 @@ public final class UserInteractionIntegration
3033

3134
private final boolean isAndroidxLifecycleAvailable;
3235

36+
// WeakReference value, because the callback chain strongly references the wrapper — a strong
37+
// value would prevent the window from ever being GC'd.
38+
//
39+
// All access must be guarded by wrappedWindowsLock — lifecycle callbacks fire on the main
40+
// thread, but close() may be called from a background thread (e.g. Sentry.close()).
41+
private final @NotNull WeakHashMap<Window, WeakReference<SentryWindowCallback>> wrappedWindows =
42+
new WeakHashMap<>();
43+
44+
private final @NotNull Object wrappedWindowsLock = new Object();
45+
3346
public UserInteractionIntegration(
3447
final @NotNull Application application, final @NotNull io.sentry.util.LoadClass classLoader) {
3548
this.application = Objects.requireNonNull(application, "Application is required");
@@ -47,19 +60,26 @@ private void startTracking(final @NotNull Activity activity) {
4760
}
4861

4962
if (scopes != null && options != null) {
63+
synchronized (wrappedWindowsLock) {
64+
final @Nullable WeakReference<SentryWindowCallback> cached = wrappedWindows.get(window);
65+
if (cached != null && cached.get() != null) {
66+
return;
67+
}
68+
}
69+
5070
Window.Callback delegate = window.getCallback();
5171
if (delegate == null) {
5272
delegate = new NoOpWindowCallback();
5373
}
5474

55-
if (delegate instanceof SentryWindowCallback) {
56-
// already instrumented
57-
return;
58-
}
59-
6075
final SentryGestureListener gestureListener =
6176
new SentryGestureListener(activity, scopes, options);
62-
window.setCallback(new SentryWindowCallback(delegate, activity, gestureListener, options));
77+
final SentryWindowCallback wrapper =
78+
new SentryWindowCallback(delegate, activity, gestureListener, options);
79+
window.setCallback(wrapper);
80+
synchronized (wrappedWindowsLock) {
81+
wrappedWindows.put(window, new WeakReference<>(wrapper));
82+
}
6383
}
6484
}
6585

@@ -71,7 +91,10 @@ private void stopTracking(final @NotNull Activity activity) {
7191
}
7292
return;
7393
}
94+
unwrapWindow(window);
95+
}
7496

97+
private void unwrapWindow(final @NotNull Window window) {
7598
final Window.Callback current = window.getCallback();
7699
if (current instanceof SentryWindowCallback) {
77100
((SentryWindowCallback) current).stopTracking();
@@ -80,6 +103,22 @@ private void stopTracking(final @NotNull Activity activity) {
80103
} else {
81104
window.setCallback(((SentryWindowCallback) current).getDelegate());
82105
}
106+
synchronized (wrappedWindowsLock) {
107+
wrappedWindows.remove(window);
108+
}
109+
return;
110+
}
111+
112+
// Another wrapper (e.g. Session Replay) sits on top of ours — cutting it out of the chain
113+
// would break its instrumentation, so we leave the chain alone and only release our
114+
// resources. The inert wrapper gets GC'd when the window is destroyed.
115+
final @Nullable SentryWindowCallback ours;
116+
synchronized (wrappedWindowsLock) {
117+
final @Nullable WeakReference<SentryWindowCallback> cached = wrappedWindows.get(window);
118+
ours = cached != null ? cached.get() : null;
119+
}
120+
if (ours != null) {
121+
ours.stopTracking();
83122
}
84123
}
85124

@@ -146,6 +185,21 @@ public void register(@NotNull IScopes scopes, @NotNull SentryOptions options) {
146185
public void close() throws IOException {
147186
application.unregisterActivityLifecycleCallbacks(this);
148187

188+
// Restore original callbacks so a subsequent Sentry.init() starts from a clean chain instead
189+
// of wrapping on top of our orphaned callback.
190+
final ArrayList<Window> snapshot;
191+
synchronized (wrappedWindowsLock) {
192+
snapshot = new ArrayList<>(wrappedWindows.keySet());
193+
}
194+
for (final Window window : snapshot) {
195+
if (window != null) {
196+
unwrapWindow(window);
197+
}
198+
}
199+
synchronized (wrappedWindowsLock) {
200+
wrappedWindows.clear();
201+
}
202+
149203
if (options != null) {
150204
options.getLogger().log(SentryLevel.DEBUG, "UserInteractionIntegration removed.");
151205
}

‎sentry-android-core/src/test/java/io/sentry/android/core/UserInteractionIntegrationTest.kt‎

Lines changed: 53 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import kotlin.test.BeforeTest
1414
import kotlin.test.Test
1515
import kotlin.test.assertIs
1616
import kotlin.test.assertIsNot
17-
import kotlin.test.assertNotEquals
1817
import kotlin.test.assertSame
1918
import org.junit.runner.RunWith
2019
import org.mockito.kotlin.any
@@ -149,15 +148,59 @@ class UserInteractionIntegrationTest {
149148
}
150149

151150
@Test
152-
fun `does not instrument if the callback is already ours`() {
153-
val existingCallback =
154-
SentryWindowCallback(NoOpWindowCallback(), fixture.activity, mock(), mock())
155-
val sut = fixture.getSut(existingCallback)
151+
fun `does not double-wrap when another callback wraps SentryWindowCallback`() {
152+
val sut = fixture.getSut()
153+
sut.register(fixture.scopes, fixture.options)
154+
155+
sut.onActivityResumed(fixture.activity)
156+
val sentryCallback = fixture.window.callback
157+
assertIs<SentryWindowCallback>(sentryCallback)
158+
159+
val outerWrapper = WrapperCallback(sentryCallback)
160+
fixture.window.callback = outerWrapper
161+
162+
sut.onActivityPaused(fixture.activity)
163+
sut.onActivityResumed(fixture.activity)
164+
165+
assertSame(outerWrapper, fixture.window.callback)
166+
}
167+
168+
@Test
169+
fun `close unwraps windows so re-init does not double-wrap`() {
170+
val mockCallback = mock<Window.Callback>()
171+
fixture.window.callback = mockCallback
156172

173+
val sutA = fixture.getSut()
174+
sutA.register(fixture.scopes, fixture.options)
175+
sutA.onActivityResumed(fixture.activity)
176+
assertIs<SentryWindowCallback>(fixture.window.callback)
177+
178+
sutA.close()
179+
assertSame(mockCallback, fixture.window.callback)
180+
181+
val sutB = UserInteractionIntegration(fixture.application, fixture.loadClass)
182+
sutB.register(fixture.scopes, fixture.options)
183+
sutB.onActivityResumed(fixture.activity)
184+
185+
val newWrapper = fixture.window.callback
186+
assertIs<SentryWindowCallback>(newWrapper)
187+
assertSame(mockCallback, newWrapper.delegate)
188+
}
189+
190+
@Test
191+
fun `paused with another wrapper on top does not cut it out of the chain`() {
192+
val sut = fixture.getSut()
157193
sut.register(fixture.scopes, fixture.options)
194+
158195
sut.onActivityResumed(fixture.activity)
196+
val sentryCallback = fixture.window.callback as SentryWindowCallback
197+
198+
val outerWrapper = WrapperCallback(sentryCallback)
199+
fixture.window.callback = outerWrapper
159200

160-
assertNotEquals(existingCallback, (fixture.window.callback as SentryWindowCallback).delegate)
201+
sut.onActivityPaused(fixture.activity)
202+
203+
assertSame(outerWrapper, fixture.window.callback)
161204
}
162205

163206
@Test
@@ -205,3 +248,7 @@ class UserInteractionIntegrationTest {
205248
private class EmptyActivity : Activity(), LifecycleOwner {
206249
override val lifecycle: Lifecycle = mock<Lifecycle>()
207250
}
251+
252+
/** Simulates a third-party callback wrapper (e.g. Session Replay's FixedWindowCallback). */
253+
private open class WrapperCallback(@JvmField val delegate: Window.Callback) :
254+
Window.Callback by delegate

‎sentry-android-replay/src/main/java/io/sentry/android/replay/gestures/GestureRecorder.kt‎

Lines changed: 37 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import io.sentry.android.replay.phoneWindow
1111
import io.sentry.android.replay.util.FixedWindowCallback
1212
import io.sentry.util.AutoClosableReentrantLock
1313
import java.lang.ref.WeakReference
14+
import java.util.WeakHashMap
1415

1516
internal class GestureRecorder(
1617
private val options: SentryOptions,
@@ -19,6 +20,11 @@ internal class GestureRecorder(
1920
private val rootViews = ArrayList<WeakReference<View>>()
2021
private val rootViewsLock = AutoClosableReentrantLock()
2122

23+
// WeakReference value, because the callback chain strongly references the wrapper — a strong
24+
// value would prevent the window from ever being GC'd.
25+
private val wrappedWindows = WeakHashMap<Window, WeakReference<SentryReplayGestureRecorder>>()
26+
private val wrappedWindowsLock = AutoClosableReentrantLock()
27+
2228
override fun onRootViewsChanged(root: View, added: Boolean) {
2329
rootViewsLock.acquire().use {
2430
if (added) {
@@ -45,10 +51,16 @@ internal class GestureRecorder(
4551
return
4652
}
4753

48-
val delegate = window.callback
49-
if (delegate !is SentryReplayGestureRecorder) {
50-
window.callback = SentryReplayGestureRecorder(options, touchRecorderCallback, delegate)
54+
wrappedWindowsLock.acquire().use {
55+
if (wrappedWindows[window]?.get() != null) {
56+
return
57+
}
5158
}
59+
60+
val delegate = window.callback
61+
val wrapper = SentryReplayGestureRecorder(options, touchRecorderCallback, delegate)
62+
window.callback = wrapper
63+
wrappedWindowsLock.acquire().use { wrappedWindows[window] = WeakReference(wrapper) }
5264
}
5365

5466
private fun View.stopGestureTracking() {
@@ -60,14 +72,25 @@ internal class GestureRecorder(
6072

6173
val callback = window.callback
6274
if (callback is SentryReplayGestureRecorder) {
63-
val delegate = callback.delegate
64-
window.callback = delegate
75+
window.callback = callback.delegate
76+
wrappedWindowsLock.acquire().use { wrappedWindows.remove(window) }
77+
return
78+
}
79+
80+
// Another wrapper (e.g. UserInteractionIntegration) sits on top of ours — cutting it out of
81+
// the chain would break its instrumentation, so we inert our buried wrapper instead. The
82+
// next replay session will then wrap on top with a fresh active instance.
83+
val ours: SentryReplayGestureRecorder?
84+
wrappedWindowsLock.acquire().use {
85+
ours = wrappedWindows[window]?.get()
86+
wrappedWindows.remove(window)
6587
}
88+
ours?.inert()
6689
}
6790

6891
internal class SentryReplayGestureRecorder(
6992
private val options: SentryOptions,
70-
private val touchRecorderCallback: TouchRecorderCallback?,
93+
@Volatile private var touchRecorderCallback: TouchRecorderCallback?,
7194
delegate: Window.Callback?,
7295
) : FixedWindowCallback(delegate) {
7396
override fun dispatchTouchEvent(event: MotionEvent?): Boolean {
@@ -83,6 +106,14 @@ internal class GestureRecorder(
83106
}
84107
return super.dispatchTouchEvent(event)
85108
}
109+
110+
/**
111+
* Turns this wrapper into a passthrough when it can't be removed from the chain (another
112+
* wrapper sits on top). Subsequent dispatches only delegate, skipping the recorder callback.
113+
*/
114+
fun inert() {
115+
touchRecorderCallback = null
116+
}
86117
}
87118
}
88119

‎sentry-android-replay/src/test/java/io/sentry/android/replay/gestures/GestureRecorderTest.kt‎

Lines changed: 38 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import android.app.Activity
55
import android.os.Bundle
66
import android.view.MotionEvent
77
import android.view.View
8+
import android.view.Window
89
import android.widget.LinearLayout
910
import androidx.test.ext.junit.runners.AndroidJUnit4
1011
import io.sentry.SentryOptions
@@ -14,6 +15,7 @@ import io.sentry.android.replay.phoneWindow
1415
import kotlin.test.Test
1516
import kotlin.test.assertEquals
1617
import kotlin.test.assertFalse
18+
import kotlin.test.assertSame
1719
import kotlin.test.assertTrue
1820
import org.junit.runner.RunWith
1921
import org.robolectric.Robolectric
@@ -37,17 +39,44 @@ class GestureRecorderTest {
3739
}
3840

3941
@Test
40-
fun `when new window added and window callback is already wrapped, does not wrap it again`() {
42+
fun `does not double-wrap when root is added twice and another callback wraps on top`() {
4143
val activity = Robolectric.buildActivity(TestActivity::class.java).setup().get()
4244
val gestureRecorder = fixture.getSut()
4345

44-
activity.root.phoneWindow?.callback = SentryReplayGestureRecorder(fixture.options, null, null)
4546
gestureRecorder.onRootViewsChanged(activity.root, true)
47+
val ourWrapper = activity.root.phoneWindow?.callback as SentryReplayGestureRecorder
4648

47-
assertFalse(
48-
(activity.root.phoneWindow?.callback as SentryReplayGestureRecorder).delegate
49-
is SentryReplayGestureRecorder
50-
)
49+
val outer = WrapperCallback(ourWrapper)
50+
activity.root.phoneWindow?.callback = outer
51+
52+
gestureRecorder.onRootViewsChanged(activity.root, true)
53+
54+
assertSame(outer, activity.root.phoneWindow?.callback)
55+
}
56+
57+
@Test
58+
fun `when stopped with another wrapper on top, inerts the buried recorder`() {
59+
var called = false
60+
val activity = Robolectric.buildActivity(TestActivity::class.java).setup().get()
61+
val gestureRecorder =
62+
fixture.getSut(
63+
touchRecorderCallback =
64+
object : TouchRecorderCallback {
65+
override fun onTouchEvent(event: MotionEvent) {
66+
called = true
67+
}
68+
}
69+
)
70+
71+
gestureRecorder.onRootViewsChanged(activity.root, true)
72+
val ourWrapper = activity.root.phoneWindow?.callback as SentryReplayGestureRecorder
73+
activity.root.phoneWindow?.callback = WrapperCallback(ourWrapper)
74+
75+
gestureRecorder.onRootViewsChanged(activity.root, false)
76+
77+
val motionEvent = MotionEvent.obtain(0, 0, MotionEvent.ACTION_DOWN, 0f, 0f, 0)
78+
ourWrapper.dispatchTouchEvent(motionEvent)
79+
assertFalse(called)
5180
}
5281

5382
@Test
@@ -109,6 +138,9 @@ class GestureRecorderTest {
109138
}
110139
}
111140

141+
private open class WrapperCallback(@JvmField val delegate: Window.Callback) :
142+
Window.Callback by delegate
143+
112144
private class TestActivity : Activity() {
113145
lateinit var root: View
114146

0 commit comments

Comments
 (0)