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

[web_ui] add missing dispose handler for MethodCalls to flutter/platform_view #12226

Merged
merged 2 commits into from
Sep 26, 2019

Conversation

tauu
Copy link
Contributor

@tauu tauu commented Sep 11, 2019

This PR adds the missing handler for the 'dispose' MethodCall to the PlatformViewChannel ( flutter/platform_view ). This MethodCall is already invoked by _HtmlElementViewController in https://github.com/flutter/flutter/blob/2c94f2b4d9247965a32a873878cabfe6d2fb7a93/packages/flutter/lib/src/widgets/platform_view.dart#L412 and yields a MissingPluginException Error without the corresponding implementation.

Fixes #12226

@yjbanov
Copy link
Contributor

yjbanov commented Sep 12, 2019

/cc @hterkelsen

final int id = methodCall.arguments;
const MethodCodec codec = StandardMethodCodec();

// remove the root element of the view from the DOM
Copy link
Contributor

Choose a reason for hiding this comment

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

Please format this comment as a full sentence with punctuation, i.e., // Remove ... DOM.

@harryterkelsen
Copy link
Contributor

Ping

@googlebot
Copy link

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

1 similar comment
@googlebot
Copy link

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

@googlebot
Copy link

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

1 similar comment
@googlebot
Copy link

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

Copy link
Contributor

@harryterkelsen harryterkelsen left a comment

Choose a reason for hiding this comment

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

LGTM. Looks like the tests are failling right now. Please wait to land this until the tests are passing again, as it seems unrelated to your change.

@tauu
Copy link
Contributor Author

tauu commented Sep 19, 2019

OK, thx! :-) Sorry for the delay. Migrating from the flutter_web fork, as well as integrating a custom flutter engine build into the build system of my team took longer than expected. (Still can't figure out how to build only the web_sdk part of the engine. :-/ )

@cbracken
Copy link
Member

cbracken commented Sep 23, 2019

Looks like compiler error on the latest run:

[ +383 ms] Dart2Js finished with:
           packages/flutter/src/semantics/semantics.dart:2060:7:
           Error: No named parameter with the name 'maxValueLength'.
                 maxValueLength: data.maxValueLength ?? -1,
                 ^^^^^^^^^^^^^^
           Error: Compilation failed.

If this is unrelated to your PR, can you try rebasing against head? It's possible this was an error we fixed in the meantime.

Thanks for your PR. We'll keep an eye out for when it's passing.

@cbracken
Copy link
Member

Looks like compiler error on the latest run:

[ +383 ms] Dart2Js finished with:
           packages/flutter/src/semantics/semantics.dart:2060:7:
           Error: No named parameter with the name 'maxValueLength'.
                 maxValueLength: data.maxValueLength ?? -1,
                 ^^^^^^^^^^^^^^
           Error: Compilation failed.

Thanks for your PR. We'll keep an eye out for when it's passing.

@tauu
Copy link
Contributor Author

tauu commented Sep 24, 2019

The compiler error was unrelated to the PR and rebasing fixed the failing tests.

@harryterkelsen harryterkelsen merged commit 84c167d into flutter:master Sep 26, 2019
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 27, 2019
engine-flutter-autoroll added a commit to flutter/flutter that referenced this pull request Sep 27, 2019
git@github.com:flutter/engine.git/compare/ce6ab8ce25fa...a206557

git log ce6ab8c..a206557 --no-merges --oneline
2019-09-27 skia-flutter-autoroll@skia.org Roll src/third_party/skia ec85f407bfee..b83cc76a5e3a (15 commits) (flutter/engine#12472)
2019-09-27 30870216+gaaclarke@users.noreply.github.com Added a default entrypoint variable to match android syntax. (flutter/engine#12370)
2019-09-26 tauu@h2overclock.de [web_ui] add missing dispose handler for MethodCalls to flutter/platform_view (flutter/engine#12226)
2019-09-26 tauu@h2overclock.de [web_ui] PersistedPlatformView attribute update handling to enable resizing (flutter/engine#12227)
2019-09-26 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/mac-amd64 from z3PaM... to HoRV8... (flutter/engine#12471)


If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC aaclarke@google.com on the revert to ensure that a human
is aware of the problem.

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 30, 2019
Inconnu08 pushed a commit to Inconnu08/flutter that referenced this pull request Sep 30, 2019
git@github.com:flutter/engine.git/compare/ce6ab8ce25fa...a206557

git log ce6ab8c..a206557 --no-merges --oneline
2019-09-27 skia-flutter-autoroll@skia.org Roll src/third_party/skia ec85f407bfee..b83cc76a5e3a (15 commits) (flutter/engine#12472)
2019-09-27 30870216+gaaclarke@users.noreply.github.com Added a default entrypoint variable to match android syntax. (flutter/engine#12370)
2019-09-26 tauu@h2overclock.de [web_ui] add missing dispose handler for MethodCalls to flutter/platform_view (flutter/engine#12226)
2019-09-26 tauu@h2overclock.de [web_ui] PersistedPlatformView attribute update handling to enable resizing (flutter/engine#12227)
2019-09-26 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/mac-amd64 from z3PaM... to HoRV8... (flutter/engine#12471)


If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC aaclarke@google.com on the revert to ensure that a human
is aware of the problem.

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md
engine-flutter-autoroll added a commit to flutter/flutter that referenced this pull request Sep 30, 2019
git@github.com:flutter/engine.git/compare/5b952f286fc0...f0c7740

git log 5b952f2..f0c7740 --no-merges --oneline
2019-09-30 skia-flutter-autoroll@skia.org Roll src/third_party/skia e242d0ea937c..89e1f600ec78 (3 commits) (flutter/engine#12696)
2019-09-30 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/mac-amd64 from ncc-K... to KH7Tv... (flutter/engine#12694)
2019-09-30 50856934+nturgut@users.noreply.github.com Refactoring text_editing.dart (flutter/engine#12479)
2019-09-30 skia-flutter-autoroll@skia.org Roll src/third_party/skia abc851eb3dcc..e242d0ea937c (2 commits) (flutter/engine#12693)
2019-09-30 skia-flutter-autoroll@skia.org Roll src/third_party/skia 0e7500021402..abc851eb3dcc (3 commits) (flutter/engine#12692)
2019-09-30 skia-flutter-autoroll@skia.org Roll src/third_party/skia 7aeabcfa6a73..0e7500021402 (1 commits) (flutter/engine#12691)
2019-09-30 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/linux-amd64 from eUBos... to sWs8N... (flutter/engine#12690)
2019-09-30 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/mac-amd64 from VACNQ... to ncc-K... (flutter/engine#12689)
2019-09-29 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/linux-amd64 from OEG20... to eUBos... (flutter/engine#12688)
2019-09-29 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/mac-amd64 from 6fMRC... to VACNQ... (flutter/engine#12687)
2019-09-29 skia-flutter-autoroll@skia.org Roll src/third_party/skia be971636cf9b..7aeabcfa6a73 (3 commits) (flutter/engine#12686)
2019-09-29 skia-flutter-autoroll@skia.org Roll src/third_party/skia 2e856451654d..be971636cf9b (1 commits) (flutter/engine#12685)
2019-09-29 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/linux-amd64 from lHiKM... to OEG20... (flutter/engine#12683)
2019-09-29 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/mac-amd64 from z3oH6... to 6fMRC... (flutter/engine#12682)
2019-09-28 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/linux-amd64 from nVAcH... to lHiKM... (flutter/engine#12680)
2019-09-28 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/mac-amd64 from Kiq-H... to z3oH6... (flutter/engine#12679)
2019-09-28 skia-flutter-autoroll@skia.org Roll src/third_party/skia bd84330ba277..2e856451654d (1 commits) (flutter/engine#12661)
2019-09-28 skia-flutter-autoroll@skia.org Roll src/third_party/skia 8ab1530cd3cd..bd84330ba277 (1 commits) (flutter/engine#12639)
2019-09-28 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/linux-amd64 from qx53U... to nVAcH... (flutter/engine#12620)
2019-09-28 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/mac-amd64 from oob8T... to Kiq-H... (flutter/engine#12616)
2019-09-28 skia-flutter-autoroll@skia.org Roll src/third_party/skia 28ad6f869822..8ab1530cd3cd (5 commits) (flutter/engine#12614)
2019-09-27 liyuqian@google.com Revert "[fuchsia] Wire up OpacityLayer to Scenic (#11322)" (flutter/engine#12610)
2019-09-27 30870216+gaaclarke@users.noreply.github.com Started asserting the FlutterEngine is running before communicating over channels. (flutter/engine#12469)
2019-09-27 30870216+gaaclarke@users.noreply.github.com Split out the logic to handle status bar touches into its own function (flutter/engine#12587)
2019-09-27 bkonyi@google.com Roll src/third_party/dart c3c31b74fd..132bee48d0 (16 commits)
2019-09-27 iska.kaushik@gmail.com [flutter_runner] Refactor thread_application pair to ActiveApplication (flutter/engine#12573)
2019-09-27 gw280@google.com Reword confusing messaging surrounding unhandled exception in flutter_runner on Fuchsia (flutter/engine#12428)
2019-09-27 skia-flutter-autoroll@skia.org Roll src/third_party/skia b23d66e10a98..28ad6f869822 (10 commits) (flutter/engine#12580)
2019-09-27 iska.kaushik@gmail.com Revert "Update linux toolchain for fuchsia" (flutter/engine#12578)
2019-09-27 jonahwilliams@google.com Add support for JIT release mode (flutter/engine#12446)
2019-09-27 iska.kaushik@gmail.com Update linux toolchain for fuchsia (flutter/engine#12572)
2019-09-27 iska.kaushik@gmail.com Remove references to topaz (flutter/engine#12565)
2019-09-27 bkonyi@google.com Roll src/third_party/dart 7901c508c8..c3c31b74fd (4 commits)
2019-09-27 skia-flutter-autoroll@skia.org Roll src/third_party/skia 3787f51c65c3..b23d66e10a98 (1 commits) (flutter/engine#12559)
2019-09-27 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/mac-amd64 from HoRV8... to oob8T... (flutter/engine#12538)
2019-09-27 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/linux-amd64 from YDv3O... to qx53U... (flutter/engine#12520)
2019-09-27 skia-flutter-autoroll@skia.org Roll src/third_party/skia 296743a86281..3787f51c65c3 (4 commits) (flutter/engine#12512)
2019-09-27 bkonyi@google.com Roll src/third_party/dart 403c4af720..7901c508c8 (7 commits)
2019-09-27 skia-flutter-autoroll@skia.org Roll src/third_party/skia b83cc76a5e3a..296743a86281 (1 commits) (flutter/engine#12491)
2019-09-27 bkonyi@google.com Roll src/third_party/dart 327bc451f8..403c4af720 (9 commits)
2019-09-27 skia-flutter-autoroll@skia.org Roll fuchsia/clang/mac-amd64 from HfPKR... to zpVtV... (flutter/engine#12473)
2019-09-27 skia-flutter-autoroll@skia.org Roll src/third_party/skia ec85f407bfee..b83cc76a5e3a (15 commits) (flutter/engine#12472)
2019-09-27 30870216+gaaclarke@users.noreply.github.com Added a default entrypoint variable to match android syntax. (flutter/engine#12370)
2019-09-26 tauu@h2overclock.de [web_ui] add missing dispose handler for MethodCalls to flutter/platform_view (flutter/engine#12226)
2019-09-26 tauu@h2overclock.de [web_ui] PersistedPlatformView attribute update handling to enable resizing (flutter/engine#12227)
...
Inconnu08 pushed a commit to Inconnu08/flutter that referenced this pull request Nov 26, 2019
git@github.com:flutter/engine.git/compare/5b952f286fc0...f0c7740

git log 5b952f2..f0c7740 --no-merges --oneline
2019-09-30 skia-flutter-autoroll@skia.org Roll src/third_party/skia e242d0ea937c..89e1f600ec78 (3 commits) (flutter/engine#12696)
2019-09-30 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/mac-amd64 from ncc-K... to KH7Tv... (flutter/engine#12694)
2019-09-30 50856934+nturgut@users.noreply.github.com Refactoring text_editing.dart (flutter/engine#12479)
2019-09-30 skia-flutter-autoroll@skia.org Roll src/third_party/skia abc851eb3dcc..e242d0ea937c (2 commits) (flutter/engine#12693)
2019-09-30 skia-flutter-autoroll@skia.org Roll src/third_party/skia 0e7500021402..abc851eb3dcc (3 commits) (flutter/engine#12692)
2019-09-30 skia-flutter-autoroll@skia.org Roll src/third_party/skia 7aeabcfa6a73..0e7500021402 (1 commits) (flutter/engine#12691)
2019-09-30 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/linux-amd64 from eUBos... to sWs8N... (flutter/engine#12690)
2019-09-30 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/mac-amd64 from VACNQ... to ncc-K... (flutter/engine#12689)
2019-09-29 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/linux-amd64 from OEG20... to eUBos... (flutter/engine#12688)
2019-09-29 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/mac-amd64 from 6fMRC... to VACNQ... (flutter/engine#12687)
2019-09-29 skia-flutter-autoroll@skia.org Roll src/third_party/skia be971636cf9b..7aeabcfa6a73 (3 commits) (flutter/engine#12686)
2019-09-29 skia-flutter-autoroll@skia.org Roll src/third_party/skia 2e856451654d..be971636cf9b (1 commits) (flutter/engine#12685)
2019-09-29 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/linux-amd64 from lHiKM... to OEG20... (flutter/engine#12683)
2019-09-29 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/mac-amd64 from z3oH6... to 6fMRC... (flutter/engine#12682)
2019-09-28 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/linux-amd64 from nVAcH... to lHiKM... (flutter/engine#12680)
2019-09-28 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/mac-amd64 from Kiq-H... to z3oH6... (flutter/engine#12679)
2019-09-28 skia-flutter-autoroll@skia.org Roll src/third_party/skia bd84330ba277..2e856451654d (1 commits) (flutter/engine#12661)
2019-09-28 skia-flutter-autoroll@skia.org Roll src/third_party/skia 8ab1530cd3cd..bd84330ba277 (1 commits) (flutter/engine#12639)
2019-09-28 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/linux-amd64 from qx53U... to nVAcH... (flutter/engine#12620)
2019-09-28 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/mac-amd64 from oob8T... to Kiq-H... (flutter/engine#12616)
2019-09-28 skia-flutter-autoroll@skia.org Roll src/third_party/skia 28ad6f869822..8ab1530cd3cd (5 commits) (flutter/engine#12614)
2019-09-27 liyuqian@google.com Revert "[fuchsia] Wire up OpacityLayer to Scenic (flutter#11322)" (flutter/engine#12610)
2019-09-27 30870216+gaaclarke@users.noreply.github.com Started asserting the FlutterEngine is running before communicating over channels. (flutter/engine#12469)
2019-09-27 30870216+gaaclarke@users.noreply.github.com Split out the logic to handle status bar touches into its own function (flutter/engine#12587)
2019-09-27 bkonyi@google.com Roll src/third_party/dart c3c31b74fd..132bee48d0 (16 commits)
2019-09-27 iska.kaushik@gmail.com [flutter_runner] Refactor thread_application pair to ActiveApplication (flutter/engine#12573)
2019-09-27 gw280@google.com Reword confusing messaging surrounding unhandled exception in flutter_runner on Fuchsia (flutter/engine#12428)
2019-09-27 skia-flutter-autoroll@skia.org Roll src/third_party/skia b23d66e10a98..28ad6f869822 (10 commits) (flutter/engine#12580)
2019-09-27 iska.kaushik@gmail.com Revert "Update linux toolchain for fuchsia" (flutter/engine#12578)
2019-09-27 jonahwilliams@google.com Add support for JIT release mode (flutter/engine#12446)
2019-09-27 iska.kaushik@gmail.com Update linux toolchain for fuchsia (flutter/engine#12572)
2019-09-27 iska.kaushik@gmail.com Remove references to topaz (flutter/engine#12565)
2019-09-27 bkonyi@google.com Roll src/third_party/dart 7901c508c8..c3c31b74fd (4 commits)
2019-09-27 skia-flutter-autoroll@skia.org Roll src/third_party/skia 3787f51c65c3..b23d66e10a98 (1 commits) (flutter/engine#12559)
2019-09-27 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/mac-amd64 from HoRV8... to oob8T... (flutter/engine#12538)
2019-09-27 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/linux-amd64 from YDv3O... to qx53U... (flutter/engine#12520)
2019-09-27 skia-flutter-autoroll@skia.org Roll src/third_party/skia 296743a86281..3787f51c65c3 (4 commits) (flutter/engine#12512)
2019-09-27 bkonyi@google.com Roll src/third_party/dart 403c4af720..7901c508c8 (7 commits)
2019-09-27 skia-flutter-autoroll@skia.org Roll src/third_party/skia b83cc76a5e3a..296743a86281 (1 commits) (flutter/engine#12491)
2019-09-27 bkonyi@google.com Roll src/third_party/dart 327bc451f8..403c4af720 (9 commits)
2019-09-27 skia-flutter-autoroll@skia.org Roll fuchsia/clang/mac-amd64 from HfPKR... to zpVtV... (flutter/engine#12473)
2019-09-27 skia-flutter-autoroll@skia.org Roll src/third_party/skia ec85f407bfee..b83cc76a5e3a (15 commits) (flutter/engine#12472)
2019-09-27 30870216+gaaclarke@users.noreply.github.com Added a default entrypoint variable to match android syntax. (flutter/engine#12370)
2019-09-26 tauu@h2overclock.de [web_ui] add missing dispose handler for MethodCalls to flutter/platform_view (flutter/engine#12226)
2019-09-26 tauu@h2overclock.de [web_ui] PersistedPlatformView attribute update handling to enable resizing (flutter/engine#12227)
...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
5 participants