Bug 92569 – Ignore visibility:hidden elements when computing compositing layer bounds

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Simon Fraser (smfr)]

Google maps has some elements with crazy offsets:

<img style="position: absolute; left: 44940px; top: 15013px; width: 19px; height: 19px; -webkit-user-select: none; border: 0px; padding: 0px; margin: 0px; z-index: 0; visibility: hidden; " crossorigin="" src="//maps.gstatic.com/mapfiles/markers2/dd-via-transparent.png" class="gmnoprint" log="miw" id="mtgt_ddwnew">

<img style="width: 11px; height: 11px; -webkit-user-select: none; border: 0px; padding: 0px; margin: 0px; position: absolute; left: 44944px; top: 15017px; z-index: 0; visibility: hidden; " crossorigin="" src="//maps.gstatic.com/mapfiles/markers2/dd-via.png">

These cause us to create crazy-big layers when getting directions (at least on iOS).We should ignore visibility:hidden layers when computing composited layer bounds.

[Simon Fraser (smfr)]

Created attachment 155109 [details]
Patch

[Simon Fraser (smfr)]

I'll add a test later.

[Dean Jackson]

Comment on attachment 155109 [details]
Patch

Did you mean to "r?" this patch, or will you upload another? Otherwise all looks good.

[Simon Fraser (smfr)]

Comment on attachment 155109 [details]
Patch

I'll do one with some tests.

[Simon Fraser (smfr)]

Filed bug 92579 about filter extents.

[Simon Fraser (smfr)]

Created attachment 155136 [details]
Patch

Comment on attachment 155136 [details]
Patch

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

> Source/WebCore/rendering/RenderLayer.cpp:4251
> +    // FIXME: should probably just pass 'flags' down to descendants. This is a defensive change.

“This is a defensive change” is an appropriate comment for the change log. It doesn’t make much sense to someone reading the code (what is the change?).

> Source/WebCore/rendering/RenderLayer.cpp:4254
> +    CalculateLayerBoundsFlags descendantFlags = DefaultCalculateLayerBoundsFlags;
> +    if (flags & ExcludeHiddenDescendants)
> +        descendantFlags |= ExcludeHiddenDescendants;

There are many ways to write this. I’d go for the one-liner
    CalculateLayerBoundsFlags descendantFlags = DefaultCalculateLayerBoundsFlags | (flags & ExcludeHiddenDescendants);

[WebKit Review Bot]

Comment on attachment 155136 [details]
Patch

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

New failing tests:
compositing/geometry/bounds-ignores-hidden-composited-descendant.html
compositing/geometry/bounds-ignores-hidden-dynamic.html

[WebKit Review Bot]

Created attachment 155137 [details]
Archive of layout-test-results from gce-cr-linux-01

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: gce-cr-linux-01  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid

[Simon Fraser (smfr)]

Chromium linux needs to stop making layers 1px bigger on all sides.

[Simon Fraser (smfr)]

http://trac.webkit.org/changeset/123971

[Radar WebKit Bug Importer]

<rdar://problem/11981260>