Summary: | [chromium] Remove WebLayerTreeView::setViewportSize call | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Alexandre Elias <aelias> | ||||||
Component: | New Bugs | Assignee: | Alexandre Elias <aelias> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | aelias, buildbot, dglazkov, jamesr, jochen, piman, rniwa, webkit.review.bot | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Alexandre Elias
2013-02-24 23:29:23 PST
Created attachment 190001 [details]
Patch
https://codereview.chromium.org/12328080/ will land before this one. 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 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 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.
Created attachment 190641 [details]
Patch
Make DumpRenderTree call setViewportSize
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? 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. (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 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 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. Comment on attachment 190641 [details]
Patch
Source/WebKit/chromium/DEPS rolled past 185621, landing this half.
Comment on attachment 190641 [details] Patch Clearing flags on attachment: 190641 Committed r144581: <http://trac.webkit.org/changeset/144581> All reviewed patches have been landed. Closing bug. This change regressed >500 tests on content_shell I have a follow-up fix https://codereview.chromium.org/12374078/ that should hopefully fix them. 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! |