Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix platform views channel regression #9185

Merged
merged 2 commits into from
Jun 4, 2019
Merged

Conversation

amirh
Copy link
Contributor

@amirh amirh commented Jun 4, 2019

This regression was introduced in #7847.

The PlatformViewsChannel method call handler was always setting the result to notImplemented even after handling a result, this resulted in a "Reply already submitted" exception being thrown.
Note that the method channel code is swallowing this exception and logging an error, so we didn't crash instead we were logging an error(this is why the integration test didn't fail), sample error that was logged whenever platform views were used on Android:

06-04 12:01:05.997 11978 11978 E DartMessenger: Uncaught exception in binary message listener
06-04 12:01:05.997 11978 11978 E DartMessenger: java.lang.IllegalStateException: Reply already submitted
06-04 12:01:05.997 11978 11978 E DartMessenger: 	at io.flutter.embedding.engine.dart.DartMessenger$Reply.reply(DartMessenger.java:126)
06-04 12:01:05.997 11978 11978 E DartMessenger: 	at io.flutter.plugin.common.MethodChannel$IncomingMethodCallHandler.onMessage(MethodChannel.java:240)
06-04 12:01:05.997 11978 11978 E DartMessenger: 	at io.flutter.embedding.engine.dart.DartMessenger.handleMessageFromDart(DartMessenger.java:90)
06-04 12:01:05.997 11978 11978 E DartMessenger: 	at io.flutter.embedding.engine.FlutterJNI.handlePlatformMessage(FlutterJNI.java:234)
06-04 12:01:05.997 11978 11978 E DartMessenger: 	at android.os.MessageQueue.nativePollOnce(Native Method)
06-04 12:01:05.997 11978 11978 E DartMessenger: 	at android.os.MessageQueue.next(MessageQueue.java:325)
06-04 12:01:05.997 11978 11978 E DartMessenger: 	at android.os.Looper.loop(Looper.java:142)
06-04 12:01:05.997 11978 11978 E DartMessenger: 	at android.app.ActivityThread.main(ActivityThread.java:6591)
06-04 12:01:05.997 11978 11978 E DartMessenger: 	at java.lang.reflect.Method.invoke(Native Method)
06-04 12:01:05.997 11978 11978 E DartMessenger: 	at com.android.internal.os.Zygote$MethodAndArgsCaller.run(Zygote.java:240)
06-04 12:01:05.997 11978 11978 E DartMessenger: 	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:772)
06-04 12:01:06.002 11978 11978 E MethodChannel#flutter/platform_views: Failed to handle method call
06-04 12:01:06.002 11978 11978 E MethodChannel#flutter/platform_views: java.lang.IllegalStateException: Reply already submitted
06-04 12:01:06.002 11978 11978 E MethodChannel#flutter/platform_views: 	at io.flutter.embedding.engine.dart.DartMessenger$Reply.reply(DartMessenger.java:126)
06-04 12:01:06.002 11978 11978 E MethodChannel#flutter/platform_views: 	at io.flutter.plugin.common.MethodChannel$IncomingMethodCallHandler$1.notImplemented(MethodChannel.java:235)
06-04 12:01:06.002 11978 11978 E MethodChannel#flutter/platform_views: 	at io.flutter.embedding.engine.systemchannels.PlatformViewsChannel$1.onMethodCall(PlatformViewsChannel.java:55)
06-04 12:01:06.002 11978 11978 E MethodChannel#flutter/platform_views: 	at io.flutter.plugin.common.MethodChannel$IncomingMethodCallHandler.onMessage(MethodChannel.java:222)
06-04 12:01:06.002 11978 11978 E MethodChannel#flutter/platform_views: 	at io.flutter.embedding.engine.dart.DartMessenger.handleMessageFromDart(DartMessenger.java:90)
06-04 12:01:06.002 11978 11978 E MethodChannel#flutter/platform_views: 	at io.flutter.embedding.engine.FlutterJNI.handlePlatformMessage(FlutterJNI.java:234)
06-04 12:01:06.002 11978 11978 E MethodChannel#flutter/platform_views: 	at android.os.MessageQueue.nativePollOnce(Native Method)
06-04 12:01:06.002 11978 11978 E MethodChannel#flutter/platform_views: 	at android.os.MessageQueue.next(MessageQueue.java:325)
06-04 12:01:06.002 11978 11978 E MethodChannel#flutter/platform_views: 	at android.os.Looper.loop(Looper.java:142)
06-04 12:01:06.002 11978 11978 E MethodChannel#flutter/platform_views: 	at android.app.ActivityThread.main(ActivityThread.java:6591)
06-04 12:01:06.002 11978 11978 E MethodChannel#flutter/platform_views: 	at java.lang.reflect.Method.invoke(Native Method)
06-04 12:01:06.002 11978 11978 E MethodChannel#flutter/platform_views: 	at com.android.internal.os.Zygote$MethodAndArgsCaller.run(Zygote.java:240)
06-04 12:01:06.002 11978 11978 E MethodChannel#flutter/platform_views: 	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:772)

Filed flutter/flutter#33863 to make sure tests fail when such exceptions are thrown.

This PR also cleans up an unused NoSuchPlatformViewException that was introduced in #7847.

flutter/flutter#33866

@amirh
Copy link
Contributor Author

amirh commented Jun 4, 2019

cc @matthew-carroll

Copy link
Contributor

@mklim mklim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@amirh amirh merged commit 86aa014 into flutter:master Jun 4, 2019
@amirh amirh deleted the platform_channel branch June 4, 2019 19:38
@matthew-carroll
Copy link
Contributor

Thanks for making that correction @amirh

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 4, 2019
engine-flutter-autoroll added a commit to flutter/flutter that referenced this pull request Jun 4, 2019
flutter/engine@661c24e...86aa014

git log 661c24e..86aa014 --no-merges --oneline
86aa014 Fix platform views channel regression (flutter/engine#9185)
bd358e7 Roll src/third_party/skia 806267973f8d..a4bb02063672 (10 commits) (flutter/engine#9184)

The AutoRoll server is located here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff (bmparr@google.com), and stop
the roller if necessary.
huqiuser pushed a commit to huqiuser/engine that referenced this pull request Jun 12, 2019
This regression was introduced in flutter#7847.

The PlatformViewsChannel method call handler was always setting the result to `notImplemented` even after handling a result, this resulted in a "Reply already submitted" exception being thrown.
Note that the method channel code is swallowing this exception and logging an error, so we didn't crash instead we were logging an error(this is why the integration test didn't fail).

Filed flutter/flutter#33863 to make sure tests fail when such exceptions are thrown.

This PR also cleans up an unused `NoSuchPlatformViewException` that was introduced in flutter#7847.

flutter/flutter#33866
kiku-jw pushed a commit to kiku-jw/flutter that referenced this pull request Jun 14, 2019
flutter/engine@661c24e...86aa014

git log 661c24e..86aa014 --no-merges --oneline
86aa014 Fix platform views channel regression (flutter/engine#9185)
bd358e7 Roll src/third_party/skia 806267973f8d..a4bb02063672 (10 commits) (flutter/engine#9184)

The AutoRoll server is located here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff (bmparr@google.com), and stop
the roller if necessary.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants