Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SR] Fix Session Replay crashes #3628

Merged
merged 23 commits into from
Aug 12, 2024
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
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
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 @@ -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 @@ -34,6 +34,7 @@ import android.graphics.Bitmap
import android.media.MediaCodec
import android.media.MediaCodecInfo
import android.media.MediaFormat
import android.os.Build
import android.view.Surface
import io.sentry.SentryLevel.DEBUG
import io.sentry.SentryOptions
Expand Down Expand Up @@ -139,10 +140,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
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
Loading