Skip to content

Sometimes the contents of a visibility:hidden iframe become visible#27306

Merged
webkit-commit-queue merged 1 commit intoWebKit:mainfrom
smfr:eng/Sometimes-the-contents-of-a-visibilityhidden-iframe-become-visible
Apr 18, 2024
Merged

Sometimes the contents of a visibility:hidden iframe become visible#27306
webkit-commit-queue merged 1 commit intoWebKit:mainfrom
smfr:eng/Sometimes-the-contents-of-a-visibilityhidden-iframe-become-visible

Conversation

@smfr
Copy link
Contributor

@smfr smfr commented Apr 16, 2024

d927d70

Sometimes the contents of a visibility:hidden iframe become visible
https://bugs.webkit.org/show_bug.cgi?id=272727
rdar://126178201

Reviewed by Matt Woodrow.

A normal composited but `visibility:hidden` layer uses `setContentsVisible(false)`
to hide its contents (by clearing the layer's `contents` property), and this works
within a document because `visibility` is inherited, and allows a visible child
inside a hidden ancestor.

For plugin or iframe layers we try to avoid making them composited when they
are `visibility:hidden`. However, if they are composited for other reasons (e.g.
`position:fixed`) then we hide their layer contents, but we fail to hide their
descendant layers, which GraphicsLayer parented via `attachWidgetContentLayers()`.

Fix this by not parenting the widget's content layers in `RenderLayerCompositor::attachWidgetContentLayersIfNecessary()`,
and enhancing the return value to distinguish between "widget has contents via child layers", and "mutated the layer hierarchy"
since the two callers were using the return value in two different ways.

Enhance an existing test to exercise this case.
* LayoutTests/compositing/visibility/iframe-visibility-hidden-expected.html:
* LayoutTests/compositing/visibility/iframe-visibility-hidden.html:
* Source/WebCore/rendering/RenderLayerBacking.cpp:
(WebCore::RenderLayerBacking::updateConfiguration):
* Source/WebCore/rendering/RenderLayerCompositor.cpp:
(WebCore::RenderLayerCompositor::updateBackingAndHierarchy):
(WebCore::RenderLayerCompositor::attachWidgetContentLayersIfNecessary):
(WebCore::RenderLayerCompositor::attachWidgetContentLayers): Deleted.
* Source/WebCore/rendering/RenderLayerCompositor.h:

Canonical link: https://commits.webkit.org/277643@main

ea4bda8

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug 🧪 wpe-wk2
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-skia
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 tv ✅ 🧪 mac-AS-debug-wk2 🧪 gtk-wk2
✅ 🛠 tv-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🛠 🧪 merge ✅ 🛠 watch
✅ 🛠 watch-sim

@smfr smfr self-assigned this Apr 16, 2024
@smfr smfr added the Compositing Bugs related to layerization for transforms, video, scrolling etc label Apr 16, 2024
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Apr 16, 2024
@smfr smfr removed the merging-blocked Applied to prevent a change from being merged label Apr 16, 2024
@smfr smfr force-pushed the eng/Sometimes-the-contents-of-a-visibilityhidden-iframe-become-visible branch from 3bcca51 to ea4bda8 Compare April 16, 2024 23:25
@smfr smfr added the merge-queue Applied to send a pull request to merge-queue label Apr 17, 2024
https://bugs.webkit.org/show_bug.cgi?id=272727
rdar://126178201

Reviewed by Matt Woodrow.

A normal composited but `visibility:hidden` layer uses `setContentsVisible(false)`
to hide its contents (by clearing the layer's `contents` property), and this works
within a document because `visibility` is inherited, and allows a visible child
inside a hidden ancestor.

For plugin or iframe layers we try to avoid making them composited when they
are `visibility:hidden`. However, if they are composited for other reasons (e.g.
`position:fixed`) then we hide their layer contents, but we fail to hide their
descendant layers, which GraphicsLayer parented via `attachWidgetContentLayers()`.

Fix this by not parenting the widget's content layers in `RenderLayerCompositor::attachWidgetContentLayersIfNecessary()`,
and enhancing the return value to distinguish between "widget has contents via child layers", and "mutated the layer hierarchy"
since the two callers were using the return value in two different ways.

Enhance an existing test to exercise this case.
* LayoutTests/compositing/visibility/iframe-visibility-hidden-expected.html:
* LayoutTests/compositing/visibility/iframe-visibility-hidden.html:
* Source/WebCore/rendering/RenderLayerBacking.cpp:
(WebCore::RenderLayerBacking::updateConfiguration):
* Source/WebCore/rendering/RenderLayerCompositor.cpp:
(WebCore::RenderLayerCompositor::updateBackingAndHierarchy):
(WebCore::RenderLayerCompositor::attachWidgetContentLayersIfNecessary):
(WebCore::RenderLayerCompositor::attachWidgetContentLayers): Deleted.
* Source/WebCore/rendering/RenderLayerCompositor.h:

Canonical link: https://commits.webkit.org/277643@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/Sometimes-the-contents-of-a-visibilityhidden-iframe-become-visible branch from ea4bda8 to d927d70 Compare April 18, 2024 00:20
@webkit-commit-queue
Copy link
Collaborator

Committed 277643@main (d927d70): https://commits.webkit.org/277643@main

Reviewed commits have been landed. Closing PR #27306 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit d927d70 into WebKit:main Apr 18, 2024
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Apr 18, 2024
@smfr smfr deleted the eng/Sometimes-the-contents-of-a-visibilityhidden-iframe-become-visible branch May 9, 2024 22:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Compositing Bugs related to layerization for transforms, video, scrolling etc

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants