From 87d393d2183890ff078b08a8c22cc02d31048871 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Thu, 28 Nov 2024 13:06:19 +0100 Subject: [PATCH 01/19] refactor: move replay integration to a separate class --- flutter/lib/src/native/c/sentry_native.dart | 3 + .../src/native/cocoa/sentry_native_cocoa.dart | 15 ++-- .../src/native/java/sentry_native_java.dart | 12 +--- .../lib/src/native/sentry_native_binding.dart | 2 + .../lib/src/native/sentry_native_channel.dart | 3 + flutter/lib/src/replay/integration.dart | 27 +++++++- flutter/lib/src/sentry_flutter.dart | 2 + flutter/test/mocks.mocks.dart | 6 ++ .../test/replay/replay_integration_test.dart | 69 +++++++++++++++++++ flutter/test/replay/replay_native_test.dart | 35 ---------- flutter/test/sentry_flutter_test.dart | 4 ++ 11 files changed, 121 insertions(+), 57 deletions(-) create mode 100644 flutter/test/replay/replay_integration_test.dart diff --git a/flutter/lib/src/native/c/sentry_native.dart b/flutter/lib/src/native/c/sentry_native.dart index d9fa5a971e..1058147c4f 100644 --- a/flutter/lib/src/native/c/sentry_native.dart +++ b/flutter/lib/src/native/c/sentry_native.dart @@ -268,6 +268,9 @@ class SentryNative with SentryNativeSafeInvoker implements SentryNativeBinding { Pointer.fromAddress(1).cast().toDartString(); } + @override + bool get supportsReplay => false; + @override FutureOr captureReplay(bool isCrash) { _logNotSupported('capturing replay'); diff --git a/flutter/lib/src/native/cocoa/sentry_native_cocoa.dart b/flutter/lib/src/native/cocoa/sentry_native_cocoa.dart index 850165fcec..dd90997a22 100644 --- a/flutter/lib/src/native/cocoa/sentry_native_cocoa.dart +++ b/flutter/lib/src/native/cocoa/sentry_native_cocoa.dart @@ -5,10 +5,8 @@ import 'dart:ui'; import 'package:meta/meta.dart'; import '../../../sentry_flutter.dart'; -import '../../event_processor/replay_event_processor.dart'; import '../../screenshot/recorder.dart'; import '../../screenshot/recorder_config.dart'; -import '../../replay/integration.dart'; import '../sentry_native_channel.dart'; import 'binding.dart' as cocoa; @@ -20,19 +18,14 @@ class SentryNativeCocoa extends SentryNativeChannel { SentryNativeCocoa(super.options); + @override + bool get supportsReplay => options.platformChecker.platform.isIOS; + @override Future init(Hub hub) async { // We only need these when replay is enabled (session or error capture) // so let's set it up conditionally. This allows Dart to trim the code. - if (options.experimental.replay.isEnabled && - options.platformChecker.platform.isIOS) { - options.sdk.addIntegration(replayIntegrationName); - - // We only need the integration when error-replay capture is enabled. - if ((options.experimental.replay.onErrorSampleRate ?? 0) > 0) { - options.addEventProcessor(ReplayEventProcessor(hub, this)); - } - + if (options.experimental.replay.isEnabled) { channel.setMethodCallHandler((call) async { switch (call.method) { case 'captureReplayScreenshot': diff --git a/flutter/lib/src/native/java/sentry_native_java.dart b/flutter/lib/src/native/java/sentry_native_java.dart index 91c127de4f..5db421baa2 100644 --- a/flutter/lib/src/native/java/sentry_native_java.dart +++ b/flutter/lib/src/native/java/sentry_native_java.dart @@ -4,8 +4,6 @@ import 'package:flutter/services.dart'; import 'package:meta/meta.dart'; import '../../../sentry_flutter.dart'; -import '../../event_processor/replay_event_processor.dart'; -import '../../replay/integration.dart'; import '../../replay/scheduled_recorder.dart'; import '../../replay/scheduled_recorder_config.dart'; import '../sentry_native_channel.dart'; @@ -19,18 +17,14 @@ class SentryNativeJava extends SentryNativeChannel { _IdleFrameFiller? _idleFrameFiller; SentryNativeJava(super.options); + @override + bool get supportsReplay => true; + @override Future init(Hub hub) async { // We only need these when replay is enabled (session or error capture) // so let's set it up conditionally. This allows Dart to trim the code. if (options.experimental.replay.isEnabled) { - options.sdk.addIntegration(replayIntegrationName); - - // We only need the integration when error-replay capture is enabled. - if ((options.experimental.replay.onErrorSampleRate ?? 0) > 0) { - options.addEventProcessor(ReplayEventProcessor(hub, this)); - } - channel.setMethodCallHandler((call) async { switch (call.method) { case 'ReplayRecorder.start': diff --git a/flutter/lib/src/native/sentry_native_binding.dart b/flutter/lib/src/native/sentry_native_binding.dart index 00fa6c4104..81bf9a132a 100644 --- a/flutter/lib/src/native/sentry_native_binding.dart +++ b/flutter/lib/src/native/sentry_native_binding.dart @@ -64,5 +64,7 @@ abstract class SentryNativeBinding { FutureOr nativeCrash(); + bool get supportsReplay; + FutureOr captureReplay(bool isCrash); } diff --git a/flutter/lib/src/native/sentry_native_channel.dart b/flutter/lib/src/native/sentry_native_channel.dart index e4b6c9470b..c527a13db8 100644 --- a/flutter/lib/src/native/sentry_native_channel.dart +++ b/flutter/lib/src/native/sentry_native_channel.dart @@ -216,6 +216,9 @@ class SentryNativeChannel @override Future nativeCrash() => channel.invokeMethod('nativeCrash'); + @override + bool get supportsReplay => false; + @override Future captureReplay(bool isCrash) => channel.invokeMethod('captureReplay', { diff --git a/flutter/lib/src/replay/integration.dart b/flutter/lib/src/replay/integration.dart index cfa7ba5caf..ee66a97b35 100644 --- a/flutter/lib/src/replay/integration.dart +++ b/flutter/lib/src/replay/integration.dart @@ -1,6 +1,29 @@ +import 'dart:async'; + import 'package:meta/meta.dart'; -/// We don't actually have an integration, just want to have a name reported -/// on events so we can filter them out. +import '../../sentry_flutter.dart'; +import '../event_processor/replay_event_processor.dart'; +import '../native/sentry_native_binding.dart'; + @internal const replayIntegrationName = 'ReplayIntegration'; + +@internal +class ReplayIntegration extends Integration { + final SentryNativeBinding _native; + + ReplayIntegration(this._native); + + @override + FutureOr call(Hub hub, SentryFlutterOptions options) { + if (_native.supportsReplay && options.experimental.replay.isEnabled) { + options.sdk.addIntegration(replayIntegrationName); + + // We only need the integration when error-replay capture is enabled. + if ((options.experimental.replay.onErrorSampleRate ?? 0) > 0) { + options.addEventProcessor(ReplayEventProcessor(hub, _native)); + } + } + } +} diff --git a/flutter/lib/src/sentry_flutter.dart b/flutter/lib/src/sentry_flutter.dart index ad6049ef2d..ffc1db073e 100644 --- a/flutter/lib/src/sentry_flutter.dart +++ b/flutter/lib/src/sentry_flutter.dart @@ -25,6 +25,7 @@ import 'native/native_scope_observer.dart'; import 'native/sentry_native_binding.dart'; import 'profiling.dart'; import 'renderer/renderer.dart'; +import 'replay/integration.dart'; import 'version.dart'; import 'view_hierarchy/view_hierarchy_integration.dart'; @@ -177,6 +178,7 @@ mixin SentryFlutter { NativeAppStartHandler(native), ), ); + integrations.add(ReplayIntegration(native)); options.enableDartSymbolication = false; } diff --git a/flutter/test/mocks.mocks.dart b/flutter/test/mocks.mocks.dart index 45c3d2eabb..ea80f545d2 100644 --- a/flutter/test/mocks.mocks.dart +++ b/flutter/test/mocks.mocks.dart @@ -1686,6 +1686,12 @@ class MockSentryNativeBinding extends _i1.Mock returnValue: false, ) as bool); + @override + bool get supportsReplay => (super.noSuchMethod( + Invocation.getter(#supportsReplay), + returnValue: false, + ) as bool); + @override _i11.FutureOr init(_i2.Hub? hub) => (super.noSuchMethod(Invocation.method( diff --git a/flutter/test/replay/replay_integration_test.dart b/flutter/test/replay/replay_integration_test.dart new file mode 100644 index 0000000000..5190610d81 --- /dev/null +++ b/flutter/test/replay/replay_integration_test.dart @@ -0,0 +1,69 @@ +// ignore_for_file: invalid_use_of_internal_member + +@TestOn('vm') +library flutter_test; + +import 'package:flutter_test/flutter_test.dart'; +import 'package:mockito/mockito.dart'; +import 'package:sentry_flutter/sentry_flutter.dart'; +import 'package:sentry_flutter/src/event_processor/replay_event_processor.dart'; +import 'package:sentry_flutter/src/replay/integration.dart'; + +import '../mocks.dart'; +import '../mocks.mocks.dart'; + +void main() { + late ReplayIntegration sut; + late MockSentryNativeBinding native; + late SentryFlutterOptions options; + late MockHub hub; + + setUp(() { + hub = MockHub(); + options = defaultTestOptions(); + native = MockSentryNativeBinding(); + when(native.supportsReplay).thenReturn(true); + sut = ReplayIntegration(native); + }); + + for (var supportsReplay in [true, false]) { + test( + '$ReplayIntegration in options.sdk.integrations when supportsReplay=$supportsReplay', + () { + when(native.supportsReplay).thenReturn(supportsReplay); + options.experimental.replay.sessionSampleRate = 1.0; + sut.call(hub, options); + var matcher = contains(replayIntegrationName); + matcher = supportsReplay ? matcher : isNot(matcher); + expect(options.sdk.integrations, matcher); + }); + } + + for (var sampleRate in [0.5, 0.0]) { + test( + '$ReplayIntegration in options.sdk.integrations when sessionSampleRate=$sampleRate', + () { + options.experimental.replay.sessionSampleRate = sampleRate; + sut.call(hub, options); + var matcher = contains(replayIntegrationName); + matcher = sampleRate > 0 ? matcher : isNot(matcher); + expect(options.sdk.integrations, matcher); + }); + } + + for (var sampleRate in [0.5, 0.0]) { + test( + '$ReplayEventProcessor in options.EventProcessors when onErrorSampleRate=$sampleRate', + () async { + options.experimental.replay.onErrorSampleRate = sampleRate; + await sut.call(hub, options); + + if (sampleRate > 0) { + expect( + options.eventProcessors, anyElement(isA())); + } else { + expect(options.eventProcessors, isEmpty); + } + }); + } +} diff --git a/flutter/test/replay/replay_native_test.dart b/flutter/test/replay/replay_native_test.dart index 3b35f53c2e..efdc97781c 100644 --- a/flutter/test/replay/replay_native_test.dart +++ b/flutter/test/replay/replay_native_test.dart @@ -10,10 +10,8 @@ import 'package:file/memory.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:mockito/mockito.dart'; import 'package:sentry_flutter/sentry_flutter.dart'; -import 'package:sentry_flutter/src/event_processor/replay_event_processor.dart'; import 'package:sentry_flutter/src/native/factory.dart'; import 'package:sentry_flutter/src/native/sentry_native_binding.dart'; -import 'package:sentry_flutter/src/replay/integration.dart'; import '../mocks.dart'; import '../mocks.mocks.dart'; @@ -71,39 +69,6 @@ void main() { await sut.close(); }); - test('init adds $replayIntegrationName when replay is enabled', () async { - options.experimental.replay.sessionSampleRate = 0.1; - await sut.init(hub); - - expect(options.sdk.integrations, contains(replayIntegrationName)); - }); - - test('init does not add $replayIntegrationName when replay is disabled', - () async { - await sut.init(hub); - - expect( - options.sdk.integrations, isNot(contains(replayIntegrationName))); - }); - - test('init sets $ReplayEventProcessor when error replay is enabled', - () async { - options.experimental.replay.onErrorSampleRate = 0.1; - await sut.init(hub); - - expect(options.eventProcessors.map((e) => e.runtimeType.toString()), - contains('$ReplayEventProcessor')); - }); - - test( - 'init does not set $ReplayEventProcessor when error replay is disabled', - () async { - await sut.init(hub); - - expect(options.eventProcessors.map((e) => e.runtimeType.toString()), - isNot(contains('$ReplayEventProcessor'))); - }); - group('replay recorder', () { setUp(() async { options.experimental.replay.sessionSampleRate = 0.1; diff --git a/flutter/test/sentry_flutter_test.dart b/flutter/test/sentry_flutter_test.dart index 2788acda65..b4a1b72ae7 100644 --- a/flutter/test/sentry_flutter_test.dart +++ b/flutter/test/sentry_flutter_test.dart @@ -13,6 +13,7 @@ import 'package:sentry_flutter/src/integrations/integrations.dart'; import 'package:sentry_flutter/src/integrations/screenshot_integration.dart'; import 'package:sentry_flutter/src/profiling.dart'; import 'package:sentry_flutter/src/renderer/renderer.dart'; +import 'package:sentry_flutter/src/replay/integration.dart'; import 'package:sentry_flutter/src/version.dart'; import 'package:sentry_flutter/src/view_hierarchy/view_hierarchy_integration.dart'; import 'mocks.dart'; @@ -101,6 +102,7 @@ void main() { ...nativeIntegrations, ...platformAgnosticIntegrations, ...nonWebIntegrations, + ReplayIntegration, ], shouldNotHaveIntegrations: [ ...iOsAndMacOsIntegrations, @@ -158,6 +160,7 @@ void main() { ...nativeIntegrations, ...platformAgnosticIntegrations, ...nonWebIntegrations, + ReplayIntegration, ], shouldNotHaveIntegrations: [ ...androidIntegrations, @@ -711,6 +714,7 @@ MockSentryNativeBinding mockNativeBinding() { final result = MockSentryNativeBinding(); when(result.supportsLoadContexts).thenReturn(true); when(result.supportsCaptureEnvelope).thenReturn(true); + when(result.supportsReplay).thenReturn(false); when(result.captureEnvelope(any, any)).thenReturn(null); when(result.init(any)).thenReturn(null); when(result.close()).thenReturn(null); From e8c067ce085301f3cddd54ffd92d3905fe4bd6c9 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Thu, 28 Nov 2024 16:47:40 +0100 Subject: [PATCH 02/19] Add onBuild callback to ScreenshotWidget --- .../screenshot/sentry_screenshot_widget.dart | 63 ++++++++++ flutter/lib/src/sentry_flutter.dart | 1 + .../sentry_screenshot_widget_test.dart | 108 ++++++++---------- .../sentry_screenshot_widget_test.mocks.dart | 47 ++++++++ 4 files changed, 159 insertions(+), 60 deletions(-) create mode 100644 flutter/test/screenshot/sentry_screenshot_widget_test.mocks.dart diff --git a/flutter/lib/src/screenshot/sentry_screenshot_widget.dart b/flutter/lib/src/screenshot/sentry_screenshot_widget.dart index 95d7e2560a..40505fa0e1 100644 --- a/flutter/lib/src/screenshot/sentry_screenshot_widget.dart +++ b/flutter/lib/src/screenshot/sentry_screenshot_widget.dart @@ -30,16 +30,79 @@ class SentryScreenshotWidget extends StatefulWidget { _SentryScreenshotWidgetState createState() => _SentryScreenshotWidgetState(); /// This is true when the [SentryScreenshotWidget] is in the widget tree. + @internal static bool get isMounted => sentryScreenshotWidgetGlobalKey.currentContext != null; + + @internal + static void reset() { + _status = null; + _onBuild.clear(); + } + + static SentryScreenshotWidgetStatus? _status; + static final _onBuild = []; + + /// Registers a persistent callback that is called whenever the widget is + /// built. The callback is called with the current and previous widget status. + /// To unregister, return false; + /// If the widget is already built, the callback is called immediately. + /// Note: the callback must not throw and it must not call onBuild(). + @internal + static void onBuild(SentryScreenshotWidgetOnBuildCallback callback) { + bool register = true; + final currentStatus = _status; + if (currentStatus != null) { + register = callback(currentStatus, null); + } + if (register) { + _onBuild.add(callback); + } + } } +typedef SentryScreenshotWidgetOnBuildCallback = bool Function( + SentryScreenshotWidgetStatus currentStatus, + SentryScreenshotWidgetStatus? previousStatus); + class _SentryScreenshotWidgetState extends State { @override Widget build(BuildContext context) { + final status = SentryScreenshotWidgetStatus( + size: MediaQuery.maybeSizeOf(context), + pixelRatio: MediaQuery.maybeDevicePixelRatioOf(context), + orientantion: MediaQuery.maybeOrientationOf(context), + ); + final prevStatus = SentryScreenshotWidget._status; + SentryScreenshotWidget._status = status; + + if (SentryScreenshotWidget._onBuild.isNotEmpty) { + final unregisterCallbacks = []; + for (final callback in SentryScreenshotWidget._onBuild) { + if (!callback(status, prevStatus)) { + unregisterCallbacks.add(callback); + } + } + unregisterCallbacks.forEach(SentryScreenshotWidget._onBuild.remove); + } + return RepaintBoundary( key: sentryScreenshotWidgetGlobalKey, child: widget.child, ); } } + +@visibleForTesting +@immutable +class SentryScreenshotWidgetStatus { + final Size? size; + final double? pixelRatio; + final Orientation? orientantion; + + const SentryScreenshotWidgetStatus({ + required this.size, + required this.pixelRatio, + required this.orientantion, + }); +} diff --git a/flutter/lib/src/sentry_flutter.dart b/flutter/lib/src/sentry_flutter.dart index ffc1db073e..003b669f30 100644 --- a/flutter/lib/src/sentry_flutter.dart +++ b/flutter/lib/src/sentry_flutter.dart @@ -56,6 +56,7 @@ mixin SentryFlutter { AppRunner? appRunner, @internal SentryFlutterOptions? options, }) async { + SentryScreenshotWidget.reset(); options ??= SentryFlutterOptions(); // ignore: invalid_use_of_internal_member diff --git a/flutter/test/screenshot/sentry_screenshot_widget_test.dart b/flutter/test/screenshot/sentry_screenshot_widget_test.dart index bedbfe8f1f..3e04e2fde2 100644 --- a/flutter/test/screenshot/sentry_screenshot_widget_test.dart +++ b/flutter/test/screenshot/sentry_screenshot_widget_test.dart @@ -1,79 +1,67 @@ @TestOn('vm') library flutter_test; -// ignore_for_file: invalid_use_of_internal_member -import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; +import 'package:mockito/annotations.dart'; +import 'package:mockito/mockito.dart'; import 'package:sentry_flutter/sentry_flutter.dart'; -import '../mocks.dart'; +import 'sentry_screenshot_widget_test.mocks.dart'; +import 'test_widget.dart'; void main() { - late Fixture fixture; - setUp(() { - fixture = Fixture(); - TestWidgetsFlutterBinding.ensureInitialized(); - }); + TestWidgetsFlutterBinding.ensureInitialized(); - testWidgets( - '$SentryScreenshotWidget does not apply when attachScreenshot is false', - (tester) async { - await tester.pumpWidget( - fixture.getSut( - attachScreenshot: false, - ), - ); + group('onBuild', () { + setUp(() { + SentryScreenshotWidget.reset(); + }); - final widget = find.byType(MyApp); - final repaintBoundaryFinder = find.descendant( - of: widget, - matching: find.byType(RepaintBoundary), - ); - expect(repaintBoundaryFinder, findsNothing); - }, - ); + testWidgets('calls immediately if it has status already', (tester) async { + await pumpTestElement(tester); + final mock = MockCallbacks().onBuild; + when(mock(any, any)).thenReturn(true); + SentryScreenshotWidget.onBuild(mock); + verify(mock(any, null)); + }); - testWidgets( - '$SentryScreenshotWidget applies when attachScreenshot is true', - (tester) async { - await tester.pumpWidget( - fixture.getSut( - attachScreenshot: true, - ), - ); + testWidgets('calls after the widget appears', (tester) async { + final mock = MockCallbacks().onBuild; + when(mock(any, any)).thenReturn(true); + SentryScreenshotWidget.onBuild(mock); - final widget = find.byType(MyApp); - final repaintBoundaryFinder = find.ancestor( - of: widget, - matching: find.byKey(sentryScreenshotWidgetGlobalKey), - ); - expect(repaintBoundaryFinder, findsOneWidget); - }, - ); -} + await pumpTestElement(tester); -class Fixture { - final _options = defaultTestOptions(); - late Hub hub; + final status = verify(mock(captureAny, null)).captured[0] + as SentryScreenshotWidgetStatus?; + expect(status, isNotNull); - SentryScreenshotWidget getSut({ - bool attachScreenshot = false, - }) { - _options.attachScreenshot = attachScreenshot; + await pumpTestElement(tester); - hub = Hub(_options); + verify(mock(any, status)); + }); - return SentryScreenshotWidget( - child: MaterialApp(home: MyApp()), - ); - } -} + testWidgets('unregisters the the callback if it returns false', + (tester) async { + bool returnValue = true; + final mock = MockCallbacks().onBuild; + when(mock(any, any)).thenAnswer((_) => returnValue); + SentryScreenshotWidget.onBuild(mock); -class MyApp extends StatelessWidget { - const MyApp({super.key}); + await pumpTestElement(tester); + await pumpTestElement(tester); + verify(mock(any, any)).called(2); + + returnValue = false; + await pumpTestElement(tester); + verify(mock(any, any)).called(1); + await pumpTestElement(tester); + verifyNever(mock(any, any)); + }); + }); +} - @override - Widget build(BuildContext context) { - return const Text('test'); - } +@GenerateMocks([Callbacks]) +abstract class Callbacks { + bool onBuild(SentryScreenshotWidgetStatus a, SentryScreenshotWidgetStatus? b); } diff --git a/flutter/test/screenshot/sentry_screenshot_widget_test.mocks.dart b/flutter/test/screenshot/sentry_screenshot_widget_test.mocks.dart new file mode 100644 index 0000000000..10ab1df8e8 --- /dev/null +++ b/flutter/test/screenshot/sentry_screenshot_widget_test.mocks.dart @@ -0,0 +1,47 @@ +// Mocks generated by Mockito 5.4.4 from annotations +// in sentry_flutter/test/screenshot/sentry_screenshot_widget_test.dart. +// Do not manually edit this file. + +// ignore_for_file: no_leading_underscores_for_library_prefixes +import 'package:mockito/mockito.dart' as _i1; +import 'package:sentry_flutter/sentry_flutter.dart' as _i3; + +import 'sentry_screenshot_widget_test.dart' as _i2; + +// ignore_for_file: type=lint +// ignore_for_file: avoid_redundant_argument_values +// ignore_for_file: avoid_setters_without_getters +// ignore_for_file: comment_references +// ignore_for_file: deprecated_member_use +// ignore_for_file: deprecated_member_use_from_same_package +// ignore_for_file: implementation_imports +// ignore_for_file: invalid_use_of_visible_for_testing_member +// ignore_for_file: prefer_const_constructors +// ignore_for_file: unnecessary_parenthesis +// ignore_for_file: camel_case_types +// ignore_for_file: subtype_of_sealed_class + +/// A class which mocks [Callbacks]. +/// +/// See the documentation for Mockito's code generation for more information. +class MockCallbacks extends _i1.Mock implements _i2.Callbacks { + MockCallbacks() { + _i1.throwOnMissingStub(this); + } + + @override + bool onBuild( + _i3.SentryScreenshotWidgetStatus? a, + _i3.SentryScreenshotWidgetStatus? b, + ) => + (super.noSuchMethod( + Invocation.method( + #onBuild, + [ + a, + b, + ], + ), + returnValue: false, + ) as bool); +} From 845d2669ca72dd8ce2568678871e869eaf746425 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Sun, 1 Dec 2024 10:33:20 +0100 Subject: [PATCH 03/19] configure replay from dart to native (java) --- .../io/sentry/flutter/SentryFlutterPlugin.kt | 27 ++++++++++++++++++- flutter/lib/src/native/c/sentry_native.dart | 6 +++++ .../lib/src/native/sentry_native_binding.dart | 3 +++ .../lib/src/native/sentry_native_channel.dart | 10 +++++++ flutter/lib/src/replay/replay_config.dart | 22 +++++++++++++++ 5 files changed, 67 insertions(+), 1 deletion(-) create mode 100644 flutter/lib/src/replay/replay_config.dart diff --git a/flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutterPlugin.kt b/flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutterPlugin.kt index d3a03efb0c..9b4560f606 100644 --- a/flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutterPlugin.kt +++ b/flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutterPlugin.kt @@ -2,6 +2,7 @@ package io.sentry.flutter import android.app.Activity import android.content.Context +import android.content.res.Configuration; import android.os.Build import android.os.Looper import android.util.Log @@ -27,6 +28,7 @@ import io.sentry.android.core.SentryAndroidOptions import io.sentry.android.core.performance.AppStartMetrics import io.sentry.android.core.performance.TimeSpan import io.sentry.android.replay.ReplayIntegration +import io.sentry.android.replay.ScreenshotRecorderConfig import io.sentry.protocol.DebugImage import io.sentry.protocol.SdkVersion import io.sentry.protocol.SentryId @@ -34,6 +36,7 @@ import io.sentry.protocol.User import io.sentry.transport.CurrentDateProvider import java.io.File import java.lang.ref.WeakReference +import kotlin.math.roundToInt private const val APP_START_MAX_DURATION_MS = 60000 @@ -42,6 +45,14 @@ class SentryFlutterPlugin : FlutterPlugin, MethodCallHandler, ActivityAware { private lateinit var context: Context private lateinit var sentryFlutter: SentryFlutter private lateinit var replay: ReplayIntegration + private var replayConfig = ScreenshotRecorderConfig( + recordingWidth = 0, + recordingHeight = 0, + scaleFactorX = 1.0f, + scaleFactorY = 1.0f, + frameRate = 0, + bitRate = 0 + ) private var activity: WeakReference? = null private var framesTracker: ActivityFramesTracker? = null @@ -86,6 +97,7 @@ class SentryFlutterPlugin : FlutterPlugin, MethodCallHandler, ActivityAware { "loadContexts" -> loadContexts(result) "displayRefreshRate" -> displayRefreshRate(result) "nativeCrash" -> crash() + "setReplayConfig" -> setReplayConfig(call, result) "addReplayScreenshot" -> addReplayScreenshot(call.argument("path"), call.argument("timestamp"), result) "captureReplay" -> captureReplay(call.argument("isCrash"), result) else -> result.notImplemented() @@ -152,7 +164,7 @@ class SentryFlutterPlugin : FlutterPlugin, MethodCallHandler, ActivityAware { context, dateProvider = CurrentDateProvider.getInstance(), recorderProvider = { SentryFlutterReplayRecorder(channel, replay) }, - recorderConfigProvider = null, + recorderConfigProvider = { replayConfig }, replayCacheProvider = null, ) replay.breadcrumbConverter = SentryFlutterReplayBreadcrumbConverter() @@ -577,6 +589,19 @@ class SentryFlutterPlugin : FlutterPlugin, MethodCallHandler, ActivityAware { result.success("") } + private fun setReplayConfig(call: MethodCall, result: Result) { + replayConfig = ScreenshotRecorderConfig( + recordingWidth = call.argument("width") as? Int ?: 0, + recordingHeight = call.argument("height") as? Int ?: 0, + scaleFactorX = 1.0f, + scaleFactorY = 1.0f, + frameRate = call.argument("frameRate") as? Int ?: 0, + bitRate = call.argument("bitRate") as? Int ?: 0 + ) + replay.onConfigurationChanged(Configuration()) + result.success("") + } + private fun captureReplay( isCrash: Boolean?, result: Result, diff --git a/flutter/lib/src/native/c/sentry_native.dart b/flutter/lib/src/native/c/sentry_native.dart index 1058147c4f..788a052cae 100644 --- a/flutter/lib/src/native/c/sentry_native.dart +++ b/flutter/lib/src/native/c/sentry_native.dart @@ -8,6 +8,7 @@ import 'package:ffi/ffi.dart'; import 'package:meta/meta.dart'; import '../../../sentry_flutter.dart'; +import '../../replay/replay_config.dart'; import '../native_app_start.dart'; import '../native_frames.dart'; import '../sentry_native_binding.dart'; @@ -271,6 +272,11 @@ class SentryNative with SentryNativeSafeInvoker implements SentryNativeBinding { @override bool get supportsReplay => false; + @override + FutureOr setReplayConfig(ReplayConfig config) { + _logNotSupported('replay config'); + } + @override FutureOr captureReplay(bool isCrash) { _logNotSupported('capturing replay'); diff --git a/flutter/lib/src/native/sentry_native_binding.dart b/flutter/lib/src/native/sentry_native_binding.dart index 81bf9a132a..0a00f385be 100644 --- a/flutter/lib/src/native/sentry_native_binding.dart +++ b/flutter/lib/src/native/sentry_native_binding.dart @@ -4,6 +4,7 @@ import 'dart:typed_data'; import 'package:meta/meta.dart'; import '../../sentry_flutter.dart'; +import '../replay/replay_config.dart'; import 'native_app_start.dart'; import 'native_frames.dart'; @@ -66,5 +67,7 @@ abstract class SentryNativeBinding { bool get supportsReplay; + FutureOr setReplayConfig(ReplayConfig config); + FutureOr captureReplay(bool isCrash); } diff --git a/flutter/lib/src/native/sentry_native_channel.dart b/flutter/lib/src/native/sentry_native_channel.dart index c527a13db8..759daa95c6 100644 --- a/flutter/lib/src/native/sentry_native_channel.dart +++ b/flutter/lib/src/native/sentry_native_channel.dart @@ -7,6 +7,7 @@ import 'package:flutter/services.dart'; import 'package:meta/meta.dart'; import '../../sentry_flutter.dart'; +import '../replay/replay_config.dart'; import 'native_app_start.dart'; import 'native_frames.dart'; import 'method_channel_helper.dart'; @@ -219,6 +220,15 @@ class SentryNativeChannel @override bool get supportsReplay => false; + @override + Future setReplayConfig(ReplayConfig config) => + channel.invokeMethod('setReplayConfig', { + 'width': config.width, + 'height': config.height, + 'frameRate': config.frameRate, + 'bitRate': config.bitRate, + }); + @override Future captureReplay(bool isCrash) => channel.invokeMethod('captureReplay', { diff --git a/flutter/lib/src/replay/replay_config.dart b/flutter/lib/src/replay/replay_config.dart new file mode 100644 index 0000000000..847ab04cc7 --- /dev/null +++ b/flutter/lib/src/replay/replay_config.dart @@ -0,0 +1,22 @@ +import 'package:meta/meta.dart'; + +import 'scheduled_recorder_config.dart'; + +@immutable +@internal +class ReplayConfig extends ScheduledScreenshotRecorderConfig { + @override + int get width => super.width!; + + @override + int get height => super.height!; + + final int bitRate; + + ReplayConfig({ + required int super.width, + required int super.height, + required super.frameRate, + required this.bitRate, + }); +} From e5df0ccc549a9dccc547976c0c00389b05d56fd4 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Sun, 1 Dec 2024 15:19:25 +0100 Subject: [PATCH 04/19] send replay config from dart to native --- flutter/lib/src/replay/integration.dart | 13 +++++++++++++ flutter/lib/src/replay/replay_config.dart | 2 +- .../lib/src/replay/scheduled_recorder_config.dart | 2 +- flutter/lib/src/screenshot/recorder_config.dart | 2 +- 4 files changed, 16 insertions(+), 3 deletions(-) diff --git a/flutter/lib/src/replay/integration.dart b/flutter/lib/src/replay/integration.dart index ee66a97b35..56211904f8 100644 --- a/flutter/lib/src/replay/integration.dart +++ b/flutter/lib/src/replay/integration.dart @@ -5,6 +5,7 @@ import 'package:meta/meta.dart'; import '../../sentry_flutter.dart'; import '../event_processor/replay_event_processor.dart'; import '../native/sentry_native_binding.dart'; +import 'replay_config.dart'; @internal const replayIntegrationName = 'ReplayIntegration'; @@ -24,6 +25,18 @@ class ReplayIntegration extends Integration { if ((options.experimental.replay.onErrorSampleRate ?? 0) > 0) { options.addEventProcessor(ReplayEventProcessor(hub, _native)); } + + SentryScreenshotWidget.onBuild((status, prevStatus) { + if (status != prevStatus) { + _native.setReplayConfig(ReplayConfig( + width: status.size?.width.round() ?? 0, + height: status.size?.height.round() ?? 0, + frameRate: 1, + bitRate: 75000, // TODO replay quality config + )); + } + return true; + }); } } } diff --git a/flutter/lib/src/replay/replay_config.dart b/flutter/lib/src/replay/replay_config.dart index 847ab04cc7..b5d7216cb2 100644 --- a/flutter/lib/src/replay/replay_config.dart +++ b/flutter/lib/src/replay/replay_config.dart @@ -13,7 +13,7 @@ class ReplayConfig extends ScheduledScreenshotRecorderConfig { final int bitRate; - ReplayConfig({ + const ReplayConfig({ required int super.width, required int super.height, required super.frameRate, diff --git a/flutter/lib/src/replay/scheduled_recorder_config.dart b/flutter/lib/src/replay/scheduled_recorder_config.dart index 8f3c3addb2..67c5d672ef 100644 --- a/flutter/lib/src/replay/scheduled_recorder_config.dart +++ b/flutter/lib/src/replay/scheduled_recorder_config.dart @@ -3,7 +3,7 @@ import '../screenshot/recorder_config.dart'; class ScheduledScreenshotRecorderConfig extends ScreenshotRecorderConfig { final int frameRate; - ScheduledScreenshotRecorderConfig({ + const ScheduledScreenshotRecorderConfig({ super.width, super.height, required this.frameRate, diff --git a/flutter/lib/src/screenshot/recorder_config.dart b/flutter/lib/src/screenshot/recorder_config.dart index 922a7cd62a..9bc84a51b8 100644 --- a/flutter/lib/src/screenshot/recorder_config.dart +++ b/flutter/lib/src/screenshot/recorder_config.dart @@ -7,7 +7,7 @@ class ScreenshotRecorderConfig { final int? width; final int? height; - ScreenshotRecorderConfig({ + const ScreenshotRecorderConfig({ this.width, this.height, }); From 51eb81ad92dbe3e441b32a4d1ada040fe13522dc Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Mon, 2 Dec 2024 17:01:40 +0100 Subject: [PATCH 05/19] fix: missing return in android plugin leading to duplicate response --- .../io/sentry/flutter/SentryFlutterPlugin.kt | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutterPlugin.kt b/flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutterPlugin.kt index 9b4560f606..e22a702baa 100644 --- a/flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutterPlugin.kt +++ b/flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutterPlugin.kt @@ -193,6 +193,7 @@ class SentryFlutterPlugin : FlutterPlugin, MethodCallHandler, ActivityAware { "Invalid app start data: app not launched in foreground or app start took too long (>60s)", ) result.success(null) + return } val appStartTimeSpan = appStartMetrics.appStartTimeSpan @@ -558,6 +559,20 @@ class SentryFlutterPlugin : FlutterPlugin, MethodCallHandler, ActivityAware { mainThread.uncaughtExceptionHandler.uncaughtException(mainThread, exception) mainThread.join(NATIVE_CRASH_WAIT_TIME) } + + /** + * Since codec block size is 16, so we have to adjust the width and height to it, otherwise + * the codec might fail to configure on some devices, see + * https://cs.android.com/android/platform/superproject/+/master:frameworks/base/media/java/android/media/MediaCodecInfo.java;l=1999-2001 + */ + private fun Int.adjustReplaySizeToBlockSize(): Int { + val remainder = this % 16 + return if (remainder <= 8) { + this - remainder + } else { + this + (16 - remainder) + } + } } private fun loadContexts(result: Result) { @@ -591,8 +606,8 @@ class SentryFlutterPlugin : FlutterPlugin, MethodCallHandler, ActivityAware { private fun setReplayConfig(call: MethodCall, result: Result) { replayConfig = ScreenshotRecorderConfig( - recordingWidth = call.argument("width") as? Int ?: 0, - recordingHeight = call.argument("height") as? Int ?: 0, + recordingWidth = (call.argument("width") as? Int)?.adjustReplaySizeToBlockSize() ?: 0, + recordingHeight = (call.argument("height") as? Int)?.adjustReplaySizeToBlockSize() ?: 0, scaleFactorX = 1.0f, scaleFactorY = 1.0f, frameRate = call.argument("frameRate") as? Int ?: 0, From 5d2ba470d64bbf846d9325389ef20690a65b08cb Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Mon, 2 Dec 2024 17:45:08 +0100 Subject: [PATCH 06/19] fix android replay start before the setting is present --- .../kotlin/io/sentry/flutter/SentryFlutterReplayRecorder.kt | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutterReplayRecorder.kt b/flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutterReplayRecorder.kt index ba285a12a0..1edc430575 100644 --- a/flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutterReplayRecorder.kt +++ b/flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutterReplayRecorder.kt @@ -13,6 +13,12 @@ internal class SentryFlutterReplayRecorder( private val integration: ReplayIntegration, ) : Recorder { override fun start(recorderConfig: ScreenshotRecorderConfig) { + // Ignore if this is the initial call before we actually got the configuration from Flutter. + // We'll get another call here when the configuration is set. + if (recorderConfig.recordingHeight == 0 && recorderConfig.recordingWidth == 0) { + return; + } + val cacheDirPath = integration.replayCacheDir?.absolutePath if (cacheDirPath == null) { Log.w("Sentry", "Replay cache directory is null, can't start replay recorder.") From ec3c2272a4d55f420185fe4bd2a97dd2db12a0b2 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Mon, 2 Dec 2024 18:22:52 +0100 Subject: [PATCH 07/19] tmp log --- .../src/main/kotlin/io/sentry/flutter/SentryFlutterPlugin.kt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutterPlugin.kt b/flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutterPlugin.kt index e22a702baa..b06b55f275 100644 --- a/flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutterPlugin.kt +++ b/flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutterPlugin.kt @@ -613,6 +613,10 @@ class SentryFlutterPlugin : FlutterPlugin, MethodCallHandler, ActivityAware { frameRate = call.argument("frameRate") as? Int ?: 0, bitRate = call.argument("bitRate") as? Int ?: 0 ) + Log.i( + "Sentry", + "Configuring replay: %dx%d at %d FPS, %d BPS".format(replayConfig.recordingWidth, replayConfig.recordingHeight, replayConfig.frameRate, replayConfig.bitRate) + ) replay.onConfigurationChanged(Configuration()) result.success("") } From 48014cca5b5bf6e419bbd89f806874a8e1b261dc Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Mon, 2 Dec 2024 18:23:59 +0100 Subject: [PATCH 08/19] logging --- .../main/kotlin/io/sentry/flutter/SentryFlutterPlugin.kt | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutterPlugin.kt b/flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutterPlugin.kt index b06b55f275..ca0721f37e 100644 --- a/flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutterPlugin.kt +++ b/flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutterPlugin.kt @@ -164,7 +164,13 @@ class SentryFlutterPlugin : FlutterPlugin, MethodCallHandler, ActivityAware { context, dateProvider = CurrentDateProvider.getInstance(), recorderProvider = { SentryFlutterReplayRecorder(channel, replay) }, - recorderConfigProvider = { replayConfig }, + recorderConfigProvider = { + Log.i( + "Sentry", + "Replay configuration requested. Returning: %dx%d at %d FPS, %d BPS".format(replayConfig.recordingWidth, replayConfig.recordingHeight, replayConfig.frameRate, replayConfig.bitRate) + ) + replayConfig + }, replayCacheProvider = null, ) replay.breadcrumbConverter = SentryFlutterReplayBreadcrumbConverter() From ea4c2b32ffd82b13045ec00208c3dbe326bee16a Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Tue, 3 Dec 2024 13:49:14 +0100 Subject: [PATCH 09/19] change screenshot dimensions to double --- .../io/sentry/flutter/SentryFlutterPlugin.kt | 27 +++++--- .../src/native/java/sentry_native_java.dart | 4 +- flutter/lib/src/replay/integration.dart | 4 +- flutter/lib/src/replay/replay_config.dart | 8 +-- .../lib/src/screenshot/recorder_config.dart | 4 +- .../screenshot/sentry_screenshot_quality.dart | 2 +- .../screenshot_event_processor_test.dart | 2 +- flutter/test/mocks.mocks.dart | 62 +++++++++++-------- flutter/test/screenshot/recorder_test.dart | 4 +- 9 files changed, 68 insertions(+), 49 deletions(-) diff --git a/flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutterPlugin.kt b/flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutterPlugin.kt index ca0721f37e..8e5beb9a1b 100644 --- a/flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutterPlugin.kt +++ b/flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutterPlugin.kt @@ -566,12 +566,7 @@ class SentryFlutterPlugin : FlutterPlugin, MethodCallHandler, ActivityAware { mainThread.join(NATIVE_CRASH_WAIT_TIME) } - /** - * Since codec block size is 16, so we have to adjust the width and height to it, otherwise - * the codec might fail to configure on some devices, see - * https://cs.android.com/android/platform/superproject/+/master:frameworks/base/media/java/android/media/MediaCodecInfo.java;l=1999-2001 - */ - private fun Int.adjustReplaySizeToBlockSize(): Int { + private fun Double.adjustReplaySizeToBlockSize(): Double { val remainder = this % 16 return if (remainder <= 8) { this - remainder @@ -611,9 +606,25 @@ class SentryFlutterPlugin : FlutterPlugin, MethodCallHandler, ActivityAware { } private fun setReplayConfig(call: MethodCall, result: Result) { + // Since codec block size is 16, so we have to adjust the width and height to it, + // otherwise the codec might fail to configure on some devices, see + // https://cs.android.com/android/platform/superproject/+/master:frameworks/base/media/java/android/media/MediaCodecInfo.java;l=1999-2001 + var width = call.argument("width") as? Double ?: 0.0 + var height = call.argument("height") as? Double ?: 0.0 + // First update the smaller dimension, as changing that will affect the screen ratio more. + if (width < height) { + val newWidth = width.adjustReplaySizeToBlockSize() + height = (height * (newWidth / width)).adjustReplaySizeToBlockSize() + width = newWidth + } else { + val newHeight = height.adjustReplaySizeToBlockSize() + width = (width * (newHeight / height)).adjustReplaySizeToBlockSize() + height = newHeight + } + replayConfig = ScreenshotRecorderConfig( - recordingWidth = (call.argument("width") as? Int)?.adjustReplaySizeToBlockSize() ?: 0, - recordingHeight = (call.argument("height") as? Int)?.adjustReplaySizeToBlockSize() ?: 0, + recordingWidth = width.roundToInt(), + recordingHeight = height.roundToInt(), scaleFactorX = 1.0f, scaleFactorY = 1.0f, frameRate = call.argument("frameRate") as? Int ?: 0, diff --git a/flutter/lib/src/native/java/sentry_native_java.dart b/flutter/lib/src/native/java/sentry_native_java.dart index 5db421baa2..b93882c22a 100644 --- a/flutter/lib/src/native/java/sentry_native_java.dart +++ b/flutter/lib/src/native/java/sentry_native_java.dart @@ -34,8 +34,8 @@ class SentryNativeJava extends SentryNativeChannel { _startRecorder( call.arguments['directory'] as String, ScheduledScreenshotRecorderConfig( - width: call.arguments['width'] as int, - height: call.arguments['height'] as int, + width: (call.arguments['width'] as num).toDouble(), + height: (call.arguments['height'] as num).toDouble(), frameRate: call.arguments['frameRate'] as int, ), ); diff --git a/flutter/lib/src/replay/integration.dart b/flutter/lib/src/replay/integration.dart index 56211904f8..ce8e0d78e2 100644 --- a/flutter/lib/src/replay/integration.dart +++ b/flutter/lib/src/replay/integration.dart @@ -29,8 +29,8 @@ class ReplayIntegration extends Integration { SentryScreenshotWidget.onBuild((status, prevStatus) { if (status != prevStatus) { _native.setReplayConfig(ReplayConfig( - width: status.size?.width.round() ?? 0, - height: status.size?.height.round() ?? 0, + width: status.size?.width ?? 0.0, + height: status.size?.height ?? 0.0, frameRate: 1, bitRate: 75000, // TODO replay quality config )); diff --git a/flutter/lib/src/replay/replay_config.dart b/flutter/lib/src/replay/replay_config.dart index b5d7216cb2..0294d9bf3e 100644 --- a/flutter/lib/src/replay/replay_config.dart +++ b/flutter/lib/src/replay/replay_config.dart @@ -6,16 +6,16 @@ import 'scheduled_recorder_config.dart'; @internal class ReplayConfig extends ScheduledScreenshotRecorderConfig { @override - int get width => super.width!; + double get width => super.width!; @override - int get height => super.height!; + double get height => super.height!; final int bitRate; const ReplayConfig({ - required int super.width, - required int super.height, + required double super.width, + required double super.height, required super.frameRate, required this.bitRate, }); diff --git a/flutter/lib/src/screenshot/recorder_config.dart b/flutter/lib/src/screenshot/recorder_config.dart index 9bc84a51b8..b388b198d6 100644 --- a/flutter/lib/src/screenshot/recorder_config.dart +++ b/flutter/lib/src/screenshot/recorder_config.dart @@ -4,8 +4,8 @@ import 'package:meta/meta.dart'; @internal class ScreenshotRecorderConfig { - final int? width; - final int? height; + final double? width; + final double? height; const ScreenshotRecorderConfig({ this.width, diff --git a/flutter/lib/src/screenshot/sentry_screenshot_quality.dart b/flutter/lib/src/screenshot/sentry_screenshot_quality.dart index 9b36add9d6..1323fb1d42 100644 --- a/flutter/lib/src/screenshot/sentry_screenshot_quality.dart +++ b/flutter/lib/src/screenshot/sentry_screenshot_quality.dart @@ -5,7 +5,7 @@ enum SentryScreenshotQuality { medium, low; - int? targetResolution() { + double? targetResolution() { switch (this) { case SentryScreenshotQuality.full: return null; // Uses the device pixel ratio to scale the screenshot diff --git a/flutter/test/event_processor/screenshot_event_processor_test.dart b/flutter/test/event_processor/screenshot_event_processor_test.dart index 6250a6557d..b544c9a114 100644 --- a/flutter/test/event_processor/screenshot_event_processor_test.dart +++ b/flutter/test/event_processor/screenshot_event_processor_test.dart @@ -27,7 +27,7 @@ void main() { FlutterRenderer? renderer, { required bool isWeb, required bool added, - int? expectedMaxWidthOrHeight, + double? expectedMaxWidthOrHeight, }) async { // Run with real async https://stackoverflow.com/a/54021863 await tester.runAsync(() async { diff --git a/flutter/test/mocks.mocks.dart b/flutter/test/mocks.mocks.dart index ea80f545d2..47dece300e 100644 --- a/flutter/test/mocks.mocks.dart +++ b/flutter/test/mocks.mocks.dart @@ -4,14 +4,14 @@ // ignore_for_file: no_leading_underscores_for_library_prefixes import 'dart:async' as _i11; -import 'dart:developer' as _i23; +import 'dart:developer' as _i24; import 'dart:typed_data' as _i19; import 'dart:ui' as _i6; import 'package:flutter/foundation.dart' as _i8; import 'package:flutter/gestures.dart' as _i7; import 'package:flutter/rendering.dart' as _i10; -import 'package:flutter/scheduler.dart' as _i22; +import 'package:flutter/scheduler.dart' as _i23; import 'package:flutter/services.dart' as _i4; import 'package:flutter/src/widgets/binding.dart' as _i5; import 'package:flutter/widgets.dart' as _i9; @@ -24,9 +24,10 @@ import 'package:sentry/src/profiling.dart' as _i16; import 'package:sentry/src/sentry_tracer.dart' as _i3; import 'package:sentry_flutter/sentry_flutter.dart' as _i2; import 'package:sentry_flutter/src/frames_tracking/sentry_delayed_frames_tracker.dart' - as _i21; + as _i22; import 'package:sentry_flutter/src/native/native_frames.dart' as _i20; import 'package:sentry_flutter/src/native/sentry_native_binding.dart' as _i18; +import 'package:sentry_flutter/src/replay/replay_config.dart' as _i21; import 'mocks.dart' as _i14; @@ -1823,6 +1824,13 @@ class MockSentryNativeBinding extends _i1.Mock [stackTrace], )) as _i11.FutureOr?>); + @override + _i11.FutureOr setReplayConfig(_i21.ReplayConfig? config) => + (super.noSuchMethod(Invocation.method( + #setReplayConfig, + [config], + )) as _i11.FutureOr); + @override _i11.FutureOr<_i2.SentryId> captureReplay(bool? isCrash) => (super.noSuchMethod( @@ -1844,16 +1852,16 @@ class MockSentryNativeBinding extends _i1.Mock /// /// See the documentation for Mockito's code generation for more information. class MockSentryDelayedFramesTracker extends _i1.Mock - implements _i21.SentryDelayedFramesTracker { + implements _i22.SentryDelayedFramesTracker { MockSentryDelayedFramesTracker() { _i1.throwOnMissingStub(this); } @override - List<_i21.SentryFrameTiming> get delayedFrames => (super.noSuchMethod( + List<_i22.SentryFrameTiming> get delayedFrames => (super.noSuchMethod( Invocation.getter(#delayedFrames), - returnValue: <_i21.SentryFrameTiming>[], - ) as List<_i21.SentryFrameTiming>); + returnValue: <_i22.SentryFrameTiming>[], + ) as List<_i22.SentryFrameTiming>); @override bool get isTrackingActive => (super.noSuchMethod( @@ -1880,7 +1888,7 @@ class MockSentryDelayedFramesTracker extends _i1.Mock ); @override - List<_i21.SentryFrameTiming> getFramesIntersecting({ + List<_i22.SentryFrameTiming> getFramesIntersecting({ required DateTime? startTimestamp, required DateTime? endTimestamp, }) => @@ -1893,8 +1901,8 @@ class MockSentryDelayedFramesTracker extends _i1.Mock #endTimestamp: endTimestamp, }, ), - returnValue: <_i21.SentryFrameTiming>[], - ) as List<_i21.SentryFrameTiming>); + returnValue: <_i22.SentryFrameTiming>[], + ) as List<_i22.SentryFrameTiming>); @override void addFrame( @@ -1923,7 +1931,7 @@ class MockSentryDelayedFramesTracker extends _i1.Mock ); @override - _i21.SpanFrameMetrics? getFrameMetrics({ + _i22.SpanFrameMetrics? getFrameMetrics({ required DateTime? spanStartTimestamp, required DateTime? spanEndTimestamp, }) => @@ -1934,7 +1942,7 @@ class MockSentryDelayedFramesTracker extends _i1.Mock #spanStartTimestamp: spanStartTimestamp, #spanEndTimestamp: spanEndTimestamp, }, - )) as _i21.SpanFrameMetrics?); + )) as _i22.SpanFrameMetrics?); @override void clear() => super.noSuchMethod( @@ -2073,17 +2081,17 @@ class MockWidgetsFlutterBinding extends _i1.Mock ) as _i7.SamplingClock); @override - _i22.SchedulingStrategy get schedulingStrategy => (super.noSuchMethod( + _i23.SchedulingStrategy get schedulingStrategy => (super.noSuchMethod( Invocation.getter(#schedulingStrategy), returnValue: ({ required int priority, - required _i22.SchedulerBinding scheduler, + required _i23.SchedulerBinding scheduler, }) => false, - ) as _i22.SchedulingStrategy); + ) as _i23.SchedulingStrategy); @override - set schedulingStrategy(_i22.SchedulingStrategy? _schedulingStrategy) => + set schedulingStrategy(_i23.SchedulingStrategy? _schedulingStrategy) => super.noSuchMethod( Invocation.setter( #schedulingStrategy, @@ -2111,10 +2119,10 @@ class MockWidgetsFlutterBinding extends _i1.Mock ) as bool); @override - _i22.SchedulerPhase get schedulerPhase => (super.noSuchMethod( + _i23.SchedulerPhase get schedulerPhase => (super.noSuchMethod( Invocation.getter(#schedulerPhase), - returnValue: _i22.SchedulerPhase.idle, - ) as _i22.SchedulerPhase); + returnValue: _i23.SchedulerPhase.idle, + ) as _i23.SchedulerPhase); @override bool get framesEnabled => (super.noSuchMethod( @@ -2687,10 +2695,10 @@ class MockWidgetsFlutterBinding extends _i1.Mock @override _i11.Future scheduleTask( - _i22.TaskCallback? task, - _i22.Priority? priority, { + _i23.TaskCallback? task, + _i23.Priority? priority, { String? debugLabel, - _i23.Flow? flow, + _i24.Flow? flow, }) => (super.noSuchMethod( Invocation.method( @@ -2748,7 +2756,7 @@ class MockWidgetsFlutterBinding extends _i1.Mock @override int scheduleFrameCallback( - _i22.FrameCallback? callback, { + _i23.FrameCallback? callback, { bool? rescheduling = false, }) => (super.noSuchMethod( @@ -2798,7 +2806,7 @@ class MockWidgetsFlutterBinding extends _i1.Mock ) as bool); @override - void addPersistentFrameCallback(_i22.FrameCallback? callback) => + void addPersistentFrameCallback(_i23.FrameCallback? callback) => super.noSuchMethod( Invocation.method( #addPersistentFrameCallback, @@ -2809,7 +2817,7 @@ class MockWidgetsFlutterBinding extends _i1.Mock @override void addPostFrameCallback( - _i22.FrameCallback? callback, { + _i23.FrameCallback? callback, { String? debugLabel = r'callback', }) => super.noSuchMethod( @@ -2885,12 +2893,12 @@ class MockWidgetsFlutterBinding extends _i1.Mock ); @override - _i22.PerformanceModeRequestHandle? requestPerformanceMode( + _i23.PerformanceModeRequestHandle? requestPerformanceMode( _i6.DartPerformanceMode? mode) => (super.noSuchMethod(Invocation.method( #requestPerformanceMode, [mode], - )) as _i22.PerformanceModeRequestHandle?); + )) as _i23.PerformanceModeRequestHandle?); @override void handleDrawFrame() => super.noSuchMethod( diff --git a/flutter/test/screenshot/recorder_test.dart b/flutter/test/screenshot/recorder_test.dart index 4ecb78cff6..9fecbb0b0f 100644 --- a/flutter/test/screenshot/recorder_test.dart +++ b/flutter/test/screenshot/recorder_test.dart @@ -103,7 +103,7 @@ void main() async { class _Fixture { late final ScreenshotRecorder sut; - _Fixture({int? width, int? height}) { + _Fixture({double? width, double? height}) { sut = ScreenshotRecorder( ScreenshotRecorderConfig(width: width, height: height), defaultTestOptions()..bindingUtils = TestBindingWrapper(), @@ -111,7 +111,7 @@ class _Fixture { } static Future<_Fixture> create(WidgetTester tester, - {int? width, int? height}) async { + {double? width, double? height}) async { final fixture = _Fixture(width: width, height: height); await pumpTestElement(tester); return fixture; From 8dcc806405e27f3bd1f8aa7c27f42ed3a469ad9a Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Tue, 3 Dec 2024 21:08:43 +0100 Subject: [PATCH 10/19] refactor android replay integration and fix timing issues during orientation changes --- .../native/java/android_replay_recorder.dart | 47 +++++ .../src/native/java/sentry_native_java.dart | 160 ++------------- .../lib/src/replay/scheduled_recorder.dart | 184 ++++++++++++++++-- flutter/lib/src/replay/scheduler.dart | 4 +- flutter/lib/src/screenshot/recorder.dart | 30 ++- 5 files changed, 258 insertions(+), 167 deletions(-) create mode 100644 flutter/lib/src/native/java/android_replay_recorder.dart diff --git a/flutter/lib/src/native/java/android_replay_recorder.dart b/flutter/lib/src/native/java/android_replay_recorder.dart new file mode 100644 index 0000000000..467dbf2819 --- /dev/null +++ b/flutter/lib/src/native/java/android_replay_recorder.dart @@ -0,0 +1,47 @@ +import 'package:meta/meta.dart'; + +import '../../../sentry_flutter.dart'; +import '../../replay/scheduled_recorder.dart'; +import '../sentry_safe_method_channel.dart'; + +@internal +class AndroidReplayRecorder extends ScheduledScreenshotRecorder { + final SentrySafeMethodChannel _channel; + final String _cacheDir; + + AndroidReplayRecorder( + super.config, super.options, this._channel, this._cacheDir) { + super.callback = _addReplayScreenshot; + } + + Future _addReplayScreenshot(ScreenshotPng screenshot) async { + final timestamp = DateTime.now().millisecondsSinceEpoch; + final filePath = "$_cacheDir/$timestamp.png"; + + options.logger( + SentryLevel.debug, + '$logName: 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, + '$logName: native call `addReplayScreenshot` failed', + exception: error, + stackTrace: stackTrace, + ); + if (options.automatedTestMode) { + rethrow; + } + } + } +} diff --git a/flutter/lib/src/native/java/sentry_native_java.dart b/flutter/lib/src/native/java/sentry_native_java.dart index b93882c22a..7b01bbded3 100644 --- a/flutter/lib/src/native/java/sentry_native_java.dart +++ b/flutter/lib/src/native/java/sentry_native_java.dart @@ -1,20 +1,13 @@ -import 'dart:ui'; - -import 'package:flutter/services.dart'; import 'package:meta/meta.dart'; import '../../../sentry_flutter.dart'; -import '../../replay/scheduled_recorder.dart'; import '../../replay/scheduled_recorder_config.dart'; import '../sentry_native_channel.dart'; +import 'android_replay_recorder.dart'; -// Note: currently this doesn't do anything. Later, it shall be used with -// generated JNI bindings. See https://github.com/getsentry/sentry-dart/issues/1444 @internal class SentryNativeJava extends SentryNativeChannel { - ScheduledScreenshotRecorder? _replayRecorder; - String? _replayCacheDir; - _IdleFrameFiller? _idleFrameFiller; + AndroidReplayRecorder? _replayRecorder; SentryNativeJava(super.options); @override @@ -31,15 +24,17 @@ class SentryNativeJava extends SentryNativeChannel { final replayId = SentryId.fromId(call.arguments['replayId'] as String); - _startRecorder( - call.arguments['directory'] as String, + final config = ScheduledScreenshotRecorderConfig( width: (call.arguments['width'] as num).toDouble(), height: (call.arguments['height'] as num).toDouble(), - frameRate: call.arguments['frameRate'] as int, - ), + frameRate: call.arguments['frameRate'] as int, ); + _replayRecorder = AndroidReplayRecorder( + config, options, channel, call.arguments['directory'] as String) + ..start(); + hub.configureScope((s) { // ignore: invalid_use_of_internal_member s.replayId = replayId; @@ -47,21 +42,21 @@ class SentryNativeJava extends SentryNativeChannel { break; case 'ReplayRecorder.stop': - await _stopRecorder(); - hub.configureScope((s) { // ignore: invalid_use_of_internal_member s.replayId = null; }); + final future = _replayRecorder?.stop(); + _replayRecorder = null; + await future; + break; case 'ReplayRecorder.pause': - await _replayRecorder?.stop(); - await _idleFrameFiller?.pause(); + await _replayRecorder?.pause(); break; case 'ReplayRecorder.resume': - _replayRecorder?.start(); - await _idleFrameFiller?.resume(); + await _replayRecorder?.resume(); break; default: throw UnimplementedError('Method ${call.method} not implemented'); @@ -74,132 +69,7 @@ class SentryNativeJava extends SentryNativeChannel { @override Future close() async { - await _stopRecorder(); - return super.close(); - } - - Future _stopRecorder() async { await _replayRecorder?.stop(); - await _idleFrameFiller?.stop(); - _replayRecorder = null; - _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 - // image size: 25401 bytes - // save to file: 3.677 ms - // onScreenshotRecorded1: 1.237 ms - // released and exiting callback: 0.021 ms - ScreenshotRecorderCallback callback = (image) async { - var imageData = await image.toByteData(format: ImageByteFormat.png); - if (imageData != null) { - 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, - ); - 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); - } - }); + return super.close(); } } diff --git a/flutter/lib/src/replay/scheduled_recorder.dart b/flutter/lib/src/replay/scheduled_recorder.dart index 70d8ff390c..925ef86c06 100644 --- a/flutter/lib/src/replay/scheduled_recorder.dart +++ b/flutter/lib/src/replay/scheduled_recorder.dart @@ -1,4 +1,5 @@ import 'dart:async'; +import 'dart:typed_data'; import 'dart:ui'; import 'package:meta/meta.dart'; @@ -9,32 +10,193 @@ import 'scheduled_recorder_config.dart'; import 'scheduler.dart'; @internal -typedef ScreenshotRecorderCallback = Future Function(Image); +typedef ScheduledScreenshotRecorderCallback = Future Function( + ScreenshotPng); @internal class ScheduledScreenshotRecorder extends ScreenshotRecorder { late final Scheduler _scheduler; - final ScreenshotRecorderCallback _callback; + late final ScheduledScreenshotRecorderCallback _callback; + var _status = _Status.running; + late final Duration _frameDuration; + late final _idleFrameFiller = _IdleFrameFiller(_frameDuration, _onScreenshot); - ScheduledScreenshotRecorder(ScheduledScreenshotRecorderConfig config, - this._callback, SentryFlutterOptions options) - : super(config, options) { + @override + @protected + ScheduledScreenshotRecorderConfig get config => + super.config as ScheduledScreenshotRecorderConfig; + + ScheduledScreenshotRecorder( + ScheduledScreenshotRecorderConfig config, SentryFlutterOptions options, + [ScheduledScreenshotRecorderCallback? callback, String? logName]) + : super(config, options, logName: logName) { assert(config.frameRate > 0); - final frameDuration = Duration(milliseconds: 1000 ~/ config.frameRate); - _scheduler = Scheduler(frameDuration, _capture, + _frameDuration = Duration(milliseconds: 1000 ~/ config.frameRate); + assert(_frameDuration.inMicroseconds > 0); + _scheduler = Scheduler(_frameDuration, _capture, options.bindingUtils.instance!.addPostFrameCallback); + if (callback != null) { + _callback = callback; + } + } + + set callback(ScheduledScreenshotRecorderCallback callback) { + _callback = callback; } void start() { - options.logger(SentryLevel.debug, "Replay: starting replay capture."); + assert(() { + // The following fails if callback hasn't been provided + // in the constructor nor set with a setter. + _callback; + return true; + }()); + + options.logger(SentryLevel.debug, + "$logName: starting capture (${config.width}x${config.height} @ ${config.frameRate} Hz)."); + _status = _Status.running; + _startScheduler(); + } + + void _startScheduler() { _scheduler.start(); + + // We need to schedule a frame because if this happens in-between user + // actions, there may not be any frame captured for a long time so even + // the IdleFrameFiller won't have anything to repeat. This would appear + // as if the replay was broken. + options.bindingUtils.instance!.scheduleFrame(); } Future stop() async { - await _scheduler.stop(); - options.logger(SentryLevel.debug, "Replay: replay capture stopped."); + options.logger(SentryLevel.debug, "$logName: stopping capture."); + _status = _Status.stopped; + await Future.wait([_scheduler.stop(), _idleFrameFiller.stop()]); + options.logger(SentryLevel.debug, "$logName: capture stopped."); + } + + Future pause() async { + if (_status == _Status.running) { + _status = _Status.paused; + _idleFrameFiller.pause(); + await _scheduler.stop(); + } + } + + Future resume() async { + if (_status == _Status.paused) { + _status = _Status.running; + _startScheduler(); + _idleFrameFiller.resume(); + } } Future _capture(Duration sinceSchedulerEpoch) async => - capture(_callback); + capture(_onImageCaptured); + + Future _onImageCaptured(Image image) async { + if (_status == _Status.running) { + var imageData = await image.toByteData(format: ImageByteFormat.png); + if (imageData != null) { + final screenshot = ScreenshotPng(image.width, image.height, imageData); + await _onScreenshot(screenshot); + _idleFrameFiller.actualFrameReceived(screenshot); + } else { + options.logger( + SentryLevel.debug, + '$logName: failed to convert screenshot to PNG, ' + 'toByteData() returned null. (${image.width}x${image.height} pixels)'); + } + } else { + // drop any screenshots from callbacks if the replay has already been stopped/paused. + options.logger(SentryLevel.debug, + '$logName: screenshot dropped because status=${_status.name}.'); + } + } + + Future _onScreenshot(ScreenshotPng screenshot) async { + if (_status == _Status.running) { + await _callback(screenshot); + } else { + // drop any screenshots from callbacks if the replay has already been stopped/paused. + options.logger(SentryLevel.debug, + '$logName: screenshot dropped because status=${_status.name}.'); + } + } } + +@internal +@immutable +class ScreenshotPng { + final int width; + final int height; + final ByteData data; + + const ScreenshotPng(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(ScreenshotPng screenshot) _callback; + var _status = _Status.running; + Future? _scheduled; + ScreenshotPng? _mostRecent; + + _IdleFrameFiller(this._interval, this._callback); + + void actualFrameReceived(ScreenshotPng 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 { + _status = _Status.stopped; + final scheduled = _scheduled; + _scheduled = null; + _mostRecent = null; + await scheduled; + } + + void pause() async { + if (_status == _Status.running) { + _status = _Status.paused; + } + } + + void resume() async { + if (_status == _Status.paused) { + _status = _Status.running; + } + } + + void repostLater(Duration delay, ScreenshotPng screenshot) { + _scheduled = Future.delayed(delay, () async { + if (_status == _Status.stopped) { + return; + } + + // Only repost if the screenshot haven't changed. + if (screenshot == _mostRecent) { + if (_status == _Status.running) { + // We don't strictly need to await here but it helps reduce load. + // If the callback takes a long time, we keep the spacing between + // calls consistent. + await _callback(screenshot); + } + // On subsequent frames, we stick to the actual frame rate. + repostLater(_interval, screenshot); + } + }); + } +} + +enum _Status { stopped, running, paused } diff --git a/flutter/lib/src/replay/scheduler.dart b/flutter/lib/src/replay/scheduler.dart index 4d246360e3..2b085ccf25 100644 --- a/flutter/lib/src/replay/scheduler.dart +++ b/flutter/lib/src/replay/scheduler.dart @@ -39,7 +39,9 @@ class Scheduler { @pragma('vm:prefer-inline') void _scheduleNext() { - _scheduled ??= Future.delayed(_interval, _runAfterNextFrame); + if (_running) { + _scheduled ??= Future.delayed(_interval, _runAfterNextFrame); + } } @pragma('vm:prefer-inline') diff --git a/flutter/lib/src/screenshot/recorder.dart b/flutter/lib/src/screenshot/recorder.dart index 2d8dfffa67..51295de814 100644 --- a/flutter/lib/src/screenshot/recorder.dart +++ b/flutter/lib/src/screenshot/recorder.dart @@ -12,13 +12,16 @@ import 'widget_filter.dart'; @internal typedef ScreenshotRecorderCallback = Future Function(Image); +var _instanceCounter = 0; + @internal class ScreenshotRecorder { @protected final ScreenshotRecorderConfig config; @protected final SentryFlutterOptions options; - final String _logName; + @protected + late final String logName; WidgetFilter? _widgetFilter; bool _warningLogged = false; @@ -27,8 +30,16 @@ class ScreenshotRecorder { bool get hasWidgetFilter => _widgetFilter != null; // TODO: remove [isReplayRecorder] parameter in the next major release, see _SentryFlutterExperimentalOptions. - ScreenshotRecorder(this.config, this.options, {bool isReplayRecorder = true}) - : _logName = isReplayRecorder ? 'ReplayRecorder' : 'ScreenshotRecorder' { + ScreenshotRecorder(this.config, this.options, + {bool isReplayRecorder = true, String? logName}) { + if (logName != null) { + this.logName = logName; + } else if (isReplayRecorder) { + _instanceCounter++; + this.logName = 'ReplayRecorder #$_instanceCounter'; + } else { + this.logName = 'ScreenshotRecorder'; + } // see `options.experimental.privacy` docs for details final privacyOptions = isReplayRecorder ? options.experimental.privacyForReplay @@ -45,7 +56,7 @@ class ScreenshotRecorder { if (context == null || renderObject == null) { if (!_warningLogged) { options.logger(SentryLevel.warning, - "$_logName: SentryScreenshotWidget is not attached, skipping capture."); + "$logName: SentryScreenshotWidget is not attached, skipping capture."); _warningLogged = true; } return; @@ -96,6 +107,10 @@ class ScreenshotRecorder { try { final finalImage = await picture.toImage( (srcWidth * pixelRatio).round(), (srcHeight * pixelRatio).round()); + options.logger( + SentryLevel.debug, + "$logName: captured a screenshot in ${watch.elapsedMilliseconds}" + " ms ($blockingTime ms blocking)."); try { await callback(finalImage); } finally { @@ -104,14 +119,9 @@ class ScreenshotRecorder { } finally { picture.dispose(); } - - options.logger( - SentryLevel.debug, - "$_logName: captured a screenshot in ${watch.elapsedMilliseconds}" - " ms ($blockingTime ms blocking)."); } catch (e, stackTrace) { options.logger( - SentryLevel.error, "$_logName: failed to capture screenshot.", + SentryLevel.error, "$logName: failed to capture screenshot.", exception: e, stackTrace: stackTrace); if (options.automatedTestMode) { rethrow; From accab5a00e986de1c0cfed92d98769b37d11aff5 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Tue, 3 Dec 2024 21:39:54 +0100 Subject: [PATCH 11/19] fix tests --- flutter/test/replay/replay_native_test.dart | 5 ++- .../test/replay/scheduled_recorder_test.dart | 35 +++++++++++-------- flutter/test/replay/scheduler_test.dart | 11 +++--- 3 files changed, 30 insertions(+), 21 deletions(-) diff --git a/flutter/test/replay/replay_native_test.dart b/flutter/test/replay/replay_native_test.dart index efdc97781c..5d292f8653 100644 --- a/flutter/test/replay/replay_native_test.dart +++ b/flutter/test/replay/replay_native_test.dart @@ -115,6 +115,8 @@ void main() { testWidgets('captures images', (tester) async { await tester.runAsync(() async { + when(hub.configureScope(captureAny)).thenReturn(null); + if (mockPlatform.isAndroid) { var callbackFinished = Completer(); @@ -200,9 +202,6 @@ void main() { expect(capturedImages, equals(fsImages())); expect(capturedImages.length, count); } else if (mockPlatform.isIOS) { - // configureScope() is called on iOS - when(hub.configureScope(captureAny)).thenReturn(null); - nextFrame() async { tester.binding.scheduleFrame(); await Future.delayed(const Duration(milliseconds: 100)); diff --git a/flutter/test/replay/scheduled_recorder_test.dart b/flutter/test/replay/scheduled_recorder_test.dart index 603c2b24b2..62a8c8e0ff 100644 --- a/flutter/test/replay/scheduled_recorder_test.dart +++ b/flutter/test/replay/scheduled_recorder_test.dart @@ -3,7 +3,7 @@ @TestOn('vm') library dart_test; -import 'dart:ui'; +import 'dart:async'; import 'package:flutter_test/flutter_test.dart'; import 'package:sentry_flutter/src/replay/scheduled_recorder.dart'; @@ -16,16 +16,18 @@ void main() async { TestWidgetsFlutterBinding.ensureInitialized(); testWidgets('captures images', (tester) async { - final fixture = await _Fixture.create(tester); - expect(fixture.capturedImages, isEmpty); - await fixture.nextFrame(); - expect(fixture.capturedImages, ['1000x750']); - await fixture.nextFrame(); - expect(fixture.capturedImages, ['1000x750', '1000x750']); - final stopFuture = fixture.sut.stop(); - await fixture.nextFrame(); - await stopFuture; - expect(fixture.capturedImages, ['1000x750', '1000x750']); + await tester.runAsync(() async { + final fixture = await _Fixture.create(tester); + expect(fixture.capturedImages, isEmpty); + await fixture.nextFrame(); + expect(fixture.capturedImages, ['1000x750']); + await fixture.nextFrame(); + expect(fixture.capturedImages, ['1000x750', '1000x750']); + final stopFuture = fixture.sut.stop(); + await fixture.nextFrame(); + await stopFuture; + expect(fixture.capturedImages, ['1000x750', '1000x750']); + }); }); } @@ -33,6 +35,7 @@ class _Fixture { final WidgetTester _tester; late final ScheduledScreenshotRecorder sut; final capturedImages = []; + late Completer _completer; _Fixture._(this._tester) { sut = ScheduledScreenshotRecorder( @@ -41,10 +44,11 @@ class _Fixture { height: 1000, frameRate: 1000, ), - (Image image) async { + defaultTestOptions()..bindingUtils = TestBindingWrapper(), + (image) async { capturedImages.add("${image.width}x${image.height}"); + _completer.complete(); }, - defaultTestOptions()..bindingUtils = TestBindingWrapper(), ); } @@ -56,7 +60,10 @@ class _Fixture { } Future nextFrame() async { + _completer = Completer(); _tester.binding.scheduleFrame(); - await _tester.pumpAndSettle(const Duration(seconds: 1)); + await _tester.pumpAndSettle(const Duration(seconds: 100)); + await _completer.future + .timeout(Duration(milliseconds: 100), onTimeout: () {}); } } diff --git a/flutter/test/replay/scheduler_test.dart b/flutter/test/replay/scheduler_test.dart index c41260c854..6c74a0691b 100644 --- a/flutter/test/replay/scheduler_test.dart +++ b/flutter/test/replay/scheduler_test.dart @@ -63,12 +63,15 @@ class _Fixture { sut = Scheduler( const Duration(milliseconds: 1), (_) async => calls++, - (FrameCallback callback, {String debugLabel = 'callback'}) { - registeredCallback = callback; - }, + _AddPostFrameCallbackMock, ); } + void _AddPostFrameCallbackMock(FrameCallback callback, + {String debugLabel = 'callback'}) { + registeredCallback = callback; + } + factory _Fixture.started() { return _Fixture()..sut.start(); } @@ -76,7 +79,7 @@ class _Fixture { Future drawFrame() async { await Future.delayed(const Duration(milliseconds: 8), () {}); _frames++; - registeredCallback!(Duration(milliseconds: _frames)); + registeredCallback?.call(Duration(milliseconds: _frames)); registeredCallback = null; } } From 0c8e6aa1917b6804aefa859c67542433ba88cb28 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Tue, 3 Dec 2024 22:15:38 +0100 Subject: [PATCH 12/19] cleanup --- .../native/java/android_replay_recorder.dart | 7 ++++--- .../src/native/java/sentry_native_java.dart | 6 ++---- .../lib/src/replay/scheduled_recorder.dart | 21 +++++++++++-------- .../test/replay/scheduled_recorder_test.dart | 18 +++++++++++----- 4 files changed, 31 insertions(+), 21 deletions(-) diff --git a/flutter/lib/src/native/java/android_replay_recorder.dart b/flutter/lib/src/native/java/android_replay_recorder.dart index 467dbf2819..60a667e6eb 100644 --- a/flutter/lib/src/native/java/android_replay_recorder.dart +++ b/flutter/lib/src/native/java/android_replay_recorder.dart @@ -14,14 +14,15 @@ class AndroidReplayRecorder extends ScheduledScreenshotRecorder { super.callback = _addReplayScreenshot; } - Future _addReplayScreenshot(ScreenshotPng screenshot) async { + Future _addReplayScreenshot( + ScreenshotPng screenshot, bool isNewlyCaptured) async { final timestamp = DateTime.now().millisecondsSinceEpoch; final filePath = "$_cacheDir/$timestamp.png"; options.logger( SentryLevel.debug, - '$logName: saving screenshot to $filePath (' - '${screenshot.width}x${screenshot.height} pixels, ' + '$logName: saving ${isNewlyCaptured ? 'new' : 'repeated'} screenshot to' + ' $filePath (${screenshot.width}x${screenshot.height} pixels, ' '${screenshot.data.lengthInBytes} bytes)'); try { await options.fileSystem diff --git a/flutter/lib/src/native/java/sentry_native_java.dart b/flutter/lib/src/native/java/sentry_native_java.dart index 7b01bbded3..7aa47f2d73 100644 --- a/flutter/lib/src/native/java/sentry_native_java.dart +++ b/flutter/lib/src/native/java/sentry_native_java.dart @@ -24,12 +24,10 @@ class SentryNativeJava extends SentryNativeChannel { final replayId = SentryId.fromId(call.arguments['replayId'] as String); - final config = - ScheduledScreenshotRecorderConfig( + final config = ScheduledScreenshotRecorderConfig( width: (call.arguments['width'] as num).toDouble(), height: (call.arguments['height'] as num).toDouble(), - frameRate: call.arguments['frameRate'] as int, - ); + frameRate: call.arguments['frameRate'] as int); _replayRecorder = AndroidReplayRecorder( config, options, channel, call.arguments['directory'] as String) diff --git a/flutter/lib/src/replay/scheduled_recorder.dart b/flutter/lib/src/replay/scheduled_recorder.dart index 925ef86c06..89299112e0 100644 --- a/flutter/lib/src/replay/scheduled_recorder.dart +++ b/flutter/lib/src/replay/scheduled_recorder.dart @@ -11,7 +11,7 @@ import 'scheduler.dart'; @internal typedef ScheduledScreenshotRecorderCallback = Future Function( - ScreenshotPng); + ScreenshotPng screenshot, bool isNewlyCaptured); @internal class ScheduledScreenshotRecorder extends ScreenshotRecorder { @@ -33,8 +33,10 @@ class ScheduledScreenshotRecorder extends ScreenshotRecorder { assert(config.frameRate > 0); _frameDuration = Duration(milliseconds: 1000 ~/ config.frameRate); assert(_frameDuration.inMicroseconds > 0); + _scheduler = Scheduler(_frameDuration, _capture, options.bindingUtils.instance!.addPostFrameCallback); + if (callback != null) { _callback = callback; } @@ -99,7 +101,7 @@ class ScheduledScreenshotRecorder extends ScreenshotRecorder { var imageData = await image.toByteData(format: ImageByteFormat.png); if (imageData != null) { final screenshot = ScreenshotPng(image.width, image.height, imageData); - await _onScreenshot(screenshot); + await _onScreenshot(screenshot, true); _idleFrameFiller.actualFrameReceived(screenshot); } else { options.logger( @@ -114,9 +116,10 @@ class ScheduledScreenshotRecorder extends ScreenshotRecorder { } } - Future _onScreenshot(ScreenshotPng screenshot) async { + Future _onScreenshot( + ScreenshotPng screenshot, bool isNewlyCaptured) async { if (_status == _Status.running) { - await _callback(screenshot); + await _callback(screenshot, isNewlyCaptured); } else { // drop any screenshots from callbacks if the replay has already been stopped/paused. options.logger(SentryLevel.debug, @@ -142,7 +145,7 @@ class ScreenshotPng { // filling the gaps between actual frames with the most recent frame. class _IdleFrameFiller { final Duration _interval; - final Future Function(ScreenshotPng screenshot) _callback; + final ScheduledScreenshotRecorderCallback _callback; var _status = _Status.running; Future? _scheduled; ScreenshotPng? _mostRecent; @@ -187,10 +190,10 @@ class _IdleFrameFiller { // Only repost if the screenshot haven't changed. if (screenshot == _mostRecent) { if (_status == _Status.running) { - // We don't strictly need to await here but it helps reduce load. - // If the callback takes a long time, we keep the spacing between - // calls consistent. - await _callback(screenshot); + // We don't strictly need to await here but it helps to reduce load. + // If the callback takes a long time, we still wait between calls, + // based on the configured rate. + await _callback(screenshot, false); } // On subsequent frames, we stick to the actual frame rate. repostLater(_interval, screenshot); diff --git a/flutter/test/replay/scheduled_recorder_test.dart b/flutter/test/replay/scheduled_recorder_test.dart index 62a8c8e0ff..003157b5f2 100644 --- a/flutter/test/replay/scheduled_recorder_test.dart +++ b/flutter/test/replay/scheduled_recorder_test.dart @@ -15,7 +15,7 @@ import '../screenshot/test_widget.dart'; void main() async { TestWidgetsFlutterBinding.ensureInitialized(); - testWidgets('captures images', (tester) async { + testWidgets('captures images on frame updates', (tester) async { await tester.runAsync(() async { final fixture = await _Fixture.create(tester); expect(fixture.capturedImages, isEmpty); @@ -45,9 +45,11 @@ class _Fixture { frameRate: 1000, ), defaultTestOptions()..bindingUtils = TestBindingWrapper(), - (image) async { - capturedImages.add("${image.width}x${image.height}"); - _completer.complete(); + (image, isNewlyCaptured) async { + capturedImages.add('${image.width}x${image.height}'); + if (!_completer.isCompleted) { + _completer.complete(); + } }, ); } @@ -62,7 +64,13 @@ class _Fixture { Future nextFrame() async { _completer = Completer(); _tester.binding.scheduleFrame(); - await _tester.pumpAndSettle(const Duration(seconds: 100)); + await _tester.pumpAndSettle(const Duration(seconds: 1)); + await _completer.future + .timeout(Duration(milliseconds: 100), onTimeout: () {}); + } + + Future tryNextImage() async { + _completer = Completer(); await _completer.future .timeout(Duration(milliseconds: 100), onTimeout: () {}); } From 20e7088c74f68f33ac09214430a98285e7350445 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Tue, 3 Dec 2024 22:19:24 +0100 Subject: [PATCH 13/19] ktlint --- .../io/sentry/flutter/SentryFlutterPlugin.kt | 72 ++++++++++++------- .../flutter/SentryFlutterReplayRecorder.kt | 2 +- .../io/sentry/samples/flutter/MainActivity.kt | 1 + 3 files changed, 47 insertions(+), 28 deletions(-) diff --git a/flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutterPlugin.kt b/flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutterPlugin.kt index 8e5beb9a1b..8b22797b70 100644 --- a/flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutterPlugin.kt +++ b/flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutterPlugin.kt @@ -2,7 +2,7 @@ package io.sentry.flutter import android.app.Activity import android.content.Context -import android.content.res.Configuration; +import android.content.res.Configuration import android.os.Build import android.os.Looper import android.util.Log @@ -40,19 +40,23 @@ import kotlin.math.roundToInt private const val APP_START_MAX_DURATION_MS = 60000 -class SentryFlutterPlugin : FlutterPlugin, MethodCallHandler, ActivityAware { +class SentryFlutterPlugin : + FlutterPlugin, + MethodCallHandler, + ActivityAware { private lateinit var channel: MethodChannel private lateinit var context: Context private lateinit var sentryFlutter: SentryFlutter private lateinit var replay: ReplayIntegration - private var replayConfig = ScreenshotRecorderConfig( - recordingWidth = 0, - recordingHeight = 0, - scaleFactorX = 1.0f, - scaleFactorY = 1.0f, - frameRate = 0, - bitRate = 0 - ) + private var replayConfig = + ScreenshotRecorderConfig( + recordingWidth = 0, + recordingHeight = 0, + scaleFactorX = 1.0f, + scaleFactorY = 1.0f, + frameRate = 0, + bitRate = 0, + ) private var activity: WeakReference? = null private var framesTracker: ActivityFramesTracker? = null @@ -167,7 +171,12 @@ class SentryFlutterPlugin : FlutterPlugin, MethodCallHandler, ActivityAware { recorderConfigProvider = { Log.i( "Sentry", - "Replay configuration requested. Returning: %dx%d at %d FPS, %d BPS".format(replayConfig.recordingWidth, replayConfig.recordingHeight, replayConfig.frameRate, replayConfig.bitRate) + "Replay configuration requested. Returning: %dx%d at %d FPS, %d BPS".format( + replayConfig.recordingWidth, + replayConfig.recordingHeight, + replayConfig.frameRate, + replayConfig.bitRate, + ), ) replayConfig }, @@ -567,12 +576,12 @@ class SentryFlutterPlugin : FlutterPlugin, MethodCallHandler, ActivityAware { } private fun Double.adjustReplaySizeToBlockSize(): Double { - val remainder = this % 16 - return if (remainder <= 8) { - this - remainder - } else { - this + (16 - remainder) - } + val remainder = this % 16 + return if (remainder <= 8) { + this - remainder + } else { + this + (16 - remainder) + } } } @@ -605,7 +614,10 @@ class SentryFlutterPlugin : FlutterPlugin, MethodCallHandler, ActivityAware { result.success("") } - private fun setReplayConfig(call: MethodCall, result: Result) { + private fun setReplayConfig( + call: MethodCall, + result: Result, + ) { // Since codec block size is 16, so we have to adjust the width and height to it, // otherwise the codec might fail to configure on some devices, see // https://cs.android.com/android/platform/superproject/+/master:frameworks/base/media/java/android/media/MediaCodecInfo.java;l=1999-2001 @@ -622,17 +634,23 @@ class SentryFlutterPlugin : FlutterPlugin, MethodCallHandler, ActivityAware { height = newHeight } - replayConfig = ScreenshotRecorderConfig( - recordingWidth = width.roundToInt(), - recordingHeight = height.roundToInt(), - scaleFactorX = 1.0f, - scaleFactorY = 1.0f, - frameRate = call.argument("frameRate") as? Int ?: 0, - bitRate = call.argument("bitRate") as? Int ?: 0 - ) + replayConfig = + ScreenshotRecorderConfig( + recordingWidth = width.roundToInt(), + recordingHeight = height.roundToInt(), + scaleFactorX = 1.0f, + scaleFactorY = 1.0f, + frameRate = call.argument("frameRate") as? Int ?: 0, + bitRate = call.argument("bitRate") as? Int ?: 0, + ) Log.i( "Sentry", - "Configuring replay: %dx%d at %d FPS, %d BPS".format(replayConfig.recordingWidth, replayConfig.recordingHeight, replayConfig.frameRate, replayConfig.bitRate) + "Configuring replay: %dx%d at %d FPS, %d BPS".format( + replayConfig.recordingWidth, + replayConfig.recordingHeight, + replayConfig.frameRate, + replayConfig.bitRate, + ), ) replay.onConfigurationChanged(Configuration()) result.success("") diff --git a/flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutterReplayRecorder.kt b/flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutterReplayRecorder.kt index 1edc430575..590f53ceb3 100644 --- a/flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutterReplayRecorder.kt +++ b/flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutterReplayRecorder.kt @@ -16,7 +16,7 @@ internal class SentryFlutterReplayRecorder( // Ignore if this is the initial call before we actually got the configuration from Flutter. // We'll get another call here when the configuration is set. if (recorderConfig.recordingHeight == 0 && recorderConfig.recordingWidth == 0) { - return; + return } val cacheDirPath = integration.replayCacheDir?.absolutePath diff --git a/flutter/example/android/app/src/main/kotlin/io/sentry/samples/flutter/MainActivity.kt b/flutter/example/android/app/src/main/kotlin/io/sentry/samples/flutter/MainActivity.kt index 7966a33655..3647986803 100644 --- a/flutter/example/android/app/src/main/kotlin/io/sentry/samples/flutter/MainActivity.kt +++ b/flutter/example/android/app/src/main/kotlin/io/sentry/samples/flutter/MainActivity.kt @@ -44,6 +44,7 @@ class MainActivity : FlutterActivity() { } private external fun crash() + private external fun message() companion object { From 7f342650a8761b0150e118c8a74b217ca1ab0a91 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Wed, 4 Dec 2024 10:13:02 +0100 Subject: [PATCH 14/19] update test coverage --- .../test/replay/replay_integration_test.dart | 23 +++++++++++++++ flutter/test/sentry_native_channel_test.dart | 29 +++++++++++++++++++ 2 files changed, 52 insertions(+) diff --git a/flutter/test/replay/replay_integration_test.dart b/flutter/test/replay/replay_integration_test.dart index 5190610d81..b1c29b771c 100644 --- a/flutter/test/replay/replay_integration_test.dart +++ b/flutter/test/replay/replay_integration_test.dart @@ -8,9 +8,11 @@ import 'package:mockito/mockito.dart'; import 'package:sentry_flutter/sentry_flutter.dart'; import 'package:sentry_flutter/src/event_processor/replay_event_processor.dart'; import 'package:sentry_flutter/src/replay/integration.dart'; +import 'package:sentry_flutter/src/replay/replay_config.dart'; import '../mocks.dart'; import '../mocks.mocks.dart'; +import '../screenshot/test_widget.dart'; void main() { late ReplayIntegration sut; @@ -26,6 +28,10 @@ void main() { sut = ReplayIntegration(native); }); + tearDown(() { + SentryScreenshotWidget.reset(); + }); + for (var supportsReplay in [true, false]) { test( '$ReplayIntegration in options.sdk.integrations when supportsReplay=$supportsReplay', @@ -66,4 +72,21 @@ void main() { } }); } + + testWidgets('Configures replay when displayed', (tester) async { + options.experimental.replay.sessionSampleRate = 1.0; + when(native.setReplayConfig(any)).thenReturn(null); + sut.call(hub, options); + + TestWidgetsFlutterBinding.ensureInitialized(); + await pumpTestElement(tester); + await tester.pumpAndSettle(Duration(seconds: 1)); + + final config = verify(native.setReplayConfig(captureAny)).captured.single + as ReplayConfig; + expect(config.bitRate, 75000); + expect(config.frameRate, 1); + expect(config.width, 800); + expect(config.height, 600); + }); } diff --git a/flutter/test/sentry_native_channel_test.dart b/flutter/test/sentry_native_channel_test.dart index 3c2811267c..c26862b6d7 100644 --- a/flutter/test/sentry_native_channel_test.dart +++ b/flutter/test/sentry_native_channel_test.dart @@ -12,6 +12,7 @@ import 'package:sentry_flutter/src/native/factory.dart'; import 'package:sentry_flutter/src/native/method_channel_helper.dart'; import 'package:sentry_flutter/src/native/sentry_native_binding.dart'; import 'package:sentry/src/platform/platform.dart' as platform; +import 'package:sentry_flutter/src/replay/replay_config.dart'; import 'mocks.dart'; import 'mocks.mocks.dart'; import 'sentry_flutter_test.dart'; @@ -325,6 +326,34 @@ void main() { verify(channel.invokeMethod('nativeCrash')); }); + + test('setReplayConfig', () async { + when(channel.invokeMethod('setReplayConfig', any)) + .thenAnswer((_) => Future.value()); + + final config = + ReplayConfig(width: 1.1, height: 2.2, frameRate: 3, bitRate: 4); + await sut.setReplayConfig(config); + + verify(channel.invokeMethod('setReplayConfig', { + 'width': config.width, + 'height': config.height, + 'frameRate': config.frameRate, + 'bitRate': config.bitRate, + })); + }); + + test('captureReplay', () async { + final sentryId = SentryId.newId(); + + when(channel.invokeMethod('captureReplay', any)) + .thenAnswer((_) => Future.value(sentryId.toString())); + + final returnedId = await sut.captureReplay(true); + + verify(channel.invokeMethod('captureReplay', {'isCrash': true})); + expect(returnedId, sentryId); + }); }); } } From b1cd9a37454de1d2fbf3f33c6500831869a6fb65 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Wed, 4 Dec 2024 10:57:39 +0100 Subject: [PATCH 15/19] linter issues --- .../main/kotlin/io/sentry/flutter/SentryFlutterPlugin.kt | 7 ++++--- flutter/lib/src/screenshot/sentry_screenshot_widget.dart | 7 ++++--- flutter/test/replay/scheduler_test.dart | 4 ++-- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutterPlugin.kt b/flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutterPlugin.kt index 8b22797b70..8a2f6cf612 100644 --- a/flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutterPlugin.kt +++ b/flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutterPlugin.kt @@ -39,6 +39,7 @@ import java.lang.ref.WeakReference import kotlin.math.roundToInt private const val APP_START_MAX_DURATION_MS = 60000 +private const val VIDEO_BLOCK_SIZE = 16 class SentryFlutterPlugin : FlutterPlugin, @@ -576,11 +577,11 @@ class SentryFlutterPlugin : } private fun Double.adjustReplaySizeToBlockSize(): Double { - val remainder = this % 16 - return if (remainder <= 8) { + val remainder = this % VIDEO_BLOCK_SIZE + return if (remainder <= VIDEO_BLOCK_SIZE / 2) { this - remainder } else { - this + (16 - remainder) + this + (VIDEO_BLOCK_SIZE - remainder) } } } diff --git a/flutter/lib/src/screenshot/sentry_screenshot_widget.dart b/flutter/lib/src/screenshot/sentry_screenshot_widget.dart index 40505fa0e1..0a1cf52fef 100644 --- a/flutter/lib/src/screenshot/sentry_screenshot_widget.dart +++ b/flutter/lib/src/screenshot/sentry_screenshot_widget.dart @@ -68,10 +68,11 @@ typedef SentryScreenshotWidgetOnBuildCallback = bool Function( class _SentryScreenshotWidgetState extends State { @override Widget build(BuildContext context) { + final mq = MediaQuery.of(context); final status = SentryScreenshotWidgetStatus( - size: MediaQuery.maybeSizeOf(context), - pixelRatio: MediaQuery.maybeDevicePixelRatioOf(context), - orientantion: MediaQuery.maybeOrientationOf(context), + size: mq.size, + pixelRatio: mq.devicePixelRatio, + orientantion: mq.orientation, ); final prevStatus = SentryScreenshotWidget._status; SentryScreenshotWidget._status = status; diff --git a/flutter/test/replay/scheduler_test.dart b/flutter/test/replay/scheduler_test.dart index 6c74a0691b..300fa25280 100644 --- a/flutter/test/replay/scheduler_test.dart +++ b/flutter/test/replay/scheduler_test.dart @@ -63,11 +63,11 @@ class _Fixture { sut = Scheduler( const Duration(milliseconds: 1), (_) async => calls++, - _AddPostFrameCallbackMock, + _addPostFrameCallbackMock, ); } - void _AddPostFrameCallbackMock(FrameCallback callback, + void _addPostFrameCallbackMock(FrameCallback callback, {String debugLabel = 'callback'}) { registeredCallback = callback; } From 5b93f8dd294647e196abaedcc41af7e0409d6649 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Wed, 4 Dec 2024 11:48:15 +0100 Subject: [PATCH 16/19] chore: changelog --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ceb9af57be..10e083da15 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## Unreleased + +### Features + +- Replay: device orientation change support & improve video size fit on Android ([#2462](https://github.com/getsentry/sentry-dart/pull/2462)) + ## 8.11.0-beta.2 ### Features From 96ece564cffa0af1fbdf64741f40631b625f893d Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Thu, 5 Dec 2024 11:59:53 +0100 Subject: [PATCH 17/19] enh: use ensureVisualUpdate instead of scheduleFrame --- flutter/lib/src/replay/scheduled_recorder.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/flutter/lib/src/replay/scheduled_recorder.dart b/flutter/lib/src/replay/scheduled_recorder.dart index 89299112e0..50b855c552 100644 --- a/flutter/lib/src/replay/scheduled_recorder.dart +++ b/flutter/lib/src/replay/scheduled_recorder.dart @@ -67,7 +67,7 @@ class ScheduledScreenshotRecorder extends ScreenshotRecorder { // actions, there may not be any frame captured for a long time so even // the IdleFrameFiller won't have anything to repeat. This would appear // as if the replay was broken. - options.bindingUtils.instance!.scheduleFrame(); + options.bindingUtils.instance!.ensureVisualUpdate(); } Future stop() async { From 2b73481926380961ce9f1c90e3b319040150f927 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Mon, 9 Dec 2024 10:07:08 +0100 Subject: [PATCH 18/19] avoid errors during initial "unconfigured" replay startup --- .../kotlin/io/sentry/flutter/SentryFlutterPlugin.kt | 12 +++++++----- .../io/sentry/flutter/SentryFlutterReplayRecorder.kt | 4 ++-- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutterPlugin.kt b/flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutterPlugin.kt index 8a2f6cf612..8314add907 100644 --- a/flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutterPlugin.kt +++ b/flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutterPlugin.kt @@ -39,7 +39,7 @@ import java.lang.ref.WeakReference import kotlin.math.roundToInt private const val APP_START_MAX_DURATION_MS = 60000 -private const val VIDEO_BLOCK_SIZE = 16 +public const val VIDEO_BLOCK_SIZE = 16 class SentryFlutterPlugin : FlutterPlugin, @@ -49,14 +49,16 @@ class SentryFlutterPlugin : private lateinit var context: Context private lateinit var sentryFlutter: SentryFlutter private lateinit var replay: ReplayIntegration + // Note: initial config because we don't yet have the numbers of the actual Flutter widget. + // See how SentryFlutterReplayRecorder.start() handles it. New settings will be set by setReplayConfig() method below. private var replayConfig = ScreenshotRecorderConfig( - recordingWidth = 0, - recordingHeight = 0, + recordingWidth = VIDEO_BLOCK_SIZE, + recordingHeight = VIDEO_BLOCK_SIZE, scaleFactorX = 1.0f, scaleFactorY = 1.0f, - frameRate = 0, - bitRate = 0, + frameRate = 1, + bitRate = 75000, ) private var activity: WeakReference? = null diff --git a/flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutterReplayRecorder.kt b/flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutterReplayRecorder.kt index 590f53ceb3..c49fc23482 100644 --- a/flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutterReplayRecorder.kt +++ b/flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutterReplayRecorder.kt @@ -14,8 +14,8 @@ internal class SentryFlutterReplayRecorder( ) : Recorder { override fun start(recorderConfig: ScreenshotRecorderConfig) { // Ignore if this is the initial call before we actually got the configuration from Flutter. - // We'll get another call here when the configuration is set. - if (recorderConfig.recordingHeight == 0 && recorderConfig.recordingWidth == 0) { + // We'll get another call here when the configuration is changed according to the widget size. + if (recorderConfig.recordingHeight <= VIDEO_BLOCK_SIZE && recorderConfig.recordingWidth <= VIDEO_BLOCK_SIZE) { return } From 4080ee5851470b3da5e21855d5ae54b92338c107 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Mon, 9 Dec 2024 10:11:22 +0100 Subject: [PATCH 19/19] linter --- .../src/main/kotlin/io/sentry/flutter/SentryFlutterPlugin.kt | 1 + 1 file changed, 1 insertion(+) diff --git a/flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutterPlugin.kt b/flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutterPlugin.kt index 8314add907..505d2e0215 100644 --- a/flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutterPlugin.kt +++ b/flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutterPlugin.kt @@ -49,6 +49,7 @@ class SentryFlutterPlugin : private lateinit var context: Context private lateinit var sentryFlutter: SentryFlutter private lateinit var replay: ReplayIntegration + // Note: initial config because we don't yet have the numbers of the actual Flutter widget. // See how SentryFlutterReplayRecorder.start() handles it. New settings will be set by setReplayConfig() method below. private var replayConfig =