Address PR feedback

Author: markushi

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

@@ -15,12 +15,10 @@ import io.sentry.android.replay.screenshot.CanvasStrategy
 import io.sentry.android.replay.screenshot.PixelCopyStrategy
 import io.sentry.android.replay.screenshot.ScreenshotStrategy
 import io.sentry.android.replay.util.DebugOverlayDrawable
-import io.sentry.android.replay.util.MainLooperHandler
 import io.sentry.android.replay.util.addOnDrawListenerSafe
 import io.sentry.android.replay.util.removeOnDrawListenerSafe
 import java.io.File
 import java.lang.ref.WeakReference
-import java.util.concurrent.ScheduledExecutorService
 import java.util.concurrent.atomic.AtomicBoolean
 import kotlin.math.roundToInt
 
@@ -29,8 +27,7 @@ import kotlin.math.roundToInt
 internal class ScreenshotRecorder(
   val config: ScreenshotRecorderConfig,
   val options: SentryOptions,
-  val handler: MainLooperHandler,
-  executorService: ScheduledExecutorService,
+  val executorProvider: ExecutorProvider,
   screenshotRecorderCallback: ScreenshotRecorderCallback?,
 ) : ViewTreeObserver.OnDrawListener {
   private var rootView: WeakReference<View>? = null
@@ -42,11 +39,10 @@ internal class ScreenshotRecorder(
   private val screenshotStrategy: ScreenshotStrategy =
     when (options.sessionReplay.screenshotStrategy) {
       ScreenshotStrategyType.CANVAS ->
-        CanvasStrategy(executorService, screenshotRecorderCallback, options, config)
+        CanvasStrategy(executorProvider, screenshotRecorderCallback, options, config)
       ScreenshotStrategyType.PIXEL_COPY ->
         PixelCopyStrategy(
-          executorService,
-          handler,
+          executorProvider,
           screenshotRecorderCallback,
           options,
           config,

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

@@ -2,6 +2,8 @@ package io.sentry.android.replay
 
 import android.annotation.TargetApi
 import android.graphics.Point
+import android.os.Handler
+import android.os.HandlerThread
 import android.view.View
 import android.view.ViewTreeObserver
 import io.sentry.SentryLevel.DEBUG
@@ -24,15 +26,20 @@ internal class WindowRecorder(
   private val windowCallback: WindowCallback,
   private val mainLooperHandler: MainLooperHandler,
   private val replayExecutor: ScheduledExecutorService,
-) : Recorder, OnRootViewsChangedListener {
+) : Recorder, OnRootViewsChangedListener, ExecutorProvider {
 
   private val isRecording = AtomicBoolean(false)
   private val rootViews = ArrayList<WeakReference<View>>()
   private var lastKnownWindowSize: Point = Point()
   private val rootViewsLock = AutoClosableReentrantLock()
   private val capturerLock = AutoClosableReentrantLock()
+  private val backgroundProcessingHandlerLock = AutoClosableReentrantLock()
+
   @Volatile private var capturer: Capturer? = null
 
+  @Volatile private var backgroundProcessingHandlerThread: HandlerThread? = null
+  @Volatile private var backgroundProcessingHandler: Handler? = null
+
   private class Capturer(
     private val options: SentryOptions,
     private val mainLooperHandler: MainLooperHandler,
@@ -174,14 +181,7 @@ internal class WindowRecorder(
     }
 
     capturer?.config = config
-    capturer?.recorder =
-      ScreenshotRecorder(
-        config,
-        options,
-        mainLooperHandler,
-        replayExecutor,
-        screenshotRecorderCallback,
-      )
+    capturer?.recorder = ScreenshotRecorder(config, options, this, screenshotRecorderCallback)
 
     val newRoot = rootViews.lastOrNull()?.get()
     if (newRoot != null) {
@@ -229,6 +229,40 @@ internal class WindowRecorder(
   override fun close() {
     reset()
     mainLooperHandler.removeCallbacks(capturer)
+    backgroundProcessingHandlerLock.acquire().use {
+      backgroundProcessingHandler?.removeCallbacksAndMessages(null)
+      backgroundProcessingHandlerThread?.quitSafely()
+    }
     stop()
   }
+
+  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 {
+  /** Returns an executor suitable for background tasks. */
+  fun getExecutor(): ScheduledExecutorService
+
+  /** Returns a handler associated with the main thread looper. */
+  fun getMainLooperHandler(): MainLooperHandler
+
+  /** Returns a handler associated with a background thread looper. */
+  fun getBackgroundHandler(): Handler
 }