Changeset 249088 in webkit

Timestamp:

Aug 24, 2019, 10:35:25 AM (7 years ago)

Author:

Simon Fraser

Message:

RenderLayer::updateLayerPositions() doesn't propagate the ancestor flags correctly
​https://bugs.webkit.org/show_bug.cgi?id=201115

Reviewed by Zalan Bujtas.

When an updateLayerPositions() traversal starts at a non-root layer, we failed to populate
the ancestor-related UpdateLayerPositionsFlag flags, leaving layers with missing flags
(e.g. the m_hasTransformedAncestor flag). This is detected by the patch in bug 201066.

Fix by having updateLayerPositionsAfterStyleChange() and updateLayerPositionsAfterLayout()
initialize the flags from the parent layer.

This is a behavior change not detected by any existing test, but will be exercised once
the patch from bug 201066 lands.

• rendering/RenderLayer.cpp:

(WebCore::RenderLayer::flagsForUpdateLayerPositions):
(WebCore::RenderLayer::updateLayerPositionsAfterStyleChange):
(WebCore::RenderLayer::updateLayerPositionsAfterLayout):
(WebCore::outputPaintOrderTreeLegend):
(WebCore::outputPaintOrderTreeRecursive): Log hasTransformedAncestor().

• rendering/RenderLayer.h:

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• rendering/RenderLayer.cpp (modified) (4 diffs)
• rendering/RenderLayer.h (modified) (3 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2019-08-23  Simon Fraser  <simon.fraser@apple.com>
+
+        RenderLayer::updateLayerPositions() doesn't propagate the ancestor flags correctly
+        https://bugs.webkit.org/show_bug.cgi?id=201115
+
+        Reviewed by Zalan Bujtas.
+
+        When an updateLayerPositions() traversal starts at a non-root layer, we failed to populate
+        the ancestor-related UpdateLayerPositionsFlag flags, leaving layers with missing flags
+        (e.g. the m_hasTransformedAncestor flag). This is detected by the patch in bug 201066.
+
+        Fix by having updateLayerPositionsAfterStyleChange() and updateLayerPositionsAfterLayout()
+        initialize the flags from the parent layer.
+
+        This is a behavior change not detected by any existing test, but will be exercised once
+        the patch from bug 201066 lands.
+
+        * rendering/RenderLayer.cpp:
+        (WebCore::RenderLayer::flagsForUpdateLayerPositions):
+        (WebCore::RenderLayer::updateLayerPositionsAfterStyleChange):
+        (WebCore::RenderLayer::updateLayerPositionsAfterLayout):
+        (WebCore::outputPaintOrderTreeLegend):
+        (WebCore::outputPaintOrderTreeRecursive): Log hasTransformedAncestor().
+        * rendering/RenderLayer.h:
+
 2019-08-24  Zalan Bujtas  <zalan@apple.com>
 

trunk/Source/WebCore/rendering/RenderLayer.cpp

@@ -876 +876 @@
 }
 
+OptionSet<RenderLayer::UpdateLayerPositionsFlag> RenderLayer::flagsForUpdateLayerPositions(RenderLayer& startingLayer)
+{
+    OptionSet<UpdateLayerPositionsFlag> flags = { CheckForRepaint };
+
+    if (auto* parent = startingLayer.parent()) {
+        if (parent->hasTransformedAncestor() || parent->transform())
+            flags.add(SeenTransformedLayer);
+
+        if (parent->has3DTransformedAncestor() || (parent->transform() && !parent->transform()->isAffine()))
+            flags.add(Seen3DTransformedLayer);
+
+        if (parent->behavesAsFixed() || (parent->renderer().isFixedPositioned() && !parent->hasTransformedAncestor()))
+            flags.add(SeenFixedLayer);
+
+        if (parent->hasCompositedScrollingAncestor() || parent->hasCompositedScrollableOverflow())
+            flags.add(SeenCompositedScrollingLayer);
+    }
+
+    return flags;
+}
+
 void RenderLayer::updateLayerPositionsAfterStyleChange()
 {
-    updateLayerPositions(nullptr, RenderLayer::updateLayerPositionsDefaultFlags());
+    updateLayerPositions(nullptr, flagsForUpdateLayerPositions(*this));
 }
 
@@ -884 +905 @@
 {
     auto updateLayerPositionFlags = [&](bool isRelayoutingSubtree, bool didFullRepaint) {
-        auto flags = RenderLayer::updateLayerPositionsDefaultFlags();
+        auto flags = flagsForUpdateLayerPositions(*this);
         if (didFullRepaint) {
             flags.remove(RenderLayer::CheckForRepaint);
@@ -6975 +6996 @@
 {
     stream.nextLine();
-    stream << "(S)tacking Context/(F)orced SC/O(P)portunistic SC, (N)ormal flow only, (O)verflow clip, (A)lpha (opacity or mask), has (B)lend mode, (I)solates blending, (T)ransform-ish, (F)ilter, Fi(X)ed position, Behaves as fi(x)ed, (C)omposited, (P)rovides backing/uses (p)rovided backing/paints to (a)ncestor, (c)omposited descendant, (s)scrolling ancestor\n"
+    stream << "(S)tacking Context/(F)orced SC/O(P)portunistic SC, (N)ormal flow only, (O)verflow clip, (A)lpha (opacity or mask), has (B)lend mode, (I)solates blending, (T)ransform-ish, (F)ilter, Fi(X)ed position, Behaves as fi(x)ed, (C)omposited, (P)rovides backing/uses (p)rovided backing/paints to (a)ncestor, (c)omposited descendant, (s)scrolling ancestor, (t)transformed ancestor\n"
         "Dirty (z)-lists, Dirty (n)ormal flow lists\n"
         "Traversal needs: requirements (t)raversal on descendants, (b)acking or hierarchy traversal on descendants, (r)equirements traversal on all descendants, requirements traversal on all (s)ubsequent layers, (h)ierarchy traversal on all descendants, update of paint (o)rder children\n"
@@ -7022 +7043 @@
     stream << (layer.hasCompositingDescendant() ? "c" : "-");
     stream << (layer.hasCompositedScrollingAncestor() ? "s" : "-");
+    stream << (layer.hasTransformedAncestor() ? "t" : "-");
 
     stream << " ";

trunk/Source/WebCore/rendering/RenderLayer.h

@@ -785 +785 @@
     bool preserves3D() const { return renderer().style().transformStyle3D() == TransformStyle3D::Preserve3D; }
     bool has3DTransform() const { return m_transform && !m_transform->isAffine(); }
+    bool hasTransformedAncestor() const { return m_hasTransformedAncestor; }
 
     void filterNeedsRepaint();
@@ -989 +990 @@
         SeenCompositedScrollingLayer        = 1 << 7,
     };
-    static constexpr OptionSet<UpdateLayerPositionsFlag> updateLayerPositionsDefaultFlags() { return { CheckForRepaint }; }
+    static OptionSet<UpdateLayerPositionsFlag> flagsForUpdateLayerPositions(RenderLayer& startingLayer);
 
     // Returns true if the position changed.
     bool updateLayerPosition(OptionSet<UpdateLayerPositionsFlag>* = nullptr);
 
-    void updateLayerPositions(RenderGeometryMap* = nullptr, OptionSet<UpdateLayerPositionsFlag> = updateLayerPositionsDefaultFlags());
+    void updateLayerPositions(RenderGeometryMap*, OptionSet<UpdateLayerPositionsFlag>);
 
     enum UpdateLayerPositionsAfterScrollFlag {
@@ -1128 +1129 @@
 
     bool has3DTransformedDescendant() const { return m_has3DTransformedDescendant; }
-
-    bool hasTransformedAncestor() const { return m_hasTransformedAncestor; }
     bool has3DTransformedAncestor() const { return m_has3DTransformedAncestor; }