Remove renderer on Dart:io platforms (#1723)

ueman · web-flow · commit 48adddf2bb66 · 2023-11-14T09:56:47.000Z
* Remove renderer on dart:io platforms

* Tests + changelog

* Update CHANGELOG.md

* Fix tests

* fix more tests
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,7 @@
 
 ### Fixes 
 
+- Flutter renderer information was removed on dart:io platforms since it didn't add the correct value ([#1723](https://github.com/getsentry/sentry-dart/pull/1723))
 - Unsupported types with Expando ([#1690](https://github.com/getsentry/sentry-dart/pull/1690))
 
 ### Features
diff --git a/flutter/lib/src/event_processor/flutter_enricher_event_processor.dart b/flutter/lib/src/event_processor/flutter_enricher_event_processor.dart
@@ -132,6 +132,8 @@ class FlutterEnricherEventProcessor implements EventProcessor {
     // ignore: deprecated_member_use
     final hasRenderView = _widgetsBinding?.renderViewElement != null;
 
+    final renderer = _options.rendererWrapper.getRenderer()?.name;
+
     return <String, String>{
       'has_render_view': hasRenderView.toString(),
       if (tempDebugBrightnessOverride != null)
@@ -149,7 +151,7 @@ class FlutterEnricherEventProcessor implements EventProcessor {
       // Also always fails in tests.
       // See https://github.com/flutter/flutter/issues/83919
       // 'window_is_visible': _window.viewConfiguration.visible,
-      'renderer': _options.rendererWrapper.getRenderer().name,
+      if (renderer != null) 'renderer': renderer,
     };
   }
 
diff --git a/flutter/lib/src/event_processor/screenshot_event_processor.dart b/flutter/lib/src/event_processor/screenshot_event_processor.dart
@@ -32,10 +32,13 @@ class ScreenshotEventProcessor implements EventProcessor {
     }
 
     final renderer = _options.rendererWrapper.getRenderer();
-    if (renderer != FlutterRenderer.skia &&
+
+    if (_options.platformChecker.isWeb &&
         renderer != FlutterRenderer.canvasKit) {
-      _options.logger(SentryLevel.debug,
-          'Cannot take screenshot with ${_options.rendererWrapper.getRenderer().name} renderer.');
+      _options.logger(
+        SentryLevel.debug,
+        'Cannot take screenshot with ${renderer?.name} renderer.',
+      );
       return event;
     }
 
diff --git a/flutter/lib/src/renderer/html_renderer.dart b/flutter/lib/src/renderer/html_renderer.dart
@@ -2,7 +2,7 @@ import 'dart:js' as js;
 
 import 'renderer.dart';
 
-FlutterRenderer getRenderer() {
+FlutterRenderer? getRenderer() {
   return isCanvasKitRenderer ? FlutterRenderer.canvasKit : FlutterRenderer.html;
 }
 
diff --git a/flutter/lib/src/renderer/io_renderer.dart b/flutter/lib/src/renderer/io_renderer.dart
@@ -1,5 +1,3 @@
 import 'renderer.dart';
 
-FlutterRenderer getRenderer() {
-  return FlutterRenderer.skia;
-}
+FlutterRenderer? getRenderer() => null;
diff --git a/flutter/lib/src/renderer/renderer.dart b/flutter/lib/src/renderer/renderer.dart
@@ -6,14 +6,22 @@ import 'unknown_renderer.dart'
 
 @internal
 class RendererWrapper {
-  FlutterRenderer getRenderer() {
+  FlutterRenderer? getRenderer() {
     return implementation.getRenderer();
   }
 }
 
 enum FlutterRenderer {
+  /// https://skia.org/
   skia,
+
+  /// https://docs.flutter.dev/perf/impeller
+  impeller,
+
+  /// https://docs.flutter.dev/platform-integration/web/renderers
   canvasKit,
+
+  /// https://docs.flutter.dev/platform-integration/web/renderers
   html,
   unknown,
 }
diff --git a/flutter/lib/src/sentry_flutter.dart b/flutter/lib/src/sentry_flutter.dart
@@ -167,8 +167,7 @@ mixin SentryFlutter {
       integrations.add(LoadImageListIntegration(channel));
     }
     final renderer = options.rendererWrapper.getRenderer();
-    if (renderer == FlutterRenderer.skia ||
-        renderer == FlutterRenderer.canvasKit) {
+    if (!platformChecker.isWeb || renderer == FlutterRenderer.canvasKit) {
       integrations.add(ScreenshotIntegration());
     }
 
diff --git a/flutter/test/event_processor/flutter_enricher_event_processor_test.dart b/flutter/test/event_processor/flutter_enricher_event_processor_test.dart
@@ -22,7 +22,38 @@ void main() {
       fixture = Fixture();
     });
 
-    testWidgets('flutter context', (WidgetTester tester) async {
+    testWidgets('flutter context on dart:io', (WidgetTester tester) async {
+      if (kIsWeb) {
+        // widget tests don't support onPlatform config
+        // https://pub.dev/packages/test#platform-specific-configuration
+        return;
+      }
+      // These two values need to be changed inside the test,
+      // otherwise the Flutter test framework complains that these
+      // values are changed outside of a test.
+      debugBrightnessOverride = Brightness.dark;
+      debugDefaultTargetPlatformOverride = TargetPlatform.android;
+      final enricher = fixture.getSut(
+        binding: () => tester.binding,
+      );
+
+      final event = await enricher.apply(SentryEvent());
+
+      debugBrightnessOverride = null;
+      debugDefaultTargetPlatformOverride = null;
+
+      final flutterContext = event?.contexts['flutter_context'];
+      expect(flutterContext, isNotNull);
+      expect(flutterContext, isA<Map<String, String>>());
+    }, skip: !kIsWeb);
+
+    testWidgets('flutter context on web', (WidgetTester tester) async {
+      if (!kIsWeb) {
+        // widget tests don't support onPlatform config
+        // https://pub.dev/packages/test#platform-specific-configuration
+        return;
+      }
+
       // These two values need to be changed inside the test,
       // otherwise the Flutter test framework complains that these
       // values are changed outside of a test.
diff --git a/flutter/test/event_processor/screenshot_event_processor_test.dart b/flutter/test/event_processor/screenshot_event_processor_test.dart
@@ -19,11 +19,15 @@ void main() {
   });
 
   Future<void> _addScreenshotAttachment(
-      WidgetTester tester, FlutterRenderer renderer, bool added,
-      {int? expectedMaxWidthOrHeight}) async {
+    WidgetTester tester,
+    FlutterRenderer? renderer, {
+    required bool isWeb,
+    required bool added,
+    int? expectedMaxWidthOrHeight,
+  }) async {
     // Run with real async https://stackoverflow.com/a/54021863
     await tester.runAsync(() async {
-      final sut = fixture.getSut(renderer);
+      final sut = fixture.getSut(renderer, isWeb);
 
       await tester.pumpWidget(SentryScreenshotWidget(
           child: Text('Catching Pokémon is a snap!',
@@ -48,55 +52,54 @@ void main() {
     });
   }
 
-  testWidgets('adds screenshot attachment with skia renderer', (tester) async {
-    await _addScreenshotAttachment(tester, FlutterRenderer.skia, true);
+  testWidgets('adds screenshot attachment dart:io', (tester) async {
+    await _addScreenshotAttachment(tester, null, added: true, isWeb: false);
   });
 
   testWidgets('adds screenshot attachment with canvasKit renderer',
       (tester) async {
-    await _addScreenshotAttachment(tester, FlutterRenderer.canvasKit, true);
+    await _addScreenshotAttachment(tester, FlutterRenderer.canvasKit,
+        added: true, isWeb: true);
   });
 
   testWidgets('does not add screenshot attachment with html renderer',
       (tester) async {
-    await _addScreenshotAttachment(tester, FlutterRenderer.html, false);
-  });
-
-  testWidgets('does not add screenshot attachment with unknown renderer',
-      (tester) async {
-    await _addScreenshotAttachment(tester, FlutterRenderer.unknown, false);
+    await _addScreenshotAttachment(tester, FlutterRenderer.html,
+        added: false, isWeb: true);
   });
 
   testWidgets('does add screenshot in correct resolution for low',
       (tester) async {
     final height = SentryScreenshotQuality.low.targetResolution()!;
     fixture.options.screenshotQuality = SentryScreenshotQuality.low;
-    await _addScreenshotAttachment(tester, FlutterRenderer.skia, true,
-        expectedMaxWidthOrHeight: height);
+    await _addScreenshotAttachment(tester, null,
+        added: true, isWeb: false, expectedMaxWidthOrHeight: height);
   });
 
   testWidgets('does add screenshot in correct resolution for medium',
       (tester) async {
     final height = SentryScreenshotQuality.medium.targetResolution()!;
     fixture.options.screenshotQuality = SentryScreenshotQuality.medium;
-    await _addScreenshotAttachment(tester, FlutterRenderer.skia, true,
-        expectedMaxWidthOrHeight: height);
+    await _addScreenshotAttachment(tester, null,
+        added: true, isWeb: false, expectedMaxWidthOrHeight: height);
   });
 
   testWidgets('does add screenshot in correct resolution for high',
       (tester) async {
     final widthOrHeight = SentryScreenshotQuality.high.targetResolution()!;
     fixture.options.screenshotQuality = SentryScreenshotQuality.high;
-    await _addScreenshotAttachment(tester, FlutterRenderer.skia, true,
-        expectedMaxWidthOrHeight: widthOrHeight);
+    await _addScreenshotAttachment(tester, null,
+        added: true, isWeb: false, expectedMaxWidthOrHeight: widthOrHeight);
   });
 }
 
 class Fixture {
   SentryFlutterOptions options = SentryFlutterOptions(dsn: fakeDsn);
 
-  ScreenshotEventProcessor getSut(FlutterRenderer flutterRenderer) {
+  ScreenshotEventProcessor getSut(
+      FlutterRenderer? flutterRenderer, bool isWeb) {
     options.rendererWrapper = MockRendererWrapper(flutterRenderer);
+    options.platformChecker = MockPlatformChecker(isWebValue: isWeb);
     return ScreenshotEventProcessor(options);
   }
 }
diff --git a/flutter/test/mocks.dart b/flutter/test/mocks.dart
@@ -396,10 +396,10 @@ class MockNativeChannel implements SentryNativeBinding {
 class MockRendererWrapper implements RendererWrapper {
   MockRendererWrapper(this._renderer);
 
-  final FlutterRenderer _renderer;
+  final FlutterRenderer? _renderer;
 
   @override
-  FlutterRenderer getRenderer() {
+  FlutterRenderer? getRenderer() {
     return _renderer;
   }
 }
diff --git a/flutter/test/sentry_flutter_test.dart b/flutter/test/sentry_flutter_test.dart
@@ -495,7 +495,7 @@ void main() {
       await Sentry.close();
     });
 
-    test('installed with skia renderer', () async {
+    test('installed on io platforms', () async {
       List<Integration> integrations = [];
 
       await SentryFlutter.init(
@@ -505,7 +505,8 @@ void main() {
           integrations = options.integrations;
         },
         appRunner: appRunner,
-        platformChecker: getPlatformChecker(platform: MockPlatform.iOs()),
+        platformChecker:
+            getPlatformChecker(platform: MockPlatform.iOs(), isWeb: false),
         rendererWrapper: MockRendererWrapper(FlutterRenderer.skia),
       );
 
@@ -528,7 +529,8 @@ void main() {
           integrations = options.integrations;
         },
         appRunner: appRunner,
-        platformChecker: getPlatformChecker(platform: MockPlatform.iOs()),
+        platformChecker:
+            getPlatformChecker(platform: MockPlatform.iOs(), isWeb: true),
         rendererWrapper: MockRendererWrapper(FlutterRenderer.canvasKit),
       );
 
@@ -551,7 +553,8 @@ void main() {
           integrations = options.integrations;
         },
         appRunner: appRunner,
-        platformChecker: getPlatformChecker(platform: MockPlatform.iOs()),
+        platformChecker:
+            getPlatformChecker(platform: MockPlatform.iOs(), isWeb: true),
         rendererWrapper: MockRendererWrapper(FlutterRenderer.html),
       );
 
@@ -563,29 +566,6 @@ void main() {
 
       await Sentry.close();
     }, testOn: 'vm');
-
-    test('not installed with unknown renderer', () async {
-      List<Integration> integrations = [];
-
-      await SentryFlutter.init(
-        (options) async {
-          options.dsn = fakeDsn;
-          options.automatedTestMode = true;
-          integrations = options.integrations;
-        },
-        appRunner: appRunner,
-        platformChecker: getPlatformChecker(platform: MockPlatform.iOs()),
-        rendererWrapper: MockRendererWrapper(FlutterRenderer.unknown),
-      );
-
-      expect(
-          integrations
-              .map((e) => e.runtimeType)
-              .contains(ScreenshotIntegration),
-          false);
-
-      await Sentry.close();
-    }, testOn: 'vm');
   });
 
   group('initial values', () {

Original file line number	Diff line number	Diff line change
`@@ -2,7 +2,7 @@ import 'dart:js' as js;`
`2`	`2`
`3`	`3`	`import 'renderer.dart';`
`4`	`4`
`5`		`-FlutterRenderer getRenderer() {`
	`5`	`+FlutterRenderer? getRenderer() {`
`6`	`6`	`return isCanvasKitRenderer ? FlutterRenderer.canvasKit : FlutterRenderer.html;`
`7`	`7`	`}`
`8`	`8`
Original file line number	Diff line number	Diff line change
`@@ -167,8 +167,7 @@ mixin SentryFlutter {`
`167`	`167`	`integrations.add(LoadImageListIntegration(channel));`
`168`	`168`	`}`
`169`	`169`	`final renderer = options.rendererWrapper.getRenderer();`
`170`		`- if (renderer == FlutterRenderer.skia \|\|`
`171`		`- renderer == FlutterRenderer.canvasKit) {`
	`170`	`+ if (!platformChecker.isWeb \|\| renderer == FlutterRenderer.canvasKit) {`
`172`	`171`	`integrations.add(ScreenshotIntegration());`
`173`	`172`	`}`
`174`	`173`
Original file line number	Diff line number	Diff line change
`@@ -396,10 +396,10 @@ class MockNativeChannel implements SentryNativeBinding {`
`396`	`396`	`class MockRendererWrapper implements RendererWrapper {`
`397`	`397`	`MockRendererWrapper(this._renderer);`
`398`	`398`
`399`		`- final FlutterRenderer _renderer;`
	`399`	`+ final FlutterRenderer? _renderer;`
`400`	`400`
`401`	`401`	`@override`
`402`		`- FlutterRenderer getRenderer() {`
	`402`	`+ FlutterRenderer? getRenderer() {`
`403`	`403`	`return _renderer;`
`404`	`404`	`}`
`405`	`405`	`}`