Moved the default BinaryMessenger instance to ServicesBinding #38464

adazh · 2019-08-13T18:28:54Z

Description

Moved the [default BinaryMessenger] instance (

flutter/packages/flutter/lib/src/services/binary_messenger.dart

Line 154 in 0cf4033

const BinaryMessenger defaultBinaryMessenger = _DefaultBinaryMessenger._();

) to ServicesBinding.

This PR reverts the revert of the original PR.

Related Issues

#37409

Tests

I added the following tests:

Unit/Widget tests updated.
Manually tested on the example platform_channel app.
Also ran the devicelab tests locally.

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/d/msg/flutter-announce/sHAL2fBtJ1Y/mGjrKH3dEwAJ
No, this is not a breaking change.

adazh · 2019-08-13T21:50:44Z

Cc @amirh @mklim

packages/flutter/lib/src/services/binary_messenger.dart

adazh · 2019-08-14T17:39:25Z

Hmm... I've seen a lot of tests complaining about " | Because flutter_gallery depends on connectivity >=0.4.2+1 which requires Flutter SDK version >=1.2.0 <2.0.0, version solving failed." Any idea what the error is about?

tvolkert · 2019-08-14T18:10:28Z

The errors you're seeing are unrelated to this PR - I'd rebase against ToT and push the rebased commit to kick off fresh tests.

packages/flutter/lib/src/services/asset_bundle.dart

packages/flutter/lib/src/services/binary_messenger.dart

tvolkert · 2019-08-14T19:48:54Z

packages/flutter/lib/src/services/platform_channel.dart

@@ -130,10 +131,16 @@ class MethodChannel {
  /// The message codec used by this channel, not null.
  final MethodCodec codec;

+<<<<<<< HEAD


Bad merge. Looks like a lot of stuff got lost too.

Ya... still familiarizing myself with git... Trying to recover the commits..

Phew... got everything back!

tvolkert · 2019-08-14T22:10:15Z

packages/flutter/lib/src/services/binary_messenger.dart

@@ -6,6 +6,7 @@ import 'dart:async';
 import 'dart:typed_data';
 import 'dart:ui' as ui;

+import '../../foundation.dart';


import 'package:flutter/foundation.dart';

Updated to absolute path. But what's the rule for imported path? @goderbauer suggested using relative paths within the same package, but seems not always true?

Relative imports within the same layer / directory. services and foundation are different layers and so are treated as different packages.

tvolkert · 2019-08-14T22:37:57Z

packages/flutter/lib/src/services/platform_messages.dart

@@ -31,6 +31,9 @@ import 'platform_channel.dart';
 class BinaryMessages {
  BinaryMessages._();

+  /// The messenger which sends the platform messages, not null.
+  static final BinaryMessenger _binaryMessenger = ServicesBinding.instance.defaultBinaryMessenger;


Is it possible for this to be called before the binding is initialized?

BinaryMessages has been deprecated. Anyway, I replaced the ServicesBinding.instance.defaultBinaryMessenger with defaultBinaryMessenger to be safe.

packages/flutter/test/semantics/semantics_service_test.dart

tvolkert

LGTM. @amirh this is intentionally a hard-breaking change with a simple 1-line forwards-compatible fix. It's likely that a few plugins will need to be updated.

tvolkert · 2019-08-14T23:29:19Z

@adazh can you post an update to the breaking change announcement (https://groups.google.com/d/msg/flutter-announce/sHAL2fBtJ1Y/mGjrKH3dEwAJ), since the original announcement didn't call out the failure case of needing to initialize the binding before calling runApp() in cases of plugin initialization (or the case of tests).

thx!

adazh · 2019-08-14T23:36:38Z

@adazh can you post an update to the breaking change announcement (https://groups.google.com/d/msg/flutter-announce/sHAL2fBtJ1Y/mGjrKH3dEwAJ), since the original announcement didn't call out the failure case of needing to initialize the binding before calling runApp() in cases of plugin initialization (or the case of tests).

thx!

Sure, will do. @tvolkert @amirh Is tomorrow morning a good time to land this change? What time do you suggest?

tvolkert · 2019-08-15T03:06:37Z

@adazh if you could sync with @darrenaustin to find out the places that broke in Google's repo when the first attempt tried to roll, and send forwards-compatible fixes first, that would be very helpful, so we can land this once it's ready to roll and don't back up the roll again.

This reverts commit 821602a.

…ests.

…recated defaultBinaryMessenger for clear assert error

basketball-ico · 2019-08-29T05:54:13Z

@adazh
With this change I need to call WidgetsFlutterBinding.ensureInitialized() in the main if I want to call some code before call runApp like Firestore (For get some preferences), but this show a black screen before runApp is called , I want to keep the native launch screen like before this change (not show other widget), how can I do that?

goderbauer · 2019-08-29T16:10:57Z

@basketball-ico Can you file an issue for this that shows some example code? Please mention me on the issue.

goderbauer · 2019-08-29T16:50:33Z

I can reproduce this, see #39494.

vikramkapoor · 2019-09-03T04:12:52Z

I am seeing this issue on Flutter main and dev channels and adding WidgetFlutterBinding.ensureInitialized() at the start of Main() doesn't make any difference. I spent hours trying to debug it and gave up switched to stable channel to make it go away.

tvolkert · 2019-09-03T04:40:41Z

@vikramkapoor are you talking about #39494?

vikramkapoor · 2019-09-03T07:11:47Z

Thanks for following up. What I meant is that I got the following exception due to the breaking change.

If you're running an application and need to access the binary messenger before `runApp()` has been called (for example, during plugin initialization), then you need to explicitly call the `WidgetsFlutterBinding.ensureInitialized()` first.
If you're running a test, you can call the `TestWidgetsFlutterBinding.ensureInitialized()` as the first line in your test's `main()` method to initialize the binding.
#0      defaultBinaryMessenger.<anonymous closure> 
package:flutter/…/services/binary_messenger.dart:73
#1      defaultBinaryMessenger 
package:flutter/…/services/binary_messenger.dart:86
#2      EventChannel.binaryMessenger 
package:flutter/…/services/platform_channel.dart:484
#3      EventChannel.receiveBroadcastStream.<anonymous closure> 
package:flutter/…/services/platform_channel.dart:504
<asynchronous suspension>
#4      _runGuarded  (dart:async/stream_controller.dart:805:24)
#5      _BroadcastStreamController._subscribe  (dart:async/broadcast_stream_controller.dart:213:7)
#6      _ControllerStream._createSubscription  (dart:async/stream_controller.dart:818:19)
#7      _StreamImpl.listen  (dart:async/stream_impl.dart:472:9)
#8      FlutterIsolate._isolateInitialize 
package:flutter_isolate/flutter_isolate.dart:104
#9      _flutterIsolateEntryPoint 
package:flutter_isolate/flutter_isolate.dart:132
#10     _runMainZoned.<anonymous closure>.<anonymous closure>  (dart:ui/hooks.dart:229:25)
#11     _rootRun  (dart:async/zone.dart:1124:13)
#12     _CustomZone.run  (dart:async/zone.dart:1021:19)
#13     _runZoned  (dart:async/zone.dart:1516:10)
#14     runZoned  (dart:async/zone.dart:1500:12)
#15     _runMainZoned.<anonymous closure>  (dart:ui/hooks.dart:221:5)
#16     _startIsolate.<anonymous closure>  (dart:isolate-patch/isolate_patch.dart:305:19)
#17     _RawReceivePortImpl._handleMessage  (dart:isolate-patch/isolate_patch.dart:172:12)

while running the following code after runApp() had already been called

 Globals.receivePort = ReceivePort();
            print("Spawning isolate 1");
            FlutterIsolate.spawn(isolate1, Globals.receivePort.sendPort).then((
                data) =>
            Globals.isolate = data);
            broadcastReceivePort =  Globals.receivePort.asBroadcastStream();

I added WidgetFlutterBinding.ensureInitialized() at the start of Main() as well as in the Isolate entry function . But the exception didn't go away after I did that on master and dev channels. It went away after switching to Stable channel. This was on an IOS build.

adazh · 2019-09-03T17:02:17Z

From the stack trace, the error was thrown when binaryMessenger.setMessageHandler was called within EventChannel's receiveBroadcastStream method.

@vikramkapoor Could you check where EventChannel(s) are initialized/receiveBroadcastStream is called in your code? This error should probably go away if WidgetFlutterBinding.ensureInitialized() is called beforehand.

vikramkapoor · 2019-09-03T21:57:06Z

@adazh
RunApp is called as first thing in my main() before receiveBroadcastStream. I had tried putting WidgetFlutterBinding.ensureInitialized() as the first line in main() before RunApp and it was still happening. This only happens on iOS and not on Android with the same code. I will investigate more a little later today and get back to this thread.

vikramkapoor · 2019-09-06T05:09:43Z

@adazh I tried calling WidgetFlutterBinding.ensureInitialized() as first thing in Main but I still get this error on iOS. receiveBroadcastStream is called in my code much after runApp has been called. It works fine on Android on the master channel but I had to go to stable channel on iOS to avoid this exception. Is there anything else I should investigate? Vikram On Tuesday, September 3, 2019, 10:04:40 AM PDT, adazh <notifications@github.com> wrote: From the stack trace, the error was thrown when binaryMessenger.setMessageHandler was called within EventChannel's receiveBroadcastStream method. @vikramkapoor Could you see where EventChannel(s) are initialized/receiveBroadcastStream is called in your code? This error should probably go away if WidgetFlutterBinding.ensureInitialized() is called beforehand. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

adazh · 2019-09-06T17:40:00Z

@adazh I tried calling WidgetFlutterBinding.ensureInitialized() as first thing in Main but I still get this error on iOS. receiveBroadcastStream is called in my code much after runApp has been called. It works fine on Android on the master channel but I had to go to stable channel on iOS to avoid this exception. Is there anything else I should investigate? Vikram On Tuesday, September 3, 2019, 10:04:40 AM PDT, adazh notifications@github.com wrote: From the stack trace, the error was thrown when binaryMessenger.setMessageHandler was called within EventChannel's receiveBroadcastStream method. @vikramkapoor Could you see where EventChannel(s) are initialized/receiveBroadcastStream is called in your code? This error should probably go away if WidgetFlutterBinding.ensureInitialized() is called beforehand. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

I'm not aware of possible differences in bindings between Android and iOS. + @dnfield to see if he has any insight.

dnfield · 2019-09-06T20:12:56Z

@vikramkapoor could you please file a new issue with a minimal reproduction for this? Feel free to tag me in it.

vikramkapoor · 2019-09-07T15:43:43Z

@dnfield Sure. I am working on minimal repro to file an issue

googlebot added the cla: yes label Aug 13, 2019

adazh requested review from Hixie, tvolkert and goderbauer August 13, 2019 21:49

tvolkert reviewed Aug 13, 2019

View reviewed changes

packages/flutter/lib/src/services/binary_messenger.dart Outdated Show resolved Hide resolved

adazh requested a review from tvolkert August 14, 2019 17:40

tvolkert reviewed Aug 14, 2019

View reviewed changes

packages/flutter/lib/src/services/asset_bundle.dart Outdated Show resolved Hide resolved

packages/flutter/lib/src/services/binary_messenger.dart Outdated Show resolved Hide resolved

adazh force-pushed the binary_messenger_revert_2 branch from f7f4a01 to 09ae26c Compare August 14, 2019 19:12

tvolkert reviewed Aug 14, 2019

View reviewed changes

adazh requested a review from tvolkert August 14, 2019 22:28

tvolkert reviewed Aug 14, 2019

View reviewed changes

packages/flutter/test/semantics/semantics_service_test.dart Outdated Show resolved Hide resolved

tvolkert approved these changes Aug 14, 2019

View reviewed changes

adazh added 4 commits August 21, 2019 13:46

Moved the default BinaryMessenger instance to ServicesBinding

cf3fd54

This reverts commit 821602a.

Added assertion in defaultBinaryMessenger. Also fixed the devicelab t…

5624f24

…ests.

Replaced ServicesBinding.instance.defaultBinaryMessenger with the dep…

b76a504

…recated defaultBinaryMessenger for clear assert error

Replaced ServicesBinding.instance.defaultBinaryMessenger with the dep…

3012eb1

…recated defaultBinaryMessenger for clear assert error

adazh added 2 commits August 21, 2019 13:52

Minor fixes

8365d3e

Moved TestWidgetsFlutterBinding.ensureInitialized() to main

0f22f8d

adazh force-pushed the binary_messenger_revert_2 branch from e25970f to 0f22f8d Compare August 21, 2019 20:54

Fixed text_input_test.dart

e63c2b7

adazh merged commit 93c32c4 into flutter:master Aug 21, 2019

collinjackson mentioned this pull request Aug 22, 2019

flutter create --template plugin has failing tests #39077

Closed

mehmetf mentioned this pull request Aug 22, 2019

[all] All plugin dart tests should call TestWidgetsFlutterBinding.ensureInitialized() firebase/flutterfire#10

Closed

collinjackson mentioned this pull request Aug 22, 2019

Fix unit tests by calling TestWidgetsFlutterBinding.ensureInitialized() firebase/flutterfire#11

Merged

mehmetf added a commit to flutter/plugins that referenced this pull request Aug 23, 2019

Fix tests after breaking change flutter/flutter#38464 (#2005)

ffe12d4

Piinks mentioned this pull request Aug 24, 2019

Fixing default BinaryMessenger Breaking Change flutter/assets-for-api-docs#103

Merged

goderbauer mentioned this pull request Aug 29, 2019

WidgetsFlutterBinding.ensureInitialized() takes down splash screen too early. #39494

Closed

BondarenkoStas mentioned this pull request Sep 11, 2019

defaultBinaryMessenger was accessed before the binding was initialized ... #40253

Closed

ifananomali pushed a commit to ifananomali/camera that referenced this pull request Sep 12, 2019

Fix tests after breaking change flutter/flutter#38464 (#2005)

955b9cc

rayliverified mentioned this pull request Oct 8, 2019

App crashes after restarting: overflow on channel: flutter/lifecycle #41527

Closed

bratan mentioned this pull request Oct 28, 2019

Fix to ensure initialized before running. Jesway/flutter_translate#12

Merged

sungmin-park pushed a commit to sungmin-park/flutter-plugins that referenced this pull request Dec 17, 2019

Fix tests after breaking change flutter/flutter#38464 (flutter#2005)

fdac11c

Piinks mentioned this pull request Jan 11, 2021

Remove deprecated BinaryMessages #73750

Merged

9 tasks

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Moved the default BinaryMessenger instance to ServicesBinding #38464

Moved the default BinaryMessenger instance to ServicesBinding #38464

adazh commented Aug 13, 2019 •

edited

adazh commented Aug 13, 2019

adazh commented Aug 14, 2019 •

edited

tvolkert commented Aug 14, 2019

tvolkert Aug 14, 2019

adazh Aug 14, 2019

adazh Aug 14, 2019

tvolkert Aug 14, 2019

adazh Aug 14, 2019

tvolkert Aug 14, 2019

tvolkert Aug 14, 2019

adazh Aug 14, 2019

tvolkert left a comment

tvolkert commented Aug 14, 2019

adazh commented Aug 14, 2019

tvolkert commented Aug 15, 2019

basketball-ico commented Aug 29, 2019

goderbauer commented Aug 29, 2019

goderbauer commented Aug 29, 2019

vikramkapoor commented Sep 3, 2019

tvolkert commented Sep 3, 2019

vikramkapoor commented Sep 3, 2019 •

edited

adazh commented Sep 3, 2019 •

edited

vikramkapoor commented Sep 3, 2019

vikramkapoor commented Sep 6, 2019 via email

adazh commented Sep 6, 2019

dnfield commented Sep 6, 2019

vikramkapoor commented Sep 7, 2019

Moved the default BinaryMessenger instance to ServicesBinding #38464

Moved the default BinaryMessenger instance to ServicesBinding #38464

Conversation

adazh commented Aug 13, 2019 • edited

Description

Related Issues

Tests

Checklist

Breaking Change

adazh commented Aug 13, 2019

adazh commented Aug 14, 2019 • edited

tvolkert commented Aug 14, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tvolkert left a comment

Choose a reason for hiding this comment

tvolkert commented Aug 14, 2019

adazh commented Aug 14, 2019

tvolkert commented Aug 15, 2019

basketball-ico commented Aug 29, 2019

goderbauer commented Aug 29, 2019

goderbauer commented Aug 29, 2019

vikramkapoor commented Sep 3, 2019

tvolkert commented Sep 3, 2019

vikramkapoor commented Sep 3, 2019 • edited

adazh commented Sep 3, 2019 • edited

vikramkapoor commented Sep 3, 2019

vikramkapoor commented Sep 6, 2019 via email

adazh commented Sep 6, 2019

dnfield commented Sep 6, 2019

vikramkapoor commented Sep 7, 2019

adazh commented Aug 13, 2019 •

edited

adazh commented Aug 14, 2019 •

edited

vikramkapoor commented Sep 3, 2019 •

edited

adazh commented Sep 3, 2019 •

edited