From 2b514dd502561b200d0a71c360fe36e47ecb2683 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Thu, 2 Jan 2025 22:34:00 +0100 Subject: [PATCH 01/28] capture screenshots in sequence until they're stable --- .../screenshot_event_processor.dart | 18 +----- .../src/native/cocoa/sentry_native_cocoa.dart | 31 ++++----- .../native/java/android_replay_recorder.dart | 1 + .../lib/src/replay/scheduled_recorder.dart | 55 +++++----------- flutter/lib/src/screenshot/recorder.dart | 39 +++++++---- flutter/lib/src/screenshot/retrier.dart | 64 +++++++++++++++++++ flutter/lib/src/sentry_flutter_options.dart | 4 ++ flutter/test/screenshot/recorder_test.dart | 5 +- 8 files changed, 130 insertions(+), 87 deletions(-) create mode 100644 flutter/lib/src/screenshot/retrier.dart diff --git a/flutter/lib/src/event_processor/screenshot_event_processor.dart b/flutter/lib/src/event_processor/screenshot_event_processor.dart index 6f8618bf28..13f6c74721 100644 --- a/flutter/lib/src/event_processor/screenshot_event_processor.dart +++ b/flutter/lib/src/event_processor/screenshot_event_processor.dart @@ -125,20 +125,6 @@ class ScreenshotEventProcessor implements EventProcessor { } @internal - Future createScreenshot() => - _recorder.capture(_convertImageToUint8List); - - Future _convertImageToUint8List(Screenshot screenshot) async { - final byteData = - await screenshot.image.toByteData(format: ImageByteFormat.png); - - final bytes = byteData?.buffer.asUint8List(); - if (bytes?.isNotEmpty == true) { - return bytes; - } else { - _options.logger( - SentryLevel.debug, 'Screenshot is 0 bytes, not attaching the image.'); - return null; - } - } + Future createScreenshot() => _recorder + .capture((ScreenshotPng screenshot) => Future.value(screenshot.data)); } diff --git a/flutter/lib/src/native/cocoa/sentry_native_cocoa.dart b/flutter/lib/src/native/cocoa/sentry_native_cocoa.dart index 621d8f8a40..6a12d5396a 100644 --- a/flutter/lib/src/native/cocoa/sentry_native_cocoa.dart +++ b/flutter/lib/src/native/cocoa/sentry_native_cocoa.dart @@ -1,7 +1,6 @@ import 'dart:async'; import 'dart:ffi'; import 'dart:typed_data'; -import 'dart:ui'; import 'package:meta/meta.dart'; @@ -10,6 +9,7 @@ import '../../replay/replay_config.dart'; import '../../replay/replay_recorder.dart'; import '../../screenshot/recorder.dart'; import '../../screenshot/recorder_config.dart'; +import '../../screenshot/retrier.dart'; import '../native_memory.dart'; import '../sentry_native_channel.dart'; import 'binding.dart' as cocoa; @@ -55,24 +55,17 @@ class SentryNativeCocoa extends SentryNativeChannel { } final completer = Completer(); - widgetsBinding.ensureVisualUpdate(); - widgetsBinding.addPostFrameCallback((_) { - _replayRecorder?.capture((screenshot) async { - final image = screenshot.image; - final imageData = - await image.toByteData(format: ImageByteFormat.png); - if (imageData != null) { - options.logger( - SentryLevel.debug, - 'Replay: captured screenshot (' - '${image.width}x${image.height} pixels, ' - '${imageData.lengthInBytes} bytes)'); - return imageData.buffer.asUint8List(); - } else { - options.logger(SentryLevel.warning, - 'Replay: failed to convert screenshot to PNG'); - } - }).then(completer.complete, onError: completer.completeError); + final retrier = + ScreenshotRetrier(_replayRecorder!, options, (image) async { + options.logger( + SentryLevel.debug, + 'Replay: captured screenshot (' + '${image.width}x${image.height} pixels, ' + '${image.data.lengthInBytes} bytes)'); + completer.complete(image.data); + }); + retrier.ensureFrameAndAddCallback((msSinceEpoch) { + retrier.capture(msSinceEpoch).onError(completer.completeError); }); final uint8List = await completer.future; diff --git a/flutter/lib/src/native/java/android_replay_recorder.dart b/flutter/lib/src/native/java/android_replay_recorder.dart index 021c2672c7..c6bddd6418 100644 --- a/flutter/lib/src/native/java/android_replay_recorder.dart +++ b/flutter/lib/src/native/java/android_replay_recorder.dart @@ -2,6 +2,7 @@ import 'package:meta/meta.dart'; import '../../../sentry_flutter.dart'; import '../../replay/scheduled_recorder.dart'; +import '../../screenshot/recorder.dart'; import '../sentry_safe_method_channel.dart'; @internal diff --git a/flutter/lib/src/replay/scheduled_recorder.dart b/flutter/lib/src/replay/scheduled_recorder.dart index 3fc4cec65c..881d7ef2ff 100644 --- a/flutter/lib/src/replay/scheduled_recorder.dart +++ b/flutter/lib/src/replay/scheduled_recorder.dart @@ -1,11 +1,10 @@ import 'dart:async'; -import 'dart:typed_data'; -import 'dart:ui'; import 'package:meta/meta.dart'; import '../../sentry_flutter.dart'; import '../screenshot/recorder.dart'; +import '../screenshot/retrier.dart'; import 'replay_recorder.dart'; import 'scheduled_recorder_config.dart'; import 'scheduler.dart'; @@ -20,6 +19,7 @@ class ScheduledScreenshotRecorder extends ReplayScreenshotRecorder { late final ScheduledScreenshotRecorderCallback _callback; var _status = _Status.running; late final Duration _frameDuration; + late final ScreenshotRetrier _retrier; // late final _idleFrameFiller = _IdleFrameFiller(_frameDuration, _onScreenshot); @override @@ -35,7 +35,9 @@ class ScheduledScreenshotRecorder extends ReplayScreenshotRecorder { _frameDuration = Duration(milliseconds: 1000 ~/ config.frameRate); assert(_frameDuration.inMicroseconds > 0); - _scheduler = Scheduler(_frameDuration, _capture, _addPostFrameCallback); + _retrier = ScreenshotRetrier(this, options, _onImageCaptured); + _scheduler = Scheduler( + _frameDuration, _retrier.capture, _retrier.ensureFrameAndAddCallback); if (callback != null) { _callback = callback; @@ -46,12 +48,6 @@ class ScheduledScreenshotRecorder extends ReplayScreenshotRecorder { _callback = callback; } - void _addPostFrameCallback(FrameCallback callback) { - options.bindingUtils.instance! - ..ensureVisualUpdate() - ..addPostFrameCallback(callback); - } - void start() { assert(() { // The following fails if callback hasn't been provided @@ -66,7 +62,13 @@ class ScheduledScreenshotRecorder extends ReplayScreenshotRecorder { _startScheduler(); } + Future _stopScheduler() { + _retrier.stopped = true; + return _scheduler.stop(); + } + void _startScheduler() { + _retrier.stopped = false; _scheduler.start(); // We need to schedule a frame because if this happens in-between user @@ -79,8 +81,8 @@ class ScheduledScreenshotRecorder extends ReplayScreenshotRecorder { Future stop() async { options.logger(SentryLevel.debug, "$logName: stopping capture."); _status = _Status.stopped; - await _scheduler.stop(); - // await Future.wait([_scheduler.stop(), _idleFrameFiller.stop()]); + await _stopScheduler(); + // await Future.wait([_stopScheduler(), _idleFrameFiller.stop()]); options.logger(SentryLevel.debug, "$logName: capture stopped."); } @@ -88,7 +90,7 @@ class ScheduledScreenshotRecorder extends ReplayScreenshotRecorder { if (_status == _Status.running) { _status = _Status.paused; // _idleFrameFiller.pause(); - await _scheduler.stop(); + await _stopScheduler(); } } @@ -100,23 +102,10 @@ class ScheduledScreenshotRecorder extends ReplayScreenshotRecorder { } } - void _capture(Duration sinceSchedulerEpoch) => capture(_onImageCaptured); - - Future _onImageCaptured(Screenshot capturedScreenshot) async { - final image = capturedScreenshot.image; + Future _onImageCaptured(ScreenshotPng screenshot) async { if (_status == _Status.running) { - var imageData = await image.toByteData(format: ImageByteFormat.png); - if (imageData != null) { - final screenshot = ScreenshotPng( - image.width, image.height, imageData, capturedScreenshot.timestamp); - await _onScreenshot(screenshot, true); - // _idleFrameFiller.actualFrameReceived(screenshot); - } else { - options.logger( - SentryLevel.debug, - '$logName: failed to convert screenshot to PNG, ' - 'toByteData() returned null. (${image.width}x${image.height} pixels)'); - } + await _onScreenshot(screenshot, true); + // _idleFrameFiller.actualFrameReceived(screenshot); } else { // drop any screenshots from callbacks if the replay has already been stopped/paused. options.logger(SentryLevel.debug, @@ -136,16 +125,6 @@ class ScheduledScreenshotRecorder extends ReplayScreenshotRecorder { } } -@internal -@immutable -class ScreenshotPng { - final int width; - final int height; - final ByteData data; - final DateTime timestamp; - - const ScreenshotPng(this.width, this.height, this.data, this.timestamp); -} // TODO this is currently unused because we've decided to capture on every // frame. Consider removing if we don't reverse the decision in the future. diff --git a/flutter/lib/src/screenshot/recorder.dart b/flutter/lib/src/screenshot/recorder.dart index 979fb6a313..9729641e55 100644 --- a/flutter/lib/src/screenshot/recorder.dart +++ b/flutter/lib/src/screenshot/recorder.dart @@ -2,6 +2,7 @@ import 'dart:async'; import 'dart:developer'; import 'dart:ui'; +import 'package:flutter/foundation.dart'; import 'package:flutter/rendering.dart'; import 'package:flutter/widgets.dart' as widgets; import 'package:flutter/material.dart' as material; @@ -23,7 +24,6 @@ class ScreenshotRecorder { @protected final String logName; - bool _warningLogged = false; late final SentryMaskingConfig? _maskingConfig; @@ -60,7 +60,7 @@ class ScreenshotRecorder { /// To prevent accidental addition of await before that happens, /// /// THIS FUNCTION MUST NOT BE ASYNC. - Future capture(Future Function(Screenshot) callback) { + Future capture(Future Function(ScreenshotPng) callback) { try { final flow = Flow.begin(); Timeline.startSync('Sentry::captureScreenshot', flow: flow); @@ -180,10 +180,11 @@ class _Capture { /// /// See [task] which is what gets completed with the callback result. void Function() createTask( - Future futureImage, - Future Function(Screenshot) callback, - List? obscureItems, - Flow flow) { + Future futureImage, + Future Function(ScreenshotPng) callback, + List? obscureItems, + Flow flow, + ) { final timestamp = DateTime.now(); return () async { Timeline.startSync('Sentry::renderScreenshot', flow: flow); @@ -208,9 +209,15 @@ class _Capture { final finalImage = await picture.toImage(width, height); Timeline.finishSync(); // Sentry::screenshotToImage try { + Timeline.startSync('Sentry::screenshotToPng', flow: flow); + final imageData = + await finalImage.toByteData(format: ImageByteFormat.png); + final png = ScreenshotPng(finalImage.width, finalImage.height, + imageData!.buffer.asUint8List(), timestamp); + Timeline.finishSync(); // Sentry::screenshotToPng + Timeline.startSync('Sentry::screenshotCallback', flow: flow); - _completer - .complete(await callback(Screenshot(finalImage, timestamp))); + _completer.complete(await callback(png)); Timeline.finishSync(); // Sentry::screenshotCallback } finally { finalImage.dispose(); // image needs to be disposed-of manually @@ -297,9 +304,19 @@ extension on widgets.BuildContext { } @internal -class Screenshot { - final Image image; +@immutable +class ScreenshotPng { + final int width; + final int height; + final Uint8List data; final DateTime timestamp; - const Screenshot(this.image, this.timestamp); + const ScreenshotPng(this.width, this.height, this.data, this.timestamp); + + bool hasSameImageAs(ScreenshotPng other) { + if (other.width != width || other.height != height) { + return false; + } + return listEquals(data, other.data); + } } diff --git a/flutter/lib/src/screenshot/retrier.dart b/flutter/lib/src/screenshot/retrier.dart new file mode 100644 index 0000000000..b77b055b3a --- /dev/null +++ b/flutter/lib/src/screenshot/retrier.dart @@ -0,0 +1,64 @@ +import 'dart:async'; + +import 'package:flutter/scheduler.dart'; +import 'package:meta/meta.dart'; + +import '../../sentry_flutter.dart'; +import 'recorder.dart'; + +/// We're facing an issue: the tree walked with visitChildElements() is out of +/// sync to what is currently rendered by RenderRepaintBoundary.toImage(), +/// even though there's no async gap between these two. This causes masks to +/// be off during repaints, e.g. when scrolling a view or when text is rendered +/// in different places between two screens. This is most easily reproducible +/// when there's no animation between the two screens. +/// For example, Spotube's Search vs Library (2nd and 3rd bottom bar buttons). +/// +/// To get around this issue, we're taking two subsequent screenshots +/// (after two frames) and only actually capture a screenshot if the +/// two are exactly the same. +/// We only do these if masking is enabled. +@internal +class ScreenshotRetrier { + final SentryFlutterOptions _options; + final ScreenshotRecorder _recorder; + final Future Function(ScreenshotPng screenshot) _callback; + ScreenshotPng? _previousScreenshot; + int _tries = 0; + bool stopped = false; + + ScreenshotRetrier(this._recorder, this._options, this._callback) { + assert(_options.screenshotRetries >= 1, + "Cannot use ScreenshotRetrier if we cannot retry at least once."); + } + + void ensureFrameAndAddCallback(FrameCallback callback) { + _options.bindingUtils.instance! + ..ensureVisualUpdate() + ..addPostFrameCallback(callback); + } + + Future capture(Duration sinceSchedulerEpoch) { + _tries++; + return _recorder.capture(_onImageCaptured); + } + + Future _onImageCaptured(ScreenshotPng screenshot) async { + if (stopped) { + _tries = 0; + return; + } + + final prevScreenshot = _previousScreenshot; + _previousScreenshot = screenshot; + if (prevScreenshot != null && prevScreenshot.hasSameImageAs(screenshot)) { + _tries = 0; + await _callback(screenshot); + } else if (_tries > _options.screenshotRetries) { + throw Exception('Failed to capture a stable screenshot. ' + 'Giving up after $_tries tries.'); + } else { + ensureFrameAndAddCallback(capture); + } + } +} diff --git a/flutter/lib/src/sentry_flutter_options.dart b/flutter/lib/src/sentry_flutter_options.dart index 56c20161b9..8d4411c7a8 100644 --- a/flutter/lib/src/sentry_flutter_options.dart +++ b/flutter/lib/src/sentry_flutter_options.dart @@ -205,6 +205,10 @@ class SentryFlutterOptions extends SentryOptions { /// from the function, no screenshot will be attached. BeforeCaptureCallback? beforeCaptureScreenshot; + /// See [ScreenshotRetrier] + @meta.internal + int screenshotRetries = 5; + /// Enable or disable automatic breadcrumbs for User interactions Using [Listener] /// /// Requires adding the [SentryUserInteractionWidget] to the widget tree. diff --git a/flutter/test/screenshot/recorder_test.dart b/flutter/test/screenshot/recorder_test.dart index 109b32e575..3f18cfc29b 100644 --- a/flutter/test/screenshot/recorder_test.dart +++ b/flutter/test/screenshot/recorder_test.dart @@ -142,8 +142,7 @@ class _Fixture { return fixture; } - Future capture() => sut.capture((Screenshot screenshot) { - final image = screenshot.image; - return Future.value("${image.width}x${image.height}"); + Future capture() => sut.capture((screenshot) { + return Future.value("${screenshot.width}x${screenshot.height}"); }); } From d0c6c01ad3e0412d8dc1dfada8b1cfcb967d8b2b Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Thu, 2 Jan 2025 22:51:38 +0100 Subject: [PATCH 02/28] improve timestamps --- .../screenshot_event_processor.dart | 4 ++-- flutter/lib/src/screenshot/recorder.dart | 11 +++++++---- flutter/lib/src/screenshot/retrier.dart | 15 +++++++++++++-- 3 files changed, 22 insertions(+), 8 deletions(-) diff --git a/flutter/lib/src/event_processor/screenshot_event_processor.dart b/flutter/lib/src/event_processor/screenshot_event_processor.dart index 13f6c74721..7666b7cbee 100644 --- a/flutter/lib/src/event_processor/screenshot_event_processor.dart +++ b/flutter/lib/src/event_processor/screenshot_event_processor.dart @@ -125,6 +125,6 @@ class ScreenshotEventProcessor implements EventProcessor { } @internal - Future createScreenshot() => _recorder - .capture((ScreenshotPng screenshot) => Future.value(screenshot.data)); + Future createScreenshot() => + _recorder.capture((screenshot) => Future.value(screenshot.data)); } diff --git a/flutter/lib/src/screenshot/recorder.dart b/flutter/lib/src/screenshot/recorder.dart index 9729641e55..174f65275a 100644 --- a/flutter/lib/src/screenshot/recorder.dart +++ b/flutter/lib/src/screenshot/recorder.dart @@ -60,9 +60,10 @@ class ScreenshotRecorder { /// To prevent accidental addition of await before that happens, /// /// THIS FUNCTION MUST NOT BE ASYNC. - Future capture(Future Function(ScreenshotPng) callback) { + Future capture(Future Function(ScreenshotPng) callback, + [Flow? flow]) { try { - final flow = Flow.begin(); + flow ??= Flow.begin(); Timeline.startSync('Sentry::captureScreenshot', flow: flow); final context = sentryScreenshotWidgetGlobalKey.currentContext; final renderObject = @@ -213,7 +214,7 @@ class _Capture { final imageData = await finalImage.toByteData(format: ImageByteFormat.png); final png = ScreenshotPng(finalImage.width, finalImage.height, - imageData!.buffer.asUint8List(), timestamp); + imageData!.buffer.asUint8List(), timestamp, flow); Timeline.finishSync(); // Sentry::screenshotToPng Timeline.startSync('Sentry::screenshotCallback', flow: flow); @@ -310,8 +311,10 @@ class ScreenshotPng { final int height; final Uint8List data; final DateTime timestamp; + final Flow flow; - const ScreenshotPng(this.width, this.height, this.data, this.timestamp); + const ScreenshotPng( + this.width, this.height, this.data, this.timestamp, this.flow); bool hasSameImageAs(ScreenshotPng other) { if (other.width != width || other.height != height) { diff --git a/flutter/lib/src/screenshot/retrier.dart b/flutter/lib/src/screenshot/retrier.dart index b77b055b3a..cdd3272b17 100644 --- a/flutter/lib/src/screenshot/retrier.dart +++ b/flutter/lib/src/screenshot/retrier.dart @@ -52,13 +52,24 @@ class ScreenshotRetrier { final prevScreenshot = _previousScreenshot; _previousScreenshot = screenshot; if (prevScreenshot != null && prevScreenshot.hasSameImageAs(screenshot)) { + // Sucessfully captured a stable screenshot (repeated at least twice). _tries = 0; - await _callback(screenshot); + if (prevScreenshot.flow.id == screenshot.flow.id) { + // If it's from the same (retry) flow, use the first screenshot timestamp. + await _callback(prevScreenshot); + } else { + // Otherwise this was called from a scheduler (in a new flow) so use + // the new timestamp. + await _callback(screenshot); + } } else if (_tries > _options.screenshotRetries) { throw Exception('Failed to capture a stable screenshot. ' 'Giving up after $_tries tries.'); } else { - ensureFrameAndAddCallback(capture); + ensureFrameAndAddCallback((Duration sinceSchedulerEpoch) { + _tries++; + _recorder.capture(_onImageCaptured, screenshot.flow); + }); } } } From 6f0f351ea1acb6ff59ccffd29569783d94783dde Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Tue, 7 Jan 2025 16:13:27 +0100 Subject: [PATCH 03/28] fix tests & future handling --- flutter/lib/src/replay/replay_recorder.dart | 4 +- flutter/lib/src/replay/scheduler.dart | 12 ++-- flutter/lib/src/screenshot/recorder.dart | 4 +- flutter/lib/src/screenshot/retrier.dart | 11 ++- flutter/test/replay/replay_native_test.dart | 20 ++---- flutter/test/replay/replay_test_util.dart | 30 ++++++++ .../test/replay/scheduled_recorder_test.dart | 27 ++----- flutter/test/replay/scheduler_test.dart | 28 +++++++- flutter/test/screenshot/recorder_test.dart | 71 +++++++++++-------- 9 files changed, 133 insertions(+), 74 deletions(-) create mode 100644 flutter/test/replay/replay_test_util.dart diff --git a/flutter/lib/src/replay/replay_recorder.dart b/flutter/lib/src/replay/replay_recorder.dart index 360ce53a5e..bb136f41e5 100644 --- a/flutter/lib/src/replay/replay_recorder.dart +++ b/flutter/lib/src/replay/replay_recorder.dart @@ -16,8 +16,8 @@ class ReplayScreenshotRecorder extends ScreenshotRecorder { @override @protected - Future executeTask(void Function() task, Flow flow) { - // Future() schedules the task to be executed asynchronously with TImer.run. + Future executeTask(Future Function() task, Flow flow) { + // Future() schedules the task to be executed asynchronously with Timer.run. return Future(task); } } diff --git a/flutter/lib/src/replay/scheduler.dart b/flutter/lib/src/replay/scheduler.dart index 4c5882356e..11e30fd516 100644 --- a/flutter/lib/src/replay/scheduler.dart +++ b/flutter/lib/src/replay/scheduler.dart @@ -2,7 +2,7 @@ import 'package:flutter/scheduler.dart'; import 'package:meta/meta.dart'; @internal -typedef SchedulerCallback = void Function(Duration); +typedef SchedulerCallback = Future Function(Duration); /// This is a low-priority scheduler. /// We're not using Timer.periodic() because it may schedule a callback @@ -16,6 +16,7 @@ class Scheduler { final Duration _interval; bool _running = false; Future? _scheduled; + Future? _runningCallback; final void Function(FrameCallback callback) _addPostFrameCallback; @@ -46,13 +47,16 @@ class Scheduler { @pragma('vm:prefer-inline') void _runAfterNextFrame() { - _scheduled = null; - _addPostFrameCallback(_run); + final runningCallback = _runningCallback ?? Future.value(); + runningCallback.whenComplete(() { + _scheduled = null; + _addPostFrameCallback(_run); + }); } void _run(Duration sinceSchedulerEpoch) { if (!_running) return; - _callback(sinceSchedulerEpoch); + _runningCallback = _callback(sinceSchedulerEpoch); _scheduleNext(); } } diff --git a/flutter/lib/src/screenshot/recorder.dart b/flutter/lib/src/screenshot/recorder.dart index 174f65275a..08b252573a 100644 --- a/flutter/lib/src/screenshot/recorder.dart +++ b/flutter/lib/src/screenshot/recorder.dart @@ -112,7 +112,7 @@ class ScreenshotRecorder { } @protected - Future executeTask(void Function() task, Flow flow) { + Future executeTask(Future Function() task, Flow flow) { // Future.sync() starts executing the function synchronously, until the // first await, i.e. it's the same as if the code was executed directly. return Future.sync(task); @@ -180,7 +180,7 @@ class _Capture { /// - call the callback /// /// See [task] which is what gets completed with the callback result. - void Function() createTask( + Future Function() createTask( Future futureImage, Future Function(ScreenshotPng) callback, List? obscureItems, diff --git a/flutter/lib/src/screenshot/retrier.dart b/flutter/lib/src/screenshot/retrier.dart index cdd3272b17..e16d7abc1e 100644 --- a/flutter/lib/src/screenshot/retrier.dart +++ b/flutter/lib/src/screenshot/retrier.dart @@ -66,10 +66,17 @@ class ScreenshotRetrier { throw Exception('Failed to capture a stable screenshot. ' 'Giving up after $_tries tries.'); } else { - ensureFrameAndAddCallback((Duration sinceSchedulerEpoch) { + final completer = Completer(); + ensureFrameAndAddCallback((Duration sinceSchedulerEpoch) async { _tries++; - _recorder.capture(_onImageCaptured, screenshot.flow); + try { + await _recorder.capture(_onImageCaptured, screenshot.flow); + completer.complete(); + } catch (e, stackTrace) { + completer.completeError(e, stackTrace); + } }); + return completer.future; } } } diff --git a/flutter/test/replay/replay_native_test.dart b/flutter/test/replay/replay_native_test.dart index 424da6706c..983f4274c6 100644 --- a/flutter/test/replay/replay_native_test.dart +++ b/flutter/test/replay/replay_native_test.dart @@ -18,6 +18,7 @@ import 'package:sentry_flutter/src/native/sentry_native_binding.dart'; import '../mocks.dart'; import '../mocks.mocks.dart'; import '../screenshot/test_widget.dart'; +import 'replay_test_util.dart'; void main() { TestWidgetsFlutterBinding.ensureInitialized(); @@ -80,6 +81,7 @@ void main() { testWidgets('sets replayID to context', (tester) async { await tester.runAsync(() async { + await pumpTestElement(tester); // verify there was no scope configured before verifyNever(hub.configureScope(any)); when(hub.configureScope(captureAny)).thenReturn(null); @@ -90,8 +92,8 @@ void main() { ? 'ReplayRecorder.start' : 'captureReplayScreenshot', replayConfig); - await tester.pumpAndSettle(const Duration(seconds: 1)); - await future; + tester.binding.scheduleFrame(); + await tester.pumpAndWaitUntil(future); // verify the replay ID was set final closure = @@ -126,20 +128,12 @@ void main() { when(hub.configureScope(captureAny)).thenReturn(null); await pumpTestElement(tester); - pumpAndSettle() => tester.pumpAndSettle(const Duration(seconds: 1)); - if (mockPlatform.isAndroid) { var callbackFinished = Completer(); nextFrame({bool wait = true}) async { final future = callbackFinished.future; - await pumpAndSettle(); - await future.timeout(Duration(milliseconds: wait ? 1000 : 100), - onTimeout: () { - if (wait) { - fail('native callback not called'); - } - }); + await tester.pumpAndWaitUntil(future, requiredToComplete: wait); } imageSizeBytes(File file) => file.readAsBytesSync().length; @@ -210,8 +204,8 @@ void main() { Future captureAndVerify() async { final future = native.invokeFromNative( 'captureReplayScreenshot', replayConfig); - await pumpAndSettle(); - final json = (await future) as Map; + final json = (await tester.pumpAndWaitUntil(future)) + as Map; expect(json['length'], greaterThan(3000)); expect(json['address'], greaterThan(0)); diff --git a/flutter/test/replay/replay_test_util.dart b/flutter/test/replay/replay_test_util.dart new file mode 100644 index 0000000000..230af1cb1e --- /dev/null +++ b/flutter/test/replay/replay_test_util.dart @@ -0,0 +1,30 @@ +import 'dart:async'; + +import 'package:flutter_test/flutter_test.dart'; + +extension ReplayWidgetTesterUtil on WidgetTester { + Future pumpAndWaitUntil(Future future, + {bool requiredToComplete = true}) async { + final timeout = + requiredToComplete ? Duration(seconds: 10) : Duration(seconds: 1); + final startTime = DateTime.now(); + bool completed = false; + do { + await pumpAndSettle(const Duration(seconds: 1)); + await Future.delayed(const Duration(milliseconds: 1)); + completed = await future + .then((v) => true) + .timeout(const Duration(milliseconds: 10), onTimeout: () => false); + } while (!completed && DateTime.now().difference(startTime) < timeout); + + if (requiredToComplete) { + if (!completed) { + throw TimeoutException( + 'Future not completed', DateTime.now().difference(startTime)); + } + return future; + } else { + return Future.value(null); + } + } +} diff --git a/flutter/test/replay/scheduled_recorder_test.dart b/flutter/test/replay/scheduled_recorder_test.dart index dffd21592d..add5c67278 100644 --- a/flutter/test/replay/scheduled_recorder_test.dart +++ b/flutter/test/replay/scheduled_recorder_test.dart @@ -4,7 +4,6 @@ library dart_test; import 'dart:async'; -import 'dart:developer'; import 'package:flutter_test/flutter_test.dart'; import 'package:sentry_flutter/src/replay/scheduled_recorder.dart'; @@ -12,6 +11,7 @@ import 'package:sentry_flutter/src/replay/scheduled_recorder_config.dart'; import '../mocks.dart'; import '../screenshot/test_widget.dart'; +import 'replay_test_util.dart'; void main() async { TestWidgetsFlutterBinding.ensureInitialized(); @@ -34,14 +34,14 @@ void main() async { class _Fixture { final WidgetTester _tester; - late final _TestScheduledRecorder _sut; + late final ScheduledScreenshotRecorder _sut; final capturedImages = []; late Completer _completer; ScheduledScreenshotRecorder get sut => _sut; _Fixture._(this._tester) { - _sut = _TestScheduledRecorder( + _sut = ScheduledScreenshotRecorder( ScheduledScreenshotRecorderConfig( width: 1000, height: 1000, @@ -50,9 +50,7 @@ class _Fixture { defaultTestOptions()..bindingUtils = TestBindingWrapper(), (image, isNewlyCaptured) async { capturedImages.add('${image.width}x${image.height}'); - if (!_completer.isCompleted) { - _completer.complete(); - } + _completer.complete(); }, ); } @@ -67,19 +65,8 @@ class _Fixture { Future nextFrame(bool imageIsExpected) async { _completer = Completer(); _tester.binding.scheduleFrame(); - await _tester.pumpAndSettle(const Duration(seconds: 1)); - if (imageIsExpected) { - await _completer.future.timeout(Duration(seconds: 10)); - } - } -} - -class _TestScheduledRecorder extends ScheduledScreenshotRecorder { - _TestScheduledRecorder(super.config, super.options, super.callback); - - @override - Future executeTask(void Function() task, Flow flow) { - task(); - return Future.value(); + await _tester.pumpAndWaitUntil(_completer.future, + requiredToComplete: imageIsExpected); + expect(_completer.isCompleted, imageIsExpected); } } diff --git a/flutter/test/replay/scheduler_test.dart b/flutter/test/replay/scheduler_test.dart index 300fa25280..14601e27b5 100644 --- a/flutter/test/replay/scheduler_test.dart +++ b/flutter/test/replay/scheduler_test.dart @@ -1,3 +1,5 @@ +import 'dart:async'; + import 'package:flutter/scheduler.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:sentry_flutter/src/replay/scheduler.dart'; @@ -51,6 +53,25 @@ void main() { await fixture.drawFrame(); expect(fixture.calls, 2); }); + + test('does not trigger until previous call finished', () async { + final guard = Completer(); + var fixture = _Fixture((_) async => guard.future); + + fixture.sut.start(); + + expect(fixture.calls, 0); + await fixture.drawFrame(); + expect(fixture.calls, 1); + await fixture.drawFrame(); + await fixture.drawFrame(); + await fixture.drawFrame(); + expect(fixture.calls, 1); + + guard.complete(); + await fixture.drawFrame(); + expect(fixture.calls, 2); + }); } class _Fixture { @@ -59,10 +80,13 @@ class _Fixture { FrameCallback? registeredCallback; var _frames = 0; - _Fixture() { + _Fixture([SchedulerCallback? callback]) { sut = Scheduler( const Duration(milliseconds: 1), - (_) async => calls++, + (timestamp) async { + calls++; + await callback?.call(timestamp); + }, _addPostFrameCallbackMock, ); } diff --git a/flutter/test/screenshot/recorder_test.dart b/flutter/test/screenshot/recorder_test.dart index 3f18cfc29b..a09fda2718 100644 --- a/flutter/test/screenshot/recorder_test.dart +++ b/flutter/test/screenshot/recorder_test.dart @@ -23,55 +23,68 @@ void main() async { // The `device screen resolution = logical resolution * devicePixelRatio` testWidgets('captures full resolution images - portrait', (tester) async { - await tester.binding.setSurfaceSize(Size(2000, 4000)); - final fixture = await _Fixture.create(tester); + await tester.runAsync(() async { + await tester.binding.setSurfaceSize(Size(20, 40)); + final fixture = await _Fixture.create(tester); - //devicePixelRatio is 3.0 therefore the resolution multiplied by 3 - expect(await fixture.capture(), '6000x12000'); + //devicePixelRatio is 3.0 therefore the resolution multiplied by 3 + expect(await fixture.capture(), '60x120'); + }); }); testWidgets('captures full resolution images - landscape', (tester) async { - await tester.binding.setSurfaceSize(Size(4000, 2000)); - final fixture = await _Fixture.create(tester); + await tester.runAsync(() async { + await tester.binding.setSurfaceSize(Size(40, 20)); + final fixture = await _Fixture.create(tester); - //devicePixelRatio is 3.0 therefore the resolution multiplied by 3 - expect(await fixture.capture(), '12000x6000'); + //devicePixelRatio is 3.0 therefore the resolution multiplied by 3 + expect(await fixture.capture(), '120x60'); + }); }); testWidgets('captures high resolution images - portrait', (tester) async { - await tester.binding.setSurfaceSize(Size(2000, 4000)); - final targetResolution = SentryScreenshotQuality.high.targetResolution(); - final fixture = await _Fixture.create(tester, - width: targetResolution, height: targetResolution); + await tester.runAsync(() async { + await tester.binding.setSurfaceSize(Size(20, 40)); + final targetResolution = SentryScreenshotQuality.high.targetResolution(); + final fixture = await _Fixture.create(tester, + width: targetResolution, height: targetResolution); - expect(await fixture.capture(), '960x1920'); + expect(await fixture.capture(), '960x1920'); + }); }); testWidgets('captures high resolution images - landscape', (tester) async { - await tester.binding.setSurfaceSize(Size(4000, 2000)); - final targetResolution = SentryScreenshotQuality.high.targetResolution(); - final fixture = await _Fixture.create(tester, - width: targetResolution, height: targetResolution); + await tester.runAsync(() async { + await tester.binding.setSurfaceSize(Size(40, 20)); + final targetResolution = SentryScreenshotQuality.high.targetResolution(); + final fixture = await _Fixture.create(tester, + width: targetResolution, height: targetResolution); - expect(await fixture.capture(), '1920x960'); + expect(await fixture.capture(), '1920x960'); + }); }); testWidgets('captures medium resolution images', (tester) async { - await tester.binding.setSurfaceSize(Size(2000, 4000)); - final targetResolution = SentryScreenshotQuality.medium.targetResolution(); - final fixture = await _Fixture.create(tester, - width: targetResolution, height: targetResolution); - - expect(await fixture.capture(), '640x1280'); + await tester.runAsync(() async { + await tester.binding.setSurfaceSize(Size(20, 40)); + final targetResolution = + SentryScreenshotQuality.medium.targetResolution(); + final fixture = await _Fixture.create(tester, + width: targetResolution, height: targetResolution); + + expect(await fixture.capture(), '640x1280'); + }); }); testWidgets('captures low resolution images', (tester) async { - await tester.binding.setSurfaceSize(Size(2000, 4000)); - final targetResolution = SentryScreenshotQuality.low.targetResolution(); - final fixture = await _Fixture.create(tester, - width: targetResolution, height: targetResolution); + await tester.runAsync(() async { + await tester.binding.setSurfaceSize(Size(20, 40)); + final targetResolution = SentryScreenshotQuality.low.targetResolution(); + final fixture = await _Fixture.create(tester, + width: targetResolution, height: targetResolution); - expect(await fixture.capture(), '427x854'); + expect(await fixture.capture(), '427x854'); + }); }); testWidgets('propagates errors in test-mode', (tester) async { From 8bc4df71fb784db0be949a244ab8420093d69fa9 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Tue, 7 Jan 2025 17:24:26 +0100 Subject: [PATCH 04/28] example: debug log only in debug mode --- flutter/example/lib/main.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/flutter/example/lib/main.dart b/flutter/example/lib/main.dart index d5c7d71d71..1f1d1d65e0 100644 --- a/flutter/example/lib/main.dart +++ b/flutter/example/lib/main.dart @@ -79,7 +79,7 @@ Future setupSentry( // We can enable Sentry debug logging during development. This is likely // going to log too much for your app, but can be useful when figuring out // configuration issues, e.g. finding out why your events are not uploaded. - options.debug = true; + options.debug = kDebugMode; options.spotlight = Spotlight(enabled: true); options.enableTimeToFullDisplayTracing = true; options.enableMetrics = true; From c990fe23687d780affae94f02d6fdcd9be5054ff Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Tue, 7 Jan 2025 17:35:32 +0100 Subject: [PATCH 05/28] formatting --- flutter/test/replay/scheduler_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/flutter/test/replay/scheduler_test.dart b/flutter/test/replay/scheduler_test.dart index 14601e27b5..f7b50a6d17 100644 --- a/flutter/test/replay/scheduler_test.dart +++ b/flutter/test/replay/scheduler_test.dart @@ -67,7 +67,7 @@ void main() { await fixture.drawFrame(); await fixture.drawFrame(); expect(fixture.calls, 1); - + guard.complete(); await fixture.drawFrame(); expect(fixture.calls, 2); From 7636b2c148f217c3f5f6eb715435d3a3674d4eb7 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Tue, 7 Jan 2025 20:34:26 +0100 Subject: [PATCH 06/28] min-version compat --- flutter/lib/src/screenshot/recorder.dart | 2 ++ 1 file changed, 2 insertions(+) diff --git a/flutter/lib/src/screenshot/recorder.dart b/flutter/lib/src/screenshot/recorder.dart index 08b252573a..e7556e27e5 100644 --- a/flutter/lib/src/screenshot/recorder.dart +++ b/flutter/lib/src/screenshot/recorder.dart @@ -1,6 +1,8 @@ import 'dart:async'; import 'dart:developer'; import 'dart:ui'; +// ignore: unnecessary_import // backcompatibility for Flutter < 3.3 +import 'dart:typed_data'; import 'package:flutter/foundation.dart'; import 'package:flutter/rendering.dart'; From ae34a38595ab622732a7207cf6b8f651f3a41d70 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Wed, 8 Jan 2025 08:13:15 +0100 Subject: [PATCH 07/28] linter issue --- flutter/lib/src/sentry_flutter_options.dart | 1 + 1 file changed, 1 insertion(+) diff --git a/flutter/lib/src/sentry_flutter_options.dart b/flutter/lib/src/sentry_flutter_options.dart index 8d4411c7a8..cccd4220ff 100644 --- a/flutter/lib/src/sentry_flutter_options.dart +++ b/flutter/lib/src/sentry_flutter_options.dart @@ -11,6 +11,7 @@ import 'binding_wrapper.dart'; import 'navigation/time_to_display_tracker.dart'; import 'renderer/renderer.dart'; import 'screenshot/sentry_screenshot_quality.dart'; +import 'screenshot/retrier.dart'; import 'event_processor/screenshot_event_processor.dart'; import 'sentry_flutter.dart'; import 'sentry_privacy_options.dart'; From 9bedc015d5653ff509ceeee0cd3566e4129ecccc Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Wed, 8 Jan 2025 08:35:04 +0100 Subject: [PATCH 08/28] fix scheduler tests --- flutter/test/replay/scheduler_test.dart | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/flutter/test/replay/scheduler_test.dart b/flutter/test/replay/scheduler_test.dart index f7b50a6d17..f53d0675cc 100644 --- a/flutter/test/replay/scheduler_test.dart +++ b/flutter/test/replay/scheduler_test.dart @@ -63,9 +63,9 @@ void main() { expect(fixture.calls, 0); await fixture.drawFrame(); expect(fixture.calls, 1); - await fixture.drawFrame(); - await fixture.drawFrame(); - await fixture.drawFrame(); + await fixture + .drawFrame() + .timeout(const Duration(milliseconds: 200), onTimeout: () => null); expect(fixture.calls, 1); guard.complete(); @@ -77,7 +77,7 @@ void main() { class _Fixture { var calls = 0; late final Scheduler sut; - FrameCallback? registeredCallback; + var registeredCallback = Completer(); var _frames = 0; _Fixture([SchedulerCallback? callback]) { @@ -93,17 +93,18 @@ class _Fixture { void _addPostFrameCallbackMock(FrameCallback callback, {String debugLabel = 'callback'}) { - registeredCallback = callback; + if (!registeredCallback.isCompleted) { + registeredCallback.complete(callback); + } } factory _Fixture.started() { return _Fixture()..sut.start(); } - Future drawFrame() async { - await Future.delayed(const Duration(milliseconds: 8), () {}); - _frames++; - registeredCallback?.call(Duration(milliseconds: _frames)); - registeredCallback = null; + Future drawFrame() { + registeredCallback = Completer(); + return registeredCallback.future + .then((fn) => fn(Duration(milliseconds: ++_frames))); } } From 32f5b3589e66a425aebafb903a851d5ffafd483d Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Wed, 8 Jan 2025 10:22:16 +0100 Subject: [PATCH 09/28] cleanup --- flutter/test/replay/scheduler_test.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/flutter/test/replay/scheduler_test.dart b/flutter/test/replay/scheduler_test.dart index f53d0675cc..6ba944266b 100644 --- a/flutter/test/replay/scheduler_test.dart +++ b/flutter/test/replay/scheduler_test.dart @@ -104,7 +104,7 @@ class _Fixture { Future drawFrame() { registeredCallback = Completer(); - return registeredCallback.future - .then((fn) => fn(Duration(milliseconds: ++_frames))); + final timestamp = Duration(milliseconds: ++_frames); + return registeredCallback.future.then((fn) => fn(timestamp)); } } From 9d4a3f914ab9962c501179d3a835a0e363374432 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Wed, 8 Jan 2025 10:27:42 +0100 Subject: [PATCH 10/28] fix: scheduler test --- flutter/test/replay/scheduler_test.dart | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/flutter/test/replay/scheduler_test.dart b/flutter/test/replay/scheduler_test.dart index 6ba944266b..bd82a28910 100644 --- a/flutter/test/replay/scheduler_test.dart +++ b/flutter/test/replay/scheduler_test.dart @@ -34,8 +34,9 @@ void main() { await fixture.drawFrame(); expect(fixture.calls, 1); await fixture.drawFrame(); + expect(fixture.calls, 2); await fixture.sut.stop(); - await fixture.drawFrame(); + await fixture.drawFrame(awaitCallback: false); expect(fixture.calls, 2); }); @@ -47,7 +48,7 @@ void main() { await fixture.drawFrame(); expect(fixture.calls, 1); await fixture.sut.stop(); - await fixture.drawFrame(); + await fixture.drawFrame(awaitCallback: false); expect(fixture.calls, 1); fixture.sut.start(); await fixture.drawFrame(); @@ -63,9 +64,7 @@ void main() { expect(fixture.calls, 0); await fixture.drawFrame(); expect(fixture.calls, 1); - await fixture - .drawFrame() - .timeout(const Duration(milliseconds: 200), onTimeout: () => null); + await fixture.drawFrame(awaitCallback: false); expect(fixture.calls, 1); guard.complete(); @@ -102,9 +101,15 @@ class _Fixture { return _Fixture()..sut.start(); } - Future drawFrame() { + Future drawFrame({bool awaitCallback = true}) async { registeredCallback = Completer(); final timestamp = Duration(milliseconds: ++_frames); - return registeredCallback.future.then((fn) => fn(timestamp)); + final future = registeredCallback.future.then((fn) => fn(timestamp)); + if (awaitCallback) { + return future; + } else { + return future.timeout(const Duration(milliseconds: 200), + onTimeout: () {}); + } } } From 8550861afe59f0ff7ae125d05096222dffedcb6f Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Thu, 9 Jan 2025 20:18:03 +0100 Subject: [PATCH 11/28] improve performance --- flutter/example/devtools_options.yaml | 3 + .../screenshot_event_processor.dart | 4 +- .../src/native/cocoa/sentry_native_cocoa.dart | 11 +-- .../native/java/android_replay_recorder.dart | 17 ++-- .../lib/src/replay/scheduled_recorder.dart | 14 ++-- flutter/lib/src/screenshot/recorder.dart | 55 ++++--------- flutter/lib/src/screenshot/retrier.dart | 62 +++++++------- flutter/lib/src/screenshot/screenshot.dart | 80 +++++++++++++++++++ 8 files changed, 155 insertions(+), 91 deletions(-) create mode 100644 flutter/example/devtools_options.yaml create mode 100644 flutter/lib/src/screenshot/screenshot.dart diff --git a/flutter/example/devtools_options.yaml b/flutter/example/devtools_options.yaml new file mode 100644 index 0000000000..fa0b357c4f --- /dev/null +++ b/flutter/example/devtools_options.yaml @@ -0,0 +1,3 @@ +description: This file stores settings for Dart & Flutter DevTools. +documentation: https://docs.flutter.dev/tools/devtools/extensions#configure-extension-enablement-states +extensions: diff --git a/flutter/lib/src/event_processor/screenshot_event_processor.dart b/flutter/lib/src/event_processor/screenshot_event_processor.dart index 7666b7cbee..23ee438948 100644 --- a/flutter/lib/src/event_processor/screenshot_event_processor.dart +++ b/flutter/lib/src/event_processor/screenshot_event_processor.dart @@ -125,6 +125,6 @@ class ScreenshotEventProcessor implements EventProcessor { } @internal - Future createScreenshot() => - _recorder.capture((screenshot) => Future.value(screenshot.data)); + Future createScreenshot() => _recorder.capture( + (screenshot) => screenshot.pngData.then((v) => v.buffer.asUint8List())); } diff --git a/flutter/lib/src/native/cocoa/sentry_native_cocoa.dart b/flutter/lib/src/native/cocoa/sentry_native_cocoa.dart index 6a12d5396a..0eef1aa495 100644 --- a/flutter/lib/src/native/cocoa/sentry_native_cocoa.dart +++ b/flutter/lib/src/native/cocoa/sentry_native_cocoa.dart @@ -55,14 +55,15 @@ class SentryNativeCocoa extends SentryNativeChannel { } final completer = Completer(); - final retrier = - ScreenshotRetrier(_replayRecorder!, options, (image) async { + final retrier = ScreenshotRetrier(_replayRecorder!, options, + (screenshot) async { + final pngData = await screenshot.pngData; options.logger( SentryLevel.debug, 'Replay: captured screenshot (' - '${image.width}x${image.height} pixels, ' - '${image.data.lengthInBytes} bytes)'); - completer.complete(image.data); + '${screenshot.width}x${screenshot.height} pixels, ' + '${pngData.lengthInBytes} bytes)'); + completer.complete(pngData.buffer.asUint8List()); }); retrier.ensureFrameAndAddCallback((msSinceEpoch) { retrier.capture(msSinceEpoch).onError(completer.completeError); diff --git a/flutter/lib/src/native/java/android_replay_recorder.dart b/flutter/lib/src/native/java/android_replay_recorder.dart index c6bddd6418..2719558fef 100644 --- a/flutter/lib/src/native/java/android_replay_recorder.dart +++ b/flutter/lib/src/native/java/android_replay_recorder.dart @@ -2,7 +2,7 @@ import 'package:meta/meta.dart'; import '../../../sentry_flutter.dart'; import '../../replay/scheduled_recorder.dart'; -import '../../screenshot/recorder.dart'; +import '../../screenshot/screenshot.dart'; import '../sentry_safe_method_channel.dart'; @internal @@ -16,19 +16,20 @@ class AndroidReplayRecorder extends ScheduledScreenshotRecorder { } Future _addReplayScreenshot( - ScreenshotPng screenshot, bool isNewlyCaptured) async { + Screenshot screenshot, bool isNewlyCaptured) async { final timestamp = screenshot.timestamp.millisecondsSinceEpoch; final filePath = "$_cacheDir/$timestamp.png"; - options.logger( - SentryLevel.debug, - '$logName: saving ${isNewlyCaptured ? 'new' : 'repeated'} screenshot to' - ' $filePath (${screenshot.width}x${screenshot.height} pixels, ' - '${screenshot.data.lengthInBytes} bytes)'); try { + final pngData = await screenshot.pngData; + options.logger( + SentryLevel.debug, + '$logName: saving ${isNewlyCaptured ? 'new' : 'repeated'} screenshot to' + ' $filePath (${screenshot.width}x${screenshot.height} pixels, ' + '${pngData.lengthInBytes} bytes)'); await options.fileSystem .file(filePath) - .writeAsBytes(screenshot.data.buffer.asUint8List(), flush: true); + .writeAsBytes(pngData.buffer.asUint8List(), flush: true); await _channel.invokeMethod( 'addReplayScreenshot', diff --git a/flutter/lib/src/replay/scheduled_recorder.dart b/flutter/lib/src/replay/scheduled_recorder.dart index 881d7ef2ff..a34c768104 100644 --- a/flutter/lib/src/replay/scheduled_recorder.dart +++ b/flutter/lib/src/replay/scheduled_recorder.dart @@ -3,15 +3,15 @@ import 'dart:async'; import 'package:meta/meta.dart'; import '../../sentry_flutter.dart'; -import '../screenshot/recorder.dart'; import '../screenshot/retrier.dart'; +import '../screenshot/screenshot.dart'; import 'replay_recorder.dart'; import 'scheduled_recorder_config.dart'; import 'scheduler.dart'; @internal typedef ScheduledScreenshotRecorderCallback = Future Function( - ScreenshotPng screenshot, bool isNewlyCaptured); + Screenshot screenshot, bool isNewlyCaptured); @internal class ScheduledScreenshotRecorder extends ReplayScreenshotRecorder { @@ -102,7 +102,7 @@ class ScheduledScreenshotRecorder extends ReplayScreenshotRecorder { } } - Future _onImageCaptured(ScreenshotPng screenshot) async { + Future _onImageCaptured(Screenshot screenshot) async { if (_status == _Status.running) { await _onScreenshot(screenshot, true); // _idleFrameFiller.actualFrameReceived(screenshot); @@ -114,7 +114,7 @@ class ScheduledScreenshotRecorder extends ReplayScreenshotRecorder { } Future _onScreenshot( - ScreenshotPng screenshot, bool isNewlyCaptured) async { + Screenshot screenshot, bool isNewlyCaptured) async { if (_status == _Status.running) { await _callback(screenshot, isNewlyCaptured); } else { @@ -138,11 +138,11 @@ class ScheduledScreenshotRecorder extends ReplayScreenshotRecorder { // final ScheduledScreenshotRecorderCallback _callback; // var _status = _Status.running; // Future? _scheduled; -// ScreenshotPng? _mostRecent; +// Screenshot? _mostRecent; // _IdleFrameFiller(this._interval, this._callback); -// void actualFrameReceived(ScreenshotPng screenshot) { +// void actualFrameReceived(Screenshot screenshot) { // // We store the most recent frame but only repost it when the most recent // // one is the same instance (unchanged). // _mostRecent = screenshot; @@ -171,7 +171,7 @@ class ScheduledScreenshotRecorder extends ReplayScreenshotRecorder { // } // } -// void repostLater(Duration delay, ScreenshotPng screenshot) { +// void repostLater(Duration delay, Screenshot screenshot) { // _scheduled = Future.delayed(delay, () async { // if (_status == _Status.stopped) { // return; diff --git a/flutter/lib/src/screenshot/recorder.dart b/flutter/lib/src/screenshot/recorder.dart index e7556e27e5..087c07ffa6 100644 --- a/flutter/lib/src/screenshot/recorder.dart +++ b/flutter/lib/src/screenshot/recorder.dart @@ -1,10 +1,7 @@ import 'dart:async'; import 'dart:developer'; import 'dart:ui'; -// ignore: unnecessary_import // backcompatibility for Flutter < 3.3 -import 'dart:typed_data'; -import 'package:flutter/foundation.dart'; import 'package:flutter/rendering.dart'; import 'package:flutter/widgets.dart' as widgets; import 'package:flutter/material.dart' as material; @@ -14,6 +11,7 @@ import 'package:meta/meta.dart'; import '../../sentry_flutter.dart'; import 'masking_config.dart'; import 'recorder_config.dart'; +import 'screenshot.dart'; import 'widget_filter.dart'; @internal @@ -62,8 +60,7 @@ class ScreenshotRecorder { /// To prevent accidental addition of await before that happens, /// /// THIS FUNCTION MUST NOT BE ASYNC. - Future capture(Future Function(ScreenshotPng) callback, - [Flow? flow]) { + Future capture(Future Function(Screenshot) callback, [Flow? flow]) { try { flow ??= Flow.begin(); Timeline.startSync('Sentry::captureScreenshot', flow: flow); @@ -184,7 +181,7 @@ class _Capture { /// See [task] which is what gets completed with the callback result. Future Function() createTask( Future futureImage, - Future Function(ScreenshotPng) callback, + Future Function(Screenshot) callback, List? obscureItems, Flow flow, ) { @@ -207,27 +204,23 @@ class _Capture { final picture = recorder.endRecording(); Timeline.finishSync(); // Sentry::renderScreenshot + late Image finalImage; try { Timeline.startSync('Sentry::screenshotToImage', flow: flow); - final finalImage = await picture.toImage(width, height); + finalImage = await picture.toImage(width, height); Timeline.finishSync(); // Sentry::screenshotToImage - try { - Timeline.startSync('Sentry::screenshotToPng', flow: flow); - final imageData = - await finalImage.toByteData(format: ImageByteFormat.png); - final png = ScreenshotPng(finalImage.width, finalImage.height, - imageData!.buffer.asUint8List(), timestamp, flow); - Timeline.finishSync(); // Sentry::screenshotToPng - - Timeline.startSync('Sentry::screenshotCallback', flow: flow); - _completer.complete(await callback(png)); - Timeline.finishSync(); // Sentry::screenshotCallback - } finally { - finalImage.dispose(); // image needs to be disposed-of manually - } } finally { picture.dispose(); } + + final screenshot = Screenshot(finalImage, timestamp, flow); + try { + Timeline.startSync('Sentry::screenshotCallback', flow: flow); + _completer.complete(await callback(screenshot)); + Timeline.finishSync(); // Sentry::screenshotCallback + } finally { + screenshot.dispose(); + } }; } @@ -305,23 +298,3 @@ extension on widgets.BuildContext { return null; } } - -@internal -@immutable -class ScreenshotPng { - final int width; - final int height; - final Uint8List data; - final DateTime timestamp; - final Flow flow; - - const ScreenshotPng( - this.width, this.height, this.data, this.timestamp, this.flow); - - bool hasSameImageAs(ScreenshotPng other) { - if (other.width != width || other.height != height) { - return false; - } - return listEquals(data, other.data); - } -} diff --git a/flutter/lib/src/screenshot/retrier.dart b/flutter/lib/src/screenshot/retrier.dart index e16d7abc1e..4b4ab92231 100644 --- a/flutter/lib/src/screenshot/retrier.dart +++ b/flutter/lib/src/screenshot/retrier.dart @@ -5,6 +5,7 @@ import 'package:meta/meta.dart'; import '../../sentry_flutter.dart'; import 'recorder.dart'; +import 'screenshot.dart'; /// We're facing an issue: the tree walked with visitChildElements() is out of /// sync to what is currently rendered by RenderRepaintBoundary.toImage(), @@ -22,8 +23,8 @@ import 'recorder.dart'; class ScreenshotRetrier { final SentryFlutterOptions _options; final ScreenshotRecorder _recorder; - final Future Function(ScreenshotPng screenshot) _callback; - ScreenshotPng? _previousScreenshot; + final Future Function(Screenshot screenshot) _callback; + Screenshot? _previousScreenshot; int _tries = 0; bool stopped = false; @@ -43,40 +44,45 @@ class ScreenshotRetrier { return _recorder.capture(_onImageCaptured); } - Future _onImageCaptured(ScreenshotPng screenshot) async { + Future _onImageCaptured(Screenshot screenshot) async { if (stopped) { _tries = 0; return; } final prevScreenshot = _previousScreenshot; - _previousScreenshot = screenshot; - if (prevScreenshot != null && prevScreenshot.hasSameImageAs(screenshot)) { - // Sucessfully captured a stable screenshot (repeated at least twice). - _tries = 0; - if (prevScreenshot.flow.id == screenshot.flow.id) { - // If it's from the same (retry) flow, use the first screenshot timestamp. - await _callback(prevScreenshot); + try { + _previousScreenshot = screenshot.clone(); + if (prevScreenshot != null && + await prevScreenshot.hasSameImageAs(screenshot)) { + // Sucessfully captured a stable screenshot (repeated at least twice). + _tries = 0; + if (prevScreenshot.flow.id == screenshot.flow.id) { + // If it's from the same (retry) flow, use the first screenshot timestamp. + await _callback(prevScreenshot); + } else { + // Otherwise this was called from a scheduler (in a new flow) so use + // the new timestamp. + await _callback(screenshot); + } + } else if (_tries > _options.screenshotRetries) { + throw Exception('Failed to capture a stable screenshot. ' + 'Giving up after $_tries tries.'); } else { - // Otherwise this was called from a scheduler (in a new flow) so use - // the new timestamp. - await _callback(screenshot); + final completer = Completer(); + ensureFrameAndAddCallback((Duration sinceSchedulerEpoch) async { + _tries++; + try { + await _recorder.capture(_onImageCaptured, screenshot.flow); + completer.complete(); + } catch (e, stackTrace) { + completer.completeError(e, stackTrace); + } + }); + return completer.future; } - } else if (_tries > _options.screenshotRetries) { - throw Exception('Failed to capture a stable screenshot. ' - 'Giving up after $_tries tries.'); - } else { - final completer = Completer(); - ensureFrameAndAddCallback((Duration sinceSchedulerEpoch) async { - _tries++; - try { - await _recorder.capture(_onImageCaptured, screenshot.flow); - completer.complete(); - } catch (e, stackTrace) { - completer.completeError(e, stackTrace); - } - }); - return completer.future; + } finally { + prevScreenshot?.dispose(); } } } diff --git a/flutter/lib/src/screenshot/screenshot.dart b/flutter/lib/src/screenshot/screenshot.dart new file mode 100644 index 0000000000..479a1d9b2b --- /dev/null +++ b/flutter/lib/src/screenshot/screenshot.dart @@ -0,0 +1,80 @@ +import 'dart:async'; +import 'dart:developer'; +import 'dart:ffi'; +import 'dart:ui'; +// ignore: unnecessary_import // backcompatibility for Flutter < 3.3 +import 'dart:typed_data'; + +import 'package:flutter/foundation.dart'; +import 'package:meta/meta.dart'; + +@internal +class Screenshot { + final Image _image; + final DateTime timestamp; + final Flow flow; + Future? _rawData; + Future? _pngData; + + Screenshot(this._image, this.timestamp, this.flow); + Screenshot._cloned( + this._image, this.timestamp, this.flow, this._rawData, this._pngData); + + int get width => _image.width; + int get height => _image.height; + + Future get rawData { + _rawData ??= _image + .toByteData(format: ImageByteFormat.rawUnmodified) + .then((data) => data!); + return _rawData!; + } + + Future get pngData { + _pngData ??= + _image.toByteData(format: ImageByteFormat.png).then((data) => data!); + return _pngData!; + } + + Future hasSameImageAs(Screenshot other) async { + if (other.width != width || other.height != height) { + return false; + } + + final thisData = await rawData; + final otherData = await other.rawData; + if (thisData.lengthInBytes != otherData.lengthInBytes) { + return false; + } + if (identical(thisData, otherData)) { + return true; + } + + // Note: listEquals() is slow because it compares each byte pair. + // Ideally, we would use memcmp with Uint8List.address but that's only + // available since Dart 3.5.0. + // For now, the best we can do is compare by chunks + var pos = 0; + while (pos + 8 < thisData.lengthInBytes) { + if (thisData.getInt64(pos) != otherData.getInt64(pos)) { + return false; + } + pos += 8; + } + while (pos < thisData.lengthInBytes) { + if (thisData.getUint8(pos) != otherData.getUint8(pos)) { + return false; + } + pos++; + } + return true; + } + + Screenshot clone() { + assert(!_image.debugDisposed); + return Screenshot._cloned( + _image.clone(), timestamp, flow, _rawData, _pngData); + } + + void dispose() => _image.dispose(); +} From 05574bb4e8fee4d900a82409e1bab409ab593bd1 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Fri, 10 Jan 2025 10:03:51 +0100 Subject: [PATCH 12/28] improve screenshot comparison performance --- flutter/lib/src/screenshot/screenshot.dart | 77 ++++++++++++---------- 1 file changed, 44 insertions(+), 33 deletions(-) diff --git a/flutter/lib/src/screenshot/screenshot.dart b/flutter/lib/src/screenshot/screenshot.dart index 479a1d9b2b..4df34e2111 100644 --- a/flutter/lib/src/screenshot/screenshot.dart +++ b/flutter/lib/src/screenshot/screenshot.dart @@ -1,9 +1,6 @@ import 'dart:async'; import 'dart:developer'; -import 'dart:ffi'; import 'dart:ui'; -// ignore: unnecessary_import // backcompatibility for Flutter < 3.3 -import 'dart:typed_data'; import 'package:flutter/foundation.dart'; import 'package:meta/meta.dart'; @@ -24,57 +21,71 @@ class Screenshot { int get height => _image.height; Future get rawData { - _rawData ??= _image - .toByteData(format: ImageByteFormat.rawUnmodified) - .then((data) => data!); + _rawData ??= _encode(ImageByteFormat.rawUnmodified); return _rawData!; } Future get pngData { - _pngData ??= - _image.toByteData(format: ImageByteFormat.png).then((data) => data!); + _pngData ??= _encode(ImageByteFormat.png); return _pngData!; } + Future _encode(ImageByteFormat format) async { + Timeline.startSync('Sentry::screenshotTo${format.name}', flow: flow); + final result = + await _image.toByteData(format: format).then((data) => data!); + Timeline.finishSync(); + return result; + } + Future hasSameImageAs(Screenshot other) async { if (other.width != width || other.height != height) { return false; } - final thisData = await rawData; - final otherData = await other.rawData; - if (thisData.lengthInBytes != otherData.lengthInBytes) { - return false; - } - if (identical(thisData, otherData)) { + return listEquals(await rawData, await other.rawData); + } + + Screenshot clone() { + assert(!_image.debugDisposed); + return Screenshot._cloned( + _image.clone(), timestamp, flow, _rawData, _pngData); + } + + void dispose() => _image.dispose(); + + /// Efficiently compares two memory regions for data equality.. + /// Ideally, we would use memcmp with Uint8List.address but that's only + /// available since Dart 3.5.0. + /// For now, the best we can do is compare by chunks of 8 bytes. + @visibleForTesting + static bool listEquals(ByteData dataA, ByteData dataB) { + if (identical(dataA, dataB)) { return true; } + if (dataA.lengthInBytes != dataB.lengthInBytes) { + return false; + } - // Note: listEquals() is slow because it compares each byte pair. - // Ideally, we would use memcmp with Uint8List.address but that's only - // available since Dart 3.5.0. - // For now, the best we can do is compare by chunks - var pos = 0; - while (pos + 8 < thisData.lengthInBytes) { - if (thisData.getInt64(pos) != otherData.getInt64(pos)) { + final numWords = dataA.lengthInBytes ~/ 8; + final wordsA = dataA.buffer.asUint64List(0, numWords); + final wordsB = dataB.buffer.asUint64List(0, numWords); + + for (var i = 0; i < wordsA.length; i++) { + if (wordsA[i] != wordsB[i]) { return false; } - pos += 8; } - while (pos < thisData.lengthInBytes) { - if (thisData.getUint8(pos) != otherData.getUint8(pos)) { + + // Compare any remaining bytes. + final bytesA = dataA.buffer.asUint8List(wordsA.lengthInBytes); + final bytesB = dataA.buffer.asUint8List(wordsA.lengthInBytes); + for (var i = 0; i < bytesA.lengthInBytes; i++) { + if (bytesA[i] != bytesB[i]) { return false; } - pos++; } - return true; - } - Screenshot clone() { - assert(!_image.debugDisposed); - return Screenshot._cloned( - _image.clone(), timestamp, flow, _rawData, _pngData); + return true; } - - void dispose() => _image.dispose(); } From ae1c5679b38e97975828ebe6671c295273bff399 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Fri, 10 Jan 2025 11:07:28 +0100 Subject: [PATCH 13/28] comments --- flutter/lib/src/screenshot/screenshot.dart | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/flutter/lib/src/screenshot/screenshot.dart b/flutter/lib/src/screenshot/screenshot.dart index 4df34e2111..354c448c25 100644 --- a/flutter/lib/src/screenshot/screenshot.dart +++ b/flutter/lib/src/screenshot/screenshot.dart @@ -55,9 +55,6 @@ class Screenshot { void dispose() => _image.dispose(); /// Efficiently compares two memory regions for data equality.. - /// Ideally, we would use memcmp with Uint8List.address but that's only - /// available since Dart 3.5.0. - /// For now, the best we can do is compare by chunks of 8 bytes. @visibleForTesting static bool listEquals(ByteData dataA, ByteData dataB) { if (identical(dataA, dataB)) { @@ -67,6 +64,12 @@ class Screenshot { return false; } + /// Ideally, we would use memcmp with Uint8List.address but that's only + /// available since Dart 3.5.0. The relevant code is commented out below and + /// Should be used once we can bump the Dart SDK in the next major version. + /// For now, the best we can do is compare by chunks of 8 bytes. + // return 0 == memcmp(dataA.address, dataB.address, dataA.lengthInBytes); + final numWords = dataA.lengthInBytes ~/ 8; final wordsA = dataA.buffer.asUint64List(0, numWords); final wordsB = dataB.buffer.asUint64List(0, numWords); @@ -89,3 +92,9 @@ class Screenshot { return true; } } + +// /// Compares the first num bytes of the block of memory pointed by ptr1 to the +// /// first num bytes pointed by ptr2, returning zero if they all match or a value +// /// different from zero representing which is greater if they do not. +// @Native(symbol: 'memcmp', isLeaf: true) +// external int memcmp(Pointer ptr1, Pointer ptr2, int len); From d010424c1c4f1eb7111d81c846d5b1225583711b Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Fri, 10 Jan 2025 11:23:28 +0100 Subject: [PATCH 14/28] screenshot list-equal fix & tests --- flutter/lib/src/screenshot/screenshot.dart | 2 +- flutter/test/screenshot/recorder_test.dart | 60 ++++++++++++++++++++++ 2 files changed, 61 insertions(+), 1 deletion(-) diff --git a/flutter/lib/src/screenshot/screenshot.dart b/flutter/lib/src/screenshot/screenshot.dart index 354c448c25..2ed11e7fcc 100644 --- a/flutter/lib/src/screenshot/screenshot.dart +++ b/flutter/lib/src/screenshot/screenshot.dart @@ -82,7 +82,7 @@ class Screenshot { // Compare any remaining bytes. final bytesA = dataA.buffer.asUint8List(wordsA.lengthInBytes); - final bytesB = dataA.buffer.asUint8List(wordsA.lengthInBytes); + final bytesB = dataB.buffer.asUint8List(wordsA.lengthInBytes); for (var i = 0; i < bytesA.lengthInBytes; i++) { if (bytesA[i] != bytesB[i]) { return false; diff --git a/flutter/test/screenshot/recorder_test.dart b/flutter/test/screenshot/recorder_test.dart index a09fda2718..85ff006748 100644 --- a/flutter/test/screenshot/recorder_test.dart +++ b/flutter/test/screenshot/recorder_test.dart @@ -3,6 +3,7 @@ @TestOn('vm') library dart_test; +import 'dart:typed_data'; import 'dart:ui'; import 'package:flutter/widgets.dart' as widgets; @@ -11,6 +12,7 @@ import 'package:sentry_flutter/sentry_flutter.dart'; import 'package:sentry_flutter/src/replay/replay_recorder.dart'; import 'package:sentry_flutter/src/screenshot/recorder.dart'; import 'package:sentry_flutter/src/screenshot/recorder_config.dart'; +import 'package:sentry_flutter/src/screenshot/screenshot.dart'; import '../mocks.dart'; import 'test_widget.dart'; @@ -136,6 +138,64 @@ void main() async { expect(sut.hasWidgetFilter, isTrue); }); }); + + group('$Screenshot', () { + test('listEquals()', () { + expect( + Screenshot.listEquals( + Uint8List(0).buffer.asByteData(), + Uint8List(0).buffer.asByteData(), + ), + isTrue); + expect( + Screenshot.listEquals( + Uint8List.fromList([1, 2, 3]).buffer.asByteData(), + Uint8List.fromList([1, 2, 3]).buffer.asByteData(), + ), + isTrue); + expect( + Screenshot.listEquals( + Uint8List.fromList([1, 0, 3]).buffer.asByteData(), + Uint8List.fromList([1, 2, 3]).buffer.asByteData(), + ), + isFalse); + expect( + Screenshot.listEquals( + Uint8List.fromList([1, 2, 3, 4, 5, 6, 7, 8]).buffer.asByteData(), + Uint8List.fromList([1, 2, 3, 4, 5, 6, 7, 8]).buffer.asByteData(), + ), + isTrue); + expect( + Screenshot.listEquals( + Uint8List.fromList([1, 2, 3, 4, 5, 6, 0, 8]).buffer.asByteData(), + Uint8List.fromList([1, 2, 3, 4, 5, 6, 7, 8]).buffer.asByteData(), + ), + isFalse); + expect( + Screenshot.listEquals( + Uint8List.fromList([1, 2, 3, 4, 5, 6, 7, 8, 9]).buffer.asByteData(), + Uint8List.fromList([1, 2, 3, 4, 5, 6, 7, 8, 9]).buffer.asByteData(), + ), + isTrue); + expect( + Screenshot.listEquals( + Uint8List.fromList([1, 2, 3, 4, 5, 6, 7, 8, 9]).buffer.asByteData(), + Uint8List.fromList([1, 2, 3, 4, 5, 6, 7, 8, 0]).buffer.asByteData(), + ), + isFalse); + + final dataA = Uint8List.fromList( + List.generate(10 * 1000 * 1000, (index) => index % 256)) + .buffer + .asByteData(); + final dataB = ByteData(dataA.lengthInBytes) + ..buffer.asUint8List().setAll(0, dataA.buffer.asUint8List()); + expect(Screenshot.listEquals(dataA, dataB), isTrue); + + dataB.setInt8(dataB.lengthInBytes >> 2, 0); + expect(Screenshot.listEquals(dataA, dataB), isFalse); + }); + }); } class _Fixture { From f62e5ee1268d2f4186842ba862210151c1e58b26 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Fri, 10 Jan 2025 11:47:44 +0100 Subject: [PATCH 15/28] min-version failure --- flutter/lib/src/screenshot/screenshot.dart | 2 ++ 1 file changed, 2 insertions(+) diff --git a/flutter/lib/src/screenshot/screenshot.dart b/flutter/lib/src/screenshot/screenshot.dart index 2ed11e7fcc..582d81853f 100644 --- a/flutter/lib/src/screenshot/screenshot.dart +++ b/flutter/lib/src/screenshot/screenshot.dart @@ -1,6 +1,8 @@ import 'dart:async'; import 'dart:developer'; import 'dart:ui'; +// ignore: unnecessary_import // backcompatibility for Flutter < 3.3 +import 'dart:typed_data'; import 'package:flutter/foundation.dart'; import 'package:meta/meta.dart'; From b78c583a78b790eb85c04a3a2e05f1da2d165845 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Fri, 10 Jan 2025 14:00:36 +0100 Subject: [PATCH 16/28] rename retrier to stabilizer --- .../lib/src/native/cocoa/sentry_native_cocoa.dart | 8 ++++---- flutter/lib/src/replay/scheduled_recorder.dart | 14 +++++++------- .../screenshot/{retrier.dart => stabilizer.dart} | 7 +++---- flutter/lib/src/sentry_flutter_options.dart | 4 ++-- 4 files changed, 16 insertions(+), 17 deletions(-) rename flutter/lib/src/screenshot/{retrier.dart => stabilizer.dart} (93%) diff --git a/flutter/lib/src/native/cocoa/sentry_native_cocoa.dart b/flutter/lib/src/native/cocoa/sentry_native_cocoa.dart index 0eef1aa495..0e08321ba5 100644 --- a/flutter/lib/src/native/cocoa/sentry_native_cocoa.dart +++ b/flutter/lib/src/native/cocoa/sentry_native_cocoa.dart @@ -9,7 +9,7 @@ import '../../replay/replay_config.dart'; import '../../replay/replay_recorder.dart'; import '../../screenshot/recorder.dart'; import '../../screenshot/recorder_config.dart'; -import '../../screenshot/retrier.dart'; +import '../../screenshot/stabilizer.dart'; import '../native_memory.dart'; import '../sentry_native_channel.dart'; import 'binding.dart' as cocoa; @@ -55,7 +55,7 @@ class SentryNativeCocoa extends SentryNativeChannel { } final completer = Completer(); - final retrier = ScreenshotRetrier(_replayRecorder!, options, + final stabilizer = ScreenshotStabilizer(_replayRecorder!, options, (screenshot) async { final pngData = await screenshot.pngData; options.logger( @@ -65,8 +65,8 @@ class SentryNativeCocoa extends SentryNativeChannel { '${pngData.lengthInBytes} bytes)'); completer.complete(pngData.buffer.asUint8List()); }); - retrier.ensureFrameAndAddCallback((msSinceEpoch) { - retrier.capture(msSinceEpoch).onError(completer.completeError); + stabilizer.ensureFrameAndAddCallback((msSinceEpoch) { + stabilizer.capture(msSinceEpoch).onError(completer.completeError); }); final uint8List = await completer.future; diff --git a/flutter/lib/src/replay/scheduled_recorder.dart b/flutter/lib/src/replay/scheduled_recorder.dart index a34c768104..781dbbaced 100644 --- a/flutter/lib/src/replay/scheduled_recorder.dart +++ b/flutter/lib/src/replay/scheduled_recorder.dart @@ -3,7 +3,7 @@ import 'dart:async'; import 'package:meta/meta.dart'; import '../../sentry_flutter.dart'; -import '../screenshot/retrier.dart'; +import '../screenshot/stabilizer.dart'; import '../screenshot/screenshot.dart'; import 'replay_recorder.dart'; import 'scheduled_recorder_config.dart'; @@ -19,7 +19,7 @@ class ScheduledScreenshotRecorder extends ReplayScreenshotRecorder { late final ScheduledScreenshotRecorderCallback _callback; var _status = _Status.running; late final Duration _frameDuration; - late final ScreenshotRetrier _retrier; + late final ScreenshotStabilizer _stabilizer; // late final _idleFrameFiller = _IdleFrameFiller(_frameDuration, _onScreenshot); @override @@ -35,9 +35,9 @@ class ScheduledScreenshotRecorder extends ReplayScreenshotRecorder { _frameDuration = Duration(milliseconds: 1000 ~/ config.frameRate); assert(_frameDuration.inMicroseconds > 0); - _retrier = ScreenshotRetrier(this, options, _onImageCaptured); - _scheduler = Scheduler( - _frameDuration, _retrier.capture, _retrier.ensureFrameAndAddCallback); + _stabilizer = ScreenshotStabilizer(this, options, _onImageCaptured); + _scheduler = Scheduler(_frameDuration, _stabilizer.capture, + _stabilizer.ensureFrameAndAddCallback); if (callback != null) { _callback = callback; @@ -63,12 +63,12 @@ class ScheduledScreenshotRecorder extends ReplayScreenshotRecorder { } Future _stopScheduler() { - _retrier.stopped = true; + _stabilizer.stopped = true; return _scheduler.stop(); } void _startScheduler() { - _retrier.stopped = false; + _stabilizer.stopped = false; _scheduler.start(); // We need to schedule a frame because if this happens in-between user diff --git a/flutter/lib/src/screenshot/retrier.dart b/flutter/lib/src/screenshot/stabilizer.dart similarity index 93% rename from flutter/lib/src/screenshot/retrier.dart rename to flutter/lib/src/screenshot/stabilizer.dart index 4b4ab92231..876519f066 100644 --- a/flutter/lib/src/screenshot/retrier.dart +++ b/flutter/lib/src/screenshot/stabilizer.dart @@ -18,9 +18,8 @@ import 'screenshot.dart'; /// To get around this issue, we're taking two subsequent screenshots /// (after two frames) and only actually capture a screenshot if the /// two are exactly the same. -/// We only do these if masking is enabled. @internal -class ScreenshotRetrier { +class ScreenshotStabilizer { final SentryFlutterOptions _options; final ScreenshotRecorder _recorder; final Future Function(Screenshot screenshot) _callback; @@ -28,9 +27,9 @@ class ScreenshotRetrier { int _tries = 0; bool stopped = false; - ScreenshotRetrier(this._recorder, this._options, this._callback) { + ScreenshotStabilizer(this._recorder, this._options, this._callback) { assert(_options.screenshotRetries >= 1, - "Cannot use ScreenshotRetrier if we cannot retry at least once."); + "Cannot use ScreenshotStabilizer if we cannot retry at least once."); } void ensureFrameAndAddCallback(FrameCallback callback) { diff --git a/flutter/lib/src/sentry_flutter_options.dart b/flutter/lib/src/sentry_flutter_options.dart index cccd4220ff..7cde2fc598 100644 --- a/flutter/lib/src/sentry_flutter_options.dart +++ b/flutter/lib/src/sentry_flutter_options.dart @@ -11,7 +11,7 @@ import 'binding_wrapper.dart'; import 'navigation/time_to_display_tracker.dart'; import 'renderer/renderer.dart'; import 'screenshot/sentry_screenshot_quality.dart'; -import 'screenshot/retrier.dart'; +import 'screenshot/stabilizer.dart'; import 'event_processor/screenshot_event_processor.dart'; import 'sentry_flutter.dart'; import 'sentry_privacy_options.dart'; @@ -206,7 +206,7 @@ class SentryFlutterOptions extends SentryOptions { /// from the function, no screenshot will be attached. BeforeCaptureCallback? beforeCaptureScreenshot; - /// See [ScreenshotRetrier] + /// See [ScreenshotStabilizer] @meta.internal int screenshotRetries = 5; From 9509e443c38658bbab791357336156d6fbfc144e Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Fri, 10 Jan 2025 14:02:11 +0100 Subject: [PATCH 17/28] chore: changelog --- CHANGELOG.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4193d61d71..f747639031 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,12 +6,16 @@ - Add `beforeCapture` for View Hierarchy ([#2523](https://github.com/getsentry/sentry-dart/pull/2523)) - View hierarchy calls are now debounced for 2 seconds. - + ### Enhancements - Replay: improve iOS native interop performance ([#2530](https://github.com/getsentry/sentry-dart/pull/2530)) - Replay: improve orientation change tracking accuracy on Android ([#2540](https://github.com/getsentry/sentry-dart/pull/2540)) +### Fixes + +- Replay: fix masking for frames captured during UI changes ([#2553](https://github.com/getsentry/sentry-dart/pull/2553)) + ### Dependencies - Bump Cocoa SDK from v8.42.0 to v8.43.0 ([#2542](https://github.com/getsentry/sentry-dart/pull/2542), [#2548](https://github.com/getsentry/sentry-dart/pull/2548)) From 0418629a69396997ebe3de0f8c271eaabec8ce2a Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Fri, 10 Jan 2025 14:47:53 +0100 Subject: [PATCH 18/28] capture stable issue screenshots when masking is enabled --- .../screenshot_event_processor.dart | 22 +++++++++++++++++-- flutter/lib/src/screenshot/stabilizer.dart | 6 ++++- 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/flutter/lib/src/event_processor/screenshot_event_processor.dart b/flutter/lib/src/event_processor/screenshot_event_processor.dart index 23ee438948..4365d0e91c 100644 --- a/flutter/lib/src/event_processor/screenshot_event_processor.dart +++ b/flutter/lib/src/event_processor/screenshot_event_processor.dart @@ -9,6 +9,7 @@ import '../screenshot/recorder.dart'; import '../screenshot/recorder_config.dart'; import 'package:flutter/widgets.dart' as widget; +import '../screenshot/stabilizer.dart'; import '../utils/debouncer.dart'; class ScreenshotEventProcessor implements EventProcessor { @@ -125,6 +126,23 @@ class ScreenshotEventProcessor implements EventProcessor { } @internal - Future createScreenshot() => _recorder.capture( - (screenshot) => screenshot.pngData.then((v) => v.buffer.asUint8List())); + Future createScreenshot() async { + if (_options.experimental.privacyForScreenshots == null) { + return _recorder.capture((screenshot) => + screenshot.pngData.then((v) => v.buffer.asUint8List())); + } else { + // If masking is enabled, we need to use [ScreenshotStabilizer]. + final completer = Completer(); + final stabilizer = + ScreenshotStabilizer(_recorder, _options, (screenshot) async { + final pngData = await screenshot.pngData; + completer.complete(pngData.buffer.asUint8List()); + }); + unawaited( + stabilizer.capture(Duration.zero).onError(completer.completeError)); + final uint8List = await completer.future; + stabilizer.dispose(); + return uint8List; + } + } } diff --git a/flutter/lib/src/screenshot/stabilizer.dart b/flutter/lib/src/screenshot/stabilizer.dart index 876519f066..28c6b40ee5 100644 --- a/flutter/lib/src/screenshot/stabilizer.dart +++ b/flutter/lib/src/screenshot/stabilizer.dart @@ -32,13 +32,17 @@ class ScreenshotStabilizer { "Cannot use ScreenshotStabilizer if we cannot retry at least once."); } + void dispose() { + _previousScreenshot?.dispose(); + } + void ensureFrameAndAddCallback(FrameCallback callback) { _options.bindingUtils.instance! ..ensureVisualUpdate() ..addPostFrameCallback(callback); } - Future capture(Duration sinceSchedulerEpoch) { + Future capture(Duration _) { _tries++; return _recorder.capture(_onImageCaptured); } From 183e995ccd09f10626572c37e7a3bb28b7c4fd77 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Fri, 10 Jan 2025 19:01:26 +0100 Subject: [PATCH 19/28] fixes and tests --- .../screenshot_event_processor.dart | 15 ++++++++++----- .../src/native/cocoa/sentry_native_cocoa.dart | 17 +++++++++++++---- flutter/lib/src/replay/scheduled_recorder.dart | 1 + flutter/lib/src/screenshot/stabilizer.dart | 16 +++++++++++++--- flutter/lib/src/sentry_flutter_options.dart | 5 ----- .../screenshot_event_processor_test.dart | 10 +++++++++- 6 files changed, 46 insertions(+), 18 deletions(-) diff --git a/flutter/lib/src/event_processor/screenshot_event_processor.dart b/flutter/lib/src/event_processor/screenshot_event_processor.dart index 4365d0e91c..5b6fd05797 100644 --- a/flutter/lib/src/event_processor/screenshot_event_processor.dart +++ b/flutter/lib/src/event_processor/screenshot_event_processor.dart @@ -133,11 +133,16 @@ class ScreenshotEventProcessor implements EventProcessor { } else { // If masking is enabled, we need to use [ScreenshotStabilizer]. final completer = Completer(); - final stabilizer = - ScreenshotStabilizer(_recorder, _options, (screenshot) async { - final pngData = await screenshot.pngData; - completer.complete(pngData.buffer.asUint8List()); - }); + final stabilizer = ScreenshotStabilizer( + _recorder, + _options, + (screenshot) async { + final pngData = await screenshot.pngData; + completer.complete(pngData.buffer.asUint8List()); + }, + // This limits the amount of time to take a stable masked screenshot. + maxTries: 5, + ); unawaited( stabilizer.capture(Duration.zero).onError(completer.completeError)); final uint8List = await completer.future; diff --git a/flutter/lib/src/native/cocoa/sentry_native_cocoa.dart b/flutter/lib/src/native/cocoa/sentry_native_cocoa.dart index 0e08321ba5..f381e16cc3 100644 --- a/flutter/lib/src/native/cocoa/sentry_native_cocoa.dart +++ b/flutter/lib/src/native/cocoa/sentry_native_cocoa.dart @@ -55,6 +55,7 @@ class SentryNativeCocoa extends SentryNativeChannel { } final completer = Completer(); + final stabilizer = ScreenshotStabilizer(_replayRecorder!, options, (screenshot) async { final pngData = await screenshot.pngData; @@ -65,10 +66,18 @@ class SentryNativeCocoa extends SentryNativeChannel { '${pngData.lengthInBytes} bytes)'); completer.complete(pngData.buffer.asUint8List()); }); - stabilizer.ensureFrameAndAddCallback((msSinceEpoch) { - stabilizer.capture(msSinceEpoch).onError(completer.completeError); - }); - final uint8List = await completer.future; + + late final Uint8List? uint8List; + try { + stabilizer.ensureFrameAndAddCallback((msSinceEpoch) { + stabilizer + .capture(msSinceEpoch) + .onError(completer.completeError); + }); + uint8List = await completer.future; + } finally { + stabilizer.dispose(); + } // Malloc memory and copy the data. Native must free it. return uint8List?.toNativeMemory().toJson(); diff --git a/flutter/lib/src/replay/scheduled_recorder.dart b/flutter/lib/src/replay/scheduled_recorder.dart index 781dbbaced..06b75a2a60 100644 --- a/flutter/lib/src/replay/scheduled_recorder.dart +++ b/flutter/lib/src/replay/scheduled_recorder.dart @@ -82,6 +82,7 @@ class ScheduledScreenshotRecorder extends ReplayScreenshotRecorder { options.logger(SentryLevel.debug, "$logName: stopping capture."); _status = _Status.stopped; await _stopScheduler(); + _stabilizer.dispose(); // await Future.wait([_stopScheduler(), _idleFrameFiller.stop()]); options.logger(SentryLevel.debug, "$logName: capture stopped."); } diff --git a/flutter/lib/src/screenshot/stabilizer.dart b/flutter/lib/src/screenshot/stabilizer.dart index 28c6b40ee5..73a0e0cead 100644 --- a/flutter/lib/src/screenshot/stabilizer.dart +++ b/flutter/lib/src/screenshot/stabilizer.dart @@ -1,4 +1,5 @@ import 'dart:async'; +import 'dart:math'; import 'package:flutter/scheduler.dart'; import 'package:meta/meta.dart'; @@ -23,17 +24,20 @@ class ScreenshotStabilizer { final SentryFlutterOptions _options; final ScreenshotRecorder _recorder; final Future Function(Screenshot screenshot) _callback; + final int? maxTries; Screenshot? _previousScreenshot; int _tries = 0; bool stopped = false; - ScreenshotStabilizer(this._recorder, this._options, this._callback) { - assert(_options.screenshotRetries >= 1, + ScreenshotStabilizer(this._recorder, this._options, this._callback, + {this.maxTries}) { + assert(maxTries == null || maxTries! > 1, "Cannot use ScreenshotStabilizer if we cannot retry at least once."); } void dispose() { _previousScreenshot?.dispose(); + _previousScreenshot = null; } void ensureFrameAndAddCallback(FrameCallback callback) { @@ -68,10 +72,16 @@ class ScreenshotStabilizer { // the new timestamp. await _callback(screenshot); } - } else if (_tries > _options.screenshotRetries) { + } else if (maxTries != null && _tries >= maxTries!) { throw Exception('Failed to capture a stable screenshot. ' 'Giving up after $_tries tries.'); } else { + // Add a delay to give the UI a chance to stabilize. + if (_tries > 0) { + await Future.delayed( + Duration(milliseconds: min(100, 10 * (_tries - 1)))); + } + final completer = Completer(); ensureFrameAndAddCallback((Duration sinceSchedulerEpoch) async { _tries++; diff --git a/flutter/lib/src/sentry_flutter_options.dart b/flutter/lib/src/sentry_flutter_options.dart index 7cde2fc598..56c20161b9 100644 --- a/flutter/lib/src/sentry_flutter_options.dart +++ b/flutter/lib/src/sentry_flutter_options.dart @@ -11,7 +11,6 @@ import 'binding_wrapper.dart'; import 'navigation/time_to_display_tracker.dart'; import 'renderer/renderer.dart'; import 'screenshot/sentry_screenshot_quality.dart'; -import 'screenshot/stabilizer.dart'; import 'event_processor/screenshot_event_processor.dart'; import 'sentry_flutter.dart'; import 'sentry_privacy_options.dart'; @@ -206,10 +205,6 @@ class SentryFlutterOptions extends SentryOptions { /// from the function, no screenshot will be attached. BeforeCaptureCallback? beforeCaptureScreenshot; - /// See [ScreenshotStabilizer] - @meta.internal - int screenshotRetries = 5; - /// Enable or disable automatic breadcrumbs for User interactions Using [Listener] /// /// Requires adding the [SentryUserInteractionWidget] to the widget tree. diff --git a/flutter/test/event_processor/screenshot_event_processor_test.dart b/flutter/test/event_processor/screenshot_event_processor_test.dart index b544c9a114..095e398892 100644 --- a/flutter/test/event_processor/screenshot_event_processor_test.dart +++ b/flutter/test/event_processor/screenshot_event_processor_test.dart @@ -11,6 +11,8 @@ import 'package:sentry_flutter/src/renderer/renderer.dart'; import '../mocks.dart'; import 'package:sentry_flutter/sentry_flutter.dart'; +import '../replay/replay_test_util.dart'; + void main() { TestWidgetsFlutterBinding.ensureInitialized(); late Fixture fixture; @@ -40,7 +42,7 @@ void main() { final throwable = Exception(); event = SentryEvent(throwable: throwable); hint = Hint(); - await sut.apply(event, hint); + await tester.pumpAndWaitUntil(sut.apply(event, hint)); expect(hint.screenshot != null, added); if (expectedMaxWidthOrHeight != null) { @@ -60,6 +62,12 @@ void main() { await _addScreenshotAttachment(tester, null, added: true, isWeb: false); }); + testWidgets('adds screenshot attachment with masking enabled dart:io', + (tester) async { + fixture.options.experimental.privacy.maskAllText = true; + await _addScreenshotAttachment(tester, null, added: true, isWeb: false); + }); + testWidgets('adds screenshot attachment with canvasKit renderer', (tester) async { await _addScreenshotAttachment(tester, FlutterRenderer.canvasKit, From 4ccd5ea9dfe05c237ec6cab3a97d7e496797ca07 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Fri, 10 Jan 2025 19:13:35 +0100 Subject: [PATCH 20/28] better logging --- flutter/lib/src/screenshot/recorder.dart | 1 - flutter/lib/src/screenshot/stabilizer.dart | 17 ++++++++++++++--- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/flutter/lib/src/screenshot/recorder.dart b/flutter/lib/src/screenshot/recorder.dart index 087c07ffa6..497eb4ebfd 100644 --- a/flutter/lib/src/screenshot/recorder.dart +++ b/flutter/lib/src/screenshot/recorder.dart @@ -22,7 +22,6 @@ class ScreenshotRecorder { @protected final SentryFlutterOptions options; - @protected final String logName; bool _warningLogged = false; late final SentryMaskingConfig? _maskingConfig; diff --git a/flutter/lib/src/screenshot/stabilizer.dart b/flutter/lib/src/screenshot/stabilizer.dart index 73a0e0cead..53622c5cc9 100644 --- a/flutter/lib/src/screenshot/stabilizer.dart +++ b/flutter/lib/src/screenshot/stabilizer.dart @@ -77,9 +77,20 @@ class ScreenshotStabilizer { 'Giving up after $_tries tries.'); } else { // Add a delay to give the UI a chance to stabilize. - if (_tries > 0) { - await Future.delayed( - Duration(milliseconds: min(100, 10 * (_tries - 1)))); + // Only do this on every other frame so that there's a greater chance + // of two subsequent frames being the same. + final sleepMs = _tries % 2 == 1 ? min(100, 10 * (_tries - 1)) : 0; + + if (_tries > 1) { + _options.logger( + SentryLevel.debug, + '${_recorder.logName}: ' + 'Retrying screenshot capture due to UI changes. ' + 'Delay before next capture: $sleepMs ms.'); + } + + if (sleepMs > 0) { + await Future.delayed(Duration(milliseconds: sleepMs)); } final completer = Completer(); From e7ab1008abb45a4c679e318f31ba9af4fed7c9ce Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Fri, 10 Jan 2025 19:55:25 +0100 Subject: [PATCH 21/28] cocoa replay fix --- .../native/cocoa/cocoa_replay_recorder.dart | 40 +++++++++ .../src/native/cocoa/sentry_native_cocoa.dart | 45 ++-------- flutter/lib/src/screenshot/stabilizer.dart | 82 ++++++++++--------- 3 files changed, 89 insertions(+), 78 deletions(-) create mode 100644 flutter/lib/src/native/cocoa/cocoa_replay_recorder.dart diff --git a/flutter/lib/src/native/cocoa/cocoa_replay_recorder.dart b/flutter/lib/src/native/cocoa/cocoa_replay_recorder.dart new file mode 100644 index 0000000000..8404dfaa9f --- /dev/null +++ b/flutter/lib/src/native/cocoa/cocoa_replay_recorder.dart @@ -0,0 +1,40 @@ +import 'dart:async'; +import 'dart:typed_data'; + +import 'package:meta/meta.dart'; + +import '../../../sentry_flutter.dart'; +import '../../replay/replay_recorder.dart'; +import '../../screenshot/recorder.dart'; +import '../../screenshot/recorder_config.dart'; +import '../../screenshot/stabilizer.dart'; + +@internal +class CocoaReplayRecorder { + final SentryFlutterOptions _options; + final ScreenshotRecorder _recorder; + late final ScreenshotStabilizer _stabilizer; + var _completer = Completer(); + + CocoaReplayRecorder(this._options) + : _recorder = + ReplayScreenshotRecorder(ScreenshotRecorderConfig(), _options) { + _stabilizer = ScreenshotStabilizer(_recorder, _options, (screenshot) async { + final pngData = await screenshot.pngData; + _options.logger( + SentryLevel.debug, + 'Replay: captured screenshot (' + '${screenshot.width}x${screenshot.height} pixels, ' + '${pngData.lengthInBytes} bytes)'); + _completer.complete(pngData.buffer.asUint8List()); + }); + } + + Future captureScreenshot() async { + _completer = Completer(); + _stabilizer.ensureFrameAndAddCallback((msSinceEpoch) { + _stabilizer.capture(msSinceEpoch).onError(_completer.completeError); + }); + return _completer.future; + } +} diff --git a/flutter/lib/src/native/cocoa/sentry_native_cocoa.dart b/flutter/lib/src/native/cocoa/sentry_native_cocoa.dart index f381e16cc3..658b5f8af0 100644 --- a/flutter/lib/src/native/cocoa/sentry_native_cocoa.dart +++ b/flutter/lib/src/native/cocoa/sentry_native_cocoa.dart @@ -1,23 +1,19 @@ import 'dart:async'; import 'dart:ffi'; -import 'dart:typed_data'; import 'package:meta/meta.dart'; import '../../../sentry_flutter.dart'; import '../../replay/replay_config.dart'; -import '../../replay/replay_recorder.dart'; -import '../../screenshot/recorder.dart'; -import '../../screenshot/recorder_config.dart'; -import '../../screenshot/stabilizer.dart'; import '../native_memory.dart'; import '../sentry_native_channel.dart'; import 'binding.dart' as cocoa; +import 'cocoa_replay_recorder.dart'; @internal class SentryNativeCocoa extends SentryNativeChannel { late final _lib = cocoa.SentryCocoa(DynamicLibrary.process()); - ScreenshotRecorder? _replayRecorder; + CocoaReplayRecorder? _replayRecorder; SentryId? _replayId; SentryNativeCocoa(super.options); @@ -33,12 +29,12 @@ class SentryNativeCocoa extends SentryNativeChannel { channel.setMethodCallHandler((call) async { switch (call.method) { case 'captureReplayScreenshot': - _replayRecorder ??= - ReplayScreenshotRecorder(ScreenshotRecorderConfig(), options); + _replayRecorder ??= CocoaReplayRecorder(options); final replayId = call.arguments['replayId'] == null ? null : SentryId.fromId(call.arguments['replayId'] as String); + if (_replayId != replayId) { _replayId = replayId; hub.configureScope((s) { @@ -47,38 +43,7 @@ class SentryNativeCocoa extends SentryNativeChannel { }); } - final widgetsBinding = options.bindingUtils.instance; - if (widgetsBinding == null) { - options.logger(SentryLevel.warning, - 'Replay: failed to capture screenshot, WidgetsBinding.instance is null'); - return null; - } - - final completer = Completer(); - - final stabilizer = ScreenshotStabilizer(_replayRecorder!, options, - (screenshot) async { - final pngData = await screenshot.pngData; - options.logger( - SentryLevel.debug, - 'Replay: captured screenshot (' - '${screenshot.width}x${screenshot.height} pixels, ' - '${pngData.lengthInBytes} bytes)'); - completer.complete(pngData.buffer.asUint8List()); - }); - - late final Uint8List? uint8List; - try { - stabilizer.ensureFrameAndAddCallback((msSinceEpoch) { - stabilizer - .capture(msSinceEpoch) - .onError(completer.completeError); - }); - uint8List = await completer.future; - } finally { - stabilizer.dispose(); - } - + final uint8List = await _replayRecorder!.captureScreenshot(); // Malloc memory and copy the data. Native must free it. return uint8List?.toNativeMemory().toJson(); default: diff --git a/flutter/lib/src/screenshot/stabilizer.dart b/flutter/lib/src/screenshot/stabilizer.dart index 53622c5cc9..7ded3eb7d4 100644 --- a/flutter/lib/src/screenshot/stabilizer.dart +++ b/flutter/lib/src/screenshot/stabilizer.dart @@ -64,49 +64,55 @@ class ScreenshotStabilizer { await prevScreenshot.hasSameImageAs(screenshot)) { // Sucessfully captured a stable screenshot (repeated at least twice). _tries = 0; - if (prevScreenshot.flow.id == screenshot.flow.id) { - // If it's from the same (retry) flow, use the first screenshot timestamp. - await _callback(prevScreenshot); - } else { - // Otherwise this was called from a scheduler (in a new flow) so use - // the new timestamp. - await _callback(screenshot); - } - } else if (maxTries != null && _tries >= maxTries!) { - throw Exception('Failed to capture a stable screenshot. ' - 'Giving up after $_tries tries.'); - } else { - // Add a delay to give the UI a chance to stabilize. - // Only do this on every other frame so that there's a greater chance - // of two subsequent frames being the same. - final sleepMs = _tries % 2 == 1 ? min(100, 10 * (_tries - 1)) : 0; - - if (_tries > 1) { - _options.logger( - SentryLevel.debug, - '${_recorder.logName}: ' - 'Retrying screenshot capture due to UI changes. ' - 'Delay before next capture: $sleepMs ms.'); - } - if (sleepMs > 0) { - await Future.delayed(Duration(milliseconds: sleepMs)); - } + // If it's from the same (retry) flow, use the first screenshot + // timestamp. Otherwise this was called from a scheduler (in a new flow) + // so use the new timestamp. + await _callback((prevScreenshot.flow.id == screenshot.flow.id) + ? prevScreenshot + : screenshot); - final completer = Completer(); - ensureFrameAndAddCallback((Duration sinceSchedulerEpoch) async { - _tries++; - try { - await _recorder.capture(_onImageCaptured, screenshot.flow); - completer.complete(); - } catch (e, stackTrace) { - completer.completeError(e, stackTrace); - } - }); - return completer.future; + // Do not just return the Future resulting from callback(). + // We need to await here so that the dispose runs ASAP. + return; } } finally { + // Note: we need to dispose (free the memory) before recursion. prevScreenshot?.dispose(); } + + if (maxTries != null && _tries >= maxTries!) { + throw Exception('Failed to capture a stable screenshot. ' + 'Giving up after $_tries tries.'); + } else { + // Add a delay to give the UI a chance to stabilize. + // Only do this on every other frame so that there's a greater chance + // of two subsequent frames being the same. + final sleepMs = _tries % 2 == 1 ? min(100, 10 * (_tries - 1)) : 0; + + if (_tries > 1) { + _options.logger( + SentryLevel.debug, + '${_recorder.logName}: ' + 'Retrying screenshot capture due to UI changes. ' + 'Delay before next capture: $sleepMs ms.'); + } + + if (sleepMs > 0) { + await Future.delayed(Duration(milliseconds: sleepMs)); + } + + final completer = Completer(); + ensureFrameAndAddCallback((Duration sinceSchedulerEpoch) async { + _tries++; + try { + await _recorder.capture(_onImageCaptured, screenshot.flow); + completer.complete(); + } catch (e, stackTrace) { + completer.completeError(e, stackTrace); + } + }); + return completer.future; + } } } From 622d859b768385d0337af72b9cc8220433d8b23a Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Fri, 10 Jan 2025 22:35:02 +0100 Subject: [PATCH 22/28] fix dart2js tests --- flutter/lib/src/screenshot/screenshot.dart | 26 ++++++++++++++-------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/flutter/lib/src/screenshot/screenshot.dart b/flutter/lib/src/screenshot/screenshot.dart index 582d81853f..762cb5eb80 100644 --- a/flutter/lib/src/screenshot/screenshot.dart +++ b/flutter/lib/src/screenshot/screenshot.dart @@ -72,19 +72,27 @@ class Screenshot { /// For now, the best we can do is compare by chunks of 8 bytes. // return 0 == memcmp(dataA.address, dataB.address, dataA.lengthInBytes); - final numWords = dataA.lengthInBytes ~/ 8; - final wordsA = dataA.buffer.asUint64List(0, numWords); - final wordsB = dataB.buffer.asUint64List(0, numWords); - - for (var i = 0; i < wordsA.length; i++) { - if (wordsA[i] != wordsB[i]) { - return false; + late final int processed; + try { + final numWords = dataA.lengthInBytes ~/ 8; + final wordsA = dataA.buffer.asUint64List(0, numWords); + final wordsB = dataB.buffer.asUint64List(0, numWords); + + for (var i = 0; i < wordsA.length; i++) { + if (wordsA[i] != wordsB[i]) { + return false; + } } + processed = wordsA.lengthInBytes; + } on UnsupportedError { + // This should only trigger on dart2js: + // Unsupported operation: Uint64List not supported by dart2js. + processed = 0; } // Compare any remaining bytes. - final bytesA = dataA.buffer.asUint8List(wordsA.lengthInBytes); - final bytesB = dataB.buffer.asUint8List(wordsA.lengthInBytes); + final bytesA = dataA.buffer.asUint8List(processed); + final bytesB = dataB.buffer.asUint8List(processed); for (var i = 0; i < bytesA.lengthInBytes; i++) { if (bytesA[i] != bytesB[i]) { return false; From e9cebf5a2765fdbdbcb9621a7bbd1c1caa0aa62f Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Fri, 10 Jan 2025 23:41:59 +0100 Subject: [PATCH 23/28] fix screenshot capture hanging --- .../screenshot_event_processor.dart | 18 ++++++++----- flutter/lib/src/screenshot/stabilizer.dart | 27 ++++++++++++++++--- 2 files changed, 34 insertions(+), 11 deletions(-) diff --git a/flutter/lib/src/event_processor/screenshot_event_processor.dart b/flutter/lib/src/event_processor/screenshot_event_processor.dart index 5b6fd05797..436bf9d84b 100644 --- a/flutter/lib/src/event_processor/screenshot_event_processor.dart +++ b/flutter/lib/src/event_processor/screenshot_event_processor.dart @@ -134,20 +134,24 @@ class ScreenshotEventProcessor implements EventProcessor { // If masking is enabled, we need to use [ScreenshotStabilizer]. final completer = Completer(); final stabilizer = ScreenshotStabilizer( - _recorder, - _options, + _recorder, _options, (screenshot) async { final pngData = await screenshot.pngData; completer.complete(pngData.buffer.asUint8List()); }, // This limits the amount of time to take a stable masked screenshot. maxTries: 5, + // We need to force the frame the frame or this could hang indefinitely. + frameSchedulingMode: FrameSchedulingMode.forced, ); - unawaited( - stabilizer.capture(Duration.zero).onError(completer.completeError)); - final uint8List = await completer.future; - stabilizer.dispose(); - return uint8List; + try { + unawaited( + stabilizer.capture(Duration.zero).onError(completer.completeError)); + // DO NOT return completer.future directly - we need to dispose first. + return await completer.future; + } finally { + stabilizer.dispose(); + } } } } diff --git a/flutter/lib/src/screenshot/stabilizer.dart b/flutter/lib/src/screenshot/stabilizer.dart index 7ded3eb7d4..089c6574d8 100644 --- a/flutter/lib/src/screenshot/stabilizer.dart +++ b/flutter/lib/src/screenshot/stabilizer.dart @@ -25,12 +25,13 @@ class ScreenshotStabilizer { final ScreenshotRecorder _recorder; final Future Function(Screenshot screenshot) _callback; final int? maxTries; + final FrameSchedulingMode frameSchedulingMode; Screenshot? _previousScreenshot; int _tries = 0; bool stopped = false; ScreenshotStabilizer(this._recorder, this._options, this._callback, - {this.maxTries}) { + {this.maxTries, this.frameSchedulingMode = FrameSchedulingMode.normal}) { assert(maxTries == null || maxTries! > 1, "Cannot use ScreenshotStabilizer if we cannot retry at least once."); } @@ -41,9 +42,16 @@ class ScreenshotStabilizer { } void ensureFrameAndAddCallback(FrameCallback callback) { - _options.bindingUtils.instance! - ..ensureVisualUpdate() - ..addPostFrameCallback(callback); + final binding = _options.bindingUtils.instance!; + switch (frameSchedulingMode) { + case FrameSchedulingMode.normal: + binding.scheduleFrame(); + break; + case FrameSchedulingMode.forced: + binding.scheduleForcedFrame(); + break; + } + binding.addPostFrameCallback(callback); } Future capture(Duration _) { @@ -116,3 +124,14 @@ class ScreenshotStabilizer { } } } + +@internal +enum FrameSchedulingMode { + /// The frame is scheduled only if the UI is visible. + /// If you await for the callback, it may take indefinitely long if the + /// app is in the background. + normal, + + /// A forced frame is scheduled immediately regardless of the UI visibility. + forced, +} From 07aec1d7854d442eee05daf459b1eeaf73591a7a Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Sat, 11 Jan 2025 13:22:40 +0100 Subject: [PATCH 24/28] fix integration test --- flutter/example/integration_test/integration_test.dart | 2 ++ 1 file changed, 2 insertions(+) diff --git a/flutter/example/integration_test/integration_test.dart b/flutter/example/integration_test/integration_test.dart index 8934deb156..6fd6916b7d 100644 --- a/flutter/example/integration_test/integration_test.dart +++ b/flutter/example/integration_test/integration_test.dart @@ -20,6 +20,8 @@ void main() { const fakeDsn = 'https://abc@def.ingest.sentry.io/1234567'; IntegrationTestWidgetsFlutterBinding.ensureInitialized(); + IntegrationTestWidgetsFlutterBinding.instance.framePolicy = + LiveTestWidgetsFlutterBindingFramePolicy.fullyLive; tearDown(() async { await Sentry.close(); From 134b0157fdef184fa536c90e9d90a032cdd2806a Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Sat, 11 Jan 2025 14:32:48 +0100 Subject: [PATCH 25/28] time out if we can't take a screenshot for events --- .../screenshot_event_processor.dart | 7 ++++++- .../screenshot_event_processor_test.dart | 21 +++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/flutter/lib/src/event_processor/screenshot_event_processor.dart b/flutter/lib/src/event_processor/screenshot_event_processor.dart index 436bf9d84b..4d2446c2a4 100644 --- a/flutter/lib/src/event_processor/screenshot_event_processor.dart +++ b/flutter/lib/src/event_processor/screenshot_event_processor.dart @@ -148,7 +148,12 @@ class ScreenshotEventProcessor implements EventProcessor { unawaited( stabilizer.capture(Duration.zero).onError(completer.completeError)); // DO NOT return completer.future directly - we need to dispose first. - return await completer.future; + return await completer.future.timeout(const Duration(seconds: 1), + onTimeout: () { + _options.logger( + SentryLevel.warning, 'Timed out taking a stable screenshot.'); + return null; + }); } finally { stabilizer.dispose(); } diff --git a/flutter/test/event_processor/screenshot_event_processor_test.dart b/flutter/test/event_processor/screenshot_event_processor_test.dart index 095e398892..dae0bf468d 100644 --- a/flutter/test/event_processor/screenshot_event_processor_test.dart +++ b/flutter/test/event_processor/screenshot_event_processor_test.dart @@ -68,6 +68,27 @@ void main() { await _addScreenshotAttachment(tester, null, added: true, isWeb: false); }); + testWidgets('does not block if the screenshot fails to stabilize', + (tester) async { + fixture.options.automatedTestMode = false; + fixture.options.experimental.privacy.maskAllText = true; + // Run with real async https://stackoverflow.com/a/54021863 + await tester.runAsync(() async { + final sut = fixture.getSut(null, false); + + await tester.pumpWidget(SentryScreenshotWidget( + child: Text('Catching Pokémon is a snap!', + textDirection: TextDirection.ltr))); + + final throwable = Exception(); + event = SentryEvent(throwable: throwable); + hint = Hint(); + await sut.apply(event, hint); + + expect(hint.screenshot, isNull); + }); + }); + testWidgets('adds screenshot attachment with canvasKit renderer', (tester) async { await _addScreenshotAttachment(tester, FlutterRenderer.canvasKit, From 7f1fbf459dc348c1dfe0e20f3c4b3b2fe2f36a7c Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Sat, 11 Jan 2025 22:16:39 +0100 Subject: [PATCH 26/28] enha: use rgba for cocoa replay --- .../SentryFlutterReplayScreenshotProvider.m | 36 ++++++++++++++++++- .../native/cocoa/cocoa_replay_recorder.dart | 19 ++++++---- .../src/native/cocoa/sentry_native_cocoa.dart | 5 +-- flutter/lib/src/native/native_memory.dart | 27 +++++++++----- flutter/lib/src/screenshot/screenshot.dart | 14 ++++---- flutter/test/native_memory_test.dart | 13 +++---- flutter/test/native_memory_web_mock.dart | 5 +-- flutter/test/replay/replay_native_test.dart | 2 ++ 8 files changed, 86 insertions(+), 35 deletions(-) diff --git a/flutter/ios/Classes/SentryFlutterReplayScreenshotProvider.m b/flutter/ios/Classes/SentryFlutterReplayScreenshotProvider.m index 2abe56ac43..6ffb969423 100644 --- a/flutter/ios/Classes/SentryFlutterReplayScreenshotProvider.m +++ b/flutter/ios/Classes/SentryFlutterReplayScreenshotProvider.m @@ -33,11 +33,45 @@ - (void)imageWithView:(UIView *_Nonnull)view NSDictionary *dict = (NSDictionary *)value; long address = ((NSNumber *)dict[@"address"]).longValue; NSNumber *length = ((NSNumber *)dict[@"length"]); + NSNumber *width = ((NSNumber *)dict[@"width"]); + NSNumber *height = ((NSNumber *)dict[@"height"]); NSData *data = [NSData dataWithBytesNoCopy:(void *)address length:length.unsignedLongValue freeWhenDone:TRUE]; - UIImage *image = [UIImage imageWithData:data]; + + // We expect rawRGBA, see from + // https://api.flutter.dev/flutter/dart-ui/ImageByteFormat.html + // Unencoded bytes, in RGBA row-primary form with premultiplied + // alpha, 8 bits per channel. + static const int kBitsPerChannel = 8; + static const int kBytesPerPixel = 4; + assert(length.unsignedLongValue % kBytesPerPixel == 0); + + // Let's create an UIImage from the raw data. + // We need to provide it the width & height and + // the info how the data is encoded. + CGDataProviderRef provider = + CGDataProviderCreateWithCFData((CFDataRef)data); + CGColorSpaceRef colorSpace = CGColorSpaceCreateDeviceRGB(); + CGBitmapInfo bitmapInfo = + kCGBitmapByteOrderDefault | kCGImageAlphaPremultipliedLast; + CGImageRef cgImage = CGImageCreate( + width.unsignedLongValue, // width + height.unsignedLongValue, // height + kBitsPerChannel, // bits per component + kBitsPerChannel * kBytesPerPixel, // bits per pixel + width.unsignedLongValue * kBytesPerPixel, // bytes per row + colorSpace, bitmapInfo, provider, NULL, false, + kCGRenderingIntentDefault); + + UIImage *image = [UIImage imageWithCGImage:cgImage]; + + // UIImage takes its own refs, we need to release these here. + CGImageRelease(cgImage); + CGColorSpaceRelease(colorSpace); + CGDataProviderRelease(provider); + onComplete(image); return; } else if ([value isKindOfClass:[FlutterError class]]) { diff --git a/flutter/lib/src/native/cocoa/cocoa_replay_recorder.dart b/flutter/lib/src/native/cocoa/cocoa_replay_recorder.dart index 8404dfaa9f..a9b0f34de8 100644 --- a/flutter/lib/src/native/cocoa/cocoa_replay_recorder.dart +++ b/flutter/lib/src/native/cocoa/cocoa_replay_recorder.dart @@ -1,5 +1,4 @@ import 'dart:async'; -import 'dart:typed_data'; import 'package:meta/meta.dart'; @@ -8,30 +7,36 @@ import '../../replay/replay_recorder.dart'; import '../../screenshot/recorder.dart'; import '../../screenshot/recorder_config.dart'; import '../../screenshot/stabilizer.dart'; +import '../native_memory.dart'; @internal class CocoaReplayRecorder { final SentryFlutterOptions _options; final ScreenshotRecorder _recorder; late final ScreenshotStabilizer _stabilizer; - var _completer = Completer(); + var _completer = Completer?>(); CocoaReplayRecorder(this._options) : _recorder = ReplayScreenshotRecorder(ScreenshotRecorderConfig(), _options) { _stabilizer = ScreenshotStabilizer(_recorder, _options, (screenshot) async { - final pngData = await screenshot.pngData; + final data = await screenshot.rawRgbaData; _options.logger( SentryLevel.debug, 'Replay: captured screenshot (' '${screenshot.width}x${screenshot.height} pixels, ' - '${pngData.lengthInBytes} bytes)'); - _completer.complete(pngData.buffer.asUint8List()); + '${data.lengthInBytes} bytes)'); + + // Malloc memory and copy the data. Native must free it. + final json = data.toNativeMemory().toJson(); + json['width'] = screenshot.width; + json['height'] = screenshot.height; + _completer.complete(json); }); } - Future captureScreenshot() async { - _completer = Completer(); + Future?> captureScreenshot() async { + _completer = Completer(); _stabilizer.ensureFrameAndAddCallback((msSinceEpoch) { _stabilizer.capture(msSinceEpoch).onError(_completer.completeError); }); diff --git a/flutter/lib/src/native/cocoa/sentry_native_cocoa.dart b/flutter/lib/src/native/cocoa/sentry_native_cocoa.dart index 658b5f8af0..56f7a4ef13 100644 --- a/flutter/lib/src/native/cocoa/sentry_native_cocoa.dart +++ b/flutter/lib/src/native/cocoa/sentry_native_cocoa.dart @@ -5,7 +5,6 @@ import 'package:meta/meta.dart'; import '../../../sentry_flutter.dart'; import '../../replay/replay_config.dart'; -import '../native_memory.dart'; import '../sentry_native_channel.dart'; import 'binding.dart' as cocoa; import 'cocoa_replay_recorder.dart'; @@ -43,9 +42,7 @@ class SentryNativeCocoa extends SentryNativeChannel { }); } - final uint8List = await _replayRecorder!.captureScreenshot(); - // Malloc memory and copy the data. Native must free it. - return uint8List?.toNativeMemory().toJson(); + return _replayRecorder!.captureScreenshot(); 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 index a7c7be0008..3cbe4698e9 100644 --- a/flutter/lib/src/native/native_memory.dart +++ b/flutter/lib/src/native/native_memory.dart @@ -12,13 +12,24 @@ class NativeMemory { 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); + factory NativeMemory.fromByteData(ByteData source) { + final lengthInBytes = source.lengthInBytes; + final ptr = pkg_ffi.malloc.allocate(lengthInBytes); + + // TODO memcpy() from source.buffer.asUint8List().address + // once we can depend on Dart SDK 3.5+ + final numWords = lengthInBytes ~/ 8; + final words = ptr.cast().asTypedList(numWords); + if (numWords > 0) { + words.setAll(0, source.buffer.asUint64List(0, numWords)); } - return NativeMemory._(ptr, length); + + final bytes = ptr.asTypedList(lengthInBytes); + for (var i = words.lengthInBytes; i < source.lengthInBytes; i++) { + bytes[i] = source.getUint8(i); + } + + return NativeMemory._(ptr, lengthInBytes); } factory NativeMemory.fromJson(Map json) { @@ -44,6 +55,6 @@ class NativeMemory { } @internal -extension Uint8ListNativeMemory on Uint8List { - NativeMemory toNativeMemory() => NativeMemory.fromUint8List(this); +extension ByteDataNativeMemory on ByteData { + NativeMemory toNativeMemory() => NativeMemory.fromByteData(this); } diff --git a/flutter/lib/src/screenshot/screenshot.dart b/flutter/lib/src/screenshot/screenshot.dart index 762cb5eb80..6e5881d28f 100644 --- a/flutter/lib/src/screenshot/screenshot.dart +++ b/flutter/lib/src/screenshot/screenshot.dart @@ -12,19 +12,19 @@ class Screenshot { final Image _image; final DateTime timestamp; final Flow flow; - Future? _rawData; + Future? _rawRgbaData; Future? _pngData; Screenshot(this._image, this.timestamp, this.flow); Screenshot._cloned( - this._image, this.timestamp, this.flow, this._rawData, this._pngData); + this._image, this.timestamp, this.flow, this._rawRgbaData, this._pngData); int get width => _image.width; int get height => _image.height; - Future get rawData { - _rawData ??= _encode(ImageByteFormat.rawUnmodified); - return _rawData!; + Future get rawRgbaData { + _rawRgbaData ??= _encode(ImageByteFormat.rawRgba); + return _rawRgbaData!; } Future get pngData { @@ -45,13 +45,13 @@ class Screenshot { return false; } - return listEquals(await rawData, await other.rawData); + return listEquals(await rawRgbaData, await other.rawRgbaData); } Screenshot clone() { assert(!_image.debugDisposed); return Screenshot._cloned( - _image.clone(), timestamp, flow, _rawData, _pngData); + _image.clone(), timestamp, flow, _rawRgbaData, _pngData); } void dispose() => _image.dispose(); diff --git a/flutter/test/native_memory_test.dart b/flutter/test/native_memory_test.dart index 055ccf90f7..185590ac69 100644 --- a/flutter/test/native_memory_test.dart +++ b/flutter/test/native_memory_test.dart @@ -8,10 +8,11 @@ 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]); + final testSrcList = Uint8List.fromList([1, 2, 3, 4, 5, 6, 7, 8, 9, 0]); + final testSrcData = testSrcList.buffer.asByteData(); test('empty list', () async { - final sut = NativeMemory.fromUint8List(Uint8List.fromList([])); + final sut = NativeMemory.fromByteData(ByteData(0)); expect(sut.length, 0); expect(sut.pointer.address, greaterThan(0)); expect(sut.asTypedList(), isEmpty); @@ -19,18 +20,18 @@ void main() { }); test('non-empty list', () async { - final sut = NativeMemory.fromUint8List(testSrcList); - expect(sut.length, 3); + final sut = NativeMemory.fromByteData(testSrcData); + expect(sut.length, 10); expect(sut.pointer.address, greaterThan(0)); expect(sut.asTypedList(), testSrcList); sut.free(); }); test('json', () async { - final sut = NativeMemory.fromUint8List(testSrcList); + final sut = NativeMemory.fromByteData(testSrcData); final json = sut.toJson(); expect(json['address'], greaterThan(0)); - expect(json['length'], 3); + expect(json['length'], 10); expect(json.entries, hasLength(2)); final sut2 = NativeMemory.fromJson(json); diff --git a/flutter/test/native_memory_web_mock.dart b/flutter/test/native_memory_web_mock.dart index 81e03fee6f..820532c84c 100644 --- a/flutter/test/native_memory_web_mock.dart +++ b/flutter/test/native_memory_web_mock.dart @@ -9,8 +9,9 @@ class NativeMemory { const NativeMemory._(this.pointer, this.length); - factory NativeMemory.fromUint8List(Uint8List source) { - return NativeMemory._(Pointer._store(source), source.length); + factory NativeMemory.fromByteData(ByteData source) { + return NativeMemory._(Pointer._store(source.buffer.asUint8List()), + source.lengthInBytes); } factory NativeMemory.fromJson(Map json) { diff --git a/flutter/test/replay/replay_native_test.dart b/flutter/test/replay/replay_native_test.dart index 983f4274c6..baa5493e70 100644 --- a/flutter/test/replay/replay_native_test.dart +++ b/flutter/test/replay/replay_native_test.dart @@ -209,6 +209,8 @@ void main() { expect(json['length'], greaterThan(3000)); expect(json['address'], greaterThan(0)); + expect(json['width'], greaterThan(0)); + expect(json['height'], greaterThan(0)); NativeMemory.fromJson(json).free(); } From c624aa659a03084553148af78186c12ca706fccb Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Mon, 13 Jan 2025 17:52:39 +0100 Subject: [PATCH 27/28] chore: improve docs --- flutter/ios/Classes/SentryFlutterReplayScreenshotProvider.m | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/flutter/ios/Classes/SentryFlutterReplayScreenshotProvider.m b/flutter/ios/Classes/SentryFlutterReplayScreenshotProvider.m index 6ffb969423..7a6d8f2422 100644 --- a/flutter/ios/Classes/SentryFlutterReplayScreenshotProvider.m +++ b/flutter/ios/Classes/SentryFlutterReplayScreenshotProvider.m @@ -40,7 +40,7 @@ - (void)imageWithView:(UIView *_Nonnull)view length:length.unsignedLongValue freeWhenDone:TRUE]; - // We expect rawRGBA, see from + // We expect rawRGBA, see docs for ImageByteFormat: // https://api.flutter.dev/flutter/dart-ui/ImageByteFormat.html // Unencoded bytes, in RGBA row-primary form with premultiplied // alpha, 8 bits per channel. From 69e689c593dbe9f1812c0479c3b6d9802b89feb8 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Mon, 13 Jan 2025 17:53:43 +0100 Subject: [PATCH 28/28] chore: update changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f747639031..4ed99cc594 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ ### Enhancements -- Replay: improve iOS native interop performance ([#2530](https://github.com/getsentry/sentry-dart/pull/2530)) +- Replay: improve iOS native interop performance ([#2530](https://github.com/getsentry/sentry-dart/pull/2530), [#2573](https://github.com/getsentry/sentry-dart/pull/2573)) - Replay: improve orientation change tracking accuracy on Android ([#2540](https://github.com/getsentry/sentry-dart/pull/2540)) ### Fixes