Refactor android replay recorder to use new isolate worker class (#3296)

* Update

* Update

Author: buenaflor

CHANGELOG.md

@@ -7,8 +7,12 @@
 - Mark file sync spans run in the main isolate with `blocked_main_thread` ([#3270](https://github.com/getsentry/sentry-dart/pull/3270))
  - This allows Sentry to create issues automatically out of file spans running a certain time on the main thread: https://docs.sentry.io/product/issues/issue-details/performance-issues/file-main-thread-io/
 
-## 9.7.0
+### Enhancements
 
+- Refactor `AndroidReplayRecorder` to use the new worker isolate api [#3296](https://github.com/getsentry/sentry-dart/pull/3296/)
+- Offload `captureEnvelope` to background isolate for Cocoa and Android [#3232](https://github.com/getsentry/sentry-dart/pull/3232)
+
+## 9.7.0
 
 ### Features
 

packages/flutter/lib/src/isolate/isolate_worker.dart

@@ -4,6 +4,8 @@ import 'dart:isolate';
 import '../../sentry_flutter.dart';
 import 'isolate_logger.dart';
 
+typedef SpawnWorkerFn = Future<Worker> Function(WorkerConfig, WorkerEntry);
+
 const _shutdownCommand = '_shutdown_';
 
 // -------------------------------------------

packages/flutter/lib/src/native/java/android_envelope_sender.dart

@@ -10,8 +10,6 @@ import '../../isolate/isolate_worker.dart';
 import '../../isolate/isolate_logger.dart';
 import 'binding.dart' as native;
 
-typedef SpawnWorkerFn = Future<Worker> Function(WorkerConfig, WorkerEntry);
-
 class AndroidEnvelopeSender {
   final SentryFlutterOptions _options;
   final WorkerConfig _config;

packages/flutter/lib/src/native/java/android_replay_recorder.dart

@@ -6,6 +6,8 @@ import 'package:jni/jni.dart';
 import 'package:meta/meta.dart';
 
 import '../../../sentry_flutter.dart';
+import '../../isolate/isolate_logger.dart';
+import '../../isolate/isolate_worker.dart';
 import '../../replay/scheduled_recorder.dart';
 import '../../screenshot/screenshot.dart';
 import 'binding.dart' as native;
@@ -14,21 +16,30 @@ import 'binding.dart' as native;
 // cumbersome, see https://github.com/dart-lang/native/issues/1794
 @internal
 class AndroidReplayRecorder extends ScheduledScreenshotRecorder {
-  _AndroidNativeReplayWorker? _worker;
+  final WorkerConfig _config;
+  final SpawnWorkerFn _spawn;
+  Worker? _worker;
 
   @internal // visible for testing, used by SentryNativeJava
   static AndroidReplayRecorder Function(SentryFlutterOptions) factory =
       AndroidReplayRecorder.new;
 
-  AndroidReplayRecorder(super.options) {
+  AndroidReplayRecorder(super.options, {SpawnWorkerFn? spawn})
+      : _config = WorkerConfig(
+          debugName: 'SentryAndroidReplayRecorder',
+          debug: options.debug,
+          diagnosticLevel: options.diagnosticLevel,
+          automatedTestMode: options.automatedTestMode,
+        ),
+        _spawn = spawn ?? spawnWorker {
     super.callback = _addReplayScreenshot;
   }
 
   @override
   Future<void> start() async {
-    final spawningWorker = _AndroidNativeReplayWorker.spawn();
+    if (_worker != null) return;
+    _worker = await _spawn(_config, _entryPoint);
     await super.start();
-    _worker = await spawningWorker;
   }
 
   @override
@@ -50,7 +61,7 @@ class AndroidReplayRecorder extends ScheduledScreenshotRecorder {
           '${screenshot.width}x${screenshot.height} pixels, '
           '${data.lengthInBytes} bytes)');
 
-      await _worker!.nativeAddScreenshot(_WorkItem(
+      await _worker!.request(_WorkItem(
         timestamp: timestamp,
         data: data.buffer.asUint8List(),
         width: screenshot.width,
@@ -68,127 +79,71 @@ class AndroidReplayRecorder extends ScheduledScreenshotRecorder {
       }
     }
   }
-}
-
-// Based on https://dart.dev/language/isolates#robust-ports-example
-class _AndroidNativeReplayWorker {
-  final SendPort _commands;
-  final ReceivePort _responses;
-  final Map<int, Completer<Object?>> _activeRequests = {};
-  int _idCounter = 0;
-  bool _closed = false;
-
-  static Future<_AndroidNativeReplayWorker> spawn() async {
-    // Create a receive port and add its initial message handler
-    final initPort = RawReceivePort();
-    final connection = Completer<(ReceivePort, SendPort)>.sync();
-    initPort.handler = (SendPort commandPort) {
-      connection.complete((
-        ReceivePort.fromRawReceivePort(initPort),
-        commandPort,
-      ));
-    };
-
-    // Spawn the isolate.
-    try {
-      await Isolate.spawn(_startRemoteIsolate, (initPort.sendPort),
-          debugName: 'SentryReplayRecorder');
-    } on Object {
-      initPort.close();
-      rethrow;
-    }
-
-    final (ReceivePort receivePort, SendPort sendPort) =
-        await connection.future;
 
-    return _AndroidNativeReplayWorker._(receivePort, sendPort);
+  static void _entryPoint((SendPort, WorkerConfig) init) {
+    final (host, config) = init;
+    runWorker(config, host, _AndroidReplayHandler(config));
   }
+}
 
-  _AndroidNativeReplayWorker._(this._responses, this._commands) {
-    _responses.listen(_handleResponsesFromIsolate);
-  }
+class _AndroidReplayHandler extends WorkerHandler {
+  final WorkerConfig _config;
+  // Android Bitmap creation is a bit costly so we reuse it between captures.
+  native.Bitmap? _bitmap;
+  late final native.ReplayIntegration _nativeReplay;
 
-  Future<Object?> nativeAddScreenshot(_WorkItem item) async {
-    if (_closed) throw StateError('Closed');
-    final completer = Completer<Object?>.sync();
-    final id = _idCounter++;
-    _activeRequests[id] = completer;
-    _commands.send((id, item));
-    return await completer.future;
+  _AndroidReplayHandler(this._config) {
+    _nativeReplay = native.SentryFlutterPlugin.Companion
+        .privateSentryGetReplayIntegration()!;
   }
 
-  void _handleResponsesFromIsolate(dynamic message) {
-    final (int id, Object? response) = message as (int, Object?);
-    final completer = _activeRequests.remove(id)!;
-
-    if (response is RemoteError) {
-      completer.completeError(response);
-    } else {
-      completer.complete(response);
-    }
-
-    if (_closed && _activeRequests.isEmpty) _responses.close();
+  @override
+  FutureOr<void> onMessage(Object? message) {
+    IsolateLogger.log(
+        SentryLevel.warning, 'Unexpected fire-and-forget message: $message');
   }
 
-  /// This is the actual Android native implementation, the rest is just plumbing.
-  static void _handleCommandsToIsolate(
-    ReceivePort receivePort,
-    SendPort sendPort,
-  ) {
-    // Android Bitmap creation is a bit costly so we reuse it between captures.
-    native.Bitmap? bitmap;
+  @override
+  FutureOr<Object?> onRequest(Object? payload) {
+    if (payload is! _WorkItem) {
+      IsolateLogger.log(
+          SentryLevel.warning, 'Unexpected payload type: $payload');
+      return null;
+    }
 
-    final _nativeReplay = native.SentryFlutterPlugin.Companion
-        .privateSentryGetReplayIntegration()!;
+    final item = payload;
+    JByteBuffer? jBuffer;
 
-    receivePort.listen((message) {
-      if (message == 'shutdown') {
-        receivePort.close();
-        return;
-      }
-      final (id, item) = message as (int, _WorkItem);
-      try {
-        if (bitmap != null) {
-          if (bitmap!.getWidth() != item.width ||
-              bitmap!.getHeight() != item.height) {
-            bitmap!.release();
-            bitmap = null;
-          }
+    try {
+      if (_bitmap != null) {
+        if (_bitmap!.getWidth() != item.width ||
+            _bitmap!.getHeight() != item.height) {
+          _bitmap!.release();
+          _bitmap = null;
         }
+      }
 
-        // https://developer.android.com/reference/android/graphics/Bitmap#createBitmap(int,%20int,%20android.graphics.Bitmap.Config)
-        // Note: while the generated API is nullable, the docs say the returned value cannot be null..
-        bitmap ??= native.Bitmap.createBitmap$3(
-            item.width, item.height, native.Bitmap$Config.ARGB_8888);
+      // https://developer.android.com/reference/android/graphics/Bitmap#createBitmap(int,%20int,%20android.graphics.Bitmap.Config)
+      // Note: while the generated API is nullable, the docs say the returned value cannot be null..
+      _bitmap ??= native.Bitmap.createBitmap$3(
+          item.width, item.height, native.Bitmap$Config.ARGB_8888);
 
-        final jBuffer = JByteBuffer.fromList(item.data);
-        try {
-          bitmap!.copyPixelsFromBuffer(jBuffer);
-        } finally {
-          jBuffer.release();
-        }
+      jBuffer = JByteBuffer.fromList(item.data);
+      _bitmap!.copyPixelsFromBuffer(jBuffer);
 
-        // TODO timestamp is currently missing in onScreenshotRecorded()
-        _nativeReplay.onScreenshotRecorded(bitmap!);
+      // TODO timestamp is currently missing in onScreenshotRecorded()
+      _nativeReplay.onScreenshotRecorded(_bitmap!);
 
-        sendPort.send((id, null));
-      } catch (e, stacktrace) {
-        sendPort.send((id, RemoteError(e.toString(), stacktrace.toString())));
+      return null;
+    } catch (exception, stackTrace) {
+      IsolateLogger.log(SentryLevel.error, 'Failed to add replay screenshot',
+          exception: exception, stackTrace: stackTrace);
+      if (_config.automatedTestMode) {
+        rethrow;
       }
-    });
-  }
-
-  static void _startRemoteIsolate(SendPort sendPort) {
-    final receivePort = ReceivePort();
-    sendPort.send(receivePort.sendPort);
-    _handleCommandsToIsolate(receivePort, sendPort);
-  }
-
-  void close() {
-    if (!_closed) {
-      _closed = true;
-      _commands.send('shutdown');
-      if (_activeRequests.isEmpty) _responses.close();
+      return null;
+    } finally {
+      jBuffer?.release();
     }
   }
 }