Skip to content

Commit

Permalink
Merge 10ead6f into 19d98e8
Browse files Browse the repository at this point in the history
  • Loading branch information
romtsn authored Aug 12, 2024
2 parents 19d98e8 + 10ead6f commit c8b47a8
Show file tree
Hide file tree
Showing 9 changed files with 86 additions and 14 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 ->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -26,7 +28,7 @@ internal fun View.isVisibleToUser(): Pair<Boolean, Rect?> {
}
// 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
Expand All @@ -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
}
}
Expand Down Expand Up @@ -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
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
}
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ class BufferCaptureStrategyTest {
(it.arguments[0] as ScopeCallback).run(scope)
}.whenever(it).configureScope(any())
}
var persistedSegment = mutableMapOf<String, String?>()
var persistedSegment = LinkedHashMap<String, String?>()
val replayCache = mock<ReplayCache> {
on { frames }.thenReturn(mutableListOf(ReplayFrame(File("1720693523997.jpg"), 1720693523997)))
on { persistSegmentValues(any(), anyOrNull()) }.then {
Expand Down Expand Up @@ -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."
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ class SessionCaptureStrategyTest {
(it.arguments[0] as ScopeCallback).run(scope)
}.whenever(it).configureScope(any())
}
var persistedSegment = mutableMapOf<String, String?>()
var persistedSegment = LinkedHashMap<String, String?>()
val replayCache = mock<ReplayCache> {
on { frames }.thenReturn(mutableListOf(ReplayFrame(File("1720693523997.jpg"), 1720693523997)))
on { persistSegmentValues(any(), anyOrNull()) }.then {
Expand Down Expand Up @@ -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."
)
}
}

0 comments on commit c8b47a8

Please sign in to comment.