RenderFlowThread's containing block cache should be invalidated before calling styleD...
authorzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 11 Nov 2016 21:20:01 +0000 (21:20 +0000)
committerzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 11 Nov 2016 21:20:01 +0000 (21:20 +0000)
https://bugs.webkit.org/show_bug.cgi?id=164646

Reviewed by Simon Fraser.

We have to invalidate the containing block cache for RenderFlowThreads soon after the containing block context
changes. Invalidating it in RenderBlock::styleDidChange is too late since we might run some code in some
of the subclasses that use this stale containing block cache.

No known behaviour change.

* rendering/RenderBlock.cpp:
(WebCore::RenderBlock::styleDidChange): This change could trigger double invalidation.
However running this code twice shouldn't impact performance greatly.
(WebCore::RenderBlock::resetFlowThreadContainingBlockAndChildInfoIncludingDescendants):
(WebCore::RenderBlock::invalidateFlowThreadContainingBlockIncludingDescendants): Deleted.
* rendering/RenderBlock.h:
* rendering/RenderElement.cpp:
(WebCore::RenderElement::setStyle): We don't need to call the invalidation from initializeStyle(), since
we don't yet have cache at that point.
* rendering/RenderInline.cpp:
(WebCore::RenderInline::splitInlines):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@208605 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderBlock.cpp
Source/WebCore/rendering/RenderBlock.h
Source/WebCore/rendering/RenderElement.cpp
Source/WebCore/rendering/RenderInline.cpp

index 9d62ac6..ecca0fb 100644 (file)
@@ -1,3 +1,28 @@
+2016-11-11  Zalan Bujtas  <zalan@apple.com>
+
+        RenderFlowThread's containing block cache should be invalidated before calling styleDidChange.
+        https://bugs.webkit.org/show_bug.cgi?id=164646
+
+        Reviewed by Simon Fraser.
+
+        We have to invalidate the containing block cache for RenderFlowThreads soon after the containing block context
+        changes. Invalidating it in RenderBlock::styleDidChange is too late since we might run some code in some
+        of the subclasses that use this stale containing block cache.  
+
+        No known behaviour change.
+
+        * rendering/RenderBlock.cpp:
+        (WebCore::RenderBlock::styleDidChange): This change could trigger double invalidation.
+        However running this code twice shouldn't impact performance greatly.
+        (WebCore::RenderBlock::resetFlowThreadContainingBlockAndChildInfoIncludingDescendants):
+        (WebCore::RenderBlock::invalidateFlowThreadContainingBlockIncludingDescendants): Deleted.
+        * rendering/RenderBlock.h:
+        * rendering/RenderElement.cpp:
+        (WebCore::RenderElement::setStyle): We don't need to call the invalidation from initializeStyle(), since
+        we don't yet have cache at that point.
+        * rendering/RenderInline.cpp:
+        (WebCore::RenderInline::splitInlines):
+
 2016-11-11  Darin Adler  <darin@apple.com>
 
         Move Node from ExceptionCode to ExceptionOr
index 87901c8..306e47a 100644 (file)
@@ -423,20 +423,13 @@ static bool borderOrPaddingLogicalWidthChanged(const RenderStyle* oldStyle, cons
 
 void RenderBlock::styleDidChange(StyleDifference diff, const RenderStyle* oldStyle)
 {
-    auto& newStyle = style();
-
     bool hadTransform = hasTransform();
-    bool flowThreadContainingBlockInvalidated = false;
-    if (oldStyle && oldStyle->position() != newStyle.position()) {
-        invalidateFlowThreadContainingBlockIncludingDescendants();
-        flowThreadContainingBlockInvalidated = true;
-    }
-
     RenderBox::styleDidChange(diff, oldStyle);
 
-    if (hadTransform != hasTransform() && !flowThreadContainingBlockInvalidated)
-        invalidateFlowThreadContainingBlockIncludingDescendants();
+    if (hadTransform != hasTransform())
+        resetFlowThreadContainingBlockAndChildInfoIncludingDescendants();
 
+    auto& newStyle = style();
     if (!isAnonymousBlock()) {
         // Ensure that all of our continuation blocks pick up the new style.
         for (RenderBlock* currCont = blockElementContinuation(); currCont; currCont = currCont->blockElementContinuation()) {
@@ -3382,7 +3375,7 @@ RenderFlowThread* RenderBlock::locateFlowThreadContainingBlock() const
     return rareData->m_flowThreadContainingBlock.value();
 }
 
-void RenderBlock::invalidateFlowThreadContainingBlockIncludingDescendants()
+void RenderBlock::resetFlowThreadContainingBlockAndChildInfoIncludingDescendants()
 {
     if (flowThreadState() == NotInsideFlowThread)
         return;
@@ -3400,7 +3393,7 @@ void RenderBlock::invalidateFlowThreadContainingBlockIncludingDescendants()
         if (flowThread)
             flowThread->removeFlowChildInfo(child);
         if (is<RenderBlock>(child))
-            downcast<RenderBlock>(child).invalidateFlowThreadContainingBlockIncludingDescendants();
+            downcast<RenderBlock>(child).resetFlowThreadContainingBlockAndChildInfoIncludingDescendants();
     }
 }
 
index c3199b0..5dca6e9 100644 (file)
@@ -307,7 +307,7 @@ public:
     RenderFlowThread* cachedFlowThreadContainingBlock() const;
     void setCachedFlowThreadContainingBlockNeedsUpdate();
     virtual bool cachedFlowThreadContainingBlockNeedsUpdate() const;
-    void invalidateFlowThreadContainingBlockIncludingDescendants();
+    void resetFlowThreadContainingBlockAndChildInfoIncludingDescendants();
 
 protected:
     RenderFlowThread* locateFlowThreadContainingBlock() const override;
index 4420750..99fdcec 100644 (file)
@@ -408,6 +408,12 @@ void RenderElement::setStyle(RenderStyle&& style, StyleDifference minimalStyleDi
     auto oldStyle = WTFMove(m_style);
     m_style = WTFMove(style);
     bool detachedFromParent = !parent();
+
+    // Make sure we invalidate the containing block cache for flows when the contianing block context changes
+    // so that styleDidChange can safely use RenderBlock::locateFlowThreadContainingBlock()
+    if (is<RenderBlock>(*this) && oldStyle.position() != m_style.position())
+        downcast<RenderBlock>(*this).resetFlowThreadContainingBlockAndChildInfoIncludingDescendants();
+
     styleDidChange(diff, &oldStyle);
 
     // Text renderers use their parent style. Notify them about the change.
index bae227e..41cd138 100644 (file)
@@ -523,7 +523,7 @@ void RenderInline::splitInlines(RenderBlock* fromBlock, RenderBlock* toBlock,
 
     // Clear the flow thread containing blocks cached during the detached state insertions.
     for (auto& cloneBlockChild : childrenOfType<RenderBlock>(*cloneInline))
-        cloneBlockChild.invalidateFlowThreadContainingBlockIncludingDescendants();
+        cloneBlockChild.resetFlowThreadContainingBlockAndChildInfoIncludingDescendants();
 
     // Now we are at the block level. We need to put the clone into the toBlock.
     toBlock->insertChildInternal(cloneInline.leakPtr(), nullptr, NotifyChildren);