diff --git a/CHANGELOG.md b/CHANGELOG.md index 972c1e20522..ca7c8eb4664 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,8 @@ - Session Replay: Fix `NoSuchElementException` in `BufferCaptureStrategy` ([#4717](https://github.com/getsentry/sentry-java/pull/4717)) - Session Replay: Fix continue recording in Session mode after Buffer is triggered ([#4719](https://github.com/getsentry/sentry-java/pull/4719)) +- Session Replay: Do not use recycled screenshots for masking ([#4790](https://github.com/getsentry/sentry-java/pull/4790)) + - This fixes native crashes seen in `Canvas.`/`ScreenshotRecorder.capture` ### Dependencies diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/ReplayIntegration.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/ReplayIntegration.kt index e6cce23aaa1..ea07d0a4292 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/ReplayIntegration.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/ReplayIntegration.kt @@ -31,8 +31,8 @@ import io.sentry.android.replay.capture.SessionCaptureStrategy import io.sentry.android.replay.gestures.GestureRecorder import io.sentry.android.replay.gestures.TouchRecorderCallback import io.sentry.android.replay.util.MainLooperHandler +import io.sentry.android.replay.util.ReplayExecutorService import io.sentry.android.replay.util.appContext -import io.sentry.android.replay.util.gracefullyShutdown import io.sentry.android.replay.util.sample import io.sentry.android.replay.util.submitSafely import io.sentry.cache.PersistingScopeObserver.BREADCRUMBS_FILENAME @@ -103,7 +103,8 @@ public class ReplayIntegration( private val random by lazy { Random() } internal val rootViewsSpy by lazy { RootViewsSpy.install() } private val replayExecutor by lazy { - Executors.newSingleThreadScheduledExecutor(ReplayExecutorServiceThreadFactory()) + val delegate = Executors.newSingleThreadScheduledExecutor(ReplayExecutorServiceThreadFactory()) + ReplayExecutorService(delegate, options) } internal val isEnabled = AtomicBoolean(false) @@ -330,7 +331,7 @@ public class ReplayIntegration( recorder?.close() recorder = null rootViewsSpy.close() - replayExecutor.gracefullyShutdown(options) + replayExecutor.shutdown() lifecycle.currentState = CLOSED } } diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/BaseCaptureStrategy.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/BaseCaptureStrategy.kt index 7e76f92aa7e..6601af1864e 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/BaseCaptureStrategy.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/BaseCaptureStrategy.kt @@ -24,7 +24,8 @@ import io.sentry.android.replay.ScreenshotRecorderConfig import io.sentry.android.replay.capture.CaptureStrategy.Companion.createSegment import io.sentry.android.replay.capture.CaptureStrategy.ReplaySegment import io.sentry.android.replay.gestures.ReplayGestureConverter -import io.sentry.android.replay.util.submitSafely +import io.sentry.android.replay.util.ReplayExecutorService +import io.sentry.android.replay.util.ReplayRunnable import io.sentry.protocol.SentryId import io.sentry.rrweb.RRWebEvent import io.sentry.transport.ICurrentDateProvider @@ -54,7 +55,9 @@ internal abstract class BaseCaptureStrategy( } private val persistingExecutor: ScheduledExecutorService by lazy { - Executors.newSingleThreadScheduledExecutor(ReplayPersistingExecutorServiceThreadFactory()) + val delegate = + Executors.newSingleThreadScheduledExecutor(ReplayPersistingExecutorServiceThreadFactory()) + ReplayExecutorService(delegate, options) } private val gestureConverter = ReplayGestureConverter(dateProvider) @@ -185,7 +188,7 @@ internal abstract class BaseCaptureStrategy( private fun runInBackground(task: () -> Unit) { if (options.threadChecker.isMainThread) { - persistingExecutor.submitSafely(options, "$TAG.runInBackground") { task() } + persistingExecutor.submit(ReplayRunnable("$TAG.runInBackground") { task() }) } else { try { task() diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/BufferCaptureStrategy.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/BufferCaptureStrategy.kt index 706a958f3f8..53394b586ea 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/BufferCaptureStrategy.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/BufferCaptureStrategy.kt @@ -14,8 +14,8 @@ import io.sentry.android.replay.ReplayCache import io.sentry.android.replay.ScreenshotRecorderConfig import io.sentry.android.replay.capture.CaptureStrategy.Companion.rotateEvents import io.sentry.android.replay.capture.CaptureStrategy.ReplaySegment +import io.sentry.android.replay.util.ReplayRunnable import io.sentry.android.replay.util.sample -import io.sentry.android.replay.util.submitSafely import io.sentry.protocol.SentryId import io.sentry.transport.ICurrentDateProvider import io.sentry.util.FileUtils @@ -62,10 +62,12 @@ internal class BufferCaptureStrategy( override fun stop() { val replayCacheDir = cache?.replayCacheDir - replayExecutor.submitSafely(options, "$TAG.stop") { - FileUtils.deleteRecursively(replayCacheDir) - currentSegment = -1 - } + replayExecutor.submit( + ReplayRunnable("$TAG.stop") { + FileUtils.deleteRecursively(replayCacheDir) + currentSegment = -1 + } + ) super.stop() } @@ -115,14 +117,16 @@ internal class BufferCaptureStrategy( // have to do it before submitting, otherwise if the queue is busy, the timestamp won't be // reflecting the exact time of when it was captured val frameTimestamp = dateProvider.currentTimeMillis - replayExecutor.submitSafely(options, "$TAG.add_frame") { - cache?.store(frameTimestamp) - - val now = dateProvider.currentTimeMillis - val bufferLimit = now - options.sessionReplay.errorReplayDuration - screenAtStart = cache?.rotate(bufferLimit) - bufferedSegments.rotate(bufferLimit) - } + replayExecutor.submit( + ReplayRunnable("$TAG.add_frame") { + cache?.store(frameTimestamp) + + val now = dateProvider.currentTimeMillis + val bufferLimit = now - options.sessionReplay.errorReplayDuration + screenAtStart = cache?.rotate(bufferLimit) + bufferedSegments.rotate(bufferLimit) + } + ) } override fun onConfigurationChanged(recorderConfig: ScreenshotRecorderConfig) { @@ -225,19 +229,21 @@ internal class BufferCaptureStrategy( val duration = now - currentSegmentTimestamp.time val replayId = currentReplayId - replayExecutor.submitSafely(options, "$TAG.$taskName") { - val segment = - createSegmentInternal( - duration, - currentSegmentTimestamp, - replayId, - currentSegment, - currentConfig.recordingHeight, - currentConfig.recordingWidth, - currentConfig.frameRate, - currentConfig.bitRate, - ) - onSegmentCreated(segment) - } + replayExecutor.submit( + ReplayRunnable("$TAG.$taskName") { + val segment = + createSegmentInternal( + duration, + currentSegmentTimestamp, + replayId, + currentSegment, + currentConfig.recordingHeight, + currentConfig.recordingWidth, + currentConfig.frameRate, + currentConfig.bitRate, + ) + onSegmentCreated(segment) + } + ) } } diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/SessionCaptureStrategy.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/SessionCaptureStrategy.kt index cc007d07067..4d3ee588f01 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/SessionCaptureStrategy.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/SessionCaptureStrategy.kt @@ -9,7 +9,7 @@ import io.sentry.SentryReplayEvent.ReplayType import io.sentry.android.replay.ReplayCache import io.sentry.android.replay.ScreenshotRecorderConfig import io.sentry.android.replay.capture.CaptureStrategy.ReplaySegment -import io.sentry.android.replay.util.submitSafely +import io.sentry.android.replay.util.ReplayRunnable import io.sentry.protocol.SentryId import io.sentry.transport.ICurrentDateProvider import io.sentry.util.FileUtils @@ -79,55 +79,57 @@ internal class SessionCaptureStrategy( // reflecting the exact time of when it was captured val currentConfig = recorderConfig val frameTimestamp = dateProvider.currentTimeMillis - replayExecutor.submitSafely(options, "$TAG.add_frame") { - cache?.store(frameTimestamp) - - val currentSegmentTimestamp = segmentTimestamp - currentSegmentTimestamp - ?: run { - options.logger.log(DEBUG, "Segment timestamp is not set, not recording frame") - return@submitSafely + replayExecutor.submit( + ReplayRunnable("$TAG.add_frame") { + cache?.store(frameTimestamp) + + val currentSegmentTimestamp = segmentTimestamp + currentSegmentTimestamp + ?: run { + options.logger.log(DEBUG, "Segment timestamp is not set, not recording frame") + return@ReplayRunnable + } + + if (isTerminating.get()) { + options.logger.log( + DEBUG, + "Not capturing segment, because the app is terminating, will be captured on next launch", + ) + return@ReplayRunnable } - if (isTerminating.get()) { - options.logger.log( - DEBUG, - "Not capturing segment, because the app is terminating, will be captured on next launch", - ) - return@submitSafely - } - - if (currentConfig == null) { - options.logger.log(DEBUG, "Recorder config is not set, not capturing a segment") - return@submitSafely - } + if (currentConfig == null) { + options.logger.log(DEBUG, "Recorder config is not set, not capturing a segment") + return@ReplayRunnable + } - val now = dateProvider.currentTimeMillis - if ((now - currentSegmentTimestamp.time >= options.sessionReplay.sessionSegmentDuration)) { - val segment = - createSegmentInternal( - options.sessionReplay.sessionSegmentDuration, - currentSegmentTimestamp, - currentReplayId, - currentSegment, - currentConfig.recordingHeight, - currentConfig.recordingWidth, - currentConfig.frameRate, - currentConfig.bitRate, - ) - if (segment is ReplaySegment.Created) { - segment.capture(scopes) - currentSegment++ - // set next segment timestamp as close to the previous one as possible to avoid gaps - segmentTimestamp = segment.replay.timestamp + val now = dateProvider.currentTimeMillis + if ((now - currentSegmentTimestamp.time >= options.sessionReplay.sessionSegmentDuration)) { + val segment = + createSegmentInternal( + options.sessionReplay.sessionSegmentDuration, + currentSegmentTimestamp, + currentReplayId, + currentSegment, + currentConfig.recordingHeight, + currentConfig.recordingWidth, + currentConfig.frameRate, + currentConfig.bitRate, + ) + if (segment is ReplaySegment.Created) { + segment.capture(scopes) + currentSegment++ + // set next segment timestamp as close to the previous one as possible to avoid gaps + segmentTimestamp = segment.replay.timestamp + } } - } - if ((now - replayStartTimestamp.get() >= options.sessionReplay.sessionDuration)) { - options.replayController.stop() - options.logger.log(INFO, "Session replay deadline exceeded (1h), stopping recording") + if ((now - replayStartTimestamp.get() >= options.sessionReplay.sessionDuration)) { + options.replayController.stop() + options.logger.log(INFO, "Session replay deadline exceeded (1h), stopping recording") + } } - } + ) } override fun onConfigurationChanged(recorderConfig: ScreenshotRecorderConfig) { @@ -161,19 +163,21 @@ internal class SessionCaptureStrategy( val currentSegmentTimestamp = segmentTimestamp ?: return val duration = now - currentSegmentTimestamp.time val replayId = currentReplayId - replayExecutor.submitSafely(options, "$TAG.$taskName") { - val segment = - createSegmentInternal( - duration, - currentSegmentTimestamp, - replayId, - currentSegment, - currentConfig.recordingHeight, - currentConfig.recordingWidth, - currentConfig.frameRate, - currentConfig.bitRate, - ) - onSegmentCreated(segment) - } + replayExecutor.submit( + ReplayRunnable("$TAG.$taskName") { + val segment = + createSegmentInternal( + duration, + currentSegmentTimestamp, + replayId, + currentSegment, + currentConfig.recordingHeight, + currentConfig.recordingWidth, + currentConfig.frameRate, + currentConfig.bitRate, + ) + onSegmentCreated(segment) + } + ) } } diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/screenshot/CanvasStrategy.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/screenshot/CanvasStrategy.kt index 57981b4382c..e9cb94ecb7c 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/screenshot/CanvasStrategy.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/screenshot/CanvasStrategy.kt @@ -31,7 +31,7 @@ import io.sentry.SentryLevel import io.sentry.SentryOptions import io.sentry.android.replay.ScreenshotRecorderCallback import io.sentry.android.replay.ScreenshotRecorderConfig -import io.sentry.android.replay.util.submitSafely +import io.sentry.android.replay.util.ReplayRunnable import io.sentry.util.IntegrationUtils import java.util.LinkedList import java.util.WeakHashMap @@ -124,7 +124,7 @@ internal class CanvasStrategy( } if (holder == null) { options.logger.log(SentryLevel.DEBUG, "No free Picture available, skipping capture") - executor.submitSafely(options, "screenshot_recorder.canvas", pictureRenderTask) + executor.submit(ReplayRunnable("screenshot_recorder.canvas", pictureRenderTask)) return } @@ -136,7 +136,7 @@ internal class CanvasStrategy( synchronized(unprocessedPictures) { unprocessedPictures.add(holder) } - executor.submitSafely(options, "screenshot_recorder.canvas", pictureRenderTask) + executor.submit(ReplayRunnable("screenshot_recorder.canvas", pictureRenderTask)) } override fun onContentChanged() { diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/screenshot/PixelCopyStrategy.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/screenshot/PixelCopyStrategy.kt index 82341405734..c47e76b17ae 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/screenshot/PixelCopyStrategy.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/screenshot/PixelCopyStrategy.kt @@ -19,8 +19,8 @@ import io.sentry.android.replay.ScreenshotRecorderConfig import io.sentry.android.replay.phoneWindow import io.sentry.android.replay.util.DebugOverlayDrawable import io.sentry.android.replay.util.MainLooperHandler +import io.sentry.android.replay.util.ReplayRunnable import io.sentry.android.replay.util.getVisibleRects -import io.sentry.android.replay.util.submitSafely import io.sentry.android.replay.util.traverse import io.sentry.android.replay.viewhierarchy.ViewHierarchyNode import io.sentry.android.replay.viewhierarchy.ViewHierarchyNode.ImageViewHierarchyNode @@ -48,11 +48,17 @@ internal class PixelCopyStrategy( private val lastCaptureSuccessful = AtomicBoolean(false) private val maskingPaint by lazy(NONE) { Paint() } private val contentChanged = AtomicBoolean(false) + private val isClosed = AtomicBoolean(false) @SuppressLint("NewApi") override fun capture(root: View) { contentChanged.set(false) + if (isClosed.get()) { + options.logger.log(DEBUG, "PixelCopyStrategy is closed, not capturing screenshot") + return + } + val window = root.phoneWindow if (window == null) { options.logger.log(DEBUG, "Window is invalid, not capturing screenshot") @@ -61,12 +67,22 @@ internal class PixelCopyStrategy( // postAtFrontOfQueue to ensure the view hierarchy and bitmap are ase close in-sync as possible mainLooperHandler.post { + if (isClosed.get()) { + options.logger.log(DEBUG, "PixelCopyStrategy is closed, not capturing screenshot") + return@post + } + try { contentChanged.set(false) PixelCopy.request( window, screenshot, { copyResult: Int -> + if (isClosed.get()) { + options.logger.log(DEBUG, "PixelCopyStrategy is closed, ignoring capture result") + return@request + } + if (copyResult != PixelCopy.SUCCESS) { options.logger.log(INFO, "Failed to capture replay recording: %d", copyResult) lastCaptureSuccessful.set(false) @@ -85,66 +101,73 @@ internal class PixelCopyStrategy( val viewHierarchy = ViewHierarchyNode.fromView(root, null, 0, options) root.traverse(viewHierarchy, options) - executor.submitSafely(options, "screenshot_recorder.mask") { - val debugMasks = mutableListOf() - - val canvas = Canvas(screenshot) - canvas.setMatrix(prescaledMatrix) - viewHierarchy.traverse { node -> - if (node.shouldMask && (node.width > 0 && node.height > 0)) { - node.visibleRect ?: return@traverse false - - // TODO: investigate why it returns true on RN when it shouldn't - // if (viewHierarchy.isObscured(node)) { - // return@traverse true - // } - - val (visibleRects, color) = - when (node) { - is ImageViewHierarchyNode -> { - listOf(node.visibleRect) to - screenshot.dominantColorForRect(node.visibleRect) - } + executor.submit( + ReplayRunnable("screenshot_recorder.mask") { + if (isClosed.get() || screenshot.isRecycled) { + options.logger.log(DEBUG, "PixelCopyStrategy is closed, skipping masking") + return@ReplayRunnable + } - is TextViewHierarchyNode -> { - val textColor = - node.layout?.dominantTextColor ?: node.dominantColor ?: Color.BLACK - node.layout.getVisibleRects( - node.visibleRect, - node.paddingLeft, - node.paddingTop, - ) to textColor + val debugMasks = mutableListOf() + + val canvas = Canvas(screenshot) + canvas.setMatrix(prescaledMatrix) + viewHierarchy.traverse { node -> + if (node.shouldMask && (node.width > 0 && node.height > 0)) { + node.visibleRect ?: return@traverse false + + // TODO: investigate why it returns true on RN when it shouldn't + // if (viewHierarchy.isObscured(node)) { + // return@traverse true + // } + + val (visibleRects, color) = + when (node) { + is ImageViewHierarchyNode -> { + listOf(node.visibleRect) to + screenshot.dominantColorForRect(node.visibleRect) + } + + is TextViewHierarchyNode -> { + val textColor = + node.layout?.dominantTextColor ?: node.dominantColor ?: Color.BLACK + node.layout.getVisibleRects( + node.visibleRect, + node.paddingLeft, + node.paddingTop, + ) to textColor + } + + else -> { + listOf(node.visibleRect) to Color.BLACK + } } - else -> { - listOf(node.visibleRect) to Color.BLACK - } + maskingPaint.setColor(color) + visibleRects.forEach { rect -> + canvas.drawRoundRect(RectF(rect), 10f, 10f, maskingPaint) + } + if (options.replayController.isDebugMaskingOverlayEnabled()) { + debugMasks.addAll(visibleRects) } - - maskingPaint.setColor(color) - visibleRects.forEach { rect -> - canvas.drawRoundRect(RectF(rect), 10f, 10f, maskingPaint) - } - if (options.replayController.isDebugMaskingOverlayEnabled()) { - debugMasks.addAll(visibleRects) } + return@traverse true } - return@traverse true - } - if (options.replayController.isDebugMaskingOverlayEnabled()) { - mainLooperHandler.post { - if (debugOverlayDrawable.callback == null) { - root.overlay.add(debugOverlayDrawable) + if (options.replayController.isDebugMaskingOverlayEnabled()) { + mainLooperHandler.post { + if (debugOverlayDrawable.callback == null) { + root.overlay.add(debugOverlayDrawable) + } + debugOverlayDrawable.updateMasks(debugMasks) + root.postInvalidate() } - debugOverlayDrawable.updateMasks(debugMasks) - root.postInvalidate() } + screenshotRecorderCallback?.onScreenshotRecorded(screenshot) + lastCaptureSuccessful.set(true) + contentChanged.set(false) } - screenshotRecorderCallback?.onScreenshotRecorded(screenshot) - lastCaptureSuccessful.set(true) - contentChanged.set(false) - } + ) }, mainLooperHandler.handler, ) @@ -170,12 +193,20 @@ internal class PixelCopyStrategy( } override fun close() { + isClosed.set(true) if (!screenshot.isRecycled) { screenshot.recycle() } + if (!singlePixelBitmap.isRecycled) { + singlePixelBitmap.recycle() + } } private fun Bitmap.dominantColorForRect(rect: Rect): Int { + if (isClosed.get() || this.isRecycled || singlePixelBitmap.isRecycled) { + return Color.BLACK + } + // TODO: maybe this ceremony can be just simplified to // TODO: multiplying the visibleRect by the prescaledMatrix val visibleRect = Rect(rect) diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/util/Executors.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/util/Executors.kt index 504280eb2aa..a5dddc3bd4d 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/util/Executors.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/util/Executors.kt @@ -1,31 +1,9 @@ package io.sentry.android.replay.util -import android.annotation.SuppressLint import io.sentry.ISentryExecutorService import io.sentry.SentryLevel.ERROR import io.sentry.SentryOptions -import java.util.concurrent.ExecutorService import java.util.concurrent.Future -import java.util.concurrent.ScheduledExecutorService -import java.util.concurrent.ScheduledFuture -import java.util.concurrent.TimeUnit -import java.util.concurrent.TimeUnit.MILLISECONDS - -internal fun ExecutorService.gracefullyShutdown(options: SentryOptions) { - synchronized(this) { - if (!isShutdown) { - shutdown() - } - try { - if (!awaitTermination(options.shutdownTimeoutMillis, MILLISECONDS)) { - shutdownNow() - } - } catch (e: InterruptedException) { - shutdownNow() - Thread.currentThread().interrupt() - } - } -} internal fun ISentryExecutorService.submitSafely( options: SentryOptions, @@ -44,54 +22,3 @@ internal fun ISentryExecutorService.submitSafely( options.logger.log(ERROR, "Failed to submit task $taskName to executor", e) null } - -internal fun ExecutorService.submitSafely( - options: SentryOptions, - taskName: String, - task: Runnable, -): Future<*>? { - if (Thread.currentThread().name.startsWith("SentryReplayIntegration")) { - // we're already on the worker thread, no need to submit - task.run() - return null - } - return try { - submit { - try { - task.run() - } catch (e: Throwable) { - options.logger.log(ERROR, "Failed to execute task $taskName", e) - } - } - } catch (e: Throwable) { - options.logger.log(ERROR, "Failed to submit task $taskName to executor", e) - null - } -} - -@SuppressLint("DiscouragedApi") -internal fun ScheduledExecutorService.scheduleAtFixedRateSafely( - options: SentryOptions, - taskName: String, - initialDelay: Long, - period: Long, - unit: TimeUnit, - task: Runnable, -): ScheduledFuture<*>? = - try { - scheduleAtFixedRate( - { - try { - task.run() - } catch (e: Throwable) { - options.logger.log(ERROR, "Failed to execute task $taskName", e) - } - }, - initialDelay, - period, - unit, - ) - } catch (e: Throwable) { - options.logger.log(ERROR, "Failed to submit task $taskName to executor", e) - null - } diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/util/ReplayExecutorService.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/util/ReplayExecutorService.kt new file mode 100644 index 00000000000..31a3279d074 --- /dev/null +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/util/ReplayExecutorService.kt @@ -0,0 +1,62 @@ +package io.sentry.android.replay.util + +import io.sentry.SentryLevel.ERROR +import io.sentry.SentryOptions +import java.util.concurrent.Future +import java.util.concurrent.ScheduledExecutorService +import java.util.concurrent.TimeUnit.MILLISECONDS + +/** + * An ExecutorService which is safe in terms of submitting tasks - it won't crash but will swallow + * and log them. + */ +internal class ReplayExecutorService( + private val delegate: ScheduledExecutorService, + private val options: SentryOptions, +) : ScheduledExecutorService by delegate { + override fun submit(task: Runnable): Future<*>? { + if (Thread.currentThread().name.startsWith("SentryReplayIntegration")) { + // we're already on the worker thread, no need to submit + task.run() + return null + } + return try { + delegate.submit { + try { + task.run() + } catch (e: Throwable) { + options.logger.log( + ERROR, + "Failed to execute task ${if (task is ReplayRunnable) task.taskName else ""}", + e, + ) + } + } + } catch (e: Throwable) { + options.logger.log( + ERROR, + "Failed to submit task ${if (task is ReplayRunnable) task.taskName else ""} to executor", + e, + ) + null + } + } + + override fun shutdown() { + synchronized(this) { + if (!isShutdown) { + delegate.shutdown() + } + try { + if (!awaitTermination(options.shutdownTimeoutMillis, MILLISECONDS)) { + shutdownNow() + } + } catch (e: InterruptedException) { + shutdownNow() + Thread.currentThread().interrupt() + } + } + } +} + +internal class ReplayRunnable(val taskName: String, delegate: Runnable) : Runnable by delegate diff --git a/sentry-android-replay/src/test/java/io/sentry/android/replay/screenshot/PixelCopyStrategyTest.kt b/sentry-android-replay/src/test/java/io/sentry/android/replay/screenshot/PixelCopyStrategyTest.kt new file mode 100644 index 00000000000..50b43bc1f64 --- /dev/null +++ b/sentry-android-replay/src/test/java/io/sentry/android/replay/screenshot/PixelCopyStrategyTest.kt @@ -0,0 +1,118 @@ +package io.sentry.android.replay.screenshot + +import android.app.Activity +import android.os.Bundle +import android.os.Looper +import android.widget.LinearLayout +import android.widget.LinearLayout.LayoutParams +import android.widget.TextView +import androidx.test.ext.junit.runners.AndroidJUnit4 +import io.sentry.SentryOptions +import io.sentry.android.replay.ScreenshotRecorderCallback +import io.sentry.android.replay.ScreenshotRecorderConfig +import io.sentry.android.replay.util.DebugOverlayDrawable +import io.sentry.android.replay.util.MainLooperHandler +import java.util.concurrent.ScheduledExecutorService +import java.util.concurrent.atomic.AtomicReference +import kotlin.test.BeforeTest +import kotlin.test.Test +import kotlin.test.assertFalse +import org.junit.runner.RunWith +import org.mockito.kotlin.any +import org.mockito.kotlin.doAnswer +import org.mockito.kotlin.mock +import org.mockito.kotlin.whenever +import org.robolectric.Robolectric.buildActivity +import org.robolectric.Shadows.shadowOf +import org.robolectric.annotation.Config +import org.robolectric.shadows.ShadowPixelCopy + +@Config(shadows = [ShadowPixelCopy::class], sdk = [30]) +@RunWith(AndroidJUnit4::class) +class PixelCopyStrategyTest { + + private class Fixture { + val options = SentryOptions() + val callback = mock() + val debugOverlayDrawable = mock() + val config = ScreenshotRecorderConfig(100, 100, 1f, 1f, 1, 1000) + + fun getSut(executor: ScheduledExecutorService = mock()): PixelCopyStrategy { + return PixelCopyStrategy( + executor, + MainLooperHandler(), + callback, + options, + config, + debugOverlayDrawable, + ) + } + } + + private val fixture = Fixture() + + @BeforeTest + fun setup() { + System.setProperty("robolectric.areWindowsMarkedVisible", "true") + System.setProperty("robolectric.pixelCopyRenderMode", "hardware") + } + + @Test + fun `when strategy is closed, lastCaptureSuccessful returns false`() { + val strategy = fixture.getSut() + + strategy.close() + + assertFalse(strategy.lastCaptureSuccessful()) + } + + @Test + fun `when close is called while executor task is running, does not crash with recycled bitmap`() { + val activity = buildActivity(SimpleActivity::class.java).setup() + shadowOf(Looper.getMainLooper()).idle() + + var strategy: PixelCopyStrategy? = null + + val failure = AtomicReference() + // Custom executor that closes the strategy before executing tasks + val executorThatClosesFirst = mock() + whenever(executorThatClosesFirst.submit(any())).doAnswer { + val task = it.getArgument(0) + strategy?.close() + try { + task.run() + } catch (e: Throwable) { + // PixelCopyStrategy swallows the exception, so we have to capture it here and rethrow later + failure.set(e) + } + null + } + + strategy = fixture.getSut(executor = executorThatClosesFirst) + strategy.capture(activity.get().findViewById(android.R.id.content)) + shadowOf(Looper.getMainLooper()).idle() + + if (failure.get() != null) throw failure.get() + } +} + +private class SimpleActivity : Activity() { + override fun onCreate(savedInstanceState: Bundle?) { + super.onCreate(savedInstanceState) + val linearLayout = + LinearLayout(this).apply { + setBackgroundColor(android.R.color.white) + orientation = LinearLayout.VERTICAL + layoutParams = LayoutParams(LayoutParams.MATCH_PARENT, LayoutParams.MATCH_PARENT) + } + + val textView = + TextView(this).apply { + text = "Hello, World!" + layoutParams = LayoutParams(LayoutParams.WRAP_CONTENT, LayoutParams.WRAP_CONTENT) + } + linearLayout.addView(textView) + + setContentView(linearLayout) + } +} diff --git a/sentry/src/test/java/io/sentry/SentryReplayOptionsTest.java b/sentry/src/test/java/io/sentry/SentryReplayOptionsTest.java deleted file mode 100644 index a0af1cddea9..00000000000 --- a/sentry/src/test/java/io/sentry/SentryReplayOptionsTest.java +++ /dev/null @@ -1,28 +0,0 @@ -package io.sentry; - -import static org.junit.Assert.assertEquals; - -import org.junit.Test; - -public final class SentryReplayOptionsTest { - - @Test - public void testDefaultScreenshotStrategy() { - SentryReplayOptions options = new SentryReplayOptions(false, null); - assertEquals(ScreenshotStrategyType.PIXEL_COPY, options.getScreenshotStrategy()); - } - - @Test - public void testSetScreenshotStrategyToCanvas() { - SentryReplayOptions options = new SentryReplayOptions(false, null); - options.setScreenshotStrategy(ScreenshotStrategyType.CANVAS); - assertEquals(ScreenshotStrategyType.CANVAS, options.getScreenshotStrategy()); - } - - @Test - public void testSetScreenshotStrategyToPixelCopy() { - SentryReplayOptions options = new SentryReplayOptions(false, null); - options.setScreenshotStrategy(ScreenshotStrategyType.PIXEL_COPY); - assertEquals(ScreenshotStrategyType.PIXEL_COPY, options.getScreenshotStrategy()); - } -} diff --git a/sentry/src/test/java/io/sentry/SentryReplayOptionsTest.kt b/sentry/src/test/java/io/sentry/SentryReplayOptionsTest.kt index f0e7b9c1fc1..e7d618c419c 100644 --- a/sentry/src/test/java/io/sentry/SentryReplayOptionsTest.kt +++ b/sentry/src/test/java/io/sentry/SentryReplayOptionsTest.kt @@ -34,4 +34,24 @@ class SentryReplayOptionsTest { assertEquals(100_000, replayOptions.quality.bitRate) assertEquals(1.0f, replayOptions.quality.sizeScale) } + + @Test + fun `default screenshot strategy is pixel copy`() { + val options = SentryReplayOptions(false, null) + assertEquals(ScreenshotStrategyType.PIXEL_COPY, options.screenshotStrategy) + } + + @Test + fun `can set screenshot strategy to canvas`() { + val options = SentryReplayOptions(false, null) + options.screenshotStrategy = ScreenshotStrategyType.CANVAS + assertEquals(ScreenshotStrategyType.CANVAS, options.screenshotStrategy) + } + + @Test + fun `can set screenshot strategy to pixel copy`() { + val options = SentryReplayOptions(false, null) + options.screenshotStrategy = ScreenshotStrategyType.PIXEL_COPY + assertEquals(ScreenshotStrategyType.PIXEL_COPY, options.screenshotStrategy) + } }