Bug 110727 – [chromium] Remove WebLayerTreeView::setViewportSize call

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Alexandre Elias]

[chromium] Remove WebLayerTreeView::setViewportSize call

[Alexandre Elias]

Created attachment 190001 [details]
Patch

[Alexandre Elias]

https://codereview.chromium.org/12328080/ will land before this one.

[Build Bot]

Comment on attachment 190001 [details]
Patch

Attachment 190001 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://queues.webkit.org/results/16752009

New failing tests:
svg/as-image/img-preserveAspectRatio-support-2.html

[WebKit Review Bot]

Comment on attachment 190001 [details]
Patch

Attachment 190001 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/16678054

New failing tests:
compositing/backface-visibility/backface-visibility-hierarchical-transform.html
animations/3d/matrix-transform-type-animation.html
animations/3d/state-at-end-event-transform.html
compositing/nested-direct-image-compositing.html
compositing/direct-image-compositing.html
compositing/backface-visibility/backface-visibility-non3d.html
animations/additive-transform-animations.html
animations/3d/replace-filling-transform.html
compositing/backface-visibility/backface-visibility-3d.html
compositing/clip-change.html
compositing/text-on-large-layer.html
compositing/layers-inside-overflow-scroll.html
compositing/animation/state-at-end-event-transform-layer.html
compositing/sibling-positioning.html
compositing/generated-content.html
compositing/fixed-position-changed-in-composited-layer.html
compositing/self-painting-layers.html
compositing/text-on-scaled-layer.html
compositing/absolute-position-changed-with-composited-parent-layer.html
compositing/absolute-position-changed-in-composited-layer.html
compositing/flat-with-transformed-child.html
compositing/scrollbar-painting.html
compositing/preserve-3d-toggle.html
compositing/fixed-position-scroll-offset-history-restore.html
compositing/backface-visibility/backface-visibility-image.html
compositing/compositing-visible-descendant.html
compositing/fixed-position-changed-within-composited-parent-layer.html
compositing/backface-visibility/backface-visibility-simple.html
compositing/backface-visibility/backface-visibility-webgl.html
compositing/text-on-scaled-surface.html

[James Robinson]

Comment on attachment 190001 [details]
Patch

This part looks fine, but you'll have to sort out the issues on the chromium-side patch and make sure that lands *and* rolls into Source/WebKit/chromium/DEPS before this part can land.

[Alexandre Elias]

Created attachment 190641 [details]
Patch

Make DumpRenderTree call setViewportSize

[James Robinson]

Comment on attachment 190641 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=190641&action=review

> Tools/DumpRenderTree/chromium/WebViewHost.cpp:940
> +    m_layerTreeView->setViewportSize(webWidget()->size(), deviceViewportSize);

Hmm, this is a bit of a bummer.  any way to avoid this?

[Alexandre Elias]

No way to avoid it comes to mind, since my patches are moving the responsibility to call this to the embedder.  DumpRenderTree is another embedder so it needs a call.

I could've left the responsibility to call it in WebKit, in which case it would need to be informed of the physical backing size via an API change.  But it doesn't need to know about this for anything else, and it's nice and clean for every coordinate given to WebKit to be in DIP pixels.

[James Robinson]

(In reply to comment #8)
> No way to avoid it comes to mind, since my patches are moving the responsibility to call this to the embedder.  DumpRenderTree is another embedder so it needs a call.
> 
> I could've left the responsibility to call it in WebKit, in which case it would need to be informed of the physical backing size via an API change.  But it doesn't need to know about this for anything else, and it's nice and clean for every coordinate given to WebKit to be in DIP pixels.

Does the viewport ever change in DRT?  If the answer is no (which I think it is) then we should pass this out as a construction-time parameter.  We shouldn't have exposed API in Platform that's only called from DRT

[Alexandre Elias]

I see, you prefer it only exposed in webkit_support.h.  Unfortunately, some layout tests do call window.resize and break if I try doing setting only at construction time, such as compositing/transitions/transform-on-large-layer.html

[James Robinson]

Yeah, I'd like to get rid of it from WebLayerTreeView completely if we don't want WebKit to be setting this generally.  But don't let that hold you up here.  We can fix it up later for DRT or just wait until DRT goes D-E-D.

[Alexandre Elias]

Comment on attachment 190641 [details]
Patch

Source/WebKit/chromium/DEPS rolled past 185621, landing this half.

[WebKit Review Bot]

Comment on attachment 190641 [details]
Patch

Clearing flags on attachment: 190641

Committed r144581: <http://trac.webkit.org/changeset/144581>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

This change regressed >500 tests on content_shell

[Alexandre Elias]

I have a follow-up fix https://codereview.chromium.org/12374078/ that should hopefully fix them.

[Alexandre Elias]

I confirmed https://codereview.chromium.org/12374078/ fixed the content_shell failures.

(In reply to comment #17)
> I confirmed https://codereview.chromium.org/12374078/ fixed the content_shell failures.

Thank you!