Move continuation teardown from subclasses to RenderBoxModelObject.
authorakling@apple.com <akling@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 26 Oct 2014 20:20:31 +0000 (20:20 +0000)
committerakling@apple.com <akling@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 26 Oct 2014 20:20:31 +0000 (20:20 +0000)
<https://webkit.org/b/138081>

Reviewed by Antti Koivisto.

Let RenderBoxModelObject::willBeDestroyed() tear down any continuation
instead of having every subclass do this themselves.

Also added a RenderElement bit tracking whether the renderer has a
continuation. This avoids a hash lookup every time we destroy a
RenderBoxModelObject that didn't have a continuation.

* rendering/RenderBlock.cpp:
(WebCore::RenderBlock::willBeDestroyed):
* rendering/RenderBlockFlow.cpp:
(WebCore::RenderBlockFlow::willBeDestroyed):
* rendering/RenderBoxModelObject.cpp:
(WebCore::continuationMap):
(WebCore::RenderBoxModelObject::willBeDestroyed):
(WebCore::RenderBoxModelObject::continuation):
(WebCore::RenderBoxModelObject::setContinuation):
* rendering/RenderElement.cpp:
(WebCore::RenderElement::RenderElement):
* rendering/RenderElement.h:
(WebCore::RenderElement::setHasContinuation):
(WebCore::RenderElement::hasContinuation):
* rendering/RenderInline.cpp:
(WebCore::RenderInline::willBeDestroyed):

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

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

index 8845654..dcd4590 100644 (file)
@@ -1,3 +1,34 @@
+2014-10-26  Andreas Kling  <akling@apple.com>
+
+        Move continuation teardown from subclasses to RenderBoxModelObject.
+        <https://webkit.org/b/138081>
+
+        Reviewed by Antti Koivisto.
+
+        Let RenderBoxModelObject::willBeDestroyed() tear down any continuation
+        instead of having every subclass do this themselves.
+
+        Also added a RenderElement bit tracking whether the renderer has a
+        continuation. This avoids a hash lookup every time we destroy a
+        RenderBoxModelObject that didn't have a continuation.
+
+        * rendering/RenderBlock.cpp:
+        (WebCore::RenderBlock::willBeDestroyed):
+        * rendering/RenderBlockFlow.cpp:
+        (WebCore::RenderBlockFlow::willBeDestroyed):
+        * rendering/RenderBoxModelObject.cpp:
+        (WebCore::continuationMap):
+        (WebCore::RenderBoxModelObject::willBeDestroyed):
+        (WebCore::RenderBoxModelObject::continuation):
+        (WebCore::RenderBoxModelObject::setContinuation):
+        * rendering/RenderElement.cpp:
+        (WebCore::RenderElement::RenderElement):
+        * rendering/RenderElement.h:
+        (WebCore::RenderElement::setHasContinuation):
+        (WebCore::RenderElement::hasContinuation):
+        * rendering/RenderInline.cpp:
+        (WebCore::RenderInline::willBeDestroyed):
+
 2014-10-25  Benjamin Poulain  <benjamin@webkit.org>
 
         Remove a useless variable from SelectorCodeGenerator::generateElementMatchesSelectorList()
index 7de181c..fe50e94 100644 (file)
@@ -247,15 +247,6 @@ void RenderBlock::willBeDestroyed()
     // properly dirty line boxes that they are removed from. Effects that do :before/:after only on hover could crash otherwise.
     destroyLeftoverChildren();
 
-    // Destroy our continuation before anything other than anonymous children.
-    // The reason we don't destroy it before anonymous children is that they may
-    // have continuations of their own that are anonymous children of our continuation.
-    RenderBoxModelObject* continuation = this->continuation();
-    if (continuation) {
-        continuation->destroy();
-        setContinuation(0);
-    }
-    
     if (!documentBeingDestroyed()) {
         if (parent())
             parent()->dirtyLinesFromChangedChild(*this);
index cf05900..5e3c71c 100644 (file)
@@ -153,15 +153,6 @@ void RenderBlockFlow::willBeDestroyed()
     // properly dirty line boxes that they are removed from. Effects that do :before/:after only on hover could crash otherwise.
     destroyLeftoverChildren();
 
-    // Destroy our continuation before anything other than anonymous children.
-    // The reason we don't destroy it before anonymous children is that they may
-    // have continuations of their own that are anonymous children of our continuation.
-    RenderBoxModelObject* continuation = this->continuation();
-    if (continuation) {
-        continuation->destroy();
-        setContinuation(0);
-    }
-
     if (!documentBeingDestroyed()) {
         if (firstRootBox()) {
             // We can't wait for RenderBox::destroy to clear the selection,
index 4074c75..7b5a528 100644 (file)
@@ -55,6 +55,7 @@
 #include "ScrollingConstraints.h"
 #include "Settings.h"
 #include "TransformState.h"
+#include <wtf/NeverDestroyed.h>
 
 namespace WebCore {
 
@@ -68,7 +69,11 @@ using namespace HTMLNames;
 // <b><i><p>Hello</p></i></b>. In this example the <i> will have a block as
 // its continuation but the <b> will just have an inline as its continuation.
 typedef HashMap<const RenderBoxModelObject*, RenderBoxModelObject*> ContinuationMap;
-static ContinuationMap* continuationMap = 0;
+static ContinuationMap& continuationMap()
+{
+    static NeverDestroyed<ContinuationMap> map;
+    return map;
+}
 
 // This HashMap is similar to the continuation map, but connects first-letter
 // renderers to their remaining text fragments.
@@ -177,8 +182,10 @@ RenderBoxModelObject::~RenderBoxModelObject()
 
 void RenderBoxModelObject::willBeDestroyed()
 {
-    // A continuation of this RenderObject should be destroyed at subclasses.
-    ASSERT(!continuation());
+    if (hasContinuation()) {
+        continuation()->destroy();
+        setContinuation(nullptr);
+    }
 
     // If this is a first-letter object with a remaining text fragment then the
     // entry needs to be cleared from the map.
@@ -2529,21 +2536,18 @@ LayoutUnit RenderBoxModelObject::containingBlockLogicalWidthForContent() const
 
 RenderBoxModelObject* RenderBoxModelObject::continuation() const
 {
-    if (!continuationMap)
-        return 0;
-    return continuationMap->get(this);
+    if (!hasContinuation())
+        return nullptr;
+    return continuationMap().get(this);
 }
 
 void RenderBoxModelObject::setContinuation(RenderBoxModelObject* continuation)
 {
-    if (continuation) {
-        if (!continuationMap)
-            continuationMap = new ContinuationMap;
-        continuationMap->set(this, continuation);
-    } else {
-        if (continuationMap)
-            continuationMap->remove(this);
-    }
+    if (continuation)
+        continuationMap().set(this, continuation);
+    else if (hasContinuation())
+        continuationMap().remove(this);
+    setHasContinuation(!!continuation);
 }
 
 RenderTextFragment* RenderBoxModelObject::firstLetterRemainingText() const
index d995266..dea813f 100644 (file)
@@ -87,6 +87,7 @@ inline RenderElement::RenderElement(ContainerNode& elementOrDocument, PassRef<Re
     , m_hasPausedImageAnimations(false)
     , m_hasCounterNodeMap(false)
     , m_isCSSAnimating(false)
+    , m_hasContinuation(false)
     , m_firstChild(nullptr)
     , m_lastChild(nullptr)
     , m_style(WTF::move(style))
index e236ddd..7052863 100644 (file)
@@ -198,6 +198,9 @@ protected:
     void setRenderInlineAlwaysCreatesLineBoxes(bool b) { m_renderInlineAlwaysCreatesLineBoxes = b; }
     bool renderInlineAlwaysCreatesLineBoxes() const { return m_renderInlineAlwaysCreatesLineBoxes; }
 
+    void setHasContinuation(bool b) { m_hasContinuation = b; }
+    bool hasContinuation() const { return m_hasContinuation; }
+
     static bool hasControlStatesForRenderer(const RenderObject*);
     static ControlStates* controlStatesForRenderer(const RenderObject*);
     static void removeControlStatesForRenderer(const RenderObject*);
@@ -237,6 +240,7 @@ private:
     unsigned m_hasPausedImageAnimations : 1;
     unsigned m_hasCounterNodeMap : 1;
     unsigned m_isCSSAnimating : 1;
+    unsigned m_hasContinuation : 1;
 
     RenderObject* m_firstChild;
     RenderObject* m_lastChild;
index ebda99c..944e0a4 100644 (file)
@@ -81,15 +81,6 @@ void RenderInline::willBeDestroyed()
     // Make sure to destroy anonymous children first while they are still connected to the rest of the tree, so that they will
     // properly dirty line boxes that they are removed from.  Effects that do :before/:after only on hover could crash otherwise.
     destroyLeftoverChildren();
-
-    // Destroy our continuation before anything other than anonymous children.
-    // The reason we don't destroy it before anonymous children is that they may
-    // have continuations of their own that are anonymous children of our continuation.
-    RenderBoxModelObject* continuation = this->continuation();
-    if (continuation) {
-        continuation->destroy();
-        setContinuation(nullptr);
-    }
     
     if (!documentBeingDestroyed()) {
         if (firstLineBox()) {