2009-11-13 Carol Szabo <carol.szabo@nokia.com>
authoreric@webkit.org <eric@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 13 Nov 2009 20:21:44 +0000 (20:21 +0000)
committereric@webkit.org <eric@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 13 Nov 2009 20:21:44 +0000 (20:21 +0000)
        Reviewed by Darin Adler.

        The CounterNode class is missing some basic tree navigation methods common in other WebKit trees such as the rendering tree
        https://bugs.webkit.org/show_bug.cgi?id=31213
        Added tree navigation methods that permit full implementation of CSS2.1
        counter feature without using recursion proportional to the counter
        tree depth.
        No new tests because I did not find any bug that is fixed by this
        commit yet, this just reduces the size of the patch for 11031 and
        helps respond to some concerns regarding that patch.

        * rendering/CounterNode.cpp:
        (WebCore::CounterNode::CounterNode):

        (WebCore::CounterNode::nextInPreOrderAfterChildren):
        (WebCore::CounterNode::nextInPreOrder):
        Added to support non-recursive tree traversal necessary for
        efficient full implementation of CSS2.1 counters.

        (WebCore::CounterNode::lastDescendant):
        (WebCore::CounterNode::previousInPreOrder):
        Moved this methods such that they occupy a place similar to that of
        identically named methods on the render tree. This allows for their
        broader use needed in full implementation of CSS2.1 counters.

        (WebCore::CounterNode::resetRenderer):
        (WebCore::CounterNode::resetRenderers):
        (WebCore::CounterNode::recount):
        (WebCore::CounterNode::insertAfter):
        (WebCore::CounterNode::removeChild):
        Changed such that insertion/removal of a counter, triggers not only
        recalculation of PrefixWidths, but also reassesment of values in
        counter nodes. This is the basis full implementation of CSS2.1
        counters. It does not change current behavior by much because of
        changes needed to the recalculation algorithm, but those are comming
        in the patch for 11031.
        (WebCore::showTreeAndMark):
        * rendering/CounterNode.h:
        * rendering/RenderCounter.cpp:
        (WebCore::counter):
        Only changed argument type to prepare for implementation of Darin
        Adler's recommendation for the patch to 11031.

        (WebCore::RenderCounter::invalidate):
        (WebCore::destroyCounterNodeChildren):
        (WebCore::RenderCounter::destroyCounterNodes):
        * rendering/RenderCounter.h:
        * rendering/RenderObjectChildList.cpp:
        (WebCore::invalidateCountersInContainer):
        (WebCore::RenderObjectChildList::invalidateCounters):
        * rendering/RenderObjectChildList.h:
        Added the ability to restrict invalidation to counters with a given
        identifier.
        Also invalidated counters that are on the child container itself
        which were missed by the previous algorithm, but were a valid case.

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

WebCore/ChangeLog
WebCore/rendering/CounterNode.cpp
WebCore/rendering/CounterNode.h
WebCore/rendering/RenderCounter.cpp
WebCore/rendering/RenderCounter.h
WebCore/rendering/RenderObjectChildList.cpp
WebCore/rendering/RenderObjectChildList.h

index 18714af49b0f985d63e5e5548521c3d35b3991f1..5503e1b5c93c711d8501ca8f9e9c38be41df4385 100644 (file)
@@ -1,3 +1,61 @@
+2009-11-13  Carol Szabo  <carol.szabo@nokia.com>
+
+        Reviewed by Darin Adler.
+
+        The CounterNode class is missing some basic tree navigation methods common in other WebKit trees such as the rendering tree
+        https://bugs.webkit.org/show_bug.cgi?id=31213
+        Added tree navigation methods that permit full implementation of CSS2.1
+        counter feature without using recursion proportional to the counter
+        tree depth.
+        No new tests because I did not find any bug that is fixed by this
+        commit yet, this just reduces the size of the patch for 11031 and
+        helps respond to some concerns regarding that patch.
+
+        * rendering/CounterNode.cpp:
+        (WebCore::CounterNode::CounterNode):
+
+        (WebCore::CounterNode::nextInPreOrderAfterChildren):
+        (WebCore::CounterNode::nextInPreOrder):
+        Added to support non-recursive tree traversal necessary for
+        efficient full implementation of CSS2.1 counters.
+
+        (WebCore::CounterNode::lastDescendant):
+        (WebCore::CounterNode::previousInPreOrder):
+        Moved this methods such that they occupy a place similar to that of
+        identically named methods on the render tree. This allows for their
+        broader use needed in full implementation of CSS2.1 counters.
+
+        (WebCore::CounterNode::resetRenderer):
+        (WebCore::CounterNode::resetRenderers):
+        (WebCore::CounterNode::recount):
+        (WebCore::CounterNode::insertAfter):
+        (WebCore::CounterNode::removeChild):
+        Changed such that insertion/removal of a counter, triggers not only
+        recalculation of PrefixWidths, but also reassesment of values in
+        counter nodes. This is the basis full implementation of CSS2.1
+        counters. It does not change current behavior by much because of
+        changes needed to the recalculation algorithm, but those are comming
+        in the patch for 11031.
+        (WebCore::showTreeAndMark):
+        * rendering/CounterNode.h:
+        * rendering/RenderCounter.cpp:
+        (WebCore::counter):
+        Only changed argument type to prepare for implementation of Darin
+        Adler's recommendation for the patch to 11031.
+
+        (WebCore::RenderCounter::invalidate):
+        (WebCore::destroyCounterNodeChildren):
+        (WebCore::RenderCounter::destroyCounterNodes):
+        * rendering/RenderCounter.h:
+        * rendering/RenderObjectChildList.cpp:
+        (WebCore::invalidateCountersInContainer):
+        (WebCore::RenderObjectChildList::invalidateCounters):
+        * rendering/RenderObjectChildList.h:
+        Added the ability to restrict invalidation to counters with a given
+        identifier.
+        Also invalidated counters that are on the child container itself
+        which were missed by the previous algorithm, but were a valid case.
+
 2009-11-13  Vitaly Repeshko  <vitalyr@chromium.org>
 
         Reviewed by Dimitri Glazkov.
index 0b4afc9649935c449817921c97f6ba207f257a78..bf45e04c4cb2ec6891c70771d82cf868018357d7 100644 (file)
@@ -44,7 +44,58 @@ CounterNode::CounterNode(RenderObject* o, bool isReset, int value)
     , m_nextSibling(0)
     , m_firstChild(0)
     , m_lastChild(0)
-{   
+{
+}
+
+CounterNode* CounterNode::nextInPreOrderAfterChildren(const CounterNode* stayWithin) const
+{
+    if (this == stayWithin)
+        return 0;
+
+    CounterNode* next = m_nextSibling;
+    if (next)
+        return next;
+    next = m_parent;
+    while (next && !next->m_nextSibling) {
+        if (next == stayWithin)
+            return 0;
+        next = next->m_parent;
+    }
+    if (next)
+        return next->m_nextSibling;
+    return 0;
+}
+
+CounterNode* CounterNode::nextInPreOrder(const CounterNode* stayWithin) const
+{
+    if (CounterNode* next = m_firstChild)
+        return next;
+
+    return nextInPreOrderAfterChildren(stayWithin);
+}
+
+CounterNode* CounterNode::lastDescendant() const
+{
+    CounterNode* last = m_lastChild;
+    if (!last)
+        return 0;
+
+    while (CounterNode* lastChild = last->m_lastChild)
+        last = lastChild;
+
+    return last;
+}
+
+CounterNode* CounterNode::previousInPreOrder() const
+{
+    CounterNode* previous = m_previousSibling;
+    if (!previous)
+        return m_parent;
+
+    while (CounterNode* lastChild = previous->m_lastChild)
+        previous = lastChild;
+
+    return previous;
 }
 
 int CounterNode::computeCountInParent() const
@@ -56,26 +107,37 @@ int CounterNode::computeCountInParent() const
     return m_parent->m_value + increment;
 }
 
-void CounterNode::recount()
+
+void CounterNode::resetRenderer(const AtomicString& identifier) const
+{
+    if (!m_renderer || m_renderer->documentBeingDestroyed())
+        return;
+    if (RenderObjectChildList* children = m_renderer->virtualChildren())
+        children->invalidateCounters(m_renderer, identifier);
+}
+
+void CounterNode::resetRenderers(const AtomicString& identifier) const
+{
+    const CounterNode* node = this;
+    do {
+        node->resetRenderer(identifier);
+        node = node->nextInPreOrder(this);
+    } while (node);
+}
+
+void CounterNode::recount(const AtomicString& identifier)
 {
-    for (CounterNode* c = this; c; c = c->m_nextSibling) {
-        int oldCount = c->m_countInParent;
-        int newCount = c->computeCountInParent();
+    for (CounterNode* node = this; node; node = node->m_nextSibling) {
+        int oldCount = node->m_countInParent;
+        int newCount = node->computeCountInParent();
         if (oldCount == newCount)
             break;
-        c->m_countInParent = newCount;
-        // m_renderer contains the parent of the render node
-        // corresponding to a CounterNode. Let's find the counter
-        // child and make this re-layout.
-        for (RenderObject* o = c->m_renderer->firstChild(); o; o = o->nextSibling())
-            if (!o->documentBeingDestroyed() && o->isCounter()) {
-                o->setNeedsLayoutAndPrefWidthsRecalc();
-                break;
-            }
+        node->m_countInParent = newCount;
+        node->resetRenderers(identifier);
     }
 }
 
-void CounterNode::insertAfter(CounterNode* newChild, CounterNode* refChild)
+void CounterNode::insertAfter(CounterNode* newChild, CounterNode* refChild, const AtomicString& identifier)
 {
     ASSERT(newChild);
     ASSERT(!newChild->m_parent);
@@ -107,62 +169,42 @@ void CounterNode::insertAfter(CounterNode* newChild, CounterNode* refChild)
 
     newChild->m_countInParent = newChild->computeCountInParent();
     if (next)
-        next->recount();
+        next->recount(identifier);
 }
 
-void CounterNode::removeChild(CounterNode* oldChild)
+void CounterNode::removeChild(CounterNode* oldChild, const AtomicString& identifier)
 {
     ASSERT(oldChild);
     ASSERT(!oldChild->m_firstChild);
     ASSERT(!oldChild->m_lastChild);
 
     CounterNode* next = oldChild->m_nextSibling;
-    CounterNode* prev = oldChild->m_previousSibling;
+    CounterNode* previous = oldChild->m_previousSibling;
 
     oldChild->m_nextSibling = 0;
     oldChild->m_previousSibling = 0;
     oldChild->m_parent = 0;
 
-    if (prev) 
-        prev->m_nextSibling = next;
+    if (previous
+        previous->m_nextSibling = next;
     else {
         ASSERT(m_firstChild == oldChild);
         m_firstChild = next;
     }
-    
+
     if (next)
-        next->m_previousSibling = prev;
+        next->m_previousSibling = previous;
     else {
         ASSERT(m_lastChild == oldChild);
-        m_lastChild = prev;
+        m_lastChild = previous;
     }
-    
+
     if (next)
-        next->recount();
+        next->recount(identifier);
 }
 
 #ifndef NDEBUG
 
-static const CounterNode* nextInPreOrderAfterChildren(const CounterNode* node)
-{
-    CounterNode* next = node->nextSibling();
-    if (!next) {
-        next = node->parent();
-        while (next && !next->nextSibling())
-            next = next->parent();
-        if (next)
-            next = next->nextSibling();
-    }
-    return next;
-}
-
-static const CounterNode* nextInPreOrder(const CounterNode* node)
-{
-    if (CounterNode* child = node->firstChild())
-        return child;
-    return nextInPreOrderAfterChildren(node);
-}
-
 static void showTreeAndMark(const CounterNode* node)
 {
     const CounterNode* root = node;
index b432e1d5ce9d5f373ee6b1ada33ae0410aac6e25..8081dc679365cf3419441b86176d333d6025d396 100644 (file)
@@ -35,6 +35,7 @@
 
 namespace WebCore {
 
+class AtomicString;
 class RenderObject;
 
 class CounterNode : public Noncopyable {
@@ -51,13 +52,19 @@ public:
     CounterNode* nextSibling() const { return m_nextSibling; }
     CounterNode* firstChild() const { return m_firstChild; }
     CounterNode* lastChild() const { return m_lastChild; }
+    CounterNode* lastDescendant() const;
+    CounterNode* previousInPreOrder() const;
+    CounterNode* nextInPreOrder(const CounterNode* stayWithin = 0) const;
+    CounterNode* nextInPreOrderAfterChildren(const CounterNode* stayWithin = 0) const;
 
-    void insertAfter(CounterNode* newChild, CounterNode* beforeChild);
-    void removeChild(CounterNode*);
+    void insertAfter(CounterNode* newChild, CounterNode* beforeChild, const AtomicString& identifier);
+    void removeChild(CounterNode*, const AtomicString& identifier);
 
 private:
     int computeCountInParent() const;
-    void recount();
+    void recount(const AtomicString& identifier);
+    void resetRenderer(const AtomicString& identifier) const;
+    void resetRenderers(const AtomicString& identifier) const;
 
     bool m_isReset;
     int m_value;
index 59123af4ec53cdc8e4cd768d1e15516f8bc9d650..39ea620b2478b66dacde1eee2a5cd5b570c93fb9 100644 (file)
@@ -53,30 +53,6 @@ static inline RenderObject* previousSiblingOrParent(RenderObject* object)
     return object->parent();
 }
 
-static CounterNode* lastDescendant(CounterNode* node)
-{
-    CounterNode* last = node->lastChild();
-    if (!last)
-        return 0;
-
-    while (CounterNode* lastChild = last->lastChild())
-        last = lastChild;
-
-    return last;
-}
-
-static CounterNode* previousInPreOrder(CounterNode* node)
-{
-    CounterNode* previous = node->previousSibling();
-    if (!previous)
-        return node->parent();
-
-    while (CounterNode* lastChild = previous->lastChild())
-        previous = lastChild;
-
-    return previous;
-}
-
 static bool planCounter(RenderObject* object, const AtomicString& counterName, bool& isReset, int& value)
 {
     ASSERT(object);
@@ -204,7 +180,7 @@ static CounterNode* makeCounterNode(RenderObject* object, const AtomicString& co
     CounterNode* newNode;
     if (findPlaceForCounter(object, counterName, isReset, newParent, newPreviousSibling)) {
         newNode = new CounterNode(object, isReset, value);
-        newParent->insertAfter(newNode, newPreviousSibling);
+        newParent->insertAfter(newNode, newPreviousSibling, counterName);
     } else {
         // Make a reset node for counters that aren't inside an existing reset node.
         newNode = new CounterNode(object, true, value);
@@ -272,24 +248,26 @@ void RenderCounter::calcPrefWidths(int lead)
     RenderText::calcPrefWidths(lead);
 }
 
-void RenderCounter::invalidate()
+void RenderCounter::invalidate(const AtomicString& identifier)
 {
+    if (m_counter.identifier() != identifier)
+        return;
     m_counterNode = 0;
     setNeedsLayoutAndPrefWidthsRecalc();
 }
 
-static void destroyCounterNodeChildren(AtomicStringImpl* identifier, CounterNode* node)
+static void destroyCounterNodeChildren(const AtomicString& identifier, CounterNode* node)
 {
     CounterNode* previous;
-    for (CounterNode* child = lastDescendant(node); child && child != node; child = previous) {
-        previous = previousInPreOrder(child);
-        child->parent()->removeChild(child);
-        ASSERT(counterMaps().get(child->renderer())->get(identifier) == child);
-        counterMaps().get(child->renderer())->remove(identifier);
+    for (CounterNode* child = node->lastDescendant(); child && child != node; child = previous) {
+        previous = child->previousInPreOrder();
+        child->parent()->removeChild(child, identifier);
+        ASSERT(counterMaps().get(child->renderer())->get(identifier->impl()) == child);
+        counterMaps().get(child->renderer())->remove(identifier.impl());
         if (!child->renderer()->documentBeingDestroyed()) {
             RenderObjectChildList* children = child->renderer()->virtualChildren();
             if (children)
-                children->invalidateCounters(child->renderer());
+                children->invalidateCounters(child->renderer(), identifier);
         }
         delete child;
     }
@@ -306,9 +284,10 @@ void RenderCounter::destroyCounterNodes(RenderObject* object)
     CounterMap::const_iterator end = map->end();
     for (CounterMap::const_iterator it = map->begin(); it != end; ++it) {
         CounterNode* node = it->second;
-        destroyCounterNodeChildren(it->first.get(), node);
+        AtomicString identifier(it->first.get());
+        destroyCounterNodeChildren(identifier, node);
         if (CounterNode* parent = node->parent())
-            parent->removeChild(node);
+            parent->removeChild(node, identifier);
         delete node;
     }
 
index 961968e16c1e58ed09688bd2827b405f8251723b..356f1bd15927a9e3c2dcaba126244d306e26b697 100644 (file)
@@ -33,7 +33,11 @@ class RenderCounter : public RenderText {
 public:
     RenderCounter(Document*, const CounterContent&);
 
-    void invalidate();
+    // Removes the reference to the CounterNode associated with this renderer
+    // if its identifier matches the argument.
+    // This is used to cause a counter display update when the CounterNode
+    // tree for identifier changes.
+    void invalidate(const AtomicString& identifier);
 
     static void destroyCounterNodes(RenderObject*);
 
index fa195470fd02ad4cb8aad53f9d4ae48c988039b3..d56a015f7677979ed98f40dcfbd5f490df663ea0 100644 (file)
@@ -271,24 +271,29 @@ static RenderObject* findBeforeAfterParent(RenderObject* object)
     return beforeAfterParent;
 }
 
-static void invalidateCountersInContainer(RenderObject* container)
+static void invalidateCountersInContainer(RenderObject* container, const AtomicString& identifier)
 {
     if (!container)
         return;
     container = findBeforeAfterParent(container);
     if (!container)
         return;
+    // Sometimes the counter is attached directly on the container.
+    if (container->isCounter()) {
+        toRenderCounter(container)->invalidate(identifier);
+        return;
+    }
     for (RenderObject* content = container->firstChild(); content; content = content->nextSibling()) {
         if (content->isCounter())
-            toRenderCounter(content)->invalidate();
+            toRenderCounter(content)->invalidate(identifier);
     }
 }
 
-void RenderObjectChildList::invalidateCounters(RenderObject* owner)
+void RenderObjectChildList::invalidateCounters(RenderObject* owner, const AtomicString& identifier)
 {
     ASSERT(!owner->documentBeingDestroyed());
-    invalidateCountersInContainer(beforeAfterContainer(owner, BEFORE));
-    invalidateCountersInContainer(beforeAfterContainer(owner, AFTER));
+    invalidateCountersInContainer(beforeAfterContainer(owner, BEFORE), identifier);
+    invalidateCountersInContainer(beforeAfterContainer(owner, AFTER), identifier);
 }
 
 void RenderObjectChildList::updateBeforeAfterContent(RenderObject* owner, PseudoId type, RenderObject* styledObject)
index bf8800a2d8a76d8089e01e38149abdfca7d3e16e..ba73c50cb4aa7e112cb41232076fca1081218739 100644 (file)
@@ -30,6 +30,7 @@
 
 namespace WebCore {
 
+class AtomicString;
 class RenderObject;
 
 class RenderObjectChildList {
@@ -55,7 +56,7 @@ public:
     void insertChildNode(RenderObject* owner, RenderObject* child, RenderObject* before, bool fullInsert = true);
 
     void updateBeforeAfterContent(RenderObject* owner, PseudoId type, RenderObject* styledObject = 0);
-    void invalidateCounters(RenderObject* owner);
+    void invalidateCounters(RenderObject* owner, const AtomicString& identifier);
 
 private:
     RenderObject* m_firstChild;