diff --git a/dev/tracing_tests/test/common.dart b/dev/tracing_tests/test/common.dart index 41e650d9ae..29a60372a3 100644 --- a/dev/tracing_tests/test/common.dart +++ b/dev/tracing_tests/test/common.dart @@ -5,7 +5,9 @@ import 'dart:developer' as developer; import 'dart:isolate' as isolate; +import 'package:flutter/foundation.dart'; import 'package:flutter/scheduler.dart'; +import 'package:flutter/widgets.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:vm_service/vm_service.dart'; import 'package:vm_service/vm_service_io.dart'; @@ -52,3 +54,29 @@ Future runFrame(VoidCallback callback) { callback(); return result; } + +// This binding skips the zones tests. These tests were written before we +// verified zones properly, and they have been legacied-in to avoid having +// to refactor them. +// +// When creating new tests, avoid relying on this class. +class ZoneIgnoringTestBinding extends WidgetsFlutterBinding { + @override + void initInstances() { + super.initInstances(); + _instance = this; + } + + @override + bool debugCheckZone(String entryPoint) { return true; } + + static ZoneIgnoringTestBinding get instance => BindingBase.checkInstance(_instance); + static ZoneIgnoringTestBinding? _instance; + + static ZoneIgnoringTestBinding ensureInitialized() { + if (ZoneIgnoringTestBinding._instance == null) { + ZoneIgnoringTestBinding(); + } + return ZoneIgnoringTestBinding.instance; + } +} diff --git a/dev/tracing_tests/test/image_painting_event_test.dart b/dev/tracing_tests/test/image_painting_event_test.dart index 774a2fa45e..e190108943 100644 --- a/dev/tracing_tests/test/image_painting_event_test.dart +++ b/dev/tracing_tests/test/image_painting_event_test.dart @@ -13,6 +13,8 @@ import 'package:vm_service/vm_service.dart'; import 'package:vm_service/vm_service_io.dart'; void main() { + LiveTestWidgetsFlutterBinding.ensureInitialized(); + late VmService vmService; late LiveTestWidgetsFlutterBinding binding; setUpAll(() async { @@ -27,7 +29,7 @@ void main() { await vmService.streamListen(EventStreams.kExtension); // Initialize bindings - binding = LiveTestWidgetsFlutterBinding(); + binding = LiveTestWidgetsFlutterBinding.instance; binding.framePolicy = LiveTestWidgetsFlutterBindingFramePolicy.fullyLive; binding.attachRootWidget(const SizedBox.expand()); expect(binding.framesEnabled, true); diff --git a/dev/tracing_tests/test/inflate_widget_tracing_test.dart b/dev/tracing_tests/test/inflate_widget_tracing_test.dart index 700be3d469..8831612e28 100644 --- a/dev/tracing_tests/test/inflate_widget_tracing_test.dart +++ b/dev/tracing_tests/test/inflate_widget_tracing_test.dart @@ -16,7 +16,7 @@ final Set interestingLabels = { }; void main() { - WidgetsFlutterBinding.ensureInitialized(); + ZoneIgnoringTestBinding.ensureInitialized(); initTimelineTests(); test('Children of MultiChildRenderObjectElement show up in tracing', () async { // We don't have expectations around the first frame because there's a race around diff --git a/dev/tracing_tests/test/inflate_widget_update_test.dart b/dev/tracing_tests/test/inflate_widget_update_test.dart index acfd014082..edd3237e55 100644 --- a/dev/tracing_tests/test/inflate_widget_update_test.dart +++ b/dev/tracing_tests/test/inflate_widget_update_test.dart @@ -9,7 +9,7 @@ import 'package:flutter_test/flutter_test.dart'; import 'common.dart'; void main() { - WidgetsFlutterBinding.ensureInitialized(); + ZoneIgnoringTestBinding.ensureInitialized(); initTimelineTests(); test('Widgets with updated keys produce well formed timelines', () async { await runFrame(() { runApp(const TestRoot()); }); diff --git a/dev/tracing_tests/test/timeline_test.dart b/dev/tracing_tests/test/timeline_test.dart index 6fd4b6cd8a..e5a915d5a6 100644 --- a/dev/tracing_tests/test/timeline_test.dart +++ b/dev/tracing_tests/test/timeline_test.dart @@ -58,7 +58,7 @@ class TestRootState extends State { } void main() { - WidgetsFlutterBinding.ensureInitialized(); + ZoneIgnoringTestBinding.ensureInitialized(); initTimelineTests(); test('Timeline', () async { // We don't have expectations around the first frame because there's a race around diff --git a/packages/flutter/lib/src/foundation/binding.dart b/packages/flutter/lib/src/foundation/binding.dart index 78b5ce1781..f1601f3558 100644 --- a/packages/flutter/lib/src/foundation/binding.dart +++ b/packages/flutter/lib/src/foundation/binding.dart @@ -2,6 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +import 'dart:async'; import 'dart:convert' show json; import 'dart:developer' as developer; import 'dart:io' show exit; @@ -265,6 +266,7 @@ abstract class BindingBase { assert(_debugInitializedType == null); assert(() { _debugInitializedType = runtimeType; + _debugBindingZone = Zone.current; return true; }()); } @@ -319,7 +321,7 @@ abstract class BindingBase { ), ErrorHint( 'It is also possible that $T does not implement "initInstances()" to assign a value to "instance". See the ' - 'documentation of the BaseBinding class for more details.', + 'documentation of the BindingBase class for more details.', ), ErrorHint( 'The binding that was initialized was of the type "$_debugInitializedType". ' @@ -399,6 +401,95 @@ abstract class BindingBase { return _debugInitializedType; } + Zone? _debugBindingZone; + + /// Whether [debugCheckZone] should throw (true) or just report the error (false). + /// + /// Setting this to true makes it easier to catch cases where the zones are + /// misconfigured, by allowing debuggers to stop at the point of error. + /// + /// Currently this defaults to false, to avoid suddenly breaking applications + /// that are affected by this check but appear to be working today. Applications + /// are encouraged to resolve any issues that cause the [debugCheckZone] message + /// to appear, as even if they appear to be working today, they are likely to be + /// hiding hard-to-find bugs, and are more brittle (likely to collect bugs in + /// the future). + /// + /// To silence the message displayed by [debugCheckZone], ensure that the same + /// zone is used when calling `ensureInitialized()` as when calling the framework + /// in any other context (e.g. via [runApp]). + static bool debugZoneErrorsAreFatal = false; + + /// Checks that the current [Zone] is the same as that which was used + /// to initialize the binding. + /// + /// If the current zone ([Zone.current]) is not the zone that was active when + /// the binding was initialized, then this method generates a [FlutterError] + /// exception with detailed information. The exception is either thrown + /// directly, or reported via [FlutterError.reportError], depending on the + /// value of [BindingBase.debugZoneErrorsAreFatal]. + /// + /// To silence the message displayed by [debugCheckZone], ensure that the same + /// zone is used when calling `ensureInitialized()` as when calling the + /// framework in any other context (e.g. via [runApp]). For example, consider + /// keeping a reference to the zone used to initialize the binding, and using + /// [Zone.run] to use it again when calling into the framework. + /// + /// ## Usage + /// + /// The binding is considered initialized once [BindingBase.initInstances] has + /// run; if this is called before then, it will throw an [AssertionError]. + /// + /// The `entryPoint` parameter is the name of the API that is checking the + /// zones are consistent, for example, `'runApp'`. + /// + /// This function always returns true (if it does not throw). It is expected + /// to be invoked via the binding instance, e.g.: + /// + /// ```dart + /// void startup() { + /// WidgetsBinding binding = WidgetsFlutterBinding.ensureInitialized(); + /// assert(binding.debugCheckZone('startup')); + /// // ... + /// } + /// ``` + /// + /// If the binding expects to be used with multiple zones, it should override + /// this method to return true always without throwing. (For example, the + /// bindings used with [flutter_test] do this as they make heavy use of zones + /// to drive the framework with an artificial clock and to catch errors and + /// report them as test failures.) + bool debugCheckZone(String entryPoint) { + assert(() { + assert(_debugBindingZone != null, 'debugCheckZone can only be used after the binding is fully initialized.'); + if (Zone.current != _debugBindingZone) { + final Error message = FlutterError( + 'Zone mismatch.\n' + 'The Flutter bindings were initialized in a different zone than is now being used. ' + 'This will likely cause confusion and bugs as any zone-specific configuration will ' + 'inconsistently use the configuration of the original binding initialization zone ' + 'or this zone based on hard-to-predict factors such as which zone was active when ' + 'a particular callback was set.\n' + 'It is important to use the same zone when calling `ensureInitialized` on the binding ' + 'as when calling `$entryPoint` later.\n' + 'To make this ${ debugZoneErrorsAreFatal ? 'error non-fatal' : 'warning fatal' }, ' + 'set BindingBase.debugZoneErrorsAreFatal to ${!debugZoneErrorsAreFatal} before the ' + 'bindings are initialized (i.e. as the first statement in `void main() { }`).', + ); + if (debugZoneErrorsAreFatal) { + throw message; + } + FlutterError.reportError(FlutterErrorDetails( + exception: message, + stack: StackTrace.current, + context: ErrorDescription('during $entryPoint'), + )); + } + return true; + }()); + return true; + } + /// Called when the binding is initialized, to register service /// extensions. /// diff --git a/packages/flutter/lib/src/widgets/binding.dart b/packages/flutter/lib/src/widgets/binding.dart index 53f595ce7e..691657feb3 100644 --- a/packages/flutter/lib/src/widgets/binding.dart +++ b/packages/flutter/lib/src/widgets/binding.dart @@ -1031,6 +1031,7 @@ mixin WidgetsBinding on BindingBase, ServicesBinding, SchedulerBinding, GestureB /// ensure the widget, element, and render trees are all built. void runApp(Widget app) { final WidgetsBinding binding = WidgetsFlutterBinding.ensureInitialized(); + assert(binding.debugCheckZone('runApp')); binding ..scheduleAttachRootWidget(binding.wrapWithDefaultView(app)) ..scheduleWarmUpFrame(); diff --git a/packages/flutter/test/foundation/binding_2_test.dart b/packages/flutter/test/foundation/binding_2_test.dart new file mode 100644 index 0000000000..fd30fdb74a --- /dev/null +++ b/packages/flutter/test/foundation/binding_2_test.dart @@ -0,0 +1,63 @@ +// Copyright 2014 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'dart:async'; + +import 'package:flutter/foundation.dart'; +import 'package:flutter_test/flutter_test.dart'; + +class TestBinding extends BindingBase { } + +void main() { + test('BindingBase.debugCheckZone', () async { + final BindingBase binding = TestBinding(); + binding.debugCheckZone('test1'); + BindingBase.debugZoneErrorsAreFatal = true; + Zone.current.fork().run(() { + try { + binding.debugCheckZone('test2'); + fail('expected an exception'); + } catch (error) { + expect(error, isA()); + expect(error.toString(), + 'Zone mismatch.\n' + 'The Flutter bindings were initialized in a different zone than is now being used. ' + 'This will likely cause confusion and bugs as any zone-specific configuration will ' + 'inconsistently use the configuration of the original binding initialization zone ' + 'or this zone based on hard-to-predict factors such as which zone was active when ' + 'a particular callback was set.\n' + 'It is important to use the same zone when calling `ensureInitialized` on the ' + 'binding as when calling `test2` later.\n' + 'To make this error non-fatal, set BindingBase.debugZoneErrorsAreFatal to false ' + 'before the bindings are initialized (i.e. as the first statement in `void main() { }`).', + ); + } + }); + BindingBase.debugZoneErrorsAreFatal = false; + Zone.current.fork().run(() { + bool sawError = false; + final FlutterExceptionHandler? lastHandler = FlutterError.onError; + FlutterError.onError = (FlutterErrorDetails details) { + final Object error = details.exception; + expect(error, isA()); + expect(error.toString(), + 'Zone mismatch.\n' + 'The Flutter bindings were initialized in a different zone than is now being used. ' + 'This will likely cause confusion and bugs as any zone-specific configuration will ' + 'inconsistently use the configuration of the original binding initialization zone ' + 'or this zone based on hard-to-predict factors such as which zone was active when ' + 'a particular callback was set.\n' + 'It is important to use the same zone when calling `ensureInitialized` on the ' + 'binding as when calling `test3` later.\n' + 'To make this warning fatal, set BindingBase.debugZoneErrorsAreFatal to true ' + 'before the bindings are initialized (i.e. as the first statement in `void main() { }`).', + ); + sawError = true; + }; + binding.debugCheckZone('test3'); + expect(sawError, isTrue); + FlutterError.onError = lastHandler; + }); + }); +} diff --git a/packages/flutter/test/foundation/binding_test.dart b/packages/flutter/test/foundation/binding_test.dart index 279038c497..c47b1acba1 100644 --- a/packages/flutter/test/foundation/binding_test.dart +++ b/packages/flutter/test/foundation/binding_test.dart @@ -27,7 +27,6 @@ class FooLibraryBinding extends BindingBase with FooBinding { } } - void main() { test('BindingBase.debugBindingType', () async { expect(BindingBase.debugBindingType(), isNull); diff --git a/packages/flutter/test/painting/binding_test.dart b/packages/flutter/test/painting/binding_test.dart index 091a76ae6a..e214e82cd1 100644 --- a/packages/flutter/test/painting/binding_test.dart +++ b/packages/flutter/test/painting/binding_test.dart @@ -47,6 +47,9 @@ class TestBindingBase implements BindingBase { @override void initInstances() {} + @override + bool debugCheckZone(String entryPoint) { return true; } + @override void initServiceExtensions() {} diff --git a/packages/flutter/test/widgets/run_app_async_test.dart b/packages/flutter/test/widgets/run_app_async_test.dart index 6dc824b7a0..d0901ec08c 100644 --- a/packages/flutter/test/widgets/run_app_async_test.dart +++ b/packages/flutter/test/widgets/run_app_async_test.dart @@ -3,12 +3,39 @@ // found in the LICENSE file. import 'package:fake_async/fake_async.dart'; +import 'package:flutter/foundation.dart'; import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; +// This test is very fragile and bypasses some zone-related checks. +// It is written this way to verify some invariants that would otherwise +// be difficult to check. +// Do not use this test as a guide for writing good Flutter code. + +class TestBinding extends WidgetsFlutterBinding { + @override + void initInstances() { + super.initInstances(); + _instance = this; + } + + @override + bool debugCheckZone(String entryPoint) { return true; } + + static TestBinding get instance => BindingBase.checkInstance(_instance); + static TestBinding? _instance; + + static TestBinding ensureInitialized() { + if (TestBinding._instance == null) { + TestBinding(); + } + return TestBinding.instance; + } +} + void main() { setUp(() { - WidgetsFlutterBinding.ensureInitialized(); + TestBinding.ensureInitialized(); WidgetsBinding.instance.resetEpoch(); }); diff --git a/packages/flutter_driver/lib/src/extension/extension.dart b/packages/flutter_driver/lib/src/extension/extension.dart index d744b5094d..2bb6705dd2 100644 --- a/packages/flutter_driver/lib/src/extension/extension.dart +++ b/packages/flutter_driver/lib/src/extension/extension.dart @@ -39,6 +39,11 @@ class _DriverBinding extends BindingBase with SchedulerBinding, ServicesBinding, final List? finders; final List? commands; + // Because you can't really control which zone a driver test uses, + // we override the test for zones here. + @override + bool debugCheckZone(String entryPoint) { return true; } + @override void initServiceExtensions() { super.initServiceExtensions(); diff --git a/packages/flutter_test/lib/src/binding.dart b/packages/flutter_test/lib/src/binding.dart index 1ca3b90553..b84d52d560 100644 --- a/packages/flutter_test/lib/src/binding.dart +++ b/packages/flutter_test/lib/src/binding.dart @@ -369,6 +369,13 @@ abstract class TestWidgetsFlutterBinding extends BindingBase // doesn't get generated for tests. } + @override + bool debugCheckZone(String entryPoint) { + // We skip all the zone checks in tests because the test framework makes heavy use + // of zones and so the zones never quite match the way the framework expects. + return true; + } + /// Whether there is currently a test executing. bool get inTest; @@ -815,9 +822,9 @@ abstract class TestWidgetsFlutterBinding extends BindingBase }; FlutterError.demangleStackTrace = (StackTrace stack) { // package:stack_trace uses ZoneSpecification.errorCallback to add useful - // information to stack traces, in this case the Trace and Chain classes - // can be present. Because these StackTrace implementations do not follow - // the format the framework expects, we covert them to a vm trace here. + // information to stack traces, meaning Trace and Chain classes can be + // present. Because these StackTrace implementations do not follow the + // format the framework expects, we convert them to a vm trace here. if (stack is stack_trace.Trace) { return stack.vmTrace; } diff --git a/packages/flutter_test/test/live_widget_controller_test.dart b/packages/flutter_test/test/live_widget_controller_test.dart index 86b2709384..448d0c75f9 100644 --- a/packages/flutter_test/test/live_widget_controller_test.dart +++ b/packages/flutter_test/test/live_widget_controller_test.dart @@ -2,11 +2,38 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +import 'package:flutter/foundation.dart'; import 'package:flutter/gestures.dart'; import 'package:flutter/material.dart'; import 'package:flutter/scheduler.dart'; import 'package:flutter_test/flutter_test.dart'; +// This test is very fragile and bypasses some zone-related checks. +// It is written this way to verify some invariants that would otherwise +// be difficult to check. +// Do not use this test as a guide for writing good Flutter code. + +class TestBinding extends WidgetsFlutterBinding { + @override + void initInstances() { + super.initInstances(); + _instance = this; + } + + @override + bool debugCheckZone(String entryPoint) { return true; } + + static TestBinding get instance => BindingBase.checkInstance(_instance); + static TestBinding? _instance; + + static TestBinding ensureInitialized() { + if (TestBinding._instance == null) { + TestBinding(); + } + return TestBinding.instance; + } +} + class CountButton extends StatefulWidget { const CountButton({super.key}); @@ -65,6 +92,8 @@ class _AnimateSampleState extends State } void main() { + TestBinding.ensureInitialized(); + test('Test pump on LiveWidgetController', () async { runApp(const MaterialApp(home: Center(child: CountButton()))); @@ -109,7 +138,7 @@ void main() { final List records = [ PointerEventRecord(Duration.zero, [ // Typically PointerAddedEvent is not used in testers, but for records - // captured on a device it is usually what start a gesture. + // captured on a device it is usually what starts a gesture. PointerAddedEvent( position: location, ),