Merge r229200 - [RenderTreeBuilder] Move styleDidChange mutation logi…

…c to RenderTreeUpdater https://bugs.webkit.org/show_bug.cgi?id=183273 <rdar://problem/38054892> Reviewed by Antti Koivisto. Source/WebCore: Covered by existing tests. * rendering/RenderBlockFlow.cpp: (WebCore::RenderBlockFlow::styleDidChange): * rendering/RenderElement.cpp: (WebCore::RenderElement::styleDidChange): (WebCore::RenderElement::noLongerAffectsParentBlock const): Deleted. * rendering/RenderElement.h: * rendering/updating/RenderTreeUpdater.cpp: (WebCore::RenderTreeUpdater::updateElementRenderer): LayoutTests: This is just a different repaint order. * fast/repaint/absolute-position-change-containing-block-expected.txt:
WebKit · Apr 9, 2018 · d46d85e · d46d85e
1 parent ec8c6c0
commit d46d85e
Show file tree

Hide file tree

Showing 10 changed files with 89 additions and 33 deletions.
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
@@ -1,3 +1,15 @@
+2018-03-03  Zalan Bujtas  <zalan@apple.com>
+
+        [RenderTreeBuilder] Move styleDidChange mutation logic to RenderTreeUpdater
+        https://bugs.webkit.org/show_bug.cgi?id=183273
+        <rdar://problem/38054892>
+
+        Reviewed by Antti Koivisto.
+
+        This is just a different repaint order.
+
+        * fast/repaint/absolute-position-change-containing-block-expected.txt:
+
 2018-03-02  Claudio Saavedra  <csaavedra@igalia.com>
 
         [GTK] Unreviewed gardening

diff --git a/LayoutTests/fast/repaint/absolute-position-change-containing-block-expected.txt b/LayoutTests/fast/repaint/absolute-position-change-containing-block-expected.txt
@@ -1,10 +1,10 @@
 (repaint rects
   (rect 8 5000 100 100)
   (rect 108 5100 100 100)
-  (rect 8 8 784 2000)
   (rect 8 5000 100 100)
   (rect 108 5100 100 100)
   (rect 100 100 100 100)
+  (rect 8 8 784 2000)
   (rect 16 5008 100 100)
   (rect 16 5008 100 100)
   (rect 8 8 100 100)

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,22 @@
+2018-03-03  Zalan Bujtas  <zalan@apple.com>
+
+        [RenderTreeBuilder] Move styleDidChange mutation logic to RenderTreeUpdater
+        https://bugs.webkit.org/show_bug.cgi?id=183273
+        <rdar://problem/38054892>
+
+        Reviewed by Antti Koivisto.
+
+        Covered by existing tests.
+
+        * rendering/RenderBlockFlow.cpp:
+        (WebCore::RenderBlockFlow::styleDidChange):
+        * rendering/RenderElement.cpp:
+        (WebCore::RenderElement::styleDidChange):
+        (WebCore::RenderElement::noLongerAffectsParentBlock const): Deleted.
+        * rendering/RenderElement.h:
+        * rendering/updating/RenderTreeUpdater.cpp:
+        (WebCore::RenderTreeUpdater::updateElementRenderer):
+
 2018-03-12  Michael Catanzaro  <mcatanzaro@igalia.com>
 
         [GTK] Crash in WebCore::PlatformDisplayWayland::~PlatformDisplayWayland

diff --git a/Source/WebCore/rendering/RenderBlockFlow.cpp b/Source/WebCore/rendering/RenderBlockFlow.cpp
@@ -2020,10 +2020,6 @@ void RenderBlockFlow::styleDidChange(StyleDifference diff, const RenderStyle* ol
         parentBlock->markAllDescendantsWithFloatsForLayout();
         parentBlock->markSiblingsWithFloatsForLayout();
     }
-    // Fresh floats need to be reparented if they actually belong to the previous anonymous block.
-    // It copies the logic of RenderBlock::addChildIgnoringContinuation
-    if (oldStyle && noLongerAffectsParentBlock(*oldStyle) && style().isFloating() && previousSibling() && previousSibling()->isAnonymousBlock())
-        RenderTreeBuilder::current()->move(downcast<RenderBoxModelObject>(*parent()), downcast<RenderBoxModelObject>(*previousSibling()), *this, RenderTreeBuilder::NormalizeAfterInsertion::No);
 
     if (diff >= StyleDifferenceRepaint) {
         // FIXME: This could use a cheaper style-only test instead of SimpleLineLayout::canUseFor.

diff --git a/Source/WebCore/rendering/RenderElement.cpp b/Source/WebCore/rendering/RenderElement.cpp
@@ -792,12 +792,6 @@ static inline bool areCursorsEqual(const RenderStyle* a, const RenderStyle* b)
 }
 #endif
 
-bool RenderElement::noLongerAffectsParentBlock(const RenderStyle& oldStyle) const
-{
-    return parent() && parent()->isRenderBlock()
-        && ((!oldStyle.isFloating() && style().isFloating()) || (!oldStyle.hasOutOfFlowPosition() && style().hasOutOfFlowPosition()));
-}
-
 void RenderElement::styleDidChange(StyleDifference diff, const RenderStyle* oldStyle)
 {
     updateFillImages(oldStyle ? &oldStyle->backgroundLayers() : nullptr, m_style.backgroundLayers());
@@ -806,21 +800,6 @@ void RenderElement::styleDidChange(StyleDifference diff, const RenderStyle* oldS
     updateImage(oldStyle ? oldStyle->maskBoxImage().image() : nullptr, m_style.maskBoxImage().image());
     updateShapeImage(oldStyle ? oldStyle->shapeOutside() : nullptr, m_style.shapeOutside());
 
-    bool affectsParentBlock = oldStyle && (oldStyle->isFloating() || oldStyle->hasOutOfFlowPosition())
-        && !style().isFloating() && !style().hasOutOfFlowPosition()
-        && parent() && (parent()->isRenderBlockFlow() || parent()->isRenderInline());
-    if (affectsParentBlock) {
-        // We have gone from not affecting the inline status of the parent flow to suddenly
-        // having an impact. See if there is a mismatch between the parent flow's
-        // childrenInline() state and our state.
-        setInline(style().isDisplayInlineType());
-        if (isInline() != parent()->childrenInline())
-            RenderTreeBuilder::current()->childFlowStateChangesAndAffectsParentBlock(*this);
-    }
-
-    if (oldStyle && noLongerAffectsParentBlock(*oldStyle))
-        RenderTreeBuilder::current()->childFlowStateChangesAndNoLongerAffectsParentBlock(*this);
-
     SVGRenderSupport::styleChanged(*this, oldStyle);
 
     if (!m_parent)

diff --git a/Source/WebCore/rendering/RenderElement.h b/Source/WebCore/rendering/RenderElement.h
@@ -282,8 +282,6 @@ class RenderElement : public RenderObject {
 
     bool isVisibleInViewport() const;
 
-    bool noLongerAffectsParentBlock(const RenderStyle& oldStyle) const;
-
 private:
     RenderElement(ContainerNode&, RenderStyle&&, BaseTypeFlags);
     void node() const = delete;

diff --git a/Source/WebCore/rendering/updating/RenderTreeBuilder.cpp b/Source/WebCore/rendering/updating/RenderTreeBuilder.cpp
@@ -513,6 +513,50 @@ void RenderTreeBuilder::moveAllChildrenIncludingFloats(RenderBlock& from, Render
     moveAllChildren(from, to, normalizeAfterInsertion);
 }
 
+void RenderTreeBuilder::normalizeTreeAfterStyleChange(RenderElement& renderer, RenderStyle& oldStyle)
+{
+    if (!renderer.parent())
+        return;
+
+    auto& parent = *renderer.parent();
+
+    bool wasFloating = oldStyle.isFloating();
+    bool wasOufOfFlowPositioned = oldStyle.hasOutOfFlowPosition();
+    bool isFloating = renderer.style().isFloating();
+    bool isOutOfFlowPositioned = renderer.style().hasOutOfFlowPosition();
+    bool startsAffectingParent = false;
+    bool noLongerAffectsParent = false;
+
+    if (is<RenderBlock>(parent))
+        noLongerAffectsParent = (!wasFloating && isFloating) || (!wasOufOfFlowPositioned && isOutOfFlowPositioned);
+
+    if (is<RenderBlockFlow>(parent) || is<RenderInline>(parent)) {
+        startsAffectingParent = (wasFloating || wasOufOfFlowPositioned) && !isFloating && !isOutOfFlowPositioned;
+        ASSERT(!startsAffectingParent || !noLongerAffectsParent);
+    }
+
+    if (startsAffectingParent) {
+        // We have gone from not affecting the inline status of the parent flow to suddenly
+        // having an impact. See if there is a mismatch between the parent flow's
+        // childrenInline() state and our state.
+        renderer.setInline(renderer.style().isDisplayInlineType());
+        if (renderer.isInline() != renderer.parent()->childrenInline())
+            childFlowStateChangesAndAffectsParentBlock(renderer);
+        return;
+    }
+
+    if (noLongerAffectsParent) {
+        childFlowStateChangesAndNoLongerAffectsParentBlock(renderer);
+
+        if (is<RenderBlockFlow>(renderer)) {
+            // Fresh floats need to be reparented if they actually belong to the previous anonymous block.
+            // It copies the logic of RenderBlock::addChildIgnoringContinuation
+            if (isFloating && renderer.previousSibling() && renderer.previousSibling()->isAnonymousBlock())
+                move(downcast<RenderBoxModelObject>(parent), downcast<RenderBoxModelObject>(*renderer.previousSibling()), renderer, RenderTreeBuilder::NormalizeAfterInsertion::No);
+        }
+    }
+}
+
 void RenderTreeBuilder::makeChildrenNonInline(RenderBlock& parent, RenderObject* insertionPoint)
 {
     // makeChildrenNonInline takes a block whose children are *all* inline and it

diff --git a/Source/WebCore/rendering/updating/RenderTreeBuilder.h b/Source/WebCore/rendering/updating/RenderTreeBuilder.h
@@ -54,15 +54,15 @@ class RenderTreeBuilder {
 
     void updateAfterDescendants(RenderElement&);
     void destroyAndCleanUpAnonymousWrappers(RenderObject& child);
+    void normalizeTreeAfterStyleChange(RenderElement&, RenderStyle& oldStyle);
 
 #if ENABLE(FULLSCREEN_API)
     void createPlaceholderForFullScreen(RenderFullScreen&, std::unique_ptr<RenderStyle>, const LayoutRect&);
 #endif
 
+private:
     void childFlowStateChangesAndAffectsParentBlock(RenderElement& child);
     void childFlowStateChangesAndNoLongerAffectsParentBlock(RenderElement& child);
-
-private:
     void attachIgnoringContinuation(RenderElement& parent, RenderPtr<RenderObject>, RenderObject* beforeChild = nullptr);
     void attachToRenderGrid(RenderGrid& parent, RenderPtr<RenderObject> child, RenderObject* beforeChild = nullptr);
     void attachToRenderElement(RenderElement& parent, RenderPtr<RenderObject> child, RenderObject* beforeChild = nullptr);

diff --git a/Source/WebCore/rendering/updating/RenderTreeUpdater.cpp b/Source/WebCore/rendering/updating/RenderTreeUpdater.cpp
@@ -294,6 +294,13 @@ static bool pseudoStyleCacheIsInvalid(RenderElement* renderer, RenderStyle* newS
     return false;
 }
 
+void RenderTreeUpdater::updateRendererStyle(RenderElement& renderer, RenderStyle&& newStyle, StyleDifference minimalStyleDifference)
+{
+    auto oldStyle = RenderStyle::clone(renderer.style());
+    renderer.setStyle(WTFMove(newStyle), minimalStyleDifference);
+    m_builder.normalizeTreeAfterStyleChange(renderer, oldStyle);
+}
+
 void RenderTreeUpdater::updateElementRenderer(Element& element, const Style::ElementUpdate& update)
 {
 #if PLATFORM(IOS)
@@ -335,19 +342,19 @@ void RenderTreeUpdater::updateElementRenderer(Element& element, const Style::Ele
     auto& renderer = *element.renderer();
 
     if (update.recompositeLayer) {
-        renderer.setStyle(RenderStyle::clone(*update.style), StyleDifferenceRecompositeLayer);
+        updateRendererStyle(renderer, RenderStyle::clone(*update.style), StyleDifferenceRecompositeLayer);
         return;
     }
 
     if (update.change == Style::NoChange) {
         if (pseudoStyleCacheIsInvalid(&renderer, update.style.get())) {
-            renderer.setStyle(RenderStyle::clone(*update.style), StyleDifferenceEqual);
+            updateRendererStyle(renderer, RenderStyle::clone(*update.style), StyleDifferenceEqual);
             return;
         }
         return;
     }
 
-    renderer.setStyle(RenderStyle::clone(*update.style), StyleDifferenceEqual);
+    updateRendererStyle(renderer, RenderStyle::clone(*update.style), StyleDifferenceEqual);
 }
 
 void RenderTreeUpdater::createRenderer(Element& element, RenderStyle&& style)

diff --git a/Source/WebCore/rendering/updating/RenderTreeUpdater.h b/Source/WebCore/rendering/updating/RenderTreeUpdater.h
@@ -58,6 +58,7 @@ class RenderTreeUpdater {
     void updateTextRenderer(Text&, const Style::TextUpdate*);
     void createTextRenderer(Text&, const Style::TextUpdate*);
     void updateElementRenderer(Element&, const Style::ElementUpdate&);
+    void updateRendererStyle(RenderElement&, RenderStyle&&, StyleDifference);
     void createRenderer(Element&, RenderStyle&&);
     void updateBeforeDescendants(Element&, const Style::ElementUpdates*);
     void updateAfterDescendants(Element&, const Style::ElementUpdates*);