[web_ui] PersistedPlatformView attribute update handling to enable resizing #12227

tauu · 2019-09-11T21:12:26Z

PersistedPlatformView lacks handling of changes to attributes of retained surfaces. Consequently the size of a PlatformView is currently never adjusted after it has been created as the root HTML Element of the PlatformView is retained unaltered from frame to frame.

This PR fixes this problem by adding an update handler to PersistedPlatformView, which applys the new dimensions to a retained surface / root HTML Element in case they have been changed.

yjbanov · 2019-09-12T03:57:06Z

/cc @hterkelsen

harryterkelsen · 2019-09-12T21:15:07Z

lib/web_ui/lib/src/engine/surface/platform_view.dart

+  void update(PersistedPlatformView oldSurface) {
+    super.update(oldSurface);
+    if (viewId != oldSurface.viewId) {
+      // the content of the surface has to be rebuild if the viewId is changed


Please make this comment a full sentence with punctuation.

harryterkelsen · 2019-09-12T21:15:35Z

lib/web_ui/lib/src/engine/surface/platform_view.dart

+    if (viewId != oldSurface.viewId) {
+      // the content of the surface has to be rebuild if the viewId is changed
+      build();
+    } else if (dx != oldSurface.dx || dy != oldSurface.dy || width != oldSurface.width || height != oldSurface.height) {


Please format this line (you can just run dartfmt on the whole file).

done, thx for the hint! :-)

harryterkelsen · 2019-09-12T21:15:57Z

lib/web_ui/lib/src/engine/surface/platform_view.dart

+      // the content of the surface has to be rebuild if the viewId is changed
+      build();
+    } else if (dx != oldSurface.dx || dy != oldSurface.dy || width != oldSurface.width || height != oldSurface.height) {
+      // a change in any of the dimensions is performed by calling apply


Please make this comment a full sentence with punctuation.

done, and will remember this format for the future ;-)

harryterkelsen · 2019-09-12T21:33:41Z

lib/web_ui/lib/src/engine/surface/platform_view.dart

  @override
  html.Element createElement() {
-    _hostElement = defaultCreateElement('flt-platform-view');
+    html.Element element = defaultCreateElement('flt-platform-view');


We should store the platform view that is created, i.e. platformView in createElement() and on update, update the size and position of the platform view, rather than the shadow root element (flt-platform-view).

This way, the created platform view is itself resized (imagine a <video> element, if we just resize the flt-platform-view element, then the <video> won't be resized, it will just be clipped).

OK, so the latest version will set the size of both the shadow root element and the PlatformView root element. In addition the overflow property is set to auto on the PlatformView root element. As this property was not set currently, the default behaviour was to let the PlatformView overflow. This will haben even if we set the size of the shadow root (e.g. if a video element is wrapped in a div element).

By setting the overflow property on the shadow root, a user may still chose a different overflow behaviour for the shadow root element. Hope that's ok this way.

Let's set the overflow to hidden, and also set it on the element in the createElement method so we don't reset it on every apply.

harryterkelsen · 2019-09-17T16:35:53Z

Ping

harryterkelsen · 2019-09-19T23:25:28Z

lib/web_ui/lib/src/engine/surface/platform_view.dart

  @override
  html.Element createElement() {
-    _hostElement = defaultCreateElement('flt-platform-view');
+    html.Element element = defaultCreateElement('flt-platform-view');


Let's set the overflow to hidden, and also set it on the element in the createElement method so we don't reset it on every apply.

harryterkelsen

LGTM but please wait for the build to be green again

tauu · 2019-09-19T23:37:29Z

OK, no problem. Thanks for the really fast review responses! :-)

cbracken · 2019-09-23T18:06:29Z

Looks like this is hitting the same dart2js error as #12226:

[ +554 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.

When the PlatformViemSurface adopts the DOM elements from a previous PlatformViewSurface the the stored value will lost.

tauu · 2019-09-24T22:14:43Z

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

…nable resizing (flutter/engine#12227)

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

…nable resizing (flutter/engine#12227)

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

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

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

googlebot added the cla: yes label Sep 11, 2019

harryterkelsen suggested changes Sep 12, 2019

View reviewed changes

harryterkelsen suggested changes Sep 19, 2019

View reviewed changes

harryterkelsen approved these changes Sep 19, 2019

View reviewed changes

harryterkelsen mentioned this pull request Sep 23, 2019

[web] [HtmlElementView] Set width and height on the registered HTML flutter/flutter#39982

Closed

tauu added 7 commits September 24, 2019 23:25

add PersistedPlatformView attribute update handling to enable resizing

584ba08

update size of root element created by a PlatformView

a0e2869

improved comments

7a59c4f

enforce effective size of PlatformView surface

76af94b

updated formating of platform_view.dart

2e1a015

stop storing root element of PlatformView in its Surface

4759afb

When the PlatformViemSurface adopts the DOM elements from a previous PlatformViewSurface the the stored value will lost.

move setting overflow property to createElement

12d94c9

tauu force-pushed the webui-platformview-resize branch from 80eeb40 to 12d94c9 Compare September 24, 2019 21:29

harryterkelsen merged commit 92d020f 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

92d020f [web_ui] PersistedPlatformView attribute update handling to e…

8a0034a

…nable resizing (flutter/engine#12227)

engine-flutter-autoroll mentioned this pull request Sep 27, 2019

Roll engine ce6ab8ce25fa..a206557f8fb8 (5 commits) flutter/flutter#41440

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 30, 2019

92d020f [web_ui] PersistedPlatformView attribute update handling to e…

6844c0b

…nable resizing (flutter/engine#12227)

engine-flutter-autoroll mentioned this pull request Sep 30, 2019

Roll engine 5b952f286fc0..f0c77405d5c2 (47 commits) flutter/flutter#41622

Merged

[web_ui] PersistedPlatformView attribute update handling to enable resizing #12227

[web_ui] PersistedPlatformView attribute update handling to enable resizing #12227

Uh oh!

Conversation

tauu commented Sep 11, 2019

Uh oh!

yjbanov commented Sep 12, 2019

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!

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!

harryterkelsen commented Sep 17, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

harryterkelsen left a comment

Choose a reason for hiding this comment

Uh oh!

tauu commented Sep 19, 2019

Uh oh!

cbracken commented Sep 23, 2019

Uh oh!

tauu commented Sep 24, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants