Skip to content

Commit afd2573

Browse files
romtsnromtsn
authored
Merge 741c925 into 4e3e79d 
2 parents 4e3e79d + 741c925 commit afd2573

9 files changed

Lines changed: 665 additions & 11 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
### Fixes
66

7+
- Session Replay: Fix replay recording freezing on screens with continuous animations ([#5489](https://github.com/getsentry/sentry-java/pull/5489))
78
- Session Replay: Populate `trace_ids` in replay events to enable searching replays by trace ID ([#5473](https://github.com/getsentry/sentry-java/pull/5473))
89

910
## 8.43.0

‎gradle/libs.versions.toml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,7 @@ launchdarkly-server = { module = "com.launchdarkly:launchdarkly-java-server-sdk"
126126
log4j-api = { module = "org.apache.logging.log4j:log4j-api", version.ref = "log4j2" }
127127
log4j-core = { module = "org.apache.logging.log4j:log4j-core", version.ref = "log4j2" }
128128
leakcanary = { module = "com.squareup.leakcanary:leakcanary-android", version = "2.14" }
129+
lottie-compose = { module = "com.airbnb.android:lottie-compose", version = "6.7.1" }
129130
logback-classic = { module = "ch.qos.logback:logback-classic", version.ref = "logback" }
130131
nopen-annotations = { module = "com.jakewharton.nopen:nopen-annotations", version.ref = "nopen" }
131132
nopen-checker = { module = "com.jakewharton.nopen:nopen-checker", version.ref = "nopen" }
@@ -248,4 +249,3 @@ msgpack = { module = "org.msgpack:msgpack-core", version = "0.9.8" }
248249
okhttp-mockwebserver = { module = "com.squareup.okhttp3:mockwebserver", version.ref = "okhttp" }
249250
okio = { module = "com.squareup.okio:okio", version = "1.13.0" }
250251
roboelectric = { module = "org.robolectric:robolectric", version = "4.15" }
251-

‎sentry-android-replay/src/main/java/io/sentry/android/replay/screenshot/PixelCopyStrategy.kt‎

Lines changed: 49 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,10 @@ internal class PixelCopyStrategy(
4040
private val markContentChanged: () -> Unit = {},
4141
) : ScreenshotStrategy {
4242

43+
private companion object {
44+
const val MAX_UNSTABLE_CAPTURES_TO_SKIP = 1
45+
}
46+
4347
private val executor = executorProvider.getExecutor()
4448
private val mainLooperHandler = executorProvider.getMainLooperHandler()
4549
private val screenshot =
@@ -49,6 +53,7 @@ internal class PixelCopyStrategy(
4953
private val lastCaptureSuccessful = AtomicBoolean(false)
5054
private val maskRenderer = MaskRenderer()
5155
private val contentChanged = AtomicBoolean(false)
56+
private val unstableCaptures = AtomicInteger(0)
5257
private val isClosed = AtomicBoolean(false)
5358
private val dstOverPaint by
5459
lazy(NONE) { Paint().apply { xfermode = PorterDuffXfermode(PorterDuff.Mode.DST_OVER) } }
@@ -86,15 +91,13 @@ internal class PixelCopyStrategy(
8691

8792
if (copyResult != PixelCopy.SUCCESS) {
8893
options.logger.log(INFO, "Failed to capture replay recording: %d", copyResult)
94+
unstableCaptures.set(0)
8995
lastCaptureSuccessful.set(false)
9096
return@request
9197
}
9298

93-
// TODO: handle animations with heuristics (e.g. if we fall under this condition 2 times
94-
// in a row, we should capture)
95-
if (contentChanged.get()) {
96-
options.logger.log(INFO, "Failed to determine view hierarchy, not capturing")
97-
lastCaptureSuccessful.set(false)
99+
val changedDuringCapture = contentChanged.get()
100+
if (changedDuringCapture && shouldSkipUnstableCapture()) {
98101
return@request
99102
}
100103

@@ -111,25 +114,48 @@ internal class PixelCopyStrategy(
111114
if (surfaceViewNodes.isNullOrEmpty()) {
112115
executor.submit(
113116
ReplayRunnable("screenshot_recorder.mask") {
114-
applyMaskingAndNotify(root, viewHierarchy)
117+
applyMaskingAndNotify(
118+
root,
119+
viewHierarchy,
120+
resetUnstableCaptures = !changedDuringCapture,
121+
)
115122
}
116123
)
117124
} else {
118125
// Re-arm the recorder's contentChanged gate; SurfaceView redraws don't trigger
119126
// ViewTreeObserver.OnDrawListener, so we'd otherwise emit the same frame forever.
120127
markContentChanged()
121-
captureSurfaceViews(root, surfaceViewNodes, viewHierarchy)
128+
captureSurfaceViews(
129+
root,
130+
surfaceViewNodes,
131+
viewHierarchy,
132+
resetUnstableCaptures = !changedDuringCapture,
133+
)
122134
}
123135
},
124136
mainLooperHandler.handler,
125137
)
126138
} catch (e: Throwable) {
127139
options.logger.log(WARNING, "Failed to capture replay recording", e)
140+
unstableCaptures.set(0)
128141
lastCaptureSuccessful.set(false)
129142
}
130143
}
131144

132-
private fun applyMaskingAndNotify(root: View, viewHierarchy: ViewHierarchyNode) {
145+
private fun shouldSkipUnstableCapture(): Boolean {
146+
if (unstableCaptures.incrementAndGet() <= MAX_UNSTABLE_CAPTURES_TO_SKIP) {
147+
options.logger.log(INFO, "Failed to determine view hierarchy, not capturing")
148+
lastCaptureSuccessful.set(false)
149+
return true
150+
}
151+
return false
152+
}
153+
154+
private fun applyMaskingAndNotify(
155+
root: View,
156+
viewHierarchy: ViewHierarchyNode,
157+
resetUnstableCaptures: Boolean,
158+
) {
133159
if (isClosed.get() || screenshot.isRecycled) {
134160
options.logger.log(DEBUG, "PixelCopyStrategy is closed, skipping masking")
135161
return
@@ -149,13 +175,17 @@ internal class PixelCopyStrategy(
149175
screenshotRecorderCallback?.onScreenshotRecorded(screenshot)
150176
lastCaptureSuccessful.set(true)
151177
contentChanged.set(false)
178+
if (resetUnstableCaptures) {
179+
unstableCaptures.set(0)
180+
}
152181
}
153182

154183
@SuppressLint("NewApi")
155184
private fun captureSurfaceViews(
156185
root: View,
157186
surfaceViewNodes: List<ViewHierarchyNode.SurfaceViewHierarchyNode>,
158187
viewHierarchy: ViewHierarchyNode,
188+
resetUnstableCaptures: Boolean,
159189
) {
160190
// Snapshot the window location into locals so the executor-side compositor reads stable
161191
// values even if a new capture cycle starts and overwrites the field.
@@ -168,7 +198,14 @@ internal class PixelCopyStrategy(
168198

169199
fun onCaptureComplete() {
170200
if (remaining.decrementAndGet() == 0) {
171-
compositeSurfaceViewsAndMask(root, captures, viewHierarchy, windowX, windowY)
201+
compositeSurfaceViewsAndMask(
202+
root,
203+
captures,
204+
viewHierarchy,
205+
windowX,
206+
windowY,
207+
resetUnstableCaptures,
208+
)
172209
}
173210
}
174211

@@ -229,6 +266,7 @@ internal class PixelCopyStrategy(
229266
viewHierarchy: ViewHierarchyNode,
230267
windowX: Int,
231268
windowY: Int,
269+
resetUnstableCaptures: Boolean,
232270
) {
233271
executor.submit(
234272
ReplayRunnable("screenshot_recorder.composite") {
@@ -258,7 +296,7 @@ internal class PixelCopyStrategy(
258296
capture.bitmap.recycle()
259297
}
260298

261-
applyMaskingAndNotify(root, viewHierarchy)
299+
applyMaskingAndNotify(root, viewHierarchy, resetUnstableCaptures)
262300
}
263301
)
264302
}
@@ -287,6 +325,7 @@ internal class PixelCopyStrategy(
287325

288326
override fun close() {
289327
isClosed.set(true)
328+
unstableCaptures.set(0)
290329
executor.submit(
291330
ReplayRunnable(
292331
"PixelCopyStrategy.close",

‎sentry-android-replay/src/test/java/io/sentry/android/replay/screenshot/PixelCopyStrategyTest.kt‎

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,10 @@ import android.graphics.RectF
1212
import android.os.Bundle
1313
import android.os.Handler
1414
import android.os.Looper
15+
import android.view.PixelCopy
1516
import android.view.SurfaceView
17+
import android.view.View
18+
import android.view.Window
1619
import android.widget.FrameLayout
1720
import android.widget.LinearLayout
1821
import android.widget.LinearLayout.LayoutParams
@@ -36,12 +39,16 @@ import org.junit.runner.RunWith
3639
import org.mockito.kotlin.any
3740
import org.mockito.kotlin.doAnswer
3841
import org.mockito.kotlin.mock
42+
import org.mockito.kotlin.never
43+
import org.mockito.kotlin.times
3944
import org.mockito.kotlin.verify
4045
import org.mockito.kotlin.whenever
4146
import org.robolectric.Robolectric.buildActivity
4247
import org.robolectric.Shadows.shadowOf
4348
import org.robolectric.annotation.Config
4449
import org.robolectric.annotation.GraphicsMode
50+
import org.robolectric.annotation.Implementation
51+
import org.robolectric.annotation.Implements
4552
import org.robolectric.shadows.ShadowPixelCopy
4653

4754
@Config(shadows = [ShadowPixelCopy::class], sdk = [30])
@@ -92,6 +99,7 @@ class PixelCopyStrategyTest {
9299
fun setup() {
93100
System.setProperty("robolectric.areWindowsMarkedVisible", "true")
94101
System.setProperty("robolectric.pixelCopyRenderMode", "hardware")
102+
DeferredWindowPixelCopyShadow.reset()
95103
}
96104

97105
@Test
@@ -132,6 +140,68 @@ class PixelCopyStrategyTest {
132140
if (failure.get() != null) throw failure.get()
133141
}
134142

143+
@Test
144+
@Config(shadows = [DeferredWindowPixelCopyShadow::class])
145+
fun `capture skips the first unstable PixelCopy result`() {
146+
val activity = buildActivity(SimpleActivity::class.java).setup()
147+
shadowOf(Looper.getMainLooper()).idle()
148+
val root = activity.get().findViewById<View>(android.R.id.content)
149+
150+
val strategy = fixture.getSut(executor = fixture.inlineExecutor())
151+
captureUnstableFrame(strategy, root)
152+
153+
assertFalse(strategy.lastCaptureSuccessful())
154+
verify(fixture.callback, never()).onScreenshotRecorded(any<Bitmap>())
155+
}
156+
157+
@Test
158+
@Config(shadows = [DeferredWindowPixelCopyShadow::class])
159+
fun `capture emits the second consecutive unstable PixelCopy result`() {
160+
val activity = buildActivity(SimpleActivity::class.java).setup()
161+
shadowOf(Looper.getMainLooper()).idle()
162+
val root = activity.get().findViewById<View>(android.R.id.content)
163+
164+
val strategy = fixture.getSut(executor = fixture.inlineExecutor())
165+
captureUnstableFrame(strategy, root)
166+
captureUnstableFrame(strategy, root)
167+
168+
assertTrue(strategy.lastCaptureSuccessful())
169+
verify(fixture.callback).onScreenshotRecorded(any<Bitmap>())
170+
}
171+
172+
@Test
173+
@Config(shadows = [DeferredWindowPixelCopyShadow::class])
174+
fun `capture keeps emitting after entering continuous instability mode`() {
175+
val activity = buildActivity(SimpleActivity::class.java).setup()
176+
shadowOf(Looper.getMainLooper()).idle()
177+
val root = activity.get().findViewById<View>(android.R.id.content)
178+
179+
val strategy = fixture.getSut(executor = fixture.inlineExecutor())
180+
captureUnstableFrame(strategy, root)
181+
captureUnstableFrame(strategy, root)
182+
captureUnstableFrame(strategy, root)
183+
184+
assertTrue(strategy.lastCaptureSuccessful())
185+
verify(fixture.callback, times(2)).onScreenshotRecorded(any<Bitmap>())
186+
}
187+
188+
@Test
189+
@Config(shadows = [DeferredWindowPixelCopyShadow::class])
190+
fun `stable capture resets the unstable PixelCopy counter`() {
191+
val activity = buildActivity(SimpleActivity::class.java).setup()
192+
shadowOf(Looper.getMainLooper()).idle()
193+
val root = activity.get().findViewById<View>(android.R.id.content)
194+
195+
val strategy = fixture.getSut(executor = fixture.inlineExecutor())
196+
captureUnstableFrame(strategy, root)
197+
captureUnstableFrame(strategy, root)
198+
captureStableFrame(strategy, root)
199+
captureUnstableFrame(strategy, root)
200+
201+
assertFalse(strategy.lastCaptureSuccessful())
202+
verify(fixture.callback, times(2)).onScreenshotRecorded(any<Bitmap>())
203+
}
204+
135205
@Test
136206
fun `capture does not call markContentChanged when option is disabled`() {
137207
val activity = buildActivity(ActivityWithSurfaceView::class.java).setup()
@@ -250,6 +320,50 @@ class PixelCopyStrategyTest {
250320
assertEquals(0, dest.getPixel(4, 4))
251321
assertEquals(0, dest.getPixel(25, 25))
252322
}
323+
324+
private fun captureUnstableFrame(strategy: PixelCopyStrategy, root: View) {
325+
strategy.capture(root)
326+
strategy.onContentChanged()
327+
DeferredWindowPixelCopyShadow.flush()
328+
shadowOf(Looper.getMainLooper()).idle()
329+
}
330+
331+
private fun captureStableFrame(strategy: PixelCopyStrategy, root: View) {
332+
strategy.capture(root)
333+
DeferredWindowPixelCopyShadow.flush()
334+
shadowOf(Looper.getMainLooper()).idle()
335+
}
336+
}
337+
338+
@Implements(PixelCopy::class)
339+
class DeferredWindowPixelCopyShadow {
340+
companion object {
341+
private val pendingCallbacks = mutableListOf<() -> Unit>()
342+
343+
fun reset() {
344+
pendingCallbacks.clear()
345+
}
346+
347+
fun flush() {
348+
val callbacks = pendingCallbacks.toList()
349+
pendingCallbacks.clear()
350+
callbacks.forEach { it.invoke() }
351+
}
352+
353+
@JvmStatic
354+
@Implementation
355+
@Suppress("UNUSED_PARAMETER")
356+
fun request(
357+
_source: Window,
358+
_dest: Bitmap,
359+
listener: PixelCopy.OnPixelCopyFinishedListener,
360+
listenerThread: Handler,
361+
) {
362+
pendingCallbacks.add {
363+
listenerThread.post { listener.onPixelCopyFinished(PixelCopy.SUCCESS) }
364+
}
365+
}
366+
}
253367
}
254368

255369
private class SimpleActivity : Activity() {

‎sentry-samples/sentry-samples-android/build.gradle.kts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,7 @@ dependencies {
150150
implementation(libs.androidx.browser)
151151
implementation(libs.coil.compose)
152152
implementation(libs.kotlinx.coroutines.android)
153+
implementation(libs.lottie.compose)
153154
implementation(libs.retrofit)
154155
implementation(libs.retrofit.gson)
155156
implementation(libs.sentry.native.ndk)

‎sentry-samples/sentry-samples-android/src/main/AndroidManifest.xml‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,10 @@
6464
android:name=".PermissionsActivity"
6565
android:exported="false" />
6666

67+
<activity
68+
android:name=".ReplayAnimationsActivity"
69+
android:exported="false" />
70+
6771
<activity
6872
android:name=".ProfilingActivity"
6973
android:exported="false" />

‎sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/MainActivity.kt‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -498,6 +498,18 @@ fun SessionReplayScreen() {
498498
}
499499
}
500500
}
501+
item {
502+
SentryTraced("open_replay_animations") {
503+
OutlinedButton(
504+
onClick = {
505+
activity.startActivity(Intent(activity, ReplayAnimationsActivity::class.java))
506+
},
507+
modifier = Modifier,
508+
) {
509+
Text("Open Animations", maxLines = 2, overflow = TextOverflow.Ellipsis)
510+
}
511+
}
512+
}
501513
item {
502514
SentryTraced("show_dialog") {
503515
OutlinedButton(onClick = { showDialog = true }, modifier = Modifier) {

0 commit comments

Comments
 (0)