Skip to content

Commit

Permalink
refactor: make screenshot recorder capture() synchronous so it's hard…
Browse files Browse the repository at this point in the history
…er to break accidentally with future changes (#2493)

* refactor: make screenshot recorder capture() synchronous so it's harder to break accidentally with future changes

* formatting

* enh: add timeline callbacks

* fix min_version_test

* try to fix ci

* try to fix ci

* try to fix ci

* fix recorder scheduling for screenshots & fix widget filter items carryover

* fix late final usage

* cleanup

* fix ci

* cleanup, ci fix

* extract replayrecorder

* test throwing in recorder task
  • Loading branch information
vaind authored Dec 18, 2024
1 parent 45d0706 commit 0b204c0
Show file tree
Hide file tree
Showing 9 changed files with 275 additions and 152 deletions.
12 changes: 2 additions & 10 deletions flutter/lib/src/event_processor/screenshot_event_processor.dart
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ class ScreenshotEventProcessor implements EventProcessor {
height: targetResolution,
),
_options,
isReplayRecorder: false,
);
_debouncer = Debouncer(
// ignore: invalid_use_of_internal_member
Expand Down Expand Up @@ -126,15 +125,8 @@ class ScreenshotEventProcessor implements EventProcessor {
}

@internal
Future<Uint8List?> createScreenshot() async {
Uint8List? screenshotData;

await _recorder.capture((Image image) async {
screenshotData = await _convertImageToUint8List(image);
});

return screenshotData;
}
Future<Uint8List?> createScreenshot() =>
_recorder.capture(_convertImageToUint8List);

Future<Uint8List?> _convertImageToUint8List(Image image) async {
final byteData = await image.toByteData(format: ImageByteFormat.png);
Expand Down
11 changes: 5 additions & 6 deletions flutter/lib/src/native/cocoa/sentry_native_cocoa.dart
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import 'dart:async';
import 'dart:ffi';
import 'dart:typed_data';
import 'dart:ui';

import 'package:meta/meta.dart';

import '../../../sentry_flutter.dart';
import '../../replay/replay_config.dart';
import '../../replay/replay_recorder.dart';
import '../../screenshot/recorder.dart';
import '../../screenshot/recorder_config.dart';
import '../sentry_native_channel.dart';
Expand All @@ -32,7 +32,8 @@ class SentryNativeCocoa extends SentryNativeChannel {
switch (call.method) {
case 'captureReplayScreenshot':
_replayRecorder ??=
ScreenshotRecorder(ScreenshotRecorderConfig(), options);
ReplayScreenshotRecorder(ScreenshotRecorderConfig(), options);

final replayId = call.arguments['replayId'] == null
? null
: SentryId.fromId(call.arguments['replayId'] as String);
Expand All @@ -44,8 +45,7 @@ class SentryNativeCocoa extends SentryNativeChannel {
});
}

Uint8List? imageBytes;
await _replayRecorder?.capture((image) async {
return _replayRecorder?.capture((image) async {
final imageData =
await image.toByteData(format: ImageByteFormat.png);
if (imageData != null) {
Expand All @@ -54,13 +54,12 @@ class SentryNativeCocoa extends SentryNativeChannel {
'Replay: captured screenshot ('
'${image.width}x${image.height} pixels, '
'${imageData.lengthInBytes} bytes)');
imageBytes = imageData.buffer.asUint8List();
return imageData.buffer.asUint8List();
} else {
options.logger(SentryLevel.warning,
'Replay: failed to convert screenshot to PNG');
}
});
return imageBytes;
default:
throw UnimplementedError('Method ${call.method} not implemented');
}
Expand Down
26 changes: 26 additions & 0 deletions flutter/lib/src/replay/replay_recorder.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import 'dart:async';
import 'dart:developer';

import 'package:flutter/scheduler.dart';
import 'package:meta/meta.dart';

import '../screenshot/recorder.dart';

var _instanceCounter = 0;

@internal
class ReplayScreenshotRecorder extends ScreenshotRecorder {
ReplayScreenshotRecorder(super.config, super.options)
: super(
privacyOptions: options.experimental.privacyForReplay,
logName: 'ReplayRecorder #${++_instanceCounter}');

@override
@protected
Future<void> executeTask(void Function() task, Flow flow) {
// Schedule the task to run between frames, when the app is idle.
return options.bindingUtils.instance
?.scheduleTask<void>(task, Priority.idle, flow: flow) ??
Future.sync(task);
}
}
15 changes: 7 additions & 8 deletions flutter/lib/src/replay/scheduled_recorder.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import 'dart:ui';
import 'package:meta/meta.dart';

import '../../sentry_flutter.dart';
import '../screenshot/recorder.dart';
import 'replay_recorder.dart';
import 'scheduled_recorder_config.dart';
import 'scheduler.dart';

Expand All @@ -14,7 +14,7 @@ typedef ScheduledScreenshotRecorderCallback = Future<void> Function(
ScreenshotPng screenshot, bool isNewlyCaptured);

@internal
class ScheduledScreenshotRecorder extends ScreenshotRecorder {
class ScheduledScreenshotRecorder extends ReplayScreenshotRecorder {
late final Scheduler _scheduler;
late final ScheduledScreenshotRecorderCallback _callback;
var _status = _Status.running;
Expand All @@ -28,8 +28,8 @@ class ScheduledScreenshotRecorder extends ScreenshotRecorder {

ScheduledScreenshotRecorder(
ScheduledScreenshotRecorderConfig config, SentryFlutterOptions options,
[ScheduledScreenshotRecorderCallback? callback, String? logName])
: super(config, options, logName: logName) {
[ScheduledScreenshotRecorderCallback? callback])
: super(config, options) {
assert(config.frameRate > 0);
_frameDuration = Duration(milliseconds: 1000 ~/ config.frameRate);
assert(_frameDuration.inMicroseconds > 0);
Expand Down Expand Up @@ -93,8 +93,7 @@ class ScheduledScreenshotRecorder extends ScreenshotRecorder {
}
}

Future<void> _capture(Duration sinceSchedulerEpoch) async =>
capture(_onImageCaptured);
void _capture(Duration sinceSchedulerEpoch) => capture(_onImageCaptured);

Future<void> _onImageCaptured(Image image) async {
if (_status == _Status.running) {
Expand Down Expand Up @@ -169,13 +168,13 @@ class _IdleFrameFiller {
await scheduled;
}

void pause() async {
void pause() {
if (_status == _Status.running) {
_status = _Status.paused;
}
}

void resume() async {
void resume() {
if (_status == _Status.paused) {
_status = _Status.running;
}
Expand Down
5 changes: 3 additions & 2 deletions flutter/lib/src/replay/scheduler.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import 'package:flutter/scheduler.dart';
import 'package:meta/meta.dart';

@internal
typedef SchedulerCallback = Future<void> Function(Duration);
typedef SchedulerCallback = void Function(Duration);

/// This is a low-priority scheduler.
/// We're not using Timer.periodic() because it may schedule a callback
Expand Down Expand Up @@ -52,6 +52,7 @@ class Scheduler {

void _run(Duration sinceSchedulerEpoch) {
if (!_running) return;
_callback(sinceSchedulerEpoch).then((_) => _scheduleNext());
_callback(sinceSchedulerEpoch);
_scheduleNext();
}
}
Loading

0 comments on commit 0b204c0

Please sign in to comment.