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

Refactor BindingUtils to enable the use of custom bindings #1184

Merged
merged 18 commits into from
Jan 10, 2023
12 changes: 0 additions & 12 deletions flutter/lib/src/binding_utils.dart

This file was deleted.

14 changes: 14 additions & 0 deletions flutter/lib/src/binding_wrapper.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import 'package:flutter/widgets.dart';

// The methods and properties are modelled after the the real binding class.
class BindingWrapper {
/// Flutter >= 2.12 throws if WidgetsBinding.instance isn't initialized.
// When this method is called, it is guaranteed that a binding was already
// initialized.
WidgetsBinding get instance => _ambiguate(WidgetsBinding.instance);

/// Initializes the Binding.
void ensureInitialized() => WidgetsFlutterBinding.ensureInitialized();
}

WidgetsBinding _ambiguate(WidgetsBinding? binding) => binding!;
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import 'package:flutter/foundation.dart';
import 'package:flutter/material.dart';
import 'package:sentry/sentry.dart';

import '../binding_utils.dart';
import '../sentry_flutter_options.dart';

typedef WidgetBindingGetter = WidgetsBinding? Function();
Expand All @@ -14,19 +13,7 @@ typedef WidgetBindingGetter = WidgetsBinding? Function();
/// FlutterEnricher only needs to add information which aren't exposed by
/// the Dart runtime.
class FlutterEnricherEventProcessor extends EventProcessor {
FlutterEnricherEventProcessor(
this._options,
this._getWidgetsBinding,
);

factory FlutterEnricherEventProcessor.simple({
required SentryFlutterOptions options,
}) {
return FlutterEnricherEventProcessor(
options,
BindingUtils.getWidgetsBindingInstance,
);
}
FlutterEnricherEventProcessor(this._options);

final SentryFlutterOptions _options;

Expand All @@ -36,9 +23,9 @@ class FlutterEnricherEventProcessor extends EventProcessor {
// We can't use `WidgetsBinding` as a direct parameter
// because it must be called inside the `runZoneGuarded`-Integration.
// Thus we call it on demand after all the initialization happened.
final WidgetBindingGetter _getWidgetsBinding;
WidgetsBinding? get _widgetsBinding => _getWidgetsBinding();
SingletonFlutterWindow? get _window => _widgetsBinding?.window;
WidgetsBinding get _widgetsBinding => _options.bindingUtils.instance;

SingletonFlutterWindow? get _window => _widgetsBinding.window;
Map<String, String> _packages = {};

@override
Expand Down Expand Up @@ -128,14 +115,14 @@ class FlutterEnricherEventProcessor extends EventProcessor {
}

Map<String, String> _getFlutterContext() {
final currentLifecycle = _widgetsBinding?.lifecycleState;
final currentLifecycle = _widgetsBinding.lifecycleState;
final debugPlatformOverride = debugDefaultTargetPlatformOverride;
final tempDebugBrightnessOverride = debugBrightnessOverride;
final initialLifecycleState = _window?.initialLifecycleState;
final defaultRouteName = _window?.defaultRouteName;
// A FlutterEngine has no renderViewElement if it was started or is
// accessed from an isolate different to the main isolate.
final hasRenderView = _widgetsBinding?.renderViewElement != null;
final hasRenderView = _widgetsBinding.renderViewElement != null;

return <String, String>{
'has_render_view': hasRenderView.toString(),
Expand Down
7 changes: 1 addition & 6 deletions flutter/lib/src/integrations/on_error_integration.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import 'dart:ui';

import 'package:flutter/widgets.dart';
import 'package:sentry/sentry.dart';
import '../binding_utils.dart';
import '../sentry_flutter_options.dart';

typedef ErrorCallback = bool Function(Object exception, StackTrace stackTrace);
Expand All @@ -29,11 +28,7 @@ class OnErrorIntegration implements Integration<SentryFlutterOptions> {
@override
void call(Hub hub, SentryFlutterOptions options) {
_options = options;
final binding = BindingUtils.getWidgetsBindingInstance();

if (binding == null) {
return;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this should stay, otherwise, it won't work anyway, so bail out early.

final binding = options.bindingUtils.instance;

// WidgetsBinding works with WidgetsFlutterBinding and other custom bindings
final wrapper = dispatchWrapper ??
Expand Down
25 changes: 9 additions & 16 deletions flutter/lib/src/integrations/widgets_binding_integration.dart
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import 'dart:async';

import 'package:sentry/sentry.dart';
import '../binding_utils.dart';
import '../sentry_flutter_options.dart';
import '../widgets_binding_observer.dart';

Expand All @@ -10,10 +9,12 @@ import '../widgets_binding_observer.dart';
/// - [SentryWidgetsBindingObserver]
/// - [WidgetsBindingObserver](https://api.flutter.dev/flutter/widgets/WidgetsBindingObserver-class.html)
class WidgetsBindingIntegration extends Integration<SentryFlutterOptions> {
SentryWidgetsBindingObserver? _observer;
late SentryWidgetsBindingObserver _observer;
late SentryFlutterOptions _options;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better to keep it nullable, so if close is called before call which is wrong, wouldn't throw, it's defensive programming against API misusing.


@override
FutureOr<void> call(Hub hub, SentryFlutterOptions options) {
_options = options;
_observer = SentryWidgetsBindingObserver(
hub: hub,
options: options,
Expand All @@ -22,23 +23,15 @@ class WidgetsBindingIntegration extends Integration<SentryFlutterOptions> {
// We don't need to call `WidgetsFlutterBinding.ensureInitialized()`
// because `WidgetsFlutterBindingIntegration` already calls it.
// If the instance is not created, we skip it to keep going.
final instance = BindingUtils.getWidgetsBindingInstance();
if (instance != null) {
instance.addObserver(_observer!);
options.sdk.addIntegration('widgetsBindingIntegration');
} else {
options.logger(
SentryLevel.error,
'widgetsBindingIntegration failed to be installed',
);
}
final instance = _options.bindingUtils.instance;

instance.addObserver(_observer);
options.sdk.addIntegration('widgetsBindingIntegration');
}

@override
FutureOr<void> close() {
final instance = BindingUtils.getWidgetsBindingInstance();
if (instance != null && _observer != null) {
instance.removeObserver(_observer!);
}
final instance = _options.bindingUtils.instance;
instance.removeObserver(_observer);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,9 @@ typedef OnWidgetsBinding = WidgetsBinding Function();
/// call into the native code.
class WidgetsFlutterBindingIntegration
extends Integration<SentryFlutterOptions> {
WidgetsFlutterBindingIntegration([OnWidgetsBinding? ensureInitialized])
: _ensureInitialized =
ensureInitialized ?? WidgetsFlutterBinding.ensureInitialized;

final OnWidgetsBinding _ensureInitialized;

@override
FutureOr<void> call(Hub hub, SentryFlutterOptions options) {
_ensureInitialized();
options.bindingUtils.ensureInitialized();
options.sdk.addIntegration('widgetsFlutterBindingIntegration');
}
}
3 changes: 1 addition & 2 deletions flutter/lib/src/sentry_flutter.dart
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,7 @@ mixin SentryFlutter {
options.transport = FileSystemTransport(channel, options);
}

var flutterEventProcessor =
FlutterEnricherEventProcessor.simple(options: options);
var flutterEventProcessor = FlutterEnricherEventProcessor(options);
options.addEventProcessor(flutterEventProcessor);

if (options.platformChecker.platform.isAndroid) {
Expand Down
4 changes: 4 additions & 0 deletions flutter/lib/src/sentry_flutter_options.dart
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import 'package:meta/meta.dart';
import 'package:sentry/sentry.dart';

import 'binding_wrapper.dart';
import 'renderer/renderer.dart';

/// This class adds options which are only availble in a Flutter environment.
Expand Down Expand Up @@ -251,4 +252,7 @@ class SentryFlutterOptions extends SentryOptions {
useFlutterBreadcrumbTracking();
}
}

@internal
BindingWrapper bindingUtils = BindingWrapper();
}
11 changes: 5 additions & 6 deletions flutter/lib/src/widgets_binding_observer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import 'dart:ui';
import 'package:flutter/foundation.dart';
import 'package:flutter/material.dart';
import '../sentry_flutter.dart';
import 'binding_utils.dart';

/// This is a `WidgetsBindingObserver` which can observe some events of a
/// Flutter application.
Expand Down Expand Up @@ -39,7 +38,7 @@ class SentryWidgetsBindingObserver with WidgetsBindingObserver {
.skip(1) // Skip initial event added below in constructor
.listen(_onScreenSizeChanged);

final window = BindingUtils.getWidgetsBindingInstance()?.window;
final window = _options.bindingUtils.instance.window;
_screenSizeStreamController.add(window);
}
}
Expand Down Expand Up @@ -87,7 +86,7 @@ class SentryWidgetsBindingObserver with WidgetsBindingObserver {
if (!_options.enableWindowMetricBreadcrumbs) {
return;
}
final window = BindingUtils.getWidgetsBindingInstance()?.window;
final window = _options.bindingUtils.instance.window;
_screenSizeStreamController.add(window);
}

Expand All @@ -109,8 +108,7 @@ class SentryWidgetsBindingObserver with WidgetsBindingObserver {
if (!_options.enableBrightnessChangeBreadcrumbs) {
return;
}
final brightness =
BindingUtils.getWidgetsBindingInstance()?.window.platformBrightness;
final brightness = _options.bindingUtils.instance.window.platformBrightness;
final brightnessDescription =
brightness == Brightness.dark ? 'dark' : 'light';

Expand All @@ -134,7 +132,8 @@ class SentryWidgetsBindingObserver with WidgetsBindingObserver {
return;
}
final newTextScaleFactor =
BindingUtils.getWidgetsBindingInstance()?.window.textScaleFactor;
_options.bindingUtils.instance.window.textScaleFactor;

_hub.addBreadcrumb(Breadcrumb(
message: 'Text scale factor changed to $newTextScaleFactor.',
type: 'system',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -295,10 +295,7 @@ class Fixture {
dsn: fakeDsn,
checker: platformChecker,
)..reportPackages = reportPackages;
return FlutterEnricherEventProcessor(
options,
binding,
);
return FlutterEnricherEventProcessor(options);
}
}

Expand Down

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import 'package:flutter/services.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:sentry_flutter/sentry_flutter.dart';
import 'package:sentry_flutter/src/binding_utils.dart';
import 'package:sentry_flutter/src/integrations/widgets_flutter_binding_integration.dart';

import '../mocks.dart';
Expand Down Expand Up @@ -33,19 +32,19 @@ void main() {
});

test('WidgetsFlutterBindingIntegration calls ensureInitialized', () async {
var called = false;
var ensureInitialized = () {
called = true;
return BindingUtils.getWidgetsBindingInstance()!;
};
final integration = WidgetsFlutterBindingIntegration(ensureInitialized);
final integration = WidgetsFlutterBindingIntegration();
await integration(fixture.hub, fixture.options);

expect(called, true);
expect(fixture.testBindingUtils.ensureBindingInitializedCalled, true);
});
}

class Fixture {
final hub = MockHub();
final options = SentryFlutterOptions(dsn: fakeDsn);

final options = SentryFlutterOptions(dsn: fakeDsn)
..bindingUtils = TestBindingWrapper();

TestBindingWrapper get testBindingUtils =>
options.bindingUtils as TestBindingWrapper;
}
Loading