Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

### Fixes

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

## 8.43.0
Expand Down
2 changes: 1 addition & 1 deletion gradle/libs.versions.toml
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ launchdarkly-server = { module = "com.launchdarkly:launchdarkly-java-server-sdk"
log4j-api = { module = "org.apache.logging.log4j:log4j-api", version.ref = "log4j2" }
log4j-core = { module = "org.apache.logging.log4j:log4j-core", version.ref = "log4j2" }
leakcanary = { module = "com.squareup.leakcanary:leakcanary-android", version = "2.14" }
lottie-compose = { module = "com.airbnb.android:lottie-compose", version = "6.7.1" }
logback-classic = { module = "ch.qos.logback:logback-classic", version.ref = "logback" }
nopen-annotations = { module = "com.jakewharton.nopen:nopen-annotations", version.ref = "nopen" }
nopen-checker = { module = "com.jakewharton.nopen:nopen-checker", version.ref = "nopen" }
Expand Down Expand Up @@ -248,4 +249,3 @@ msgpack = { module = "org.msgpack:msgpack-core", version = "0.9.8" }
okhttp-mockwebserver = { module = "com.squareup.okhttp3:mockwebserver", version.ref = "okhttp" }
okio = { module = "com.squareup.okio:okio", version = "1.13.0" }
roboelectric = { module = "org.robolectric:robolectric", version = "4.15" }

Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,10 @@ internal class PixelCopyStrategy(
private val markContentChanged: () -> Unit = {},
) : ScreenshotStrategy {

private companion object {
const val MAX_UNSTABLE_CAPTURES_TO_SKIP = 1
Comment thread
romtsn marked this conversation as resolved.
Comment thread
romtsn marked this conversation as resolved.
}

private val executor = executorProvider.getExecutor()
private val mainLooperHandler = executorProvider.getMainLooperHandler()
private val screenshot =
Expand All @@ -49,6 +53,7 @@ internal class PixelCopyStrategy(
private val lastCaptureSuccessful = AtomicBoolean(false)
private val maskRenderer = MaskRenderer()
private val contentChanged = AtomicBoolean(false)
private val unstableCaptures = AtomicInteger(0)
private val isClosed = AtomicBoolean(false)
private val dstOverPaint by
lazy(NONE) { Paint().apply { xfermode = PorterDuffXfermode(PorterDuff.Mode.DST_OVER) } }
Expand Down Expand Up @@ -86,15 +91,13 @@ internal class PixelCopyStrategy(

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

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

Expand All @@ -111,25 +114,48 @@ internal class PixelCopyStrategy(
if (surfaceViewNodes.isNullOrEmpty()) {
executor.submit(
ReplayRunnable("screenshot_recorder.mask") {
applyMaskingAndNotify(root, viewHierarchy)
applyMaskingAndNotify(
root,
viewHierarchy,
resetUnstableCaptures = !changedDuringCapture,
)
}
)
} else {
// Re-arm the recorder's contentChanged gate; SurfaceView redraws don't trigger
// ViewTreeObserver.OnDrawListener, so we'd otherwise emit the same frame forever.
markContentChanged()
captureSurfaceViews(root, surfaceViewNodes, viewHierarchy)
captureSurfaceViews(
root,
surfaceViewNodes,
viewHierarchy,
resetUnstableCaptures = !changedDuringCapture,
)
}
},
mainLooperHandler.handler,
)
} catch (e: Throwable) {
options.logger.log(WARNING, "Failed to capture replay recording", e)
unstableCaptures.set(0)
lastCaptureSuccessful.set(false)
}
}

private fun applyMaskingAndNotify(root: View, viewHierarchy: ViewHierarchyNode) {
private fun shouldSkipUnstableCapture(): Boolean {
if (unstableCaptures.incrementAndGet() <= MAX_UNSTABLE_CAPTURES_TO_SKIP) {
options.logger.log(INFO, "Failed to determine view hierarchy, not capturing")
lastCaptureSuccessful.set(false)
return true
}
return false
}

private fun applyMaskingAndNotify(
root: View,
viewHierarchy: ViewHierarchyNode,
resetUnstableCaptures: Boolean,
) {
if (isClosed.get() || screenshot.isRecycled) {
options.logger.log(DEBUG, "PixelCopyStrategy is closed, skipping masking")
return
Expand All @@ -149,13 +175,17 @@ internal class PixelCopyStrategy(
screenshotRecorderCallback?.onScreenshotRecorded(screenshot)
lastCaptureSuccessful.set(true)
contentChanged.set(false)
if (resetUnstableCaptures) {
unstableCaptures.set(0)
}
}

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

fun onCaptureComplete() {
if (remaining.decrementAndGet() == 0) {
compositeSurfaceViewsAndMask(root, captures, viewHierarchy, windowX, windowY)
compositeSurfaceViewsAndMask(
root,
captures,
viewHierarchy,
windowX,
windowY,
resetUnstableCaptures,
)
}
}

Expand Down Expand Up @@ -229,6 +266,7 @@ internal class PixelCopyStrategy(
viewHierarchy: ViewHierarchyNode,
windowX: Int,
windowY: Int,
resetUnstableCaptures: Boolean,
) {
executor.submit(
ReplayRunnable("screenshot_recorder.composite") {
Expand Down Expand Up @@ -258,7 +296,7 @@ internal class PixelCopyStrategy(
capture.bitmap.recycle()
}

applyMaskingAndNotify(root, viewHierarchy)
applyMaskingAndNotify(root, viewHierarchy, resetUnstableCaptures)
}
)
}
Expand Down Expand Up @@ -287,6 +325,7 @@ internal class PixelCopyStrategy(

override fun close() {
isClosed.set(true)
unstableCaptures.set(0)
executor.submit(
ReplayRunnable(
"PixelCopyStrategy.close",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,10 @@ import android.graphics.RectF
import android.os.Bundle
import android.os.Handler
import android.os.Looper
import android.view.PixelCopy
import android.view.SurfaceView
import android.view.View
import android.view.Window
import android.widget.FrameLayout
import android.widget.LinearLayout
import android.widget.LinearLayout.LayoutParams
Expand All @@ -36,12 +39,16 @@ import org.junit.runner.RunWith
import org.mockito.kotlin.any
import org.mockito.kotlin.doAnswer
import org.mockito.kotlin.mock
import org.mockito.kotlin.never
import org.mockito.kotlin.times
import org.mockito.kotlin.verify
import org.mockito.kotlin.whenever
import org.robolectric.Robolectric.buildActivity
import org.robolectric.Shadows.shadowOf
import org.robolectric.annotation.Config
import org.robolectric.annotation.GraphicsMode
import org.robolectric.annotation.Implementation
import org.robolectric.annotation.Implements
import org.robolectric.shadows.ShadowPixelCopy

@Config(shadows = [ShadowPixelCopy::class], sdk = [30])
Expand Down Expand Up @@ -92,6 +99,7 @@ class PixelCopyStrategyTest {
fun setup() {
System.setProperty("robolectric.areWindowsMarkedVisible", "true")
System.setProperty("robolectric.pixelCopyRenderMode", "hardware")
DeferredWindowPixelCopyShadow.reset()
}

@Test
Expand Down Expand Up @@ -132,6 +140,68 @@ class PixelCopyStrategyTest {
if (failure.get() != null) throw failure.get()
}

@Test
@Config(shadows = [DeferredWindowPixelCopyShadow::class])
fun `capture skips the first unstable PixelCopy result`() {
val activity = buildActivity(SimpleActivity::class.java).setup()
shadowOf(Looper.getMainLooper()).idle()
val root = activity.get().findViewById<View>(android.R.id.content)

val strategy = fixture.getSut(executor = fixture.inlineExecutor())
captureUnstableFrame(strategy, root)

assertFalse(strategy.lastCaptureSuccessful())
verify(fixture.callback, never()).onScreenshotRecorded(any<Bitmap>())
}

@Test
@Config(shadows = [DeferredWindowPixelCopyShadow::class])
fun `capture emits the second consecutive unstable PixelCopy result`() {
val activity = buildActivity(SimpleActivity::class.java).setup()
shadowOf(Looper.getMainLooper()).idle()
val root = activity.get().findViewById<View>(android.R.id.content)

val strategy = fixture.getSut(executor = fixture.inlineExecutor())
captureUnstableFrame(strategy, root)
captureUnstableFrame(strategy, root)

assertTrue(strategy.lastCaptureSuccessful())
verify(fixture.callback).onScreenshotRecorded(any<Bitmap>())
}

@Test
@Config(shadows = [DeferredWindowPixelCopyShadow::class])
fun `capture keeps emitting after entering continuous instability mode`() {
val activity = buildActivity(SimpleActivity::class.java).setup()
shadowOf(Looper.getMainLooper()).idle()
val root = activity.get().findViewById<View>(android.R.id.content)

val strategy = fixture.getSut(executor = fixture.inlineExecutor())
captureUnstableFrame(strategy, root)
captureUnstableFrame(strategy, root)
captureUnstableFrame(strategy, root)

assertTrue(strategy.lastCaptureSuccessful())
verify(fixture.callback, times(2)).onScreenshotRecorded(any<Bitmap>())
}

@Test
@Config(shadows = [DeferredWindowPixelCopyShadow::class])
fun `stable capture resets the unstable PixelCopy counter`() {
val activity = buildActivity(SimpleActivity::class.java).setup()
shadowOf(Looper.getMainLooper()).idle()
val root = activity.get().findViewById<View>(android.R.id.content)

val strategy = fixture.getSut(executor = fixture.inlineExecutor())
captureUnstableFrame(strategy, root)
captureUnstableFrame(strategy, root)
captureStableFrame(strategy, root)
captureUnstableFrame(strategy, root)

assertFalse(strategy.lastCaptureSuccessful())
verify(fixture.callback, times(2)).onScreenshotRecorded(any<Bitmap>())
}

@Test
fun `capture does not call markContentChanged when option is disabled`() {
val activity = buildActivity(ActivityWithSurfaceView::class.java).setup()
Expand Down Expand Up @@ -250,6 +320,50 @@ class PixelCopyStrategyTest {
assertEquals(0, dest.getPixel(4, 4))
assertEquals(0, dest.getPixel(25, 25))
}

private fun captureUnstableFrame(strategy: PixelCopyStrategy, root: View) {
strategy.capture(root)
strategy.onContentChanged()
DeferredWindowPixelCopyShadow.flush()
shadowOf(Looper.getMainLooper()).idle()
}

private fun captureStableFrame(strategy: PixelCopyStrategy, root: View) {
strategy.capture(root)
DeferredWindowPixelCopyShadow.flush()
shadowOf(Looper.getMainLooper()).idle()
}
}

@Implements(PixelCopy::class)
class DeferredWindowPixelCopyShadow {
companion object {
private val pendingCallbacks = mutableListOf<() -> Unit>()

fun reset() {
pendingCallbacks.clear()
}

fun flush() {
val callbacks = pendingCallbacks.toList()
pendingCallbacks.clear()
callbacks.forEach { it.invoke() }
}

@JvmStatic
@Implementation
@Suppress("UNUSED_PARAMETER")
fun request(
_source: Window,
_dest: Bitmap,
listener: PixelCopy.OnPixelCopyFinishedListener,
listenerThread: Handler,
) {
pendingCallbacks.add {
listenerThread.post { listener.onPixelCopyFinished(PixelCopy.SUCCESS) }
}
}
}
}

private class SimpleActivity : Activity() {
Expand Down
1 change: 1 addition & 0 deletions sentry-samples/sentry-samples-android/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,7 @@ dependencies {
implementation(libs.androidx.browser)
implementation(libs.coil.compose)
implementation(libs.kotlinx.coroutines.android)
implementation(libs.lottie.compose)
implementation(libs.retrofit)
implementation(libs.retrofit.gson)
implementation(libs.sentry.native.ndk)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,10 @@
android:name=".PermissionsActivity"
android:exported="false" />

<activity
android:name=".ReplayAnimationsActivity"
android:exported="false" />

<activity
android:name=".ProfilingActivity"
android:exported="false" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -498,6 +498,18 @@ fun SessionReplayScreen() {
}
}
}
item {
SentryTraced("open_replay_animations") {
OutlinedButton(
onClick = {
activity.startActivity(Intent(activity, ReplayAnimationsActivity::class.java))
},
modifier = Modifier,
) {
Text("Open Animations", maxLines = 2, overflow = TextOverflow.Ellipsis)
}
}
}
item {
SentryTraced("show_dialog") {
OutlinedButton(onClick = { showDialog = true }, modifier = Modifier) {
Expand Down
Loading
Loading