Fix ios black screen during splash screen if widget binding initializ… by chunhtai · Pull Request #52913 · flutter/flutter

chunhtai · 2020-03-19T23:17:18Z

…ed before runapp

Description

When ios engine starts up, it will send semantics enable message to framework if the binding has been initialized. Framework will schedule a new frame to make sure the semantics tree gets sent to the engine. This makes engine thinks the framework is ready to produce frame and takes down the splash screen.

This pr make sure scheduler binding only schedule frame when widget binding think it is ready by overriding the frameEnabled getter.

Related Issues

Fixes #39494

Tests

I added the following tests:

"The frames will only be enabled after runApp has bootstrapped the app"

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.

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I signed the CLA.
I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I updated/added relevant documentation (doc comments with ///).
All existing and new tests are passing.
The analyzer (flutter analyze --flutter-repo) does not report any problems on my PR.
I am willing to follow-up on review comments in a timely manner.

Breaking Change

Did any tests fail when you ran them? Please read Handling breaking changes.

No, no existing tests failed, so this is not a breaking change.
Yes, this is a breaking change. If not, delete the remainder of this section.
- I wrote a design doc: https://flutter.dev/go/template Replace this with a link to your design doc's short link
- I got input from the developer relations team, specifically from: Replace with the names of who gave advice
- I wrote a migration guide: Replace with a link to your migration guide

…ed before runapp

fluttergithubbot · 2020-03-19T23:17:21Z

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.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

skia-gold · 2020-03-19T23:28:53Z

Gold has detected one or more untriaged digests on patchset 4.
Please triage them at https://flutter-gold.skia.org/search?issue=52913.

chunhtai · 2020-03-19T23:39:50Z

apparently it broke all the tests, I will add a WIP label

Hixie · 2020-03-20T01:04:32Z

packages/flutter/lib/src/widgets/binding.dart

+  bool _appHasBootStapped = false;
+
+  @override
+  bool get framesEnabled => super.framesEnabled && _appHasBootStapped;


"BootStapped" isn't english... what did you mean? :-)

maybe "rootWidgetAttached"?

I misspelled bootstrapped. but your suggestion may be better. will update

fwiw you ended up going with a different name -- i'm fine with either, just wanted to mention it since you said above that you like the specific name i'd suggested.

The rootWidgetAttached will confuse with the existing https://api.flutter.dev/flutter/widgets/WidgetsBinding/isRootWidgetAttached.html. This field actually need to be set to true right before attaching the root widget. That is why i change the name.

chunhtai · 2020-03-20T17:10:31Z

ready for review

goderbauer · 2020-03-20T20:35:01Z

packages/flutter/lib/src/widgets/binding.dart

  Element get renderViewElement => _renderViewElement;
  Element _renderViewElement;

+  bool _rootWidgetAttached = false;


We already have https://master-api.flutter.dev/flutter/widgets/WidgetsBinding/isRootWidgetAttached.html, what's the difference between isRootWidgetAttached and _rootWidgetAttached?

I see, the name is confusing. I cannot use isRootWidgetAttached because it schedules the first frame during attaching the root widget, the frames should be enabled before that.

I actually have some question about #40105. What is the issue behind this pr?

isRootWidgetAttached is just a getter. How does it schedule a frame?

I actually have some question about #40105. What is the issue behind this pr?

The issue is described in the PR's description. What's your question?

After the change in #39535 we were only scheduling the first frame on accident as a side-effect of the warm-up frame (SchedulerBinding. scheduleWarmUpFrame awaits SchedulerBinding.endOfFrame, which implicitly schedules a frame)

I know it is a fact, but what problem will it cause if we keep this behavior?

Everything will continue to work as before because the warmup frame schedules the first frame as a side effect. However, if we ever were to remove the warmup frame, things would start to break. Therefore, we decided to make scheduling the first frame more explicit.

goderbauer · 2020-03-20T20:36:21Z

packages/flutter/test/widgets/binding_cannot_schedule_frame_test.dart

+
+void main() {
+  test('The frames will only be enabled after runApp attaches the root widget', () async {
+    final WidgetsFlutterBinding binding = WidgetsFlutterBinding.ensureInitialized() as WidgetsFlutterBinding;


Why does this need to be casted to a WidgetsFlutterBinding? ensureInitialized returns a WidgetsBinding, which should give you access to everything you may want to call.

I copied from the other test. will update

goderbauer · 2020-03-20T20:36:50Z

packages/flutter/test/widgets/binding_cannot_schedule_frame_test.dart

+  test('The frames will only be enabled after runApp attaches the root widget', () async {
+    final WidgetsFlutterBinding binding = WidgetsFlutterBinding.ensureInitialized() as WidgetsFlutterBinding;
+    expect(SchedulerBinding.instance.framesEnabled, isFalse);
+    // Framework starts with detached statue. Sends resumed signal to enable frame.


statue? Do you mean state?

packages/flutter/test/widgets/binding_cannot_schedule_frame_test.dart

chunhtai · 2020-03-20T21:59:47Z

ready for review @goderbauer

goderbauer

LGTM

chunhtai added 3 commits March 19, 2020 16:09

Fix ios black screen during splash screen if widget binding initializ…

3959fb5

…ed before runapp

update

56335d7

update

a0f28fe

fluttergithubbot added the framework flutter/packages/flutter repository. See also f: labels. label Mar 19, 2020

googlebot added the cla: yes label Mar 19, 2020

append _test to test file

3482240

chunhtai added the work in progress; do not review label Mar 19, 2020

chunhtai changed the title ~~Fix ios black screen during splash screen if widget binding initializ…~~ [WIP] Fix ios black screen during splash screen if widget binding initializ… Mar 19, 2020

chunhtai added 2 commits March 19, 2020 16:43

fix test

1db6cff

fixes test

003a2e8

Hixie reviewed Mar 20, 2020

View reviewed changes

update

fc74d32

chunhtai removed the work in progress; do not review label Mar 20, 2020

chunhtai changed the title ~~[WIP] Fix ios black screen during splash screen if widget binding initializ…~~ Fix ios black screen during splash screen if widget binding initializ… Mar 20, 2020

goderbauer reviewed Mar 20, 2020

View reviewed changes

address comments

d077af5

goderbauer approved these changes Mar 20, 2020

View reviewed changes

iapicca mentioned this pull request Mar 22, 2020

Black screen flashes between the LaunchImage and app's first screen on iOS #20827

Closed

chunhtai added the waiting for tree to go green label Mar 23, 2020

fluttergithubbot merged commit 9f4e5ad into flutter:master Mar 23, 2020

Piinks mentioned this pull request Apr 3, 2020

flutter 1.12.13+hotfix5 Black screen appears when loading #47582

Closed

iapicca mentioned this pull request Apr 6, 2020

Black screen before opening the first screen (Android) #47024

Closed

github-actions bot locked as resolved and limited conversation to collaborators Aug 1, 2021

Conversation

chunhtai commented Mar 19, 2020

Description

Related Issues

Tests

Checklist

Breaking Change

Uh oh!

fluttergithubbot commented Mar 19, 2020

Uh oh!

skia-gold commented Mar 19, 2020

Uh oh!

chunhtai commented Mar 19, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chunhtai commented Mar 20, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

chunhtai commented Mar 20, 2020

Uh oh!

goderbauer left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants