- Revision
- 242248
- Author
- simon.fra...@apple.com
- Date
- 2019-02-28 17:54:59 -0800 (Thu, 28 Feb 2019)
Log Message
[iOS] Dark flash when opening Google AMP pages
https://bugs.webkit.org/show_bug.cgi?id=195193
rdar://problem/48326442
Reviewed by Zalan Bujtas.
Source/WebCore:
After the incremental compositing updates changes, it was possible for a change in the size
of an overflow:hidden element to fail to update the "ancestor clipping layer" geometry on
a composited descendant that is not a descendant in z-order. When Google search results
create the <iframe> that contain AMP contents, we'd fail to update a zero-sized clipping layer,
leaving the #222 background of an intermediate element visible.
Fix by setting a flag in RenderLayer::updateLayerPosition() (which is called in containing block order)
that sets the "needs geometry update" dirty bit on containing-block-descendant layers. Currently
this flag affects all descendants; in future, we might be able to clear it for grand-children.
Tests: compositing/geometry/ancestor-clip-change-interleaved-stacking-context.html
compositing/geometry/ancestor-clip-change.html
* rendering/RenderLayer.cpp:
(WebCore::RenderLayer::updateLayerPositions):
(WebCore::RenderLayer::updateLayerPosition):
* rendering/RenderLayer.h:
* rendering/RenderLayerBacking.cpp:
(WebCore::RenderLayerBacking::updateAfterLayout):
* rendering/RenderLayerBacking.h:
LayoutTests:
Tests that change the size of a clipping layer with non-z-order composited descendant, with
a couple of layer tree configurations.
* compositing/geometry/ancestor-clip-change-expected.html: Added.
* compositing/geometry/ancestor-clip-change-interleaved-stacking-context-expected.html: Added.
* compositing/geometry/ancestor-clip-change-interleaved-stacking-context.html: Added.
* compositing/geometry/ancestor-clip-change.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (242247 => 242248)
--- trunk/LayoutTests/ChangeLog 2019-03-01 01:44:22 UTC (rev 242247)
+++ trunk/LayoutTests/ChangeLog 2019-03-01 01:54:59 UTC (rev 242248)
@@ -1,3 +1,19 @@
+2019-02-28 Simon Fraser <simon.fra...@apple.com>
+
+ [iOS] Dark flash when opening Google AMP pages
+ https://bugs.webkit.org/show_bug.cgi?id=195193
+ rdar://problem/48326442
+
+ Reviewed by Zalan Bujtas.
+
+ Tests that change the size of a clipping layer with non-z-order composited descendant, with
+ a couple of layer tree configurations.
+
+ * compositing/geometry/ancestor-clip-change-expected.html: Added.
+ * compositing/geometry/ancestor-clip-change-interleaved-stacking-context-expected.html: Added.
+ * compositing/geometry/ancestor-clip-change-interleaved-stacking-context.html: Added.
+ * compositing/geometry/ancestor-clip-change.html: Added.
+
2019-02-28 Joseph Pecoraro <pecor...@apple.com>
Web Inspector: View.removeSubview not removing the element properly when not parented
Added: trunk/LayoutTests/compositing/geometry/ancestor-clip-change-expected.html (0 => 242248)
--- trunk/LayoutTests/compositing/geometry/ancestor-clip-change-expected.html (rev 0)
+++ trunk/LayoutTests/compositing/geometry/ancestor-clip-change-expected.html 2019-03-01 01:54:59 UTC (rev 242248)
@@ -0,0 +1,30 @@
+<!DOCTYPE html>
+<html>
+<head>
+ <style>
+ body {
+ height: 1000px;
+ }
+ .clipper {
+ margin: 20px;
+ padding: 10px;
+ width: 300px;
+ height: 300px;
+ overflow: hidden;
+ background-color: gray;
+ }
+ .content {
+ transform: translateZ(0);
+ background-color: green;
+ height: 300px;
+ }
+ </style>
+</head>
+<body>
+ <div class="clipper">
+ <div class="content">
+
+ </div>
+ </div>
+</body>
+</html>
Added: trunk/LayoutTests/compositing/geometry/ancestor-clip-change-interleaved-stacking-context-expected.html (0 => 242248)
--- trunk/LayoutTests/compositing/geometry/ancestor-clip-change-interleaved-stacking-context-expected.html (rev 0)
+++ trunk/LayoutTests/compositing/geometry/ancestor-clip-change-interleaved-stacking-context-expected.html 2019-03-01 01:54:59 UTC (rev 242248)
@@ -0,0 +1,38 @@
+<!DOCTYPE html>
+<html>
+<head>
+ <style>
+ body {
+ height: 1000px;
+ }
+ .outer-clipper {
+ position: relative;
+ width: 360px;
+ height: 360px;
+ border: 2px solid orange;
+ }
+ .clipper {
+ margin: 20px;
+ padding: 10px;
+ width: 300px;
+ height: 300px;
+ overflow: hidden;
+ background-color: gray;
+ }
+ .content {
+ transform: translateZ(0);
+ background-color: green;
+ height: 300px;
+ }
+ </style>
+</head>
+<body>
+ <div class="outer-clipper">
+ <div class="clipper">
+ <div class="content">
+
+ </div>
+ </div>
+ </div>
+</body>
+</html>
Added: trunk/LayoutTests/compositing/geometry/ancestor-clip-change-interleaved-stacking-context.html (0 => 242248)
--- trunk/LayoutTests/compositing/geometry/ancestor-clip-change-interleaved-stacking-context.html (rev 0)
+++ trunk/LayoutTests/compositing/geometry/ancestor-clip-change-interleaved-stacking-context.html 2019-03-01 01:54:59 UTC (rev 242248)
@@ -0,0 +1,54 @@
+<!DOCTYPE html>
+<html>
+<head>
+ <style>
+ body {
+ height: 1000px;
+ }
+ .outer-clipper {
+ position: relative;
+ width: 360px;
+ height: 360px;
+ border: 2px solid orange;
+ }
+ .clipper {
+ margin: 20px;
+ padding: 10px;
+ width: 300px;
+ height: 100px;
+ overflow: hidden;
+ background-color: gray;
+ }
+ .changed .clipper {
+ height: 300px;
+ }
+ .content {
+ transform: translateZ(0);
+ background-color: green;
+ height: 300px;
+ }
+ </style>
+ <script>
+ if (window.testRunner)
+ testRunner.waitUntilDone();
+
+ window.addEventListener('load', () => {
+ requestAnimationFrame(() => {
+ document.body.classList.add('changed');
+
+ if (window.testRunner)
+ testRunner.notifyDone();
+ });
+ }, false);
+ </script>
+</head>
+<body>
+ <div class="outer-clipper">
+ <div class="clipper">
+ <div class="content">
+
+ </div>
+ </div>
+ </div>
+</body>
+</html>
Added: trunk/LayoutTests/compositing/geometry/ancestor-clip-change.html (0 => 242248)
--- trunk/LayoutTests/compositing/geometry/ancestor-clip-change.html (rev 0)
+++ trunk/LayoutTests/compositing/geometry/ancestor-clip-change.html 2019-03-01 01:54:59 UTC (rev 242248)
@@ -0,0 +1,46 @@
+<!DOCTYPE html>
+<html>
+<head>
+ <style>
+ body {
+ height: 1000px;
+ }
+ .clipper {
+ margin: 20px;
+ padding: 10px;
+ width: 300px;
+ height: 100px;
+ overflow: hidden;
+ background-color: gray;
+ }
+ .changed .clipper {
+ height: 300px;
+ }
+ .content {
+ transform: translateZ(0);
+ background-color: green;
+ height: 300px;
+ }
+ </style>
+ <script>
+ if (window.testRunner)
+ testRunner.waitUntilDone();
+
+ window.addEventListener('load', () => {
+ requestAnimationFrame(() => {
+ document.body.classList.add('changed');
+
+ if (window.testRunner)
+ testRunner.notifyDone();
+ });
+ }, false);
+ </script>
+</head>
+<body>
+ <div class="clipper">
+ <div class="content">
+
+ </div>
+ </div>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (242247 => 242248)
--- trunk/Source/WebCore/ChangeLog 2019-03-01 01:44:22 UTC (rev 242247)
+++ trunk/Source/WebCore/ChangeLog 2019-03-01 01:54:59 UTC (rev 242248)
@@ -1,3 +1,32 @@
+2019-02-28 Simon Fraser <simon.fra...@apple.com>
+
+ [iOS] Dark flash when opening Google AMP pages
+ https://bugs.webkit.org/show_bug.cgi?id=195193
+ rdar://problem/48326442
+
+ Reviewed by Zalan Bujtas.
+
+ After the incremental compositing updates changes, it was possible for a change in the size
+ of an overflow:hidden element to fail to update the "ancestor clipping layer" geometry on
+ a composited descendant that is not a descendant in z-order. When Google search results
+ create the <iframe> that contain AMP contents, we'd fail to update a zero-sized clipping layer,
+ leaving the #222 background of an intermediate element visible.
+
+ Fix by setting a flag in RenderLayer::updateLayerPosition() (which is called in containing block order)
+ that sets the "needs geometry update" dirty bit on containing-block-descendant layers. Currently
+ this flag affects all descendants; in future, we might be able to clear it for grand-children.
+
+ Tests: compositing/geometry/ancestor-clip-change-interleaved-stacking-context.html
+ compositing/geometry/ancestor-clip-change.html
+
+ * rendering/RenderLayer.cpp:
+ (WebCore::RenderLayer::updateLayerPositions):
+ (WebCore::RenderLayer::updateLayerPosition):
+ * rendering/RenderLayer.h:
+ * rendering/RenderLayerBacking.cpp:
+ (WebCore::RenderLayerBacking::updateAfterLayout):
+ * rendering/RenderLayerBacking.h:
+
2019-02-28 Myles C. Maxfield <mmaxfi...@apple.com>
Use-after-move in RenderCombineText::combineTextIfNeeded()
Modified: trunk/Source/WebCore/rendering/RenderLayer.cpp (242247 => 242248)
--- trunk/Source/WebCore/rendering/RenderLayer.cpp 2019-03-01 01:44:22 UTC (rev 242247)
+++ trunk/Source/WebCore/rendering/RenderLayer.cpp 2019-03-01 01:54:59 UTC (rev 242248)
@@ -848,10 +848,7 @@
void RenderLayer::updateLayerPositions(RenderGeometryMap* geometryMap, OptionSet<UpdateLayerPositionsFlag> flags)
{
- updateLayerPosition(); // For relpositioned layers or non-positioned layers,
- // we need to keep in sync, since we may have shifted relative
- // to our parent layer.
-
+ updateLayerPosition(&flags);
applyPostLayoutScrollPositionIfNeeded();
if (geometryMap)
@@ -952,7 +949,7 @@
}
if (isComposited())
- backing()->updateAfterLayout(flags.contains(NeedsFullRepaintInBacking));
+ backing()->updateAfterLayout(flags.contains(ContainingClippingLayerChangedSize), flags.contains(NeedsFullRepaintInBacking));
if (geometryMap)
geometryMap->popMappingsToAncestor(parent());
@@ -1498,7 +1495,7 @@
return has3DTransform();
}
-bool RenderLayer::updateLayerPosition()
+bool RenderLayer::updateLayerPosition(OptionSet<UpdateLayerPositionsFlag>* flags)
{
LayoutPoint localPoint;
LayoutSize inlineBoundingBoxOffset; // We don't put this into the RenderLayer x/y for inlines, so we need to subtract it out when done.
@@ -1516,6 +1513,10 @@
// Trigger RenderLayerCompositor::requiresCompositingForFrame() which depends on the contentBoxRect size.
setNeedsPostLayoutCompositingUpdate();
}
+
+ if (flags && renderer().hasOverflowClip())
+ flags->add(ContainingClippingLayerChangedSize);
+
setSize(newSize);
}
Modified: trunk/Source/WebCore/rendering/RenderLayer.h (242247 => 242248)
--- trunk/Source/WebCore/rendering/RenderLayer.h 2019-03-01 01:44:22 UTC (rev 242247)
+++ trunk/Source/WebCore/rendering/RenderLayer.h 2019-03-01 01:54:59 UTC (rev 242248)
@@ -505,11 +505,12 @@
bool canRender3DTransforms() const;
enum UpdateLayerPositionsFlag {
- CheckForRepaint = 1 << 0,
- NeedsFullRepaintInBacking = 1 << 1,
- UpdatePagination = 1 << 2,
- SeenTransformedLayer = 1 << 3,
- Seen3DTransformedLayer = 1 << 4,
+ CheckForRepaint = 1 << 0,
+ NeedsFullRepaintInBacking = 1 << 1,
+ ContainingClippingLayerChangedSize = 1 << 2,
+ UpdatePagination = 1 << 3,
+ SeenTransformedLayer = 1 << 4,
+ Seen3DTransformedLayer = 1 << 5,
};
static constexpr OptionSet<UpdateLayerPositionsFlag> updateLayerPositionsDefaultFlags() { return { CheckForRepaint }; }
@@ -931,7 +932,7 @@
void updateScrollbarsAfterLayout();
// Returns true if the position changed.
- bool updateLayerPosition();
+ bool updateLayerPosition(OptionSet<UpdateLayerPositionsFlag>* = nullptr);
void updateLayerPositions(RenderGeometryMap* = nullptr, OptionSet<UpdateLayerPositionsFlag> = updateLayerPositionsDefaultFlags());
Modified: trunk/Source/WebCore/rendering/RenderLayerBacking.cpp (242247 => 242248)
--- trunk/Source/WebCore/rendering/RenderLayerBacking.cpp 2019-03-01 01:44:22 UTC (rev 242247)
+++ trunk/Source/WebCore/rendering/RenderLayerBacking.cpp 2019-03-01 01:54:59 UTC (rev 242248)
@@ -651,7 +651,7 @@
}
}
-void RenderLayerBacking::updateAfterLayout(bool needsFullRepaint)
+void RenderLayerBacking::updateAfterLayout(bool needsClippingUpdate, bool needsFullRepaint)
{
LOG(Compositing, "RenderLayerBacking %p updateAfterLayout (layer %p)", this, &m_owningLayer);
@@ -662,7 +662,8 @@
m_owningLayer.setNeedsCompositingGeometryUpdate();
// This layer's geometry affects those of its children.
m_owningLayer.setChildrenNeedCompositingGeometryUpdate();
- }
+ } else if (needsClippingUpdate)
+ m_owningLayer.setNeedsCompositingGeometryUpdate();
if (needsFullRepaint && canIssueSetNeedsDisplay())
setContentsNeedDisplay();
Modified: trunk/Source/WebCore/rendering/RenderLayerBacking.h (242247 => 242248)
--- trunk/Source/WebCore/rendering/RenderLayerBacking.h 2019-03-01 01:44:22 UTC (rev 242247)
+++ trunk/Source/WebCore/rendering/RenderLayerBacking.h 2019-03-01 01:54:59 UTC (rev 242248)
@@ -83,7 +83,7 @@
// Update contents and clipping structure.
void updateDrawsContent();
- void updateAfterLayout(bool needsFullRepaint);
+ void updateAfterLayout(bool needsClippingUpdate, bool needsFullRepaint);
GraphicsLayer* graphicsLayer() const { return m_graphicsLayer.get(); }
