Moved the default BinaryMessenger instance to ServicesBinding#38464
Moved the default BinaryMessenger instance to ServicesBinding#38464adazh merged 8 commits intoflutter:masterfrom
Conversation
|
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? |
|
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. |
f7f4a01 to
09ae26c
Compare
There was a problem hiding this comment.
Bad merge. Looks like a lot of stuff got lost too.
There was a problem hiding this comment.
Ya... still familiarizing myself with git... Trying to recover the commits..
There was a problem hiding this comment.
Phew... got everything back!
There was a problem hiding this comment.
import 'package:flutter/foundation.dart';There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Relative imports within the same layer / directory. services and foundation are different layers and so are treated as different packages.
There was a problem hiding this comment.
Is it possible for this to be called before the binding is initialized?
There was a problem hiding this comment.
BinaryMessages has been deprecated. Anyway, I replaced the ServicesBinding.instance.defaultBinaryMessenger with defaultBinaryMessenger to be safe.
|
@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 thx! |
Sure, will do. @tvolkert @amirh Is tomorrow morning a good time to land this change? What time do you suggest? |
|
@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.
…recated defaultBinaryMessenger for clear assert error
…recated defaultBinaryMessenger for clear assert error
e25970f to
0f22f8d
Compare
|
@adazh |
|
@basketball-ico Can you file an issue for this that shows some example code? Please mention me on the issue. |
|
I can reproduce this, see #39494. |
|
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. |
|
@vikramkapoor are you talking about #39494? |
|
Thanks for following up. What I meant is that I got the following exception due to the breaking change. while running the following code after runApp() had already been called 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. |
|
From the stack trace, the error was thrown when @vikramkapoor Could you check where |
|
@adazh |
|
@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 <[email protected]> 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. |
|
@vikramkapoor could you please file a new issue with a minimal reproduction for this? Feel free to tag me in it. |
|
@dnfield Sure. I am working on minimal repro to file an issue |
Description
Moved the [default BinaryMessenger] instance (
flutter/packages/flutter/lib/src/services/binary_messenger.dart
Line 154 in 0cf4033
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.///).flutter analyze --flutter-repo) does not report any problems on my PR.Breaking Change
Does your PR require Flutter developers to manually update their apps to accommodate your change?