Do not render any frames when just initializing Bindings by goderbauer · Pull Request #39535 · flutter/flutter

goderbauer · 2019-08-30T00:47:44Z

Description

Prior to this change, initializing the WidgetsFlutterBinding (e.g. via WidgetsFlutterBinding.ensureInitialized) would register the frame callbacks onBeginFrame and onDrawFrame with the engine (this allowed to request the engine to render a frame whenever it wanted) and we would actively ask the engine to request the next frame via Window.scheduleFrame. In certain situations this caused the engine to request a frame (by calling onBeginFrame) when the framework wasn't ready to produce a meaningful frame (e. i. before the root widget was attached via a call to runApp). This would cause the framework to produce an empty frame, which in turn caused the engine to take down any splash screen and show a black empty screen.

With this change, initializing the bindings does not register the frame callbacks with the engine so the engine has no chance of requesting a frame from the framework. Instead, the frame callbacks are lazily registered when the first frame is scheduled by the framework via scheduleFrame. Additionally, initializing the bindings will no longer schedule a frame. The first frame gets scheduled when the root widget is attached and the framework is therefore ready to produce the first meaningful frame (or you can schedule a frame manually by calling scheduleFrame whenever you want).

Related Issues

Fixes #39494.

Tests

I added the following tests:

tests to ensure that the binding neither register any callbacks nor schedules a frame when instantiated.

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 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

Does your PR require Flutter developers to manually update their apps to accommodate your change?

Yes, this is a breaking change (Please read Handling breaking changes): https://groups.google.com/forum/#!topic/flutter-announce/sB8Hxs8lrKI
No, this is not a breaking change.

dnfield · 2019-08-30T17:50:46Z

Could we make this less breaking by instead making a different ensureInitialized method that people who just want method channels could call?

My thinking is that here we're breaking some pretty old and established API, whereas the ensureInitialized logic was more recently changed for people who are more likely to be able to respond to breaks. I think it would involve similar changes to what you've done here, but perhaps on ServicesBinding instead of WidgetsBinding.

dnfield · 2019-08-30T17:51:28Z

IOW - it seems strange to me to ensure that the widgets binding is initialized but to have to take an extra step to actually render widgets. Whereas I would expect that I could initialize the service bindings without necessarily rendering.

goderbauer · 2019-08-30T19:55:47Z

Adding a new method seems more breaking to me as that will require people to change the API they are calling. This change as-is does not require any changes from most people who are using Flutter in the intended way.

After initializing the bindings you always had to take an extra step (call runApp to attach your root widget) to actually render widgets. This PR is not changing this. This PR is just making sure that no frame gets rendered until this step has been taken.

Hixie · 2019-08-30T19:57:58Z

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

        debugPrintStack(label: 'scheduleFrame() called. Current phase is $schedulerPhase.');
      return true;
    }());
+    if (window.onBeginFrame == null) {


the omission of onDrawFrame on this line hurts

Hixie · 2019-08-30T19:58:18Z

packages/flutter/test/foundation/service_extensions_test.dart

  bool frameScheduled = false;
  @override
  void scheduleFrame() {
+    if (ui.window.onBeginFrame == null) {


and the code duplication here hurts...

Hixie · 2019-08-30T19:59:03Z

packages/flutter/test/rendering/independent_layout_test.dart

    renderView.child = offscreen.root;
-    renderView.scheduleInitialFrame();
+    renderView.prepareInitialFrame();
+    renderView.owner.requestVisualUpdate();


why not just pipelineOwner.requestVisualUpdate?

Hixie · 2019-08-30T19:59:08Z

packages/flutter/test/rendering/independent_layout_test.dart

    renderView.child = offscreen.root;
-    renderView.scheduleInitialFrame();
+    renderView.prepareInitialFrame();
+    renderView.owner.requestVisualUpdate();


Hixie · 2019-08-30T19:59:25Z

packages/flutter/test/widgets/independent_widget_layout_test.dart

    renderView.attach(pipelineOwner);
-    renderView.scheduleInitialFrame();
+    renderView.prepareInitialFrame();
+    renderView.owner.requestVisualUpdate();


Hixie · 2019-08-30T20:03:03Z

packages/flutter_test/lib/src/binding.dart

-    window.onBeginFrame = null;
-    window.onDrawFrame = null;
+    window.onBeginFrame = (Duration _) {};
+    window.onDrawFrame = () {};


maybe we should track if we've registered our callbacks separately from whether they're null, to avoid this kind of breakage...
Also, we could override registerFrameCallbacks here to enable the test harness and leave window entirely alone, thus avoiding this issue.

goderbauer · 2019-08-30T21:01:22Z

Comments addressed. PTAL @Hixie

chunhtai

this looks good, and I don't think it will interfere with my pr #39079
It will probably be good to run through internal test suite to make sure nothing break

chunhtai · 2019-08-30T22:55:05Z

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

        debugPrintStack(label: 'scheduleFrame() called. Current phase is $schedulerPhase.');
      return true;
    }());
+    ensureFrameCallbacksRegistered();


It feels a little bit hacky to me that we prevent the engine to achieve frame by not attaching listener, but i can't think of a better way, either.

Hixie · 2019-09-04T18:03:58Z

packages/flutter_test/lib/src/binding.dart


+  @override
+  void ensureFrameCallbacksRegistered() {
+    // Leave Window alone, do nothing.


maybe assert that window.onBeginFrame and window.onDrawFrame are null...

Hixie · 2019-09-04T18:04:04Z

shinyford · 2020-01-07T13:47:07Z

Hi - I'm currently on Flutter 1.12.13+hotfix.5 stable, and I'm still seeing a frame rendered when WidgetsFlutterBinding.ensureInitialized is called early.

Is this expected? Any info gratefully received. Cheers!

dnfield · 2020-01-07T16:01:28Z

@shinyford see #39494

shinyford · 2020-01-07T16:33:40Z

@shinyford see #39494

Thanks - yes, I found that. The suggested fix (#39494 (comment)) works for me; but I'll be more comfortable when it's properly part of the Flutter codebase.

hb0nes · 2020-01-12T23:08:44Z

I have a massive black screen during initialization of the application. Any update on this?

goderbauer · 2020-01-13T17:21:38Z

@hb0nes I recommend upvoting the issue #39494 and adding details if you have them (e.g. what device).

fluttergithubbot added a: tests "flutter test", flutter_test, or one of our tests framework flutter/packages/flutter repository. See also f: labels. labels Aug 30, 2019

googlebot added the cla: yes label Aug 30, 2019

goderbauer added 3 commits August 30, 2019 09:42

Do not render any frames when just initializing Bindings

b5c2bcf

remove deprecated member

d7262f5

add test

92ebd77

goderbauer force-pushed the fixsplash branch from f95c485 to 92ebd77 Compare August 30, 2019 17:03

goderbauer marked this pull request as ready for review August 30, 2019 17:13

goderbauer requested a review from Hixie August 30, 2019 17:42

Piinks added the c: API break Backwards-incompatible API changes label Aug 30, 2019

Hixie reviewed Aug 30, 2019

View reviewed changes

review

5c60d57

chunhtai reviewed Aug 30, 2019

View reviewed changes

Hixie reviewed Sep 4, 2019

View reviewed changes

review

e7b84a4

goderbauer added the waiting for tree to go green label Sep 5, 2019

goderbauer merged commit e232560 into flutter:master Sep 6, 2019

goderbauer deleted the fixsplash branch September 6, 2019 00:08

liammendes mentioned this pull request Sep 6, 2019

Calling WidgetsFlutterBinding.ensureInitialized() in main() before runApp() causes launch image to disappear and a black screen to be displayed momentarily #32736

Closed

goderbauer mentioned this pull request Sep 9, 2019

Ensure frame is scheduled when root widget is attached #40105

Merged

9 tasks

chunhtai mentioned this pull request Mar 20, 2020

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

Merged

13 tasks

Piinks mentioned this pull request Jan 11, 2021

Remove deprecated RenderView.scheduleInitialFrame #73748

Merged

9 tasks

dnfield mentioned this pull request May 19, 2021

examples/layout/rendering/custom_coordinate_system is broken #82954

Closed

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

Conversation

goderbauer commented Aug 30, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issues

Tests

Checklist

Breaking Change

Uh oh!

dnfield commented Aug 30, 2019

Uh oh!

dnfield commented Aug 30, 2019

Uh oh!

goderbauer commented Aug 30, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

goderbauer commented Aug 30, 2019

Uh oh!

chunhtai left a comment

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!

Hixie commented Sep 4, 2019

Uh oh!

shinyford commented Jan 7, 2020

Uh oh!

dnfield commented Jan 7, 2020

Uh oh!

shinyford commented Jan 7, 2020

Uh oh!

hb0nes commented Jan 12, 2020

Uh oh!

goderbauer commented Jan 13, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

goderbauer commented Aug 30, 2019 •

edited

Loading

goderbauer commented Aug 30, 2019 •

edited

Loading