Changeset 172730 in webkit

Timestamp:

Aug 18, 2014 3:16:47 PM (10 years ago)

Author:

Antti Koivisto

Message:

Tighten RenderCounter typing
​https://bugs.webkit.org/show_bug.cgi?id=136049

Reviewed by Andreas Kling.

RenderObject* -> RenderElement&

• rendering/CounterNode.cpp:

(WebCore::CounterNode::CounterNode):
(WebCore::CounterNode::create):
(WebCore::showTreeAndMark):

• rendering/CounterNode.h:

(WebCore::CounterNode::owner):

• rendering/RenderCounter.cpp:

(WebCore::previousInPreOrder):
(WebCore::parentOrPseudoHostElement):
(WebCore::previousSiblingOrParent):
(WebCore::areRenderersElementsSiblings):
(WebCore::nextInPreOrder):
(WebCore::planCounter):
(WebCore::findPlaceForCounter):
(WebCore::makeCounterNode):
(WebCore::RenderCounter::originalText):
(WebCore::destroyCounterNodeWithoutMapRemoval):
(WebCore::RenderCounter::destroyCounterNodes):
(WebCore::RenderCounter::destroyCounterNode):
(WebCore::RenderCounter::rendererRemovedFromTree):
(WebCore::updateCounters):
(WebCore::RenderCounter::rendererSubtreeAttached):
(WebCore::RenderCounter::rendererStyleChanged):
(showCounterRendererTree):

• rendering/RenderCounter.h:
• rendering/RenderElement.cpp:

(WebCore::RenderElement::RenderElement):
(WebCore::RenderElement::insertChildInternal):
(WebCore::RenderElement::removeChildInternal):
(WebCore::RenderElement::styleDidChange):
(WebCore::RenderElement::willBeDestroyed):

• rendering/RenderElement.h:

(WebCore::RenderElement::hasCounterNodeMap):
(WebCore::RenderElement::setHasCounterNodeMap):

Move CounterNodeMap to RenderElement from RenderObject.

• rendering/RenderObject.cpp:

(WebCore::RenderObject::willBeDestroyed):

• rendering/RenderObject.h:

(WebCore::RenderObject::RenderObjectBitfields::RenderObjectBitfields):
(WebCore::RenderObject::hasCounterNodeMap): Deleted.
(WebCore::RenderObject::setHasCounterNodeMap): Deleted.

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• rendering/CounterNode.cpp (modified) (3 diffs)
• rendering/CounterNode.h (modified) (4 diffs)
• rendering/RenderCounter.cpp (modified) (30 diffs)
• rendering/RenderCounter.h (modified) (1 diff)
• rendering/RenderElement.cpp (modified) (5 diffs)
• rendering/RenderElement.h (modified) (2 diffs)
• rendering/RenderObject.cpp (modified) (1 diff)
• rendering/RenderObject.h (modified) (4 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2014-08-18  Antti Koivisto  <antti@apple.com>
+
+        Tighten RenderCounter typing
+        https://bugs.webkit.org/show_bug.cgi?id=136049
+
+        Reviewed by Andreas Kling.
+
+        RenderObject* -> RenderElement&
+
+        * rendering/CounterNode.cpp:
+        (WebCore::CounterNode::CounterNode):
+        (WebCore::CounterNode::create):
+        (WebCore::showTreeAndMark):
+        * rendering/CounterNode.h:
+        (WebCore::CounterNode::owner):
+        * rendering/RenderCounter.cpp:
+        (WebCore::previousInPreOrder):
+        (WebCore::parentOrPseudoHostElement):
+        (WebCore::previousSiblingOrParent):
+        (WebCore::areRenderersElementsSiblings):
+        (WebCore::nextInPreOrder):
+        (WebCore::planCounter):
+        (WebCore::findPlaceForCounter):
+        (WebCore::makeCounterNode):
+        (WebCore::RenderCounter::originalText):
+        (WebCore::destroyCounterNodeWithoutMapRemoval):
+        (WebCore::RenderCounter::destroyCounterNodes):
+        (WebCore::RenderCounter::destroyCounterNode):
+        (WebCore::RenderCounter::rendererRemovedFromTree):
+        (WebCore::updateCounters):
+        (WebCore::RenderCounter::rendererSubtreeAttached):
+        (WebCore::RenderCounter::rendererStyleChanged):
+        (showCounterRendererTree):
+        * rendering/RenderCounter.h:
+        * rendering/RenderElement.cpp:
+        (WebCore::RenderElement::RenderElement):
+        (WebCore::RenderElement::insertChildInternal):
+        (WebCore::RenderElement::removeChildInternal):
+        (WebCore::RenderElement::styleDidChange):
+        (WebCore::RenderElement::willBeDestroyed):
+        * rendering/RenderElement.h:
+        (WebCore::RenderElement::hasCounterNodeMap):
+        (WebCore::RenderElement::setHasCounterNodeMap):
+        
+            Move CounterNodeMap to RenderElement from RenderObject.
+
+        * rendering/RenderObject.cpp:
+        (WebCore::RenderObject::willBeDestroyed):
+        * rendering/RenderObject.h:
+        (WebCore::RenderObject::RenderObjectBitfields::RenderObjectBitfields):
+        (WebCore::RenderObject::hasCounterNodeMap): Deleted.
+        (WebCore::RenderObject::setHasCounterNodeMap): Deleted.
+
 2014-08-18  Peyton Randolph  <prandolph@apple.com>
 

trunk/Source/WebCore/rendering/CounterNode.cpp

@@ -29 +29 @@
 namespace WebCore {
 
-CounterNode::CounterNode(RenderElement* o, bool hasResetType, int value)
+CounterNode::CounterNode(RenderElement& owner, bool hasResetType, int value)
     : m_hasResetType(hasResetType)
     , m_value(value)
     , m_countInParent(0)
-    , m_owner(o)
+    , m_owner(owner)
     , m_rootRenderer(0)
     , m_parent(0)
@@ -91 +91 @@
 }
 
-PassRefPtr<CounterNode> CounterNode::create(RenderElement* owner, bool hasResetType, int value)
+PassRefPtr<CounterNode> CounterNode::create(RenderElement& owner, bool hasResetType, int value)
 {
     return adoptRef(new CounterNode(owner, hasResetType, value));
@@ -366 +366 @@
             current, current->actsAsReset() ? "reset____" : "increment", current->value(),
             current->countInParent(), current->parent(), current->previousSibling(),
-            current->nextSibling(), current->owner());
+            current->nextSibling(), &current->owner());
     }
     fflush(stderr);

trunk/Source/WebCore/rendering/CounterNode.h

@@ -43 +43 @@
 class CounterNode : public RefCounted<CounterNode> {
 public:
-    static PassRefPtr<CounterNode> create(RenderElement*, bool isReset, int value);
+    static PassRefPtr<CounterNode> create(RenderElement&, bool isReset, int value);
     ~CounterNode();
     bool actsAsReset() const { return m_hasResetType || !m_parent; }
@@ -49 +49 @@
     int value() const { return m_value; }
     int countInParent() const { return m_countInParent; }
-    RenderElement* owner() const { return m_owner; }
+    RenderElement& owner() const { return m_owner; }
     void addRenderer(RenderCounter*);
     void removeRenderer(RenderCounter*);
@@ -72 +72 @@
 
 private:
-    CounterNode(RenderElement*, bool isReset, int value);
+    CounterNode(RenderElement&, bool isReset, int value);
     int computeCountInParent() const;
     // Invalidates the text in the renderer of this counter, if any,
@@ -82 +82 @@
     int m_value;
     int m_countInParent;
-    RenderElement* m_owner;
+    RenderElement& m_owner;
     RenderCounter* m_rootRenderer;
 

trunk/Source/WebCore/rendering/RenderCounter.cpp

@@ -45 +45 @@
 
 typedef HashMap<AtomicString, RefPtr<CounterNode>> CounterMap;
-typedef HashMap<const RenderObject*, std::unique_ptr<CounterMap>> CounterMaps;
-
-static CounterNode* makeCounterNode(RenderObject*, const AtomicString& identifier, bool alwaysCreateCounter);
+typedef HashMap<const RenderElement*, std::unique_ptr<CounterMap>> CounterMaps;
+
+static CounterNode* makeCounterNode(RenderElement&, const AtomicString& identifier, bool alwaysCreateCounter);
 
 static CounterMaps& counterMaps()
@@ -57 +57 @@
 // This function processes the renderer tree in the order of the DOM tree
 // including pseudo elements as defined in CSS 2.1.
-static RenderObject* previousInPreOrder(const RenderObject* object)
-{
-    Element* self = toElement(object->node());
-    Element* previous = ElementTraversal::previousIncludingPseudo(self);
+static RenderElement* previousInPreOrder(const RenderElement& renderer)
+{
+    Element* previous = ElementTraversal::previousIncludingPseudo(renderer.element());
     while (previous && !previous->renderer())
         previous = ElementTraversal::previousIncludingPseudo(previous);
@@ -66 +65 @@
 }
 
-static inline Element* parentOrPseudoHostElement(const RenderObject* object)
-{
-    if (object->node()->isPseudoElement())
-        return toPseudoElement(object->node())->hostElement();
-    return toElement(object->node())->parentElement();
+static inline Element* parentOrPseudoHostElement(const RenderElement& renderer)
+{
+    if (renderer.isPseudoElement())
+        return renderer.generatingElement();
+    return renderer.element() ? renderer.element()->parentElement() : nullptr;
 }
 
 // This function processes the renderer tree in the order of the DOM tree
 // including pseudo elements as defined in CSS 2.1.
-static RenderObject* previousSiblingOrParent(const RenderObject* object)
-{
-    Element* self = toElement(object->node());
-    Element* previous = ElementTraversal::pseudoAwarePreviousSibling(self);
+static RenderElement* previousSiblingOrParent(const RenderElement& renderer)
+{
+    Element* previous = ElementTraversal::pseudoAwarePreviousSibling(renderer.element());
     while (previous && !previous->renderer())
         previous = ElementTraversal::pseudoAwarePreviousSibling(previous);
     if (previous)
         return previous->renderer();
-    previous = parentOrPseudoHostElement(object);
-    return previous ? previous->renderer() : 0;
-}
-
-static inline bool areRenderersElementsSiblings(RenderObject* first, RenderObject* second)
+    previous = parentOrPseudoHostElement(renderer);
+    return previous ? previous->renderer() : nullptr;
+}
+
+static inline bool areRenderersElementsSiblings(const RenderElement& first, const RenderElement& second)
 {
     return parentOrPseudoHostElement(first) == parentOrPseudoHostElement(second);
@@ -94 +92 @@
 // This function processes the renderer tree in the order of the DOM tree
 // including pseudo elements as defined in CSS 2.1.
-static RenderElement* nextInPreOrder(const RenderElement* element, const Element* stayWithin, bool skipDescendants = false)
-{
-    Element* self = element->element();
+static RenderElement* nextInPreOrder(const RenderElement& renderer, const Element* stayWithin, bool skipDescendants = false)
+{
+    Element* self = renderer.element();
     Element* next = skipDescendants ? ElementTraversal::nextIncludingPseudoSkippingChildren(self, stayWithin) : ElementTraversal::nextIncludingPseudo(self, stayWithin);
     while (next && !next->renderer())
@@ -103 +101 @@
 }
 
-static bool planCounter(RenderElement* object, const AtomicString& identifier, bool& isReset, int& value)
-{
-    ASSERT(object);
-
+static bool planCounter(RenderElement& renderer, const AtomicString& identifier, bool& isReset, int& value)
+{
     // We must have a generating node or else we cannot have a counter.
-    Element* generatingElement = object->generatingElement();
+    Element* generatingElement = renderer.generatingElement();
     if (!generatingElement)
         return false;
 
-    const RenderStyle& style = object->style();
+    const RenderStyle& style = renderer.style();
 
     switch (style.styleType()) {
@@ -118 +114 @@
         // Sometimes elements have more then one renderer. Only the first one gets the counter
         // LayoutTests/http/tests/css/counter-crash.html
-        if (generatingElement->renderer() != object)
+        if (generatingElement->renderer() != &renderer)
             return false;
         break;
@@ -136 +132 @@
 
     if (identifier == "list-item") {
-        if (object->isListItem()) {
-            if (toRenderListItem(object)->hasExplicitValue()) {
-                value = toRenderListItem(object)->explicitValue();
+        if (renderer.isListItem()) {
+            if (toRenderListItem(renderer).hasExplicitValue()) {
+                value = toRenderListItem(renderer).explicitValue();
                 isReset = true;
                 return true;
@@ -146 +142 @@
             return true;
         }
-        if (Element* e = object->element()) {
-            if (e->hasTagName(olTag)) {
-                value = toHTMLOListElement(e)->start();
+        if (Element* element = renderer.element()) {
+            if (element->hasTagName(olTag)) {
+                value = toHTMLOListElement(element)->start();
                 isReset = true;
                 return true;
             }
-            if (e->hasTagName(ulTag) || e->hasTagName(menuTag) || e->hasTagName(dirTag)) {
+            if (element->hasTagName(ulTag) || element->hasTagName(menuTag) || element->hasTagName(dirTag)) {
                 value = 0;
                 isReset = true;
@@ -179 +175 @@
 // - Non-reset CounterNodes cannot have descendants.
 
-static bool findPlaceForCounter(RenderObject* counterOwner, const AtomicString& identifier, bool isReset, RefPtr<CounterNode>& parent, RefPtr<CounterNode>& previousSibling)
+static bool findPlaceForCounter(RenderElement& counterOwner, const AtomicString& identifier, bool isReset, RefPtr<CounterNode>& parent, RefPtr<CounterNode>& previousSibling)
 {
     // We cannot stop searching for counters with the same identifier before we also
     // check this renderer, because it may affect the positioning in the tree of our counter.
-    RenderObject* searchEndRenderer = previousSiblingOrParent(counterOwner);
+    RenderElement* searchEndRenderer = previousSiblingOrParent(counterOwner);
     // We check renderers in preOrder from the renderer that our counter is attached to
     // towards the begining of the document for counters with the same identifier as the one
     // we are trying to find a place for. This is the next renderer to be checked.
-    RenderObject* currentRenderer = previousInPreOrder(counterOwner);
+    RenderElement* currentRenderer = previousInPreOrder(counterOwner);
     previousSibling = 0;
     RefPtr<CounterNode> previousSiblingProtector = 0;
 
     while (currentRenderer) {
-        CounterNode* currentCounter = makeCounterNode(currentRenderer, identifier, false);
+        CounterNode* currentCounter = makeCounterNode(*currentRenderer, identifier, false);
         if (searchEndRenderer == currentRenderer) {
             // We may be at the end of our search.
@@ -200 +196 @@
                     if (currentCounter->actsAsReset()) {
                         // We found a reset counter that is on a renderer that is a sibling of ours or a parent.
-                        if (isReset && areRenderersElementsSiblings(currentRenderer, counterOwner)) {
+                        if (isReset && areRenderersElementsSiblings(*currentRenderer, counterOwner)) {
                             // We are also a reset counter and the previous reset was on a sibling renderer
                             // hence we are the next sibling of that counter if that reset is not a root or
@@ -221 +217 @@
                     }
                     // CurrentCounter, the counter at the EndSearchRenderer, is not reset.
-                    if (!isReset || !areRenderersElementsSiblings(currentRenderer, counterOwner)) {
+                    if (!isReset || !areRenderersElementsSiblings(*currentRenderer, counterOwner)) {
                         // If the node we are placing is not reset or we have found a counter that is attached
                         // to an ancestor of the placed counter's owner renderer we know we are a sibling of that node.
@@ -237 +233 @@
                     // to currentCounter.
                     if (currentCounter->actsAsReset()) {
-                        if (isReset && areRenderersElementsSiblings(currentRenderer, counterOwner)) {
+                        if (isReset && areRenderersElementsSiblings(*currentRenderer, counterOwner)) {
                             parent = currentCounter->parent();
                             previousSibling = currentCounter;
@@ -246 +242 @@
                         return true;
                     }
-                    if (!isReset || !areRenderersElementsSiblings(currentRenderer, counterOwner)) {
+                    if (!isReset || !areRenderersElementsSiblings(*currentRenderer, counterOwner)) {
                         parent = currentCounter->parent();
                         previousSibling = currentCounter;
@@ -258 +254 @@
             // of our owner renderer was not a reset counter.
             // Set a new goal for the end of the search.
-            searchEndRenderer = previousSiblingOrParent(currentRenderer);
+            searchEndRenderer = previousSiblingOrParent(*currentRenderer);
         } else {
             // We are searching descendants of a previous sibling of the renderer that the
@@ -271 +267 @@
                         // We are no longer interested in previous siblings of the currentRenderer or their children
                         // as counters they may have attached cannot be the previous sibling of the counter we are placing.
-                        currentRenderer = parentOrPseudoHostElement(currentRenderer)->renderer();
+                        currentRenderer = parentOrPseudoHostElement(*currentRenderer)->renderer();
                         continue;
                     }
                 } else
                     previousSiblingProtector = currentCounter;
-                currentRenderer = previousSiblingOrParent(currentRenderer);
+                currentRenderer = previousSiblingOrParent(*currentRenderer);
                 continue;
             }
@@ -285 +281 @@
         // impede the readability of this already complex algorithm.
         if (previousSiblingProtector)
-            currentRenderer = previousSiblingOrParent(currentRenderer);
+            currentRenderer = previousSiblingOrParent(*currentRenderer);
         else
-            currentRenderer = previousInPreOrder(currentRenderer);
+            currentRenderer = previousInPreOrder(*currentRenderer);
     }
     return false;
 }
 
-static CounterNode* makeCounterNode(RenderObject* object, const AtomicString& identifier, bool alwaysCreateCounter)
-{
-    ASSERT(object);
-
-    // Real text nodes don't have their own style so they can't have counters.
-    // We can't even look at their styles or we'll see extra resets and increments!
-    if (object->isText())
-        return nullptr;
-
-    RenderElement* element = toRenderElement(object);
-
-    if (element->hasCounterNodeMap()) {
-        if (CounterMap* nodeMap = counterMaps().get(element)) {
+static CounterNode* makeCounterNode(RenderElement& renderer, const AtomicString& identifier, bool alwaysCreateCounter)
+{
+    if (renderer.hasCounterNodeMap()) {
+        if (CounterMap* nodeMap = counterMaps().get(&renderer)) {
             if (CounterNode* node = nodeMap->get(identifier))
                 return node;
@@ -312 +299 @@
     bool isReset = false;
     int value = 0;
-    if (!planCounter(element, identifier, isReset, value) && !alwaysCreateCounter)
+    if (!planCounter(renderer, identifier, isReset, value) && !alwaysCreateCounter)
         return nullptr;
 
     RefPtr<CounterNode> newParent = 0;
     RefPtr<CounterNode> newPreviousSibling = 0;
-    RefPtr<CounterNode> newNode = CounterNode::create(element, isReset, value);
-    if (findPlaceForCounter(element, identifier, isReset, newParent, newPreviousSibling))
+    RefPtr<CounterNode> newNode = CounterNode::create(renderer, isReset, value);
+    if (findPlaceForCounter(renderer, identifier, isReset, newParent, newPreviousSibling))
         newParent->insertAfter(newNode.get(), newPreviousSibling.get(), identifier);
     CounterMap* nodeMap;
-    if (element->hasCounterNodeMap())
-        nodeMap = counterMaps().get(element);
+    if (renderer.hasCounterNodeMap())
+        nodeMap = counterMaps().get(&renderer);
     else {
         nodeMap = new CounterMap;
-        counterMaps().set(element, std::unique_ptr<CounterMap>(nodeMap));
-        element->setHasCounterNodeMap(true);
+        counterMaps().set(&renderer, std::unique_ptr<CounterMap>(nodeMap));
+        renderer.setHasCounterNodeMap(true);
     }
     nodeMap->set(identifier, newNode);
@@ -334 +321 @@
     // should become children of this node now.
     CounterMaps& maps = counterMaps();
-    Element* stayWithin = parentOrPseudoHostElement(element);
+    Element* stayWithin = parentOrPseudoHostElement(renderer);
     bool skipDescendants;
-    for (RenderElement* currentRenderer = nextInPreOrder(element, stayWithin); currentRenderer; currentRenderer = nextInPreOrder(currentRenderer, stayWithin, skipDescendants)) {
+    for (RenderElement* currentRenderer = nextInPreOrder(renderer, stayWithin); currentRenderer; currentRenderer = nextInPreOrder(*currentRenderer, stayWithin, skipDescendants)) {
         skipDescendants = false;
         if (!currentRenderer->hasCounterNodeMap())
@@ -346 +333 @@
         if (currentCounter->parent())
             continue;
-        if (stayWithin == parentOrPseudoHostElement(currentRenderer) && currentCounter->hasResetType())
+        if (stayWithin == parentOrPseudoHostElement(*currentRenderer) && currentCounter->hasResetType())
             break;
         newNode->insertAfter(currentCounter, newNode->lastChild(), identifier);
@@ -400 +387 @@
             beforeAfterContainer = beforeAfterContainer->parent();
         }
-        makeCounterNode(beforeAfterContainer, m_counter.identifier(), true)->addRenderer(const_cast<RenderCounter*>(this));
+        makeCounterNode(*beforeAfterContainer, m_counter.identifier(), true)->addRenderer(const_cast<RenderCounter*>(this));
         ASSERT(m_counterNode);
     }
@@ -457 +444 @@
         previous = child->previousInPreOrder();
         child->parent()->removeChild(child.get());
-        ASSERT(counterMaps().get(child->owner())->get(identifier) == child);
-        counterMaps().get(child->owner())->remove(identifier);
+        ASSERT(counterMaps().get(&child->owner())->get(identifier) == child);
+        counterMaps().get(&child->owner())->remove(identifier);
     }
     if (CounterNode* parent = node->parent())
@@ -464 +451 @@
 }
 
-void RenderCounter::destroyCounterNodes(RenderObject* owner)
+void RenderCounter::destroyCounterNodes(RenderElement& owner)
 {
     CounterMaps& maps = counterMaps();
-    CounterMaps::iterator mapsIterator = maps.find(owner);
+    CounterMaps::iterator mapsIterator = maps.find(&owner);
     if (mapsIterator == maps.end())
         return;
@@ -476 +463 @@
     }
     maps.remove(mapsIterator);
-    owner->setHasCounterNodeMap(false);
-}
-
-void RenderCounter::destroyCounterNode(RenderObject* owner, const AtomicString& identifier)
-{
-    CounterMap* map = counterMaps().get(owner);
+    owner.setHasCounterNodeMap(false);
+}
+
+void RenderCounter::destroyCounterNode(RenderElement& owner, const AtomicString& identifier)
+{
+    CounterMap* map = counterMaps().get(&owner);
     if (!map)
         return;
@@ -502 +489 @@
 }
 
-void RenderCounter::rendererRemovedFromTree(RenderObject& renderer)
+void RenderCounter::rendererRemovedFromTree(RenderElement& renderer)
 {
     if (!renderer.view().hasRenderCounters())
@@ -510 +497 @@
         currentRenderer = &renderer;
     while (true) {
-        destroyCounterNodes(currentRenderer);
+        if (currentRenderer->isRenderElement())
+            destroyCounterNodes(toRenderElement(*currentRenderer));
         if (currentRenderer == &renderer)
             break;
@@ -517 +505 @@
 }
 
-static void updateCounters(RenderObject* renderer)
-{
-    const CounterDirectiveMap* directiveMap = renderer->style().counterDirectives();
+static void updateCounters(RenderElement& renderer)
+{
+    const CounterDirectiveMap* directiveMap = renderer.style().counterDirectives();
     if (!directiveMap)
         return;
     CounterDirectiveMap::const_iterator end = directiveMap->end();
-    if (!renderer->hasCounterNodeMap()) {
+    if (!renderer.hasCounterNodeMap()) {
         for (CounterDirectiveMap::const_iterator it = directiveMap->begin(); it != end; ++it)
             makeCounterNode(renderer, it->key, false);
         return;
     }
-    CounterMap* counterMap = counterMaps().get(renderer);
+    CounterMap* counterMap = counterMaps().get(&renderer);
     ASSERT(counterMap);
     for (CounterDirectiveMap::const_iterator it = directiveMap->begin(); it != end; ++it) {
@@ -552 +540 @@
 }
 
-void RenderCounter::rendererSubtreeAttached(RenderObject* renderer)
-{
-    if (!renderer->view().hasRenderCounters())
-        return;
-    Node* node = renderer->node();
-    if (node && !node->isPseudoElement())
-        node = node->parentNode();
+void RenderCounter::rendererSubtreeAttached(RenderElement& renderer)
+{
+    if (!renderer.view().hasRenderCounters())
+        return;
+    Element* element = renderer.element();
+    if (element && !element->isPseudoElement())
+        element = element->parentElement();
     else
-        node = renderer->generatingNode();
-    if (node && !node->renderer())
+        element = renderer.generatingElement();
+    if (element && !element->renderer())
         return; // No need to update if the parent is not attached yet
-    for (RenderObject* descendant = renderer; descendant; descendant = descendant->nextInPreOrder(renderer))
-        updateCounters(descendant);
-}
-
-void RenderCounter::rendererStyleChanged(RenderObject* renderer, const RenderStyle* oldStyle, const RenderStyle* newStyle)
-{
-    Node* node = renderer->generatingNode();
-    if (!node || !node->renderer())
+    for (RenderObject* descendant = &renderer; descendant; descendant = descendant->nextInPreOrder(&renderer)) {
+        if (descendant->isRenderElement())
+            updateCounters(toRenderElement(*descendant));
+    }
+}
+
+void RenderCounter::rendererStyleChanged(RenderElement& renderer, const RenderStyle* oldStyle, const RenderStyle* newStyle)
+{
+    Element* element = renderer.generatingElement();
+    if (!element || !element->renderer())
         return; // cannot have generated content or if it can have, it will be handled during attaching
     const CounterDirectiveMap* newCounterDirectives;
@@ -596 +586 @@
             }
         } else {
-            if (renderer->hasCounterNodeMap())
+            if (renderer.hasCounterNodeMap())
                 RenderCounter::destroyCounterNodes(renderer);
         }
@@ -624 +614 @@
     AtomicString identifier(counterName);
     for (const WebCore::RenderObject* current = root; current; current = current->nextInPreOrder()) {
+        if (!current->isRenderElement())
+            continue;
         fprintf(stderr, "%c", (current == renderer) ? '*' : ' ');
         for (const WebCore::RenderObject* parent = current; parent && parent != root; parent = parent->parent())
@@ -629 +621 @@
         fprintf(stderr, "%p N:%p P:%p PS:%p NS:%p C:%p\n",
             current, current->node(), current->parent(), current->previousSibling(),
-            current->nextSibling(), current->hasCounterNodeMap() ?
-            counterName ? WebCore::counterMaps().get(current)->get(identifier) : (WebCore::CounterNode*)1 : (WebCore::CounterNode*)0);
+            current->nextSibling(), toRenderElement(current)->hasCounterNodeMap() ?
+            counterName ? WebCore::counterMaps().get(toRenderElement(current))->get(identifier) : (WebCore::CounterNode*)1 : (WebCore::CounterNode*)0);
     }
     fflush(stderr);

trunk/Source/WebCore/rendering/RenderCounter.h

@@ -35 +35 @@
     virtual ~RenderCounter();
 
-    static void destroyCounterNodes(RenderObject*);
-    static void destroyCounterNode(RenderObject*, const AtomicString& identifier);
-    static void rendererSubtreeAttached(RenderObject*);
-    static void rendererRemovedFromTree(RenderObject&);
-    static void rendererStyleChanged(RenderObject*, const RenderStyle* oldStyle, const RenderStyle* newStyle);
+    static void destroyCounterNodes(RenderElement&);
+    static void destroyCounterNode(RenderElement&, const AtomicString& identifier);
+    static void rendererSubtreeAttached(RenderElement&);
+    static void rendererRemovedFromTree(RenderElement&);
+    static void rendererStyleChanged(RenderElement&, const RenderStyle* oldStyle, const RenderStyle* newStyle);
 
     void updateCounter();

trunk/Source/WebCore/rendering/RenderElement.cpp

@@ -86 +86 @@
     , m_renderBoxNeedsLazyRepaint(false)
     , m_hasPausedImageAnimations(false)
+    , m_hasCounterNodeMap(false)
     , m_firstChild(nullptr)
     , m_lastChild(nullptr)
@@ -569 +570 @@
         if (notifyChildren == NotifyChildren)
             newChild->insertedIntoTree();
-        RenderCounter::rendererSubtreeAttached(newChild);
+        if (newChild->isRenderElement())
+            RenderCounter::rendererSubtreeAttached(toRenderElement(*newChild));
     }
 
@@ -637 +639 @@
     // rendererRemovedFromTree walks the whole subtree. We can improve performance
     // by skipping this step when destroying the entire tree.
-    if (!documentBeingDestroyed())
-        RenderCounter::rendererRemovedFromTree(oldChild);
+    if (!documentBeingDestroyed() && oldChild.isRenderElement())
+        RenderCounter::rendererRemovedFromTree(toRenderElement(oldChild));
 
     if (AXObjectCache* cache = document().existingAXObjectCache())
@@ -921 +923 @@
     
     if (diff == StyleDifferenceLayout || diff == StyleDifferenceSimplifiedLayout) {
-        RenderCounter::rendererStyleChanged(this, oldStyle, &m_style.get());
+        RenderCounter::rendererStyleChanged(*this, oldStyle, &m_style.get());
 
         // If the object already needs layout, then setNeedsLayout won't do
@@ -1007 +1009 @@
 
     destroyLeftoverChildren();
+
+    if (hasCounterNodeMap())
+        RenderCounter::destroyCounterNodes(*this);
 
     RenderObject::willBeDestroyed();

trunk/Source/WebCore/rendering/RenderElement.h

@@ -160 +160 @@
     bool renderBoxNeedsLazyRepaint() const { return m_renderBoxNeedsLazyRepaint; }
 
+    bool hasCounterNodeMap() const { return m_hasCounterNodeMap; }
+    void setHasCounterNodeMap(bool f) { m_hasCounterNodeMap = f; }
+
 protected:
     enum BaseTypeFlags {
@@ -232 +235 @@
     bool m_renderBoxNeedsLazyRepaint : 1;
     bool m_hasPausedImageAnimations : 1;
+    bool m_hasCounterNodeMap : 1;
 
     RenderObject* m_firstChild;

trunk/Source/WebCore/rendering/RenderObject.cpp

@@ -1942 +1942 @@
         cache->remove(this);
 
-    // If this renderer had a parent, remove should have destroyed any counters
-    // attached to this renderer and marked the affected other counters for
-    // reevaluation. This apparently redundant check is here for the case when
-    // this renderer had no parent at the time remove() was called.
-
-    if (hasCounterNodeMap())
-        RenderCounter::destroyCounterNodes(this);
-
     // FIXME: Would like to do this in RenderBoxModelObject, but the timing is so complicated that this can't easily
     // be moved into RenderBoxModelObject::destroy.

trunk/Source/WebCore/rendering/RenderObject.h

@@ -391 +391 @@
     static inline bool isBeforeOrAfterContent(const RenderObject* obj) { return obj && obj->isBeforeOrAfterContent(); }
 
-    bool hasCounterNodeMap() const { return m_bitfields.hasCounterNodeMap(); }
-    void setHasCounterNodeMap(bool hasCounterNodeMap) { m_bitfields.setHasCounterNodeMap(hasCounterNodeMap); }
     bool everHadLayout() const { return m_bitfields.everHadLayout(); }
 
@@ -962 +960 @@
             , m_hasTransform(false)
             , m_hasReflection(false)
-            , m_hasCounterNodeMap(false)
             , m_everHadLayout(false)
             , m_childrenInline(false)
@@ -972 +969 @@
         }
         
-        // 32 bits have been used here. There are no bits available.
+        // 31 bits have been used here. There is one bit available.
         ADD_BOOLEAN_BITFIELD(needsLayout, NeedsLayout);
         ADD_BOOLEAN_BITFIELD(needsPositionedMovementLayout, NeedsPositionedMovementLayout);
@@ -995 +992 @@
         ADD_BOOLEAN_BITFIELD(hasReflection, HasReflection);
 
-        ADD_BOOLEAN_BITFIELD(hasCounterNodeMap, HasCounterNodeMap);
         ADD_BOOLEAN_BITFIELD(everHadLayout, EverHadLayout);