diff --git a/CHANGELOG.md b/CHANGELOG.md index a6750c068..f38b7b8e0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## Unreleased + +### Enhancements + +- Replay: improve performance of screenshot data to native recorder ([#2530](https://github.com/getsentry/sentry-dart/pull/2530)) + ## 8.12.0 ### Deprecations diff --git a/flutter/ios/Classes/SentryFlutterReplayScreenshotProvider.m b/flutter/ios/Classes/SentryFlutterReplayScreenshotProvider.m index b363cf534..61d9e87e5 100644 --- a/flutter/ios/Classes/SentryFlutterReplayScreenshotProvider.m +++ b/flutter/ios/Classes/SentryFlutterReplayScreenshotProvider.m @@ -30,17 +30,23 @@ - (void)imageWithView:(UIView *_Nonnull)view NSLog(@"SentryFlutterReplayScreenshotProvider received null " @"result. " @"Cannot capture a replay screenshot."); - } else if ([value - isKindOfClass:[FlutterStandardTypedData class]]) { - FlutterStandardTypedData *typedData = - (FlutterStandardTypedData *)value; - UIImage *image = [UIImage imageWithData:typedData.data]; + } else if ([value isKindOfClass:[NSDictionary class]]) { + NSDictionary *dict = (NSDictionary *)value; + long address = ((NSNumber *)dict[@"address"]).longValue; + NSNumber *length = ((NSNumber *)dict[@"length"]); + NSData *data = + [NSData dataWithBytesNoCopy:(void *)address + length:length.unsignedLongValue + freeWhenDone:TRUE]; + UIImage *image = [UIImage imageWithData:data]; onComplete(image); + return; } else if ([value isKindOfClass:[FlutterError class]]) { FlutterError *error = (FlutterError *)value; NSLog(@"SentryFlutterReplayScreenshotProvider received an " @"error: %@. Cannot capture a replay screenshot.", error.message); + return; } else { NSLog(@"SentryFlutterReplayScreenshotProvider received an " @"unexpected result. " diff --git a/flutter/lib/src/native/cocoa/sentry_native_cocoa.dart b/flutter/lib/src/native/cocoa/sentry_native_cocoa.dart index 5a89d6fb4..621d8f8a4 100644 --- a/flutter/lib/src/native/cocoa/sentry_native_cocoa.dart +++ b/flutter/lib/src/native/cocoa/sentry_native_cocoa.dart @@ -10,6 +10,7 @@ import '../../replay/replay_config.dart'; import '../../replay/replay_recorder.dart'; import '../../screenshot/recorder.dart'; import '../../screenshot/recorder_config.dart'; +import '../native_memory.dart'; import '../sentry_native_channel.dart'; import 'binding.dart' as cocoa; @@ -73,7 +74,10 @@ class SentryNativeCocoa extends SentryNativeChannel { } }).then(completer.complete, onError: completer.completeError); }); - return completer.future; + final uint8List = await completer.future; + + // Malloc memory and copy the data. Native must free it. + return uint8List?.toNativeMemory().toJson(); default: throw UnimplementedError('Method ${call.method} not implemented'); } diff --git a/flutter/lib/src/native/native_memory.dart b/flutter/lib/src/native/native_memory.dart new file mode 100644 index 000000000..a7c7be000 --- /dev/null +++ b/flutter/lib/src/native/native_memory.dart @@ -0,0 +1,49 @@ +import 'dart:ffi'; +import 'dart:typed_data'; + +import 'package:meta/meta.dart'; +import 'package:ffi/ffi.dart' as pkg_ffi; + +@internal +@immutable +class NativeMemory { + final Pointer pointer; + final int length; + + const NativeMemory._(this.pointer, this.length); + + factory NativeMemory.fromUint8List(Uint8List source) { + final length = source.length; + final ptr = pkg_ffi.malloc.allocate(length); + if (length > 0) { + ptr.asTypedList(length).setAll(0, source); + } + return NativeMemory._(ptr, length); + } + + factory NativeMemory.fromJson(Map json) { + final length = json['length'] as int; + final ptr = Pointer.fromAddress(json['address'] as int); + return NativeMemory._(ptr, length); + } + + /// Frees the underlying native memory. + /// You must not use this object after freeing. + /// + /// Currently, we only need to do this in tests because there's no native + /// counterpart to free the memory. + @visibleForTesting + void free() => pkg_ffi.malloc.free(pointer); + + Uint8List asTypedList() => pointer.asTypedList(length); + + Map toJson() => { + 'address': pointer.address, + 'length': length, + }; +} + +@internal +extension Uint8ListNativeMemory on Uint8List { + NativeMemory toNativeMemory() => NativeMemory.fromUint8List(this); +} diff --git a/flutter/test/mocks.dart b/flutter/test/mocks.dart index 66e05b68e..b13467e7c 100644 --- a/flutter/test/mocks.dart +++ b/flutter/test/mocks.dart @@ -202,10 +202,11 @@ class NativeChannelFixture { handler; static TestDefaultBinaryMessenger get _messenger => TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger; + late final codec = StandardMethodCodec(); NativeChannelFixture() { TestWidgetsFlutterBinding.ensureInitialized(); - channel = MethodChannel('test.channel', StandardMethodCodec(), _messenger); + channel = MethodChannel('test.channel', codec, _messenger); handler = MockCallbacks().methodCallHandler; when(handler('initNativeSdk', any)).thenAnswer((_) => Future.value()); when(handler('closeNativeSdk', any)).thenAnswer((_) => Future.value()); @@ -214,11 +215,15 @@ class NativeChannelFixture { } // Mock this call as if it was invoked by the native side. - Future invokeFromNative(String method, [dynamic arguments]) async { - final call = - StandardMethodCodec().encodeMethodCall(MethodCall(method, arguments)); - return _messenger.handlePlatformMessage( + Future invokeFromNative(String method, [dynamic arguments]) async { + final call = codec.encodeMethodCall(MethodCall(method, arguments)); + final byteData = await _messenger.handlePlatformMessage( channel.name, call, (ByteData? data) {}); + if (byteData != null) { + return codec.decodeEnvelope(byteData); + } else { + return null; + } } } diff --git a/flutter/test/native_memory_test.dart b/flutter/test/native_memory_test.dart new file mode 100644 index 000000000..055ccf90f --- /dev/null +++ b/flutter/test/native_memory_test.dart @@ -0,0 +1,44 @@ +@TestOn('vm') +library flutter_test; + +import 'dart:typed_data'; + +import 'package:flutter_test/flutter_test.dart'; +import 'native_memory_web_mock.dart' + if (dart.library.io) 'package:sentry_flutter/src/native/native_memory.dart'; + +void main() { + final testSrcList = Uint8List.fromList([1, 2, 3]); + + test('empty list', () async { + final sut = NativeMemory.fromUint8List(Uint8List.fromList([])); + expect(sut.length, 0); + expect(sut.pointer.address, greaterThan(0)); + expect(sut.asTypedList(), isEmpty); + sut.free(); + }); + + test('non-empty list', () async { + final sut = NativeMemory.fromUint8List(testSrcList); + expect(sut.length, 3); + expect(sut.pointer.address, greaterThan(0)); + expect(sut.asTypedList(), testSrcList); + sut.free(); + }); + + test('json', () async { + final sut = NativeMemory.fromUint8List(testSrcList); + final json = sut.toJson(); + expect(json['address'], greaterThan(0)); + expect(json['length'], 3); + expect(json.entries, hasLength(2)); + + final sut2 = NativeMemory.fromJson(json); + expect(sut2.toJson(), json); + expect(sut2.asTypedList(), testSrcList); + + expect(sut.pointer, sut2.pointer); + expect(sut.length, sut2.length); + sut2.free(); + }); +} diff --git a/flutter/test/native_memory_web_mock.dart b/flutter/test/native_memory_web_mock.dart new file mode 100644 index 000000000..81e03fee6 --- /dev/null +++ b/flutter/test/native_memory_web_mock.dart @@ -0,0 +1,60 @@ +import 'dart:math'; +import 'dart:typed_data'; + +// This is just a mock so `flutter test --platform chrome` works. +// See https://github.com/flutter/flutter/issues/160675 +class NativeMemory { + final Pointer pointer; + final int length; + + const NativeMemory._(this.pointer, this.length); + + factory NativeMemory.fromUint8List(Uint8List source) { + return NativeMemory._(Pointer._store(source), source.length); + } + + factory NativeMemory.fromJson(Map json) { + return NativeMemory._( + Pointer._load(json['address'] as int), json['length'] as int); + } + + void free() {} + + Uint8List asTypedList() => _memory[pointer.address]!; + + Map toJson() => { + 'address': pointer.address, + 'length': length, + }; +} + +class Pointer { + final int address; + + const Pointer(this.address); + + factory Pointer._store(Uint8List data) { + final address = Random().nextInt(999999); + _memory[address] = data; + return Pointer(address); + } + + factory Pointer._load(int address) { + return Pointer(address); + } + + /// Equality for Pointers only depends on their address. + @override + bool operator ==(Object other) { + if (other is! Pointer) return false; + return address == other.address; + } + + /// The hash code for a Pointer only depends on its address. + @override + int get hashCode => address.hashCode; +} + +class Uint8 {} + +final _memory = {}; diff --git a/flutter/test/replay/replay_native_test.dart b/flutter/test/replay/replay_native_test.dart index d40402c00..424da6706 100644 --- a/flutter/test/replay/replay_native_test.dart +++ b/flutter/test/replay/replay_native_test.dart @@ -11,6 +11,8 @@ import 'package:flutter_test/flutter_test.dart'; import 'package:mockito/mockito.dart'; import 'package:sentry_flutter/sentry_flutter.dart'; import 'package:sentry_flutter/src/native/factory.dart'; +import '../native_memory_web_mock.dart' + if (dart.library.io) 'package:sentry_flutter/src/native/native_memory.dart'; import 'package:sentry_flutter/src/native/sentry_native_binding.dart'; import '../mocks.dart'; @@ -76,29 +78,35 @@ void main() { await sut.init(hub); }); - test('sets replay ID to context', () async { - // verify there was no scope configured before - verifyNever(hub.configureScope(any)); - - // emulate the native platform invoking the method - await native.invokeFromNative( - mockPlatform.isAndroid - ? 'ReplayRecorder.start' - : 'captureReplayScreenshot', - replayConfig); + testWidgets('sets replayID to context', (tester) async { + await tester.runAsync(() async { + // verify there was no scope configured before + verifyNever(hub.configureScope(any)); + when(hub.configureScope(captureAny)).thenReturn(null); - // verify the replay ID was set - final closure = - verify(hub.configureScope(captureAny)).captured.single; - final scope = Scope(options); - expect(scope.replayId, isNull); - await closure(scope); - expect(scope.replayId.toString(), replayConfig['replayId']); + // emulate the native platform invoking the method + final future = native.invokeFromNative( + mockPlatform.isAndroid + ? 'ReplayRecorder.start' + : 'captureReplayScreenshot', + replayConfig); + await tester.pumpAndSettle(const Duration(seconds: 1)); + await future; + + // verify the replay ID was set + final closure = + verify(hub.configureScope(captureAny)).captured.single; + final scope = Scope(options); + expect(scope.replayId, isNull); + await closure(scope); + expect(scope.replayId.toString(), replayConfig['replayId']); + }); }); test('clears replay ID from context', () async { // verify there was no scope configured before verifyNever(hub.configureScope(any)); + when(hub.configureScope(captureAny)).thenReturn(null); // emulate the native platform invoking the method await native.invokeFromNative('ReplayRecorder.stop'); @@ -116,6 +124,7 @@ void main() { testWidgets('captures images', (tester) async { await tester.runAsync(() async { when(hub.configureScope(captureAny)).thenReturn(null); + await pumpTestElement(tester); pumpAndSettle() => tester.pumpAndSettle(const Duration(seconds: 1)); @@ -198,17 +207,23 @@ void main() { expect(capturedImages, equals(fsImages())); expect(capturedImages.length, count); } else if (mockPlatform.isIOS) { - var imagaData = native.invokeFromNative( - 'captureReplayScreenshot', replayConfig); - await pumpAndSettle(); - expect((await imagaData)?.lengthInBytes, greaterThan(3000)); + Future captureAndVerify() async { + final future = native.invokeFromNative( + 'captureReplayScreenshot', replayConfig); + await pumpAndSettle(); + final json = (await future) as Map; + + expect(json['length'], greaterThan(3000)); + expect(json['address'], greaterThan(0)); + NativeMemory.fromJson(json).free(); + } + + await captureAndVerify(); - // Happens if the session-replay rate is 0. + // Check everything works if session-replay rate is 0, + // which causes replayId to be 0 as well. replayConfig['replayId'] = null; - imagaData = native.invokeFromNative( - 'captureReplayScreenshot', replayConfig); - await pumpAndSettle(); - expect((await imagaData)?.lengthInBytes, greaterThan(3000)); + await captureAndVerify(); } else { fail('unsupported platform'); }