Remove extra background executor, as it's not needed anymore

Author: markushi

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

@@ -244,21 +244,6 @@ internal class WindowRecorder(
   override fun getExecutor(): ScheduledExecutorService = replayExecutor
 
   override fun getMainLooperHandler(): MainLooperHandler = mainLooperHandler
-
-  override fun getBackgroundHandler(): Handler {
-    // only start the background thread if it's actually needed, as it's only used by Canvas Capture
-    // Strategy
-    if (backgroundProcessingHandler == null) {
-      backgroundProcessingHandlerLock.acquire().use {
-        if (backgroundProcessingHandler == null) {
-          backgroundProcessingHandlerThread = HandlerThread("SentryReplayBackgroundProcessing")
-          backgroundProcessingHandlerThread?.start()
-          backgroundProcessingHandler = Handler(backgroundProcessingHandlerThread!!.looper)
-        }
-      }
-    }
-    return backgroundProcessingHandler!!
-  }
 }
 
 internal interface ExecutorProvider {
@@ -267,7 +252,4 @@ internal interface ExecutorProvider {
 
   /** Returns a handler associated with the main thread looper. */
   fun getMainLooperHandler(): MainLooperHandler
-
-  /** Returns a handler associated with a background thread looper. */
-  fun getBackgroundHandler(): Handler
 }

sentry-android-replay/src/main/java/io/sentry/android/replay/screenshot/CanvasStrategy.kt

@@ -29,6 +29,7 @@ import android.view.Surface
 import android.view.View
 import androidx.annotation.RequiresApi
 import io.sentry.SentryLevel
+import io.sentry.SentryLevel.DEBUG
 import io.sentry.SentryOptions
 import io.sentry.android.replay.ExecutorProvider
 import io.sentry.android.replay.ScreenshotRecorderCallback
@@ -71,15 +72,14 @@ internal class CanvasStrategy(
   @SuppressLint("NewApi")
   private val pictureRenderTask = Runnable {
     if (isClosed.get()) {
-      options.logger.log(
-        SentryLevel.DEBUG,
-        "Canvas Strategy already closed, skipping picture render",
-      )
+      options.logger.log(DEBUG, "Canvas Strategy already closed, skipping picture render")
       return@Runnable
     }
     val picture = unprocessedPictureRef.getAndSet(null) ?: return@Runnable
 
     try {
+      // It's safe to access the surface because the render task,
+      // as well as surface release are executed on the same single threaded executor
       // Draw picture to the Surface for PixelCopy
       val surfaceCanvas = surface.lockHardwareCanvas()
       try {
@@ -96,30 +96,45 @@ internal class CanvasStrategy(
           }
         }
       }
-
-      // Trigger PixelCopy capture
       PixelCopy.request(
         surface,
         screenshot!!,
         { result ->
-          if (result == PixelCopy.SUCCESS) {
-            lastCaptureSuccessful.set(true)
-            val bitmap = screenshot
-            if (bitmap != null && !bitmap.isRecycled) {
-              screenshotRecorderCallback?.onScreenshotRecorded(bitmap)
-            }
-          } else {
-            options.logger.log(
-              SentryLevel.ERROR,
-              "Canvas Strategy: PixelCopy failed with code $result",
-            )
-            lastCaptureSuccessful.set(false)
+          if (isClosed.get()) {
+            options.logger.log(DEBUG, "CanvasStrategy is closed, ignoring capture result")
+            return@request
           }
+          executor
+            .getExecutor()
+            .submit(
+              ReplayRunnable("screenshot_recorder.mask") {
+                if (isClosed.get()) {
+                  options.logger.log(DEBUG, "CanvasStrategy is closed, ignoring capture result")
+                  return@ReplayRunnable
+                }
+                if (result == PixelCopy.SUCCESS) {
+                  lastCaptureSuccessful.set(true)
+                  val bitmap = screenshot
+                  if (bitmap != null) {
+                    synchronized(bitmap) {
+                      if (!bitmap.isRecycled)
+                        screenshotRecorderCallback?.onScreenshotRecorded(bitmap)
+                    }
+                  }
+                } else {
+                  options.logger.log(
+                    SentryLevel.ERROR,
+                    "Canvas Strategy: PixelCopy failed with code $result",
+                  )
+                  lastCaptureSuccessful.set(false)
+                }
+              }
+            )
         },
-        executor.getBackgroundHandler(),
+        executor.getMainLooperHandler().handler,
       )
     } catch (t: Throwable) {
-      options.logger.log(SentryLevel.ERROR, "Canvas Strategy: picture render failed")
+      options.logger.log(SentryLevel.ERROR, "Canvas Strategy: picture render failed", t)
       lastCaptureSuccessful.set(false)
     }
   }
@@ -139,10 +154,7 @@ internal class CanvasStrategy(
 
     if (!isClosed.get()) {
       unprocessedPictureRef.set(picture)
-      // use the same handler for PixelCopy and pictureRenderTask
-      executor
-        .getBackgroundHandler()
-        .post(ReplayRunnable("screenshot_recorder.canvas", pictureRenderTask))
+      executor.getExecutor().submit(ReplayRunnable("screenshot_recorder.canvas", pictureRenderTask))
     }
   }
 

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

@@ -1,6 +1,5 @@
 package io.sentry.android.replay
 
-import android.os.Handler
 import androidx.test.ext.junit.runners.AndroidJUnit4
 import io.sentry.ScreenshotStrategyType
 import io.sentry.SentryOptions
@@ -30,8 +29,6 @@ class ScreenshotRecorderTest {
           override fun getExecutor(): ScheduledExecutorService = mock<ScheduledExecutorService>()
 
           override fun getMainLooperHandler(): MainLooperHandler = mock<MainLooperHandler>()
-
-          override fun getBackgroundHandler(): Handler = mock<Handler>()
         },
         null,
       )

sentry-android-replay/src/test/java/io/sentry/android/replay/screenshot/PixelCopyStrategyTest.kt

@@ -2,7 +2,6 @@ package io.sentry.android.replay.screenshot
 
 import android.app.Activity
 import android.os.Bundle
-import android.os.Handler
 import android.os.Looper
 import android.widget.LinearLayout
 import android.widget.LinearLayout.LayoutParams
@@ -45,8 +44,6 @@ class PixelCopyStrategyTest {
           override fun getExecutor(): ScheduledExecutorService = executor
 
           override fun getMainLooperHandler(): MainLooperHandler = MainLooperHandler()
-
-          override fun getBackgroundHandler(): Handler = mock()
         },
         callback,
         options,