Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix duplicative Screen size changed breadcrumbs #888

Merged
merged 10 commits into from
Jul 21, 2022
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

* Breadcrumbs should not be duplicated if there is no mechanism on Android ([#936](https://github.com/getsentry/sentry-dart/pull/936))
* 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))

## 6.6.3

Expand Down
1 change: 1 addition & 0 deletions flutter/example/lib/main.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
38 changes: 31 additions & 7 deletions flutter/lib/src/widgets_binding_observer.dart
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -20,11 +24,31 @@ class SentryWidgetsBindingObserver with WidgetsBindingObserver {
Hub? hub,
required SentryFlutterOptions options,
}) : _hub = hub ?? HubAdapter(),
_options = options;
_options = options,
_screenSizeStreamController = StreamController(sync: true) {
marandaneto marked this conversation as resolved.
Show resolved Hide resolved
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
marandaneto marked this conversation as resolved.
Show resolved Hide resolved
.listen(_onScreenSizeChanged);

final window = BindingUtils.getWidgetsBindingInstance()?.window;
_screenSizeStreamController.add(window);
}
}

final Hub _hub;
final SentryFlutterOptions _options;

final StreamController<SingletonFlutterWindow?> _screenSizeStreamController;
marandaneto marked this conversation as resolved.
Show resolved Hide resolved

/// This method records lifecycle events.
/// It tries to mimic the behavior of ActivityBreadcrumbsIntegration of Sentry
/// Android for lifecycle events.
Expand Down Expand Up @@ -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,
));
}

Expand Down
63 changes: 60 additions & 3 deletions flutter/test/widgets_binding_observer_test.dart
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import 'dart:ui';

import 'package:flutter/services.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:mockito/mockito.dart';
Expand Down Expand Up @@ -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;
Expand All @@ -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();
Expand Down