Properly store screen name at start for buffer mode

sentry-android-replay/api/sentry-android-replay.api

@@ -36,12 +36,13 @@ public abstract interface class io/sentry/android/replay/Recorder : java/io/Clos
 public final class io/sentry/android/replay/ReplayCache : java/io/Closeable {
 	public static final field Companion Lio/sentry/android/replay/ReplayCache$Companion;
 	public fun <init> (Lio/sentry/SentryOptions;Lio/sentry/protocol/SentryId;Lio/sentry/android/replay/ScreenshotRecorderConfig;)V
-	public final fun addFrame (Ljava/io/File;J)V
+	public final fun addFrame (Ljava/io/File;JLjava/lang/String;)V
+	public static synthetic fun addFrame$default (Lio/sentry/android/replay/ReplayCache;Ljava/io/File;JLjava/lang/String;ILjava/lang/Object;)V
 	public fun close ()V
 	public final fun createVideoOf (JJIIILjava/io/File;)Lio/sentry/android/replay/GeneratedVideo;
 	public static synthetic fun createVideoOf$default (Lio/sentry/android/replay/ReplayCache;JJIIILjava/io/File;ILjava/lang/Object;)Lio/sentry/android/replay/GeneratedVideo;
 	public final fun persistSegmentValues (Ljava/lang/String;Ljava/lang/String;)V
-	public final fun rotate (J)V
+	public final fun rotate (J)Ljava/lang/String;
 }
 
 public final class io/sentry/android/replay/ReplayCache$Companion {

sentry-android-replay/src/main/java/io/sentry/android/replay/ReplayCache.kt

@@ -76,7 +76,7 @@ public class ReplayCache(
      * @param bitmap the frame screenshot
      * @param frameTimestamp the timestamp when the frame screenshot was taken
      */
-    internal fun addFrame(bitmap: Bitmap, frameTimestamp: Long) {
+    internal fun addFrame(bitmap: Bitmap, frameTimestamp: Long, screen: String? = null) {
         if (replayCacheDir == null || bitmap.isRecycled) {
             return
         }
@@ -89,7 +89,7 @@ public class ReplayCache(
             it.flush()
         }
 
-        addFrame(screenshot, frameTimestamp)
+        addFrame(screenshot, frameTimestamp, screen)
     }
 
     /**
@@ -101,8 +101,8 @@ public class ReplayCache(
      * @param screenshot file containing the frame screenshot
      * @param frameTimestamp the timestamp when the frame screenshot was taken
      */
-    public fun addFrame(screenshot: File, frameTimestamp: Long) {
-        val frame = ReplayFrame(screenshot, frameTimestamp)
+    public fun addFrame(screenshot: File, frameTimestamp: Long, screen: String? = null) {
+        val frame = ReplayFrame(screenshot, frameTimestamp, screen)
         frames += frame
     }
 
@@ -233,15 +233,20 @@ public class ReplayCache(
      * Removes frames from the in-memory and disk cache from start to [until].
      *
      * @param until value until whose the frames should be removed, represented as unix timestamp
+     * @return the first screen in the rotated buffer, if any
      */
-    fun rotate(until: Long) {
+    fun rotate(until: Long): String? {
+        var screen: String? = null
         frames.removeAll {
             if (it.timestamp < until) {
                 deleteFile(it.screenshot)
                 return@removeAll true
+            } else if (screen == null) {
+                screen = it.screen
             }
             return@removeAll false
         }
+        return screen
     }
 
     override fun close() {
@@ -426,7 +431,8 @@ internal data class LastSegmentData(
 
 internal data class ReplayFrame(
     val screenshot: File,
-    val timestamp: Long
+    val timestamp: Long,
+    val screen: String? = null
 )
 
 public data class GeneratedVideo(

sentry-android-replay/src/main/java/io/sentry/android/replay/ReplayIntegration.kt

@@ -12,7 +12,6 @@ import io.sentry.Integration
 import io.sentry.NoOpReplayBreadcrumbConverter
 import io.sentry.ReplayBreadcrumbConverter
 import io.sentry.ReplayController
-import io.sentry.ScopeObserverAdapter
 import io.sentry.SentryIntegrationPackageStorage
 import io.sentry.SentryLevel.DEBUG
 import io.sentry.SentryLevel.INFO
@@ -28,7 +27,6 @@ import io.sentry.cache.PersistingScopeObserver
 import io.sentry.cache.PersistingScopeObserver.BREADCRUMBS_FILENAME
 import io.sentry.cache.PersistingScopeObserver.REPLAY_FILENAME
 import io.sentry.hints.Backfillable
-import io.sentry.protocol.Contexts
 import io.sentry.protocol.SentryId
 import io.sentry.transport.ICurrentDateProvider
 import io.sentry.util.FileUtils
@@ -102,12 +100,6 @@ public class ReplayIntegration(
         }
 
         this.hub = hub
-        this.options.addScopeObserver(object : ScopeObserverAdapter() {
-            override fun setContexts(contexts: Contexts) {
-                // scope screen has fully-qualified name
-                captureStrategy?.onScreenChanged(contexts.app?.viewNames?.lastOrNull()?.substringAfterLast('.'))
-            }
-        })
         recorder = recorderProvider?.invoke() ?: WindowRecorder(options, this, this, mainLooperHandler)
         isEnabled.set(true)
 
@@ -213,8 +205,10 @@ public class ReplayIntegration(
     }
 
     override fun onScreenshotRecorded(bitmap: Bitmap) {
+        var screen: String? = null
+        hub?.configureScope { screen = it.screen?.substringAfterLast('.') }
         captureStrategy?.onScreenshotRecorded(bitmap) { frameTimeStamp ->
-            addFrame(bitmap, frameTimeStamp)
+            addFrame(bitmap, frameTimeStamp, screen)
         }
     }
 

sentry-android-replay/src/main/java/io/sentry/android/replay/capture/BufferCaptureStrategy.kt

@@ -8,7 +8,6 @@ import io.sentry.SentryLevel.DEBUG
 import io.sentry.SentryLevel.ERROR
 import io.sentry.SentryLevel.INFO
 import io.sentry.SentryOptions
-import io.sentry.SentryReplayEvent.ReplayType
 import io.sentry.SentryReplayEvent.ReplayType.BUFFER
 import io.sentry.android.replay.ReplayCache
 import io.sentry.android.replay.ScreenshotRecorderConfig
@@ -36,42 +35,11 @@ internal class BufferCaptureStrategy(
     // TODO: capture envelopes for buffered segments instead, but don't send them until buffer is triggered
     private val bufferedSegments = mutableListOf<ReplaySegment.Created>()
 
-    // TODO: rework this bs, it doesn't work with sending replay on restart
-    private val bufferedScreensLock = Any()
-    private val bufferedScreens = mutableListOf<Pair<String, Long>>()
-
     internal companion object {
         private const val TAG = "BufferCaptureStrategy"
         private const val ENVELOPE_PROCESSING_DELAY: Long = 100L
     }
 
-    override fun start(
-        recorderConfig: ScreenshotRecorderConfig,
-        segmentId: Int,
-        replayId: SentryId,
-        replayType: ReplayType?
-    ) {
-        super.start(recorderConfig, segmentId, replayId, replayType)
-
-        hub?.configureScope {
-            val screen = it.screen?.substringAfterLast('.')
-            if (screen != null) {
-                synchronized(bufferedScreensLock) {
-                    bufferedScreens.add(screen to dateProvider.currentTimeMillis)
-                }
-            }
-        }
-    }
-
-    override fun onScreenChanged(screen: String?) {
-        synchronized(bufferedScreensLock) {
-            val lastKnownScreen = bufferedScreens.lastOrNull()?.first
-            if (screen != null && lastKnownScreen != screen) {
-                bufferedScreens.add(screen to dateProvider.currentTimeMillis)
-            }
-        }
-    }
-
     override fun pause() {
         createCurrentSegment("pause") { segment ->
             if (segment is ReplaySegment.Created) {
@@ -138,7 +106,7 @@ internal class BufferCaptureStrategy(
 
             val now = dateProvider.currentTimeMillis
             val bufferLimit = now - options.experimental.sessionReplay.errorReplayDuration
-            cache?.rotate(bufferLimit)
+            screenAtStart = cache?.rotate(bufferLimit)
             bufferedSegments.rotate(bufferLimit)
         }
     }
@@ -171,21 +139,6 @@ internal class BufferCaptureStrategy(
         rotateEvents(currentEvents, bufferLimit)
     }
 
-    private fun findAndSetStartScreen(segmentStart: Long) {
-        synchronized(bufferedScreensLock) {
-            val startScreen = bufferedScreens.lastOrNull { (_, timestamp) ->
-                timestamp <= segmentStart
-            }?.first
-            // if no screen is found before the segment start, this likely means the buffer is from the
-            // app start, and the start screen will be taken from the navigation crumbs
-            if (startScreen != null) {
-                screenAtStart = startScreen
-            }
-            // can clear as we switch to session mode and don't care anymore about buffering
-            bufferedScreens.clear()
-        }
-    }
-
     private fun deleteFile(file: File?) {
         if (file == null) {
             return
@@ -248,8 +201,6 @@ internal class BufferCaptureStrategy(
         val height = this.recorderConfig.recordingHeight
         val width = this.recorderConfig.recordingWidth
 
-        findAndSetStartScreen(currentSegmentTimestamp.time)
-
         replayExecutor.submitSafely(options, "$TAG.$taskName") {
             val segment =
                 createSegmentInternal(duration, currentSegmentTimestamp, replayId, segmentId, height, width)

sentry-android-replay/src/main/java/io/sentry/android/replay/capture/SessionCaptureStrategy.kt

@@ -40,7 +40,7 @@ internal class SessionCaptureStrategy(
         // tagged with the replay that might never be sent when we're recording in buffer mode
         hub?.configureScope {
             it.replayId = currentReplayId
-            screenAtStart = it.screen
+            screenAtStart = it.screen?.substringAfterLast('.')
         }
     }
 

sentry-android-replay/src/test/java/io/sentry/android/replay/ReplayCacheTest.kt

@@ -266,6 +266,23 @@ class ReplayCacheTest {
         assertTrue(replayCache.replayCacheDir!!.listFiles()!!.none { it.name == "1.jpg" || it.name == "1001.jpg" })
     }
 
+    @Test
+    fun `rotate returns first screen in buffer`() {
+        val replayCache = fixture.getSut(
+            tmpDir,
+            frameRate = 1
+        )
+
+        val bitmap = Bitmap.createBitmap(1, 1, ARGB_8888)
+        replayCache.addFrame(bitmap, 1, "MainActivity")
+        replayCache.addFrame(bitmap, 1001, "SecondActivity")
+        replayCache.addFrame(bitmap, 2001, "ThirdActivity")
+        replayCache.addFrame(bitmap, 3001, "FourthActivity")
+
+        val screen = replayCache.rotate(2000)
+        assertEquals("ThirdActivity", screen)
+    }
+
     @Test
     fun `does not persist segment if already closed`() {
         val replayId = SentryId()

sentry-android-replay/src/test/java/io/sentry/android/replay/ReplayIntegrationTest.kt

@@ -10,6 +10,8 @@ import io.sentry.Breadcrumb
 import io.sentry.DateUtils
 import io.sentry.Hint
 import io.sentry.IHub
+import io.sentry.Scope
+import io.sentry.ScopeCallback
 import io.sentry.SentryEvent
 import io.sentry.SentryIntegrationPackageStorage
 import io.sentry.SentryOptions
@@ -78,7 +80,12 @@ class ReplayIntegrationTest {
                 }.whenever(mock).submit(any<Runnable>())
             }
         }
-        val hub = mock<IHub>()
+        val scope = Scope(options)
+        val hub = mock<IHub> {
+            doAnswer {
+                ((it.arguments[0]) as ScopeCallback).run(scope)
+            }.whenever(mock).configureScope(any<ScopeCallback>())
+        }
 
         val replayCache = mock<ReplayCache> {
             on { frames }.thenReturn(mutableListOf(ReplayFrame(File("1720693523997.jpg"), 1720693523997)))
@@ -149,7 +156,6 @@ class ReplayIntegrationTest {
         replay.register(fixture.hub, fixture.options)
 
         assertTrue(replay.isEnabled.get())
-        assertEquals(1, fixture.options.scopeObservers.size)
         assertTrue(recorderCreated)
         assertTrue(SentryIntegrationPackageStorage.getInstance().integrations.contains("Replay"))
     }
@@ -533,4 +539,22 @@ class ReplayIntegrationTest {
         assertTrue(scopeCache.exists())
         assertFalse(evenOlderReplay.exists())
     }
+
+    @Test
+    fun `onScreenshotRecorded supplies screen from scope to replay cache`() {
+        val captureStrategy = mock<CaptureStrategy> {
+            doAnswer {
+                ((it.arguments[1] as ReplayCache.(frameTimestamp: Long) -> Unit)).invoke(fixture.replayCache, 1720693523997)
+            }.whenever(mock).onScreenshotRecorded(anyOrNull<Bitmap>(), any())
+        }
+        val replay = fixture.getSut(context, replayCaptureStrategyProvider = { captureStrategy })
+
+        fixture.hub.configureScope { it.screen = "MainActivity" }
+        replay.register(fixture.hub, fixture.options)
+        replay.start()
+
+        replay.onScreenshotRecorded(mock<Bitmap>())
+
+        verify(fixture.replayCache).addFrame(any<Bitmap>(), any(), eq("MainActivity"))
+    }
 }