Unverified Commit 245d6d45 authored by Ian Hickson's avatar Ian Hickson Committed by GitHub

Assert that runApp is called in the same zone as binding.ensureInitialized (#122836)

Assert that runApp is called in the same zone as binding.ensureInitialized
parent 9b8f3c8b
...@@ -5,7 +5,9 @@ ...@@ -5,7 +5,9 @@
import 'dart:developer' as developer; import 'dart:developer' as developer;
import 'dart:isolate' as isolate; import 'dart:isolate' as isolate;
import 'package:flutter/foundation.dart';
import 'package:flutter/scheduler.dart'; import 'package:flutter/scheduler.dart';
import 'package:flutter/widgets.dart';
import 'package:flutter_test/flutter_test.dart'; import 'package:flutter_test/flutter_test.dart';
import 'package:vm_service/vm_service.dart'; import 'package:vm_service/vm_service.dart';
import 'package:vm_service/vm_service_io.dart'; import 'package:vm_service/vm_service_io.dart';
...@@ -52,3 +54,29 @@ Future<void> runFrame(VoidCallback callback) { ...@@ -52,3 +54,29 @@ Future<void> runFrame(VoidCallback callback) {
callback(); callback();
return result; 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;
}
}
...@@ -13,6 +13,8 @@ import 'package:vm_service/vm_service.dart'; ...@@ -13,6 +13,8 @@ import 'package:vm_service/vm_service.dart';
import 'package:vm_service/vm_service_io.dart'; import 'package:vm_service/vm_service_io.dart';
void main() { void main() {
LiveTestWidgetsFlutterBinding.ensureInitialized();
late VmService vmService; late VmService vmService;
late LiveTestWidgetsFlutterBinding binding; late LiveTestWidgetsFlutterBinding binding;
setUpAll(() async { setUpAll(() async {
...@@ -27,7 +29,7 @@ void main() { ...@@ -27,7 +29,7 @@ void main() {
await vmService.streamListen(EventStreams.kExtension); await vmService.streamListen(EventStreams.kExtension);
// Initialize bindings // Initialize bindings
binding = LiveTestWidgetsFlutterBinding(); binding = LiveTestWidgetsFlutterBinding.instance;
binding.framePolicy = LiveTestWidgetsFlutterBindingFramePolicy.fullyLive; binding.framePolicy = LiveTestWidgetsFlutterBindingFramePolicy.fullyLive;
binding.attachRootWidget(const SizedBox.expand()); binding.attachRootWidget(const SizedBox.expand());
expect(binding.framesEnabled, true); expect(binding.framesEnabled, true);
......
...@@ -16,7 +16,7 @@ final Set<String> interestingLabels = <String>{ ...@@ -16,7 +16,7 @@ final Set<String> interestingLabels = <String>{
}; };
void main() { void main() {
WidgetsFlutterBinding.ensureInitialized(); ZoneIgnoringTestBinding.ensureInitialized();
initTimelineTests(); initTimelineTests();
test('Children of MultiChildRenderObjectElement show up in tracing', () async { test('Children of MultiChildRenderObjectElement show up in tracing', () async {
// We don't have expectations around the first frame because there's a race around // We don't have expectations around the first frame because there's a race around
......
...@@ -9,7 +9,7 @@ import 'package:flutter_test/flutter_test.dart'; ...@@ -9,7 +9,7 @@ import 'package:flutter_test/flutter_test.dart';
import 'common.dart'; import 'common.dart';
void main() { void main() {
WidgetsFlutterBinding.ensureInitialized(); ZoneIgnoringTestBinding.ensureInitialized();
initTimelineTests(); initTimelineTests();
test('Widgets with updated keys produce well formed timelines', () async { test('Widgets with updated keys produce well formed timelines', () async {
await runFrame(() { runApp(const TestRoot()); }); await runFrame(() { runApp(const TestRoot()); });
......
...@@ -58,7 +58,7 @@ class TestRootState extends State<TestRoot> { ...@@ -58,7 +58,7 @@ class TestRootState extends State<TestRoot> {
} }
void main() { void main() {
WidgetsFlutterBinding.ensureInitialized(); ZoneIgnoringTestBinding.ensureInitialized();
initTimelineTests(); initTimelineTests();
test('Timeline', () async { test('Timeline', () async {
// We don't have expectations around the first frame because there's a race around // We don't have expectations around the first frame because there's a race around
......
...@@ -2,6 +2,7 @@ ...@@ -2,6 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
import 'dart:async';
import 'dart:convert' show json; import 'dart:convert' show json;
import 'dart:developer' as developer; import 'dart:developer' as developer;
import 'dart:io' show exit; import 'dart:io' show exit;
...@@ -265,6 +266,7 @@ abstract class BindingBase { ...@@ -265,6 +266,7 @@ abstract class BindingBase {
assert(_debugInitializedType == null); assert(_debugInitializedType == null);
assert(() { assert(() {
_debugInitializedType = runtimeType; _debugInitializedType = runtimeType;
_debugBindingZone = Zone.current;
return true; return true;
}()); }());
} }
...@@ -319,7 +321,7 @@ abstract class BindingBase { ...@@ -319,7 +321,7 @@ abstract class BindingBase {
), ),
ErrorHint( ErrorHint(
'It is also possible that $T does not implement "initInstances()" to assign a value to "instance". See the ' '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( ErrorHint(
'The binding that was initialized was of the type "$_debugInitializedType". ' 'The binding that was initialized was of the type "$_debugInitializedType". '
...@@ -399,6 +401,95 @@ abstract class BindingBase { ...@@ -399,6 +401,95 @@ abstract class BindingBase {
return _debugInitializedType; 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 /// Called when the binding is initialized, to register service
/// extensions. /// extensions.
/// ///
......
...@@ -1031,6 +1031,7 @@ mixin WidgetsBinding on BindingBase, ServicesBinding, SchedulerBinding, GestureB ...@@ -1031,6 +1031,7 @@ mixin WidgetsBinding on BindingBase, ServicesBinding, SchedulerBinding, GestureB
/// ensure the widget, element, and render trees are all built. /// ensure the widget, element, and render trees are all built.
void runApp(Widget app) { void runApp(Widget app) {
final WidgetsBinding binding = WidgetsFlutterBinding.ensureInitialized(); final WidgetsBinding binding = WidgetsFlutterBinding.ensureInitialized();
assert(binding.debugCheckZone('runApp'));
binding binding
..scheduleAttachRootWidget(binding.wrapWithDefaultView(app)) ..scheduleAttachRootWidget(binding.wrapWithDefaultView(app))
..scheduleWarmUpFrame(); ..scheduleWarmUpFrame();
......
// 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<FlutterError>());
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<FlutterError>());
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;
});
});
}
...@@ -27,7 +27,6 @@ class FooLibraryBinding extends BindingBase with FooBinding { ...@@ -27,7 +27,6 @@ class FooLibraryBinding extends BindingBase with FooBinding {
} }
} }
void main() { void main() {
test('BindingBase.debugBindingType', () async { test('BindingBase.debugBindingType', () async {
expect(BindingBase.debugBindingType(), isNull); expect(BindingBase.debugBindingType(), isNull);
......
...@@ -47,6 +47,9 @@ class TestBindingBase implements BindingBase { ...@@ -47,6 +47,9 @@ class TestBindingBase implements BindingBase {
@override @override
void initInstances() {} void initInstances() {}
@override
bool debugCheckZone(String entryPoint) { return true; }
@override @override
void initServiceExtensions() {} void initServiceExtensions() {}
......
...@@ -3,12 +3,39 @@ ...@@ -3,12 +3,39 @@
// found in the LICENSE file. // found in the LICENSE file.
import 'package:fake_async/fake_async.dart'; import 'package:fake_async/fake_async.dart';
import 'package:flutter/foundation.dart';
import 'package:flutter/material.dart'; import 'package:flutter/material.dart';
import 'package:flutter_test/flutter_test.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() { void main() {
setUp(() { setUp(() {
WidgetsFlutterBinding.ensureInitialized(); TestBinding.ensureInitialized();
WidgetsBinding.instance.resetEpoch(); WidgetsBinding.instance.resetEpoch();
}); });
......
...@@ -39,6 +39,11 @@ class _DriverBinding extends BindingBase with SchedulerBinding, ServicesBinding, ...@@ -39,6 +39,11 @@ class _DriverBinding extends BindingBase with SchedulerBinding, ServicesBinding,
final List<FinderExtension>? finders; final List<FinderExtension>? finders;
final List<CommandExtension>? commands; final List<CommandExtension>? 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 @override
void initServiceExtensions() { void initServiceExtensions() {
super.initServiceExtensions(); super.initServiceExtensions();
......
...@@ -369,6 +369,13 @@ abstract class TestWidgetsFlutterBinding extends BindingBase ...@@ -369,6 +369,13 @@ abstract class TestWidgetsFlutterBinding extends BindingBase
// doesn't get generated for tests. // 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. /// Whether there is currently a test executing.
bool get inTest; bool get inTest;
...@@ -815,9 +822,9 @@ abstract class TestWidgetsFlutterBinding extends BindingBase ...@@ -815,9 +822,9 @@ abstract class TestWidgetsFlutterBinding extends BindingBase
}; };
FlutterError.demangleStackTrace = (StackTrace stack) { FlutterError.demangleStackTrace = (StackTrace stack) {
// package:stack_trace uses ZoneSpecification.errorCallback to add useful // package:stack_trace uses ZoneSpecification.errorCallback to add useful
// information to stack traces, in this case the Trace and Chain classes // information to stack traces, meaning Trace and Chain classes can be
// can be present. Because these StackTrace implementations do not follow // present. Because these StackTrace implementations do not follow the
// the format the framework expects, we covert them to a vm trace here. // format the framework expects, we convert them to a vm trace here.
if (stack is stack_trace.Trace) { if (stack is stack_trace.Trace) {
return stack.vmTrace; return stack.vmTrace;
} }
......
...@@ -2,11 +2,38 @@ ...@@ -2,11 +2,38 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
import 'package:flutter/foundation.dart';
import 'package:flutter/gestures.dart'; import 'package:flutter/gestures.dart';
import 'package:flutter/material.dart'; import 'package:flutter/material.dart';
import 'package:flutter/scheduler.dart'; import 'package:flutter/scheduler.dart';
import 'package:flutter_test/flutter_test.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 { class CountButton extends StatefulWidget {
const CountButton({super.key}); const CountButton({super.key});
...@@ -65,6 +92,8 @@ class _AnimateSampleState extends State<AnimateSample> ...@@ -65,6 +92,8 @@ class _AnimateSampleState extends State<AnimateSample>
} }
void main() { void main() {
TestBinding.ensureInitialized();
test('Test pump on LiveWidgetController', () async { test('Test pump on LiveWidgetController', () async {
runApp(const MaterialApp(home: Center(child: CountButton()))); runApp(const MaterialApp(home: Center(child: CountButton())));
...@@ -109,7 +138,7 @@ void main() { ...@@ -109,7 +138,7 @@ void main() {
final List<PointerEventRecord> records = <PointerEventRecord>[ final List<PointerEventRecord> records = <PointerEventRecord>[
PointerEventRecord(Duration.zero, <PointerEvent>[ PointerEventRecord(Duration.zero, <PointerEvent>[
// Typically PointerAddedEvent is not used in testers, but for records // 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( PointerAddedEvent(
position: location, position: location,
), ),
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment