Fix duplicative Screen size changed breadcrumbs (#888)

getsentry · Jul 21, 2022 · 5a35e4a · 5a35e4a
1 parent bdc6e0f
commit 5a35e4a
Show file tree

Hide file tree

Showing 4 changed files with 93 additions and 10 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,6 +5,7 @@
 ### Fixes
 
 * Maps with Key Object, Object would fail during serialization if not String, Object ([#935](https://github.com/getsentry/sentry-dart/pull/935))
+* Fix duplicative Screen size changed breadcrumbs ([#888](https://github.com/getsentry/sentry-dart/pull/888))
 
 ### Features
 

diff --git a/flutter/example/lib/main.dart b/flutter/example/lib/main.dart
@@ -27,6 +27,7 @@ Future<void> main() async {
       options.addInAppInclude('sentry_flutter_example');
       options.considerInAppFramesByDefault = false;
       options.attachThreads = true;
+      options.enableWindowMetricBreadcrumbs = true;
     },
     // Init your App.
     appRunner: () => runApp(

diff --git a/flutter/lib/src/widgets_binding_observer.dart b/flutter/lib/src/widgets_binding_observer.dart
@@ -1,3 +1,7 @@
+import 'dart:async';
+import 'dart:ui';
+
+import 'package:flutter/foundation.dart';
 import 'package:flutter/material.dart';
 import '../sentry_flutter.dart';
 import 'binding_utils.dart';
@@ -20,11 +24,31 @@ class SentryWidgetsBindingObserver with WidgetsBindingObserver {
     Hub? hub,
     required SentryFlutterOptions options,
   })  : _hub = hub ?? HubAdapter(),
-        _options = options;
+        _options = options,
+        _screenSizeStreamController = StreamController(sync: true) {
+    if (_options.enableWindowMetricBreadcrumbs) {
+      _screenSizeStreamController.stream
+          .map(
+            (window) => {
+              'new_pixel_ratio': window?.devicePixelRatio,
+              'new_height': window?.physicalSize.height,
+              'new_width': window?.physicalSize.width,
+            },
+          )
+          .distinct(mapEquals)
+          .skip(1) // Skip initial event added below in constructor
+          .listen(_onScreenSizeChanged);
+
+      final window = BindingUtils.getWidgetsBindingInstance()?.window;
+      _screenSizeStreamController.add(window);
+    }
+  }
 
   final Hub _hub;
   final SentryFlutterOptions _options;
 
+  final StreamController<SingletonFlutterWindow?> _screenSizeStreamController;
+
   /// This method records lifecycle events.
   /// It tries to mimic the behavior of ActivityBreadcrumbsIntegration of Sentry
   /// Android for lifecycle events.
@@ -55,22 +79,22 @@ class SentryWidgetsBindingObserver with WidgetsBindingObserver {
   /// when a phone is rotated or an application window is resized.
   ///
   /// See also:
-  ///   - [Window.onMetricsChanged](https://api.flutter.dev/flutter/dart-ui/Window/onMetricsChanged.html)
+  ///   - [SingletonFlutterWindow.onMetricsChanged](https://api.flutter.dev/flutter/dart-ui/SingletonFlutterWindow/onMetricsChanged.html)
   @override
   void didChangeMetrics() {
     if (!_options.enableWindowMetricBreadcrumbs) {
       return;
     }
     final window = BindingUtils.getWidgetsBindingInstance()?.window;
+    _screenSizeStreamController.add(window);
+  }
+
+  void _onScreenSizeChanged(Map<String, dynamic> data) {
     _hub.addBreadcrumb(Breadcrumb(
       message: 'Screen size changed',
       category: 'device.screen',
       type: 'navigation',
-      data: <String, dynamic>{
-        'new_pixel_ratio': window?.devicePixelRatio,
-        'new_height': window?.physicalSize.height,
-        'new_width': window?.physicalSize.width,
-      },
+      data: data,
     ));
   }
 

diff --git a/flutter/test/widgets_binding_observer_test.dart b/flutter/test/widgets_binding_observer_test.dart
@@ -1,3 +1,5 @@
+import 'dart:ui';
+
 import 'package:flutter/services.dart';
 import 'package:flutter_test/flutter_test.dart';
 import 'package:mockito/mockito.dart';
@@ -175,12 +177,14 @@ void main() {
         hub: hub,
         options: flutterTrackingEnabledOptions,
       );
-      final instance = BindingUtils.getWidgetsBindingInstance();
-      instance!.addObserver(observer);
+      final instance = tester.binding;
+      instance.addObserver(observer);
 
       final window = instance.window;
 
-      window.onMetricsChanged!();
+      const newWidth = 123.0;
+      const newHeight = 456.0;
+      window.physicalSizeTestValue = Size(newWidth, newHeight);
 
       final breadcrumb =
           verify(hub.addBreadcrumb(captureAny)).captured.single as Breadcrumb;
@@ -191,13 +195,66 @@ void main() {
       expect(breadcrumb.level, SentryLevel.info);
       expect(breadcrumb.data, <String, dynamic>{
         'new_pixel_ratio': window.devicePixelRatio,
+        'new_height': newHeight,
+        'new_width': newWidth,
+      });
+
+      instance.removeObserver(observer);
+    });
+
+    testWidgets('only unique metrics emit events', (WidgetTester tester) async {
+      final hub = MockHub();
+
+      final observer = SentryWidgetsBindingObserver(
+        hub: hub,
+        options: flutterTrackingEnabledOptions,
+      );
+      final instance = tester.binding;
+      instance.addObserver(observer);
+
+      final window = instance.window;
+
+      window.physicalSizeTestValue = window.physicalSize;
+
+      const newPixelRatio = 1.618;
+      window.devicePixelRatioTestValue = newPixelRatio;
+
+      final breadcrumb =
+          verify(hub.addBreadcrumb(captureAny)).captured.single as Breadcrumb;
+
+      expect(breadcrumb.message, 'Screen size changed');
+      expect(breadcrumb.category, 'device.screen');
+      expect(breadcrumb.type, 'navigation');
+      expect(breadcrumb.level, SentryLevel.info);
+      expect(breadcrumb.data, <String, dynamic>{
+        'new_pixel_ratio': newPixelRatio,
         'new_height': window.physicalSize.height,
         'new_width': window.physicalSize.width,
       });
 
       instance.removeObserver(observer);
     });
 
+    testWidgets('no breadcrumb on unrelated metrics changes',
+        (WidgetTester tester) async {
+      final hub = MockHub();
+
+      final observer = SentryWidgetsBindingObserver(
+        hub: hub,
+        options: flutterTrackingEnabledOptions,
+      );
+      final instance = tester.binding;
+      instance.addObserver(observer);
+
+      final window = instance.window;
+
+      window.viewInsetsTestValue = WindowPadding.zero;
+
+      verifyNever(hub.addBreadcrumb(captureAny));
+
+      instance.removeObserver(observer);
+    });
+
     testWidgets('disable metrics changed breadcrumb',
         (WidgetTester tester) async {
       final hub = MockHub();