From 3d722baa8568c3e80f98967b54089f3dbb888625 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Thu, 5 Sep 2024 17:07:50 +0200 Subject: [PATCH 1/3] fix: repost replay screenshots on android while idle --- .../src/native/java/sentry_native_java.dart | 145 ++++++++++++++---- flutter/test/replay/replay_native_test.dart | 39 +++-- 2 files changed, 142 insertions(+), 42 deletions(-) diff --git a/flutter/lib/src/native/java/sentry_native_java.dart b/flutter/lib/src/native/java/sentry_native_java.dart index 5ccd3a1c67..be6bd55937 100644 --- a/flutter/lib/src/native/java/sentry_native_java.dart +++ b/flutter/lib/src/native/java/sentry_native_java.dart @@ -1,5 +1,6 @@ import 'dart:ui'; +import 'package:flutter/services.dart'; import 'package:meta/meta.dart'; import '../../../sentry_flutter.dart'; @@ -13,6 +14,8 @@ import '../sentry_native_channel.dart'; @internal class SentryNativeJava extends SentryNativeChannel { ScheduledScreenshotRecorder? _replayRecorder; + String? _replayCacheDir; + _IdleFrameFiller? _idleFrameFiller; SentryNativeJava(super.options, super.channel); @override @@ -47,8 +50,7 @@ class SentryNativeJava extends SentryNativeChannel { break; case 'ReplayRecorder.stop': - await _replayRecorder?.stop(); - _replayRecorder = null; + await _stopRecorder(); hub.configureScope((s) { // ignore: invalid_use_of_internal_member @@ -58,9 +60,11 @@ class SentryNativeJava extends SentryNativeChannel { break; case 'ReplayRecorder.pause': await _replayRecorder?.stop(); + await _idleFrameFiller?.pause(); break; case 'ReplayRecorder.resume': _replayRecorder?.start(); + await _idleFrameFiller?.resume(); break; default: throw UnimplementedError('Method ${call.method} not implemented'); @@ -73,13 +77,22 @@ class SentryNativeJava extends SentryNativeChannel { @override Future close() async { + await _stopRecorder(); + return super.close(); + } + + Future _stopRecorder() async { await _replayRecorder?.stop(); + await _idleFrameFiller?.stop(); _replayRecorder = null; - return super.close(); + _idleFrameFiller = null; } void _startRecorder( String cacheDir, ScheduledScreenshotRecorderConfig config) { + _idleFrameFiller = _IdleFrameFiller( + Duration(milliseconds: 1000 ~/ config.frameRate), _addReplayScreenshot); + // Note: time measurements using a Stopwatch in a debug build: // save as rawRgba (1230876 bytes): 0.257 ms -- discarded // save as PNG (25401 bytes): 43.110 ms -- used for the final image @@ -90,39 +103,107 @@ class SentryNativeJava extends SentryNativeChannel { ScreenshotRecorderCallback callback = (image) async { var imageData = await image.toByteData(format: ImageByteFormat.png); if (imageData != null) { - final timestamp = DateTime.now().millisecondsSinceEpoch; - final filePath = "$cacheDir/$timestamp.png"; - - options.logger( - SentryLevel.debug, - 'Replay: Saving screenshot to $filePath (' - '${image.width}x${image.height} pixels, ' - '${imageData.lengthInBytes} bytes)'); - try { - await options.fileSystem - .file(filePath) - .writeAsBytes(imageData.buffer.asUint8List(), flush: true); - - await channel.invokeMethod( - 'addReplayScreenshot', - {'path': filePath, 'timestamp': timestamp}, - ); - } catch (error, stackTrace) { - options.logger( - SentryLevel.error, - 'Native call `addReplayScreenshot` failed', - exception: error, - stackTrace: stackTrace, - ); - // ignore: invalid_use_of_internal_member - if (options.automatedTestMode) { - rethrow; - } - } + final screenshot = _Screenshot(image.width, image.height, imageData); + await _addReplayScreenshot(screenshot); + _idleFrameFiller?.actualFrameReceived(screenshot); } }; + _replayCacheDir = cacheDir; _replayRecorder = ScheduledScreenshotRecorder(config, callback, options) ..start(); } + + Future _addReplayScreenshot(_Screenshot screenshot) async { + final timestamp = DateTime.now().millisecondsSinceEpoch; + final filePath = "$_replayCacheDir/$timestamp.png"; + + options.logger( + SentryLevel.debug, + 'Replay: Saving screenshot to $filePath (' + '${screenshot.width}x${screenshot.height} pixels, ' + '${screenshot.data.lengthInBytes} bytes)'); + try { + await options.fileSystem + .file(filePath) + .writeAsBytes(screenshot.data.buffer.asUint8List(), flush: true); + + await channel.invokeMethod( + 'addReplayScreenshot', + {'path': filePath, 'timestamp': timestamp}, + ); + } catch (error, stackTrace) { + options.logger( + SentryLevel.error, + 'Native call `addReplayScreenshot` failed', + exception: error, + stackTrace: stackTrace, + ); + // ignore: invalid_use_of_internal_member + if (options.automatedTestMode) { + rethrow; + } + } + } +} + +class _Screenshot { + final int width; + final int height; + final ByteData data; + + _Screenshot(this.width, this.height, this.data); +} + +// Workaround for https://github.com/getsentry/sentry-java/issues/3677 +// In short: when there are no postFrameCallbacks issued by Flutter (because +// there are no animations or user interactions), the replay recorder will +// need to get screenshots at a fixed frame rate. This class is responsible for +// filling the gaps between actual frames with the most recent frame. +class _IdleFrameFiller { + final Duration _interval; + final Future Function(_Screenshot screenshot) _callback; + bool running = true; + Future? _scheduled; + _Screenshot? _mostRecent; + + _IdleFrameFiller(this._interval, this._callback); + + 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; + // Also, the initial reposted frame will be delayed to allow actual frames + // to cancel the reposting. + repostLater(_interval * 1.5, screenshot); + } + + Future stop() async { + // Clearing [_mostRecent] stops the delayed callback from posting the image. + _mostRecent = null; + running = false; + await _scheduled; + _scheduled = null; + } + + Future pause() async { + running = false; + } + + Future resume() async { + running = true; + } + + void repostLater(Duration delay, _Screenshot screenshot) { + _scheduled = Future.delayed(delay, () async { + // Only repost if the screenshot haven't changed. + if (screenshot == _mostRecent) { + if (running) { + await _callback(screenshot); + } + // On subsequent frames, we stick to the actual frame rate. + repostLater(_interval, screenshot); + } + }); + } } diff --git a/flutter/test/replay/replay_native_test.dart b/flutter/test/replay/replay_native_test.dart index 319bb5f88f..81c6f456c3 100644 --- a/flutter/test/replay/replay_native_test.dart +++ b/flutter/test/replay/replay_native_test.dart @@ -46,7 +46,7 @@ void main() { 'directory': 'dir', 'width': 800, 'height': 600, - 'frameRate': 1000, + 'frameRate': 10, }; } @@ -142,16 +142,15 @@ void main() { var callbackFinished = Completer(); nextFrame({bool wait = true}) async { + final future = callbackFinished.future; tester.binding.scheduleFrame(); - await Future.delayed(const Duration(milliseconds: 100)); await tester.pumpAndSettle(const Duration(seconds: 1)); - await callbackFinished.future.timeout( - Duration(milliseconds: wait ? 1000 : 100), onTimeout: () { + await future.timeout(Duration(milliseconds: wait ? 1000 : 100), + onTimeout: () { if (wait) { fail('native callback not called'); } }); - callbackFinished = Completer(); } imageInfo(File file) => file.readAsBytesSync().length; @@ -162,10 +161,11 @@ void main() { final capturedImages = {}; when(native.handler('addReplayScreenshot', any)) .thenAnswer((invocation) async { - callbackFinished.complete(); final path = invocation.positionalArguments[1]["path"] as String; capturedImages[path] = imageInfo(fs.file(path)); + callbackFinished.complete(); + callbackFinished = Completer(); return null; }); @@ -191,18 +191,37 @@ void main() { expect(capturedImages, equals(fsImages())); await nextFrame(); - expect(fsImages().values, [size, size]); + fsImages().values.forEach((s) => expect(s, size)); expect(capturedImages, equals(fsImages())); - await native.invokeFromNative('ReplayRecorder.stop'); + await native.invokeFromNative('ReplayRecorder.pause'); + var count = capturedImages.length; + + await nextFrame(wait: false); + await Future.delayed(const Duration(milliseconds: 100)); + fsImages().values.forEach((s) => expect(s, size)); + expect(capturedImages, equals(fsImages())); + expect(capturedImages.length, count); await nextFrame(wait: false); - expect(fsImages().values, [size, size]); + fsImages().values.forEach((s) => expect(s, size)); expect(capturedImages, equals(fsImages())); + expect(capturedImages.length, count); + await native.invokeFromNative('ReplayRecorder.resume'); + + await nextFrame(); + fsImages().values.forEach((s) => expect(s, size)); + expect(capturedImages, equals(fsImages())); + expect(capturedImages.length, greaterThan(count)); + + await native.invokeFromNative('ReplayRecorder.stop'); + count = capturedImages.length; + await Future.delayed(const Duration(milliseconds: 100)); await nextFrame(wait: false); - expect(fsImages().values, [size, size]); + fsImages().values.forEach((s) => expect(s, size)); expect(capturedImages, equals(fsImages())); + expect(capturedImages.length, count); } else if (mockPlatform.isIOS) { // configureScope() is called on iOS when(hub.configureScope(captureAny)).thenReturn(null); From 384ef86c1dddfc3bbcc2595c309db3f6b9472ffe Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Thu, 5 Sep 2024 17:08:35 +0200 Subject: [PATCH 2/3] chore: changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1d2fb4e2b7..865e7cb8e6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ ### Features -- Session replay Alpha for Android and iOS ([#2208](https://github.com/getsentry/sentry-dart/pull/2208), [#2269](https://github.com/getsentry/sentry-dart/pull/2269), [#2236](https://github.com/getsentry/sentry-dart/pull/2236)). +- Session replay Alpha for Android and iOS ([#2208](https://github.com/getsentry/sentry-dart/pull/2208), [#2269](https://github.com/getsentry/sentry-dart/pull/2269), [#2236](https://github.com/getsentry/sentry-dart/pull/2236), [#2275](https://github.com/getsentry/sentry-dart/pull/2275)). To try out replay, you can set following options (access is limited to early access orgs on Sentry. If you're interested, [sign up for the waitlist](https://sentry.io/lp/mobile-replay-beta/)): From 63ffb5807df65e21c957febab91dee9d97f55fba Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Mon, 9 Sep 2024 14:02:54 +0200 Subject: [PATCH 3/3] review change --- flutter/lib/src/native/java/sentry_native_java.dart | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/flutter/lib/src/native/java/sentry_native_java.dart b/flutter/lib/src/native/java/sentry_native_java.dart index be6bd55937..df5e11b814 100644 --- a/flutter/lib/src/native/java/sentry_native_java.dart +++ b/flutter/lib/src/native/java/sentry_native_java.dart @@ -163,7 +163,7 @@ class _Screenshot { class _IdleFrameFiller { final Duration _interval; final Future Function(_Screenshot screenshot) _callback; - bool running = true; + bool _running = true; Future? _scheduled; _Screenshot? _mostRecent; @@ -181,24 +181,24 @@ class _IdleFrameFiller { Future stop() async { // Clearing [_mostRecent] stops the delayed callback from posting the image. _mostRecent = null; - running = false; + _running = false; await _scheduled; _scheduled = null; } Future pause() async { - running = false; + _running = false; } Future resume() async { - running = true; + _running = true; } void repostLater(Duration delay, _Screenshot screenshot) { _scheduled = Future.delayed(delay, () async { // Only repost if the screenshot haven't changed. if (screenshot == _mostRecent) { - if (running) { + if (_running) { await _callback(screenshot); } // On subsequent frames, we stick to the actual frame rate.