migrate scheduler to nullsafety #61570
Conversation
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat. Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
fluttergithubbot
added
the
framework
| late StackTrace debugStack; | ||
| late Completer<T> completer; |
There was a problem hiding this comment.
Could these two be late final?
There was a problem hiding this comment.
Actually, it looks like completer could just be final and initialized right here instead of in the constructor?
| @@ -340,8 +338,8 @@ mixin SchedulerBinding on BindingBase { | |||
| /// | |||
| /// The preferred way to watch for changes to this value is using | |||
| /// [WidgetsBindingObserver.didChangeAppLifecycleState]. | |||
| AppLifecycleState get lifecycleState => _lifecycleState; | |||
| AppLifecycleState _lifecycleState; | |||
| AppLifecycleState? get lifecycleState => _lifecycleState; | |||
There was a problem hiding this comment.
I would expect this one to always return a non-null value. At the time when you can read it, _lifecycleState has always been initialized.
There was a problem hiding this comment.
Changing this line to:
AppLifecycleState get lifecycleState => _lifecycleState!;
causes a lot of failures in tests. For instance:
Null check operator used on a null value
package:flutter/src/scheduler/binding.dart 340:58 SchedulerBinding.lifecycleState
package:flutter/src/services/binding.dart 180:9 ServicesBinding.readInitialLifecycleStateFromNativeWindow
package:flutter/src/services/binding.dart 34:5 ServicesBinding.initInstances
package:flutter/src/gestures/binding.dart 66:11 GestureBinding.initInstances
package:flutter/src/semantics/binding.dart 24:11 SemanticsBinding.initInstances
package:flutter/src/rendering/binding.dart 32:11 RendererBinding.initInstances
package:flutter/src/painting/binding.dart 23:11 PaintingBinding.initInstances
package:flutter/src/widgets/binding.dart 257:11 WidgetsBinding.initInstances
package:flutter_test/src/binding.dart 302:11 TestWidgetsFlutterBinding.initInstances
package:flutter_test/src/binding.dart 898:11 AutomatedTestWidgetsFlutterBinding.initInstances
package:flutter/src/foundation/binding.dart 57:5 new BindingBase
package:flutter_test/src/binding.dart new _TestWidgetsFlutterBinding&BindingBase&SchedulerBinding
package:flutter_test/src/binding.dart new _TestWidgetsFlutterBinding&BindingBase&SchedulerBinding&ServicesBinding
package:flutter_test/src/binding.dart new _TestWidgetsFlutterBinding&BindingBase&SchedulerBinding&ServicesBinding&GestureBinding
package:flutter_test/src/binding.dart new _TestWidgetsFlutterBinding&BindingBase&SchedulerBinding&ServicesBinding&GestureBinding&SemanticsBinding
package:flutter_test/src/binding.dart new _TestWidgetsFlutterBinding&BindingBase&SchedulerBinding&ServicesBinding&GestureBinding&SemanticsBinding&RendererBinding
package:flutter_test/src/binding.dart new _TestWidgetsFlutterBinding&BindingBase&SchedulerBinding&ServicesBinding&GestureBinding&SemanticsBinding&RendererBinding&PaintingBinding
package:flutter_test/src/binding.dart new _TestWidgetsFlutterBinding&BindingBase&SchedulerBinding&ServicesBinding&GestureBinding&SemanticsBinding&RendererBinding&PaintingBinding&WidgetsBinding
package:flutter_test/src/binding.dart new TestWidgetsFlutterBinding
package:flutter_test/src/binding.dart new AutomatedTestWidgetsFlutterBinding
package:flutter_test/src/_binding_io.dart 25:7 ensureInitialized
package:flutter_test/src/binding.dart 298:100 TestWidgetsFlutterBinding.ensureInitialized
package:flutter_test/src/widget_tester.dart 123:71 testWidgets
material/tabbed_scrollview_warp_test.dart 79:3 main
package:flutter_goldens/flutter_goldens.dart 43:17 main
dart:async _completeOnAsyncReturn
package:flutter_goldens/flutter_goldens.dart FlutterLocalFileComparator.fromDefaultComparator
dart:async _completeOnAsyncReturn
package:flutter_goldens_client/skia_client.dart SkiaGoldClient.getExpectations
dart:async _completeOnAsyncReturn
package:flutter_goldens_client/skia_client.dart SkiaGoldClient.getExpectations.<fn>
| return _primaryCompleter.future.then<R>(onValue, onError: onError); | ||
| } | ||
|
|
||
| @override | ||
| Future<void> timeout(Duration timeLimit, { dynamic onTimeout() }) { | ||
| Future<void> timeout(Duration timeLimit, {FutureOr<void> onTimeout()?}) { |
There was a problem hiding this comment.
Since you changed it above: Change this one also to the new function syntax?
|
PTAL |
|
@goderbauer can I merge regarding this job failure :
|
|
Let me ask internally what the plan for NNBD is there. I'll get back to you. |
|
Let's wait with merging this patch until the other NNBD patch has landed in google and this check is green. |
|
Google internally is now ready for NNBD flutter. I restarted the "google testing" check. If it passes, we're ready to submit this. |
Description
NNBD migration for scheduler
Related Issues
Tests
I added the following tests:
None
Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]). This will ensure a smooth and quick review process.///).flutter analyze --flutter-repo) does not report any problems on my PR.Breaking Change
Did any tests fail when you ran them? Please read Handling breaking changes.