diff --git a/CHANGELOG.md b/CHANGELOG.md index 7b500b004f..f5576ba86d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,11 @@ - Align next segment timestamp with the end of the buffered segment when converting from buffer mode to session mode - Persist `buffer` replay type for the entire replay when converting from buffer mode to session mode - Properly store screen names for `buffer` mode +- Session Replay: fix various crashes and issues ([#3628](https://github.com/getsentry/sentry-java/pull/3628)) + - Fix video not being encoded on Pixel devices + - Fix SIGABRT native crashes on Xiaomi devices when encoding a video + - Fix `RejectedExecutionException` when redacting a screenshot + - Fix `FileNotFoundException` when persisting segment values ### Chores diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/ScreenshotRecorder.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/ScreenshotRecorder.kt index 9680c2a3ad..5ec142b3e5 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/ScreenshotRecorder.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/ScreenshotRecorder.kt @@ -26,6 +26,7 @@ import io.sentry.SentryReplayOptions import io.sentry.android.replay.util.MainLooperHandler import io.sentry.android.replay.util.getVisibleRects import io.sentry.android.replay.util.gracefullyShutdown +import io.sentry.android.replay.util.submitSafely import io.sentry.android.replay.viewhierarchy.ViewHierarchyNode import io.sentry.android.replay.viewhierarchy.ViewHierarchyNode.ImageViewHierarchyNode import io.sentry.android.replay.viewhierarchy.ViewHierarchyNode.TextViewHierarchyNode @@ -122,7 +123,7 @@ internal class ScreenshotRecorder( val viewHierarchy = ViewHierarchyNode.fromView(root, null, 0, options) root.traverse(viewHierarchy) - recorder.submit { + recorder.submitSafely(options, "screenshot_recorder.redact") { val canvas = Canvas(bitmap) canvas.setMatrix(prescaledMatrix) viewHierarchy.traverse { node -> 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 97cb386109..fcd2d11293 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 @@ -100,10 +100,10 @@ internal abstract class BaseCaptureStrategy( ) { cache = replayCacheProvider?.invoke(replayId, recorderConfig) ?: ReplayCache(options, replayId, recorderConfig) + this.currentReplayId = replayId + this.currentSegment = segmentId this.replayType = replayType ?: (if (this is SessionCaptureStrategy) SESSION else BUFFER) this.recorderConfig = recorderConfig - this.currentSegment = segmentId - this.currentReplayId = replayId segmentTimestamp = DateUtils.getCurrentDateTime() replayStartTimestamp.set(dateProvider.currentTimeMillis) 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 61a98b6914..7b416d18b7 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 @@ -124,7 +124,6 @@ internal class SessionCaptureStrategy( } override fun onConfigurationChanged(recorderConfig: ScreenshotRecorderConfig) { - val currentSegmentTimestamp = segmentTimestamp ?: return createCurrentSegment("onConfigurationChanged") { segment -> if (segment is ReplaySegment.Created) { segment.capture(hub) diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/util/Views.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/util/Views.kt index 58accf0b77..a44508eac6 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/util/Views.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/util/Views.kt @@ -14,6 +14,8 @@ import android.os.Build.VERSION import android.os.Build.VERSION_CODES import android.text.Layout import android.view.View +import android.widget.TextView +import java.lang.NullPointerException /** * Adapted copy of AccessibilityNodeInfo from https://cs.android.com/android/platform/superproject/+/master:frameworks/base/core/java/android/view/View.java;l=10718 @@ -26,7 +28,7 @@ internal fun View.isVisibleToUser(): Pair { } // An invisible predecessor or one with alpha zero means // that this view is not visible to the user. - var current: Any = this + var current: Any? = this while (current is View) { val view = current val transitionAlpha = if (VERSION.SDK_INT >= VERSION_CODES.Q) view.transitionAlpha else 1f @@ -53,7 +55,10 @@ internal fun Drawable?.isRedactable(): Boolean { // TODO: otherwise maybe check for the bitmap size and don't redact those that take a lot of height (e.g. a background of a whatsapp chat) return when (this) { is InsetDrawable, is ColorDrawable, is VectorDrawable, is GradientDrawable -> false - is BitmapDrawable -> !bitmap.isRecycled && bitmap.height > 10 && bitmap.width > 10 + is BitmapDrawable -> { + val bmp = bitmap ?: return false + return !bmp.isRecycled && bmp.height > 10 && bmp.width > 10 + } else -> true } } @@ -84,3 +89,15 @@ internal fun Layout?.getVisibleRects(globalRect: Rect, paddingLeft: Int, padding } return rects } + +/** + * [TextView.getVerticalOffset] which is used by [TextView.getTotalPaddingTop] may throw an NPE on + * some devices (Redmi), so we try-catch it specifically for an NPE and then fallback to + * [TextView.getExtendedPaddingTop] + */ +internal val TextView.totalPaddingTopSafe: Int + get() = try { + totalPaddingTop + } catch (e: NullPointerException) { + extendedPaddingTop + } diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/video/SimpleVideoEncoder.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/video/SimpleVideoEncoder.kt index fd770131d8..baf521a2e6 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/video/SimpleVideoEncoder.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/video/SimpleVideoEncoder.kt @@ -33,7 +33,9 @@ import android.annotation.TargetApi import android.graphics.Bitmap import android.media.MediaCodec import android.media.MediaCodecInfo +import android.media.MediaCodecList import android.media.MediaFormat +import android.os.Build import android.view.Surface import io.sentry.SentryLevel.DEBUG import io.sentry.SentryOptions @@ -50,8 +52,22 @@ internal class SimpleVideoEncoder( val onClose: (() -> Unit)? = null ) { + private val hasExynosCodec: Boolean by lazy(NONE) { + // MediaCodecList ctor will initialize an internal in-memory static cache of codecs, so this + // call is only expensive the first time + MediaCodecList(MediaCodecList.REGULAR_CODECS) + .codecInfos + .any { it.name.contains("c2.exynos") } + } + internal val mediaCodec: MediaCodec = run { - val codec = MediaCodec.createEncoderByType(muxerConfig.mimeType) + // c2.exynos.h264.encoder seems to have problems encoding the video (Pixel and Samsung devices) + // so we use the default encoder instead + val codec = if (hasExynosCodec) { + MediaCodec.createByCodecName("c2.android.avc.encoder") + } else { + MediaCodec.createEncoderByType(muxerConfig.mimeType) + } codec } @@ -139,10 +155,13 @@ internal class SimpleVideoEncoder( } fun encode(image: Bitmap) { - // NOTE do not use `lockCanvas` like what is done in bitmap2video - // This is because https://developer.android.com/reference/android/media/MediaCodec#createInputSurface() - // says that, "Surface.lockCanvas(android.graphics.Rect) may fail or produce unexpected results." - val canvas = surface?.lockHardwareCanvas() + // it seems that Xiaomi devices have problems with hardware canvas, so we have to use + // lockCanvas instead https://stackoverflow.com/a/73520742 + val canvas = if (Build.MANUFACTURER.contains("xiaomi", ignoreCase = true)) { + surface?.lockCanvas(null) + } else { + surface?.lockHardwareCanvas() + } canvas?.drawBitmap(image, 0f, 0f, null) surface?.unlockCanvasAndPost(canvas) drainCodec(false) diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/viewhierarchy/ViewHierarchyNode.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/viewhierarchy/ViewHierarchyNode.kt index 1a94b295f7..145cefff3d 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/viewhierarchy/ViewHierarchyNode.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/viewhierarchy/ViewHierarchyNode.kt @@ -9,6 +9,7 @@ import android.widget.TextView import io.sentry.SentryOptions import io.sentry.android.replay.util.isRedactable import io.sentry.android.replay.util.isVisibleToUser +import io.sentry.android.replay.util.totalPaddingTopSafe @TargetApi(26) sealed class ViewHierarchyNode( @@ -245,7 +246,7 @@ sealed class ViewHierarchyNode( layout = view.layout, dominantColor = view.currentTextColor.toOpaque(), paddingLeft = view.totalPaddingLeft, - paddingTop = view.totalPaddingTop, + paddingTop = view.totalPaddingTopSafe, x = view.x, y = view.y, width = view.width, diff --git a/sentry-android-replay/src/test/java/io/sentry/android/replay/capture/BufferCaptureStrategyTest.kt b/sentry-android-replay/src/test/java/io/sentry/android/replay/capture/BufferCaptureStrategyTest.kt index bbad02444d..1cd266ecb2 100644 --- a/sentry-android-replay/src/test/java/io/sentry/android/replay/capture/BufferCaptureStrategyTest.kt +++ b/sentry-android-replay/src/test/java/io/sentry/android/replay/capture/BufferCaptureStrategyTest.kt @@ -64,7 +64,7 @@ class BufferCaptureStrategyTest { (it.arguments[0] as ScopeCallback).run(scope) }.whenever(it).configureScope(any()) } - var persistedSegment = mutableMapOf() + var persistedSegment = LinkedHashMap() val replayCache = mock { on { frames }.thenReturn(mutableListOf(ReplayFrame(File("1720693523997.jpg"), 1720693523997))) on { persistSegmentValues(any(), anyOrNull()) }.then { @@ -293,4 +293,19 @@ class BufferCaptureStrategyTest { verify(fixture.hub).captureReplay(any(), any()) } + + @Test + fun `replayId should be set and serialized first`() { + val strategy = fixture.getSut() + val replayId = SentryId() + + strategy.start(fixture.recorderConfig, 0, replayId) + + assertEquals( + replayId.toString(), + fixture.persistedSegment.values.first(), + "The replayId must be set first, so when we clean up stale replays" + + "the current replay cache folder is not being deleted." + ) + } } diff --git a/sentry-android-replay/src/test/java/io/sentry/android/replay/capture/SessionCaptureStrategyTest.kt b/sentry-android-replay/src/test/java/io/sentry/android/replay/capture/SessionCaptureStrategyTest.kt index ac593f6c27..12eb10c3f4 100644 --- a/sentry-android-replay/src/test/java/io/sentry/android/replay/capture/SessionCaptureStrategyTest.kt +++ b/sentry-android-replay/src/test/java/io/sentry/android/replay/capture/SessionCaptureStrategyTest.kt @@ -70,7 +70,7 @@ class SessionCaptureStrategyTest { (it.arguments[0] as ScopeCallback).run(scope) }.whenever(it).configureScope(any()) } - var persistedSegment = mutableMapOf() + var persistedSegment = LinkedHashMap() val replayCache = mock { on { frames }.thenReturn(mutableListOf(ReplayFrame(File("1720693523997.jpg"), 1720693523997))) on { persistSegmentValues(any(), anyOrNull()) }.then { @@ -352,4 +352,19 @@ class SessionCaptureStrategyTest { } ) } + + @Test + fun `replayId should be set and serialized first`() { + val strategy = fixture.getSut() + val replayId = SentryId() + + strategy.start(fixture.recorderConfig, 0, replayId) + + assertEquals( + replayId.toString(), + fixture.persistedSegment.values.first(), + "The replayId must be set first, so when we clean up stale replays" + + "the current replay cache folder is not being deleted." + ) + } }