2011-01-27 Carol Szabo <carol.szabo@nokia.com>

        Reviewed by David Hyatt.

        A corrupted counter tree is created when renderers are added to the
        tree bypassing RenderObject::addChild
        https://bugs.webkit.org/show_bug.cgi?id=51270

        No new tests. This patch reimplements the fix for bugs 43812 and
        51637 and hence all tests are already there as part of the original
        fixes for those bugs.

        * rendering/RenderCounter.cpp:
        (WebCore::findPlaceForCounter):
        Removed old workaround as this patch hopefully fixes the real
        problem.
        * rendering/RenderObject.cpp:
        (WebCore::RenderObject::addChild):
        Removed call to counter updater as it was moved to a lower level.
        (WebCore::RenderObject::destroy):
        Moved attached counter nodes destruction to after the node is
        removed from the tree.
        * rendering/RenderObjectChildList.cpp:
        (WebCore::RenderObjectChildList::removeChildNode):
        (WebCore::RenderObjectChildList::appendChildNode):
        (WebCore::RenderObjectChildList::insertChildNode):
        Added notifications to the Counter system such that the
        CounterForest reflects the changes to the RendererTree.
        * rendering/RenderWidget.cpp:
        (WebCore::RenderWidget::destroy):
        Applied the same changes as for RenderObject::destroy()
        since RenderObject::destroy() is not called from here.

Canonical link: https://commits.webkit.org/67069@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@76859 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog

@@ -1,3 +1,36 @@
+2011-01-27  Carol Szabo <carol.szabo@nokia.com>
+
+        Reviewed by David Hyatt.
+
+        A corrupted counter tree is created when renderers are added to the
+        tree bypassing RenderObject::addChild
+        https://bugs.webkit.org/show_bug.cgi?id=51270
+
+        No new tests. This patch reimplements the fix for bugs 43812 and
+        51637 and hence all tests are already there as part of the original
+        fixes for those bugs.
+
+        * rendering/RenderCounter.cpp:
+        (WebCore::findPlaceForCounter):
+        Removed old workaround as this patch hopefully fixes the real
+        problem.
+        * rendering/RenderObject.cpp:
+        (WebCore::RenderObject::addChild):
+        Removed call to counter updater as it was moved to a lower level.
+        (WebCore::RenderObject::destroy):
+        Moved attached counter nodes destruction to after the node is
+        removed from the tree.
+        * rendering/RenderObjectChildList.cpp:
+        (WebCore::RenderObjectChildList::removeChildNode):
+        (WebCore::RenderObjectChildList::appendChildNode):
+        (WebCore::RenderObjectChildList::insertChildNode):
+        Added notifications to the Counter system such that the
+        CounterForest reflects the changes to the RendererTree.
+        * rendering/RenderWidget.cpp:
+        (WebCore::RenderWidget::destroy):
+        Applied the same changes as for RenderObject::destroy()
+        since RenderObject::destroy() is not called from here.
+
 2011-01-27  Adam Roben  <aroben@apple.com>
 
         Add WKCACFViewLayerTreeHost

Source/WebCore/rendering/RenderCounter.cpp

@@ -136,11 +136,6 @@ static bool findPlaceForCounter(RenderObject* counterOwner, const AtomicString&
     RenderObject* currentRenderer = counterOwner->previousInPreOrder();
     previousSibling = 0;
     while (currentRenderer) {
-        // A sibling without a parent means that the counter node tree was not constructed correctly so we stop 
-        // traversing. In the future RenderCounter should handle RenderObjects that are not connected to the 
-        // render tree at counter node creation. See bug 43812. 
-        if (previousSibling && !previousSibling->parent())
-            return false;
         CounterNode* currentCounter = makeCounterNode(currentRenderer, identifier, false);
         if (searchEndRenderer == currentRenderer) {
             // We may be at the end of our search.

Source/WebCore/rendering/RenderObject.cpp

@@ -316,7 +316,6 @@ void RenderObject::addChild(RenderObject* newChild, RenderObject* beforeChild)
         // Just add it...
         children->insertChildNode(this, newChild, beforeChild);
     }
-    RenderCounter::rendererSubtreeAttached(newChild); 
     if (newChild->isText() && newChild->style()->textTransform() == CAPITALIZE) {
         RefPtr<StringImpl> textToTransform = toRenderText(newChild)->originalText();
         if (textToTransform)
@@ -2158,9 +2157,6 @@ void RenderObject::destroy()
     if (frame() && frame()->eventHandler()->autoscrollRenderer() == this)
         frame()->eventHandler()->stopAutoscrollTimer(true);
 
-    if (m_hasCounterNodeMap)
-        RenderCounter::destroyCounterNodes(this);
-
     if (AXObjectCache::accessibilityEnabled()) {
         document()->axObjectCache()->childrenChanged(this->parent());
         document()->axObjectCache()->remove(this);
@@ -2173,6 +2169,14 @@ void RenderObject::destroy()
 
     remove();
 
+    // 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 (m_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.
     if (hasLayer()) {

Source/WebCore/rendering/RenderObjectChildList.cpp

@@ -128,6 +128,9 @@ RenderObject* RenderObjectChildList::removeChildNode(RenderObject* owner, Render
     oldChild->setNextSibling(0);
     oldChild->setParent(0);
 
+    if (oldChild->m_hasCounterNodeMap)
+        RenderCounter::destroyCounterNodes(oldChild);
+
     if (AXObjectCache::accessibilityEnabled())
         owner->document()->axObjectCache()->childrenChanged(owner);
 
@@ -175,6 +178,7 @@ void RenderObjectChildList::appendChildNode(RenderObject* owner, RenderObject* n
             owner->dirtyLinesFromChangedChild(newChild);
     }
 
+    RenderCounter::rendererSubtreeAttached(newChild);
     newChild->setNeedsLayoutAndPrefWidthsRecalc(); // Goes up the containing block hierarchy.
     if (!owner->normalChildNeedsLayout())
         owner->setChildNeedsLayout(true); // We may supply the static position for an absolute positioned child.
@@ -234,6 +238,7 @@ void RenderObjectChildList::insertChildNode(RenderObject* owner, RenderObject* c
             owner->dirtyLinesFromChangedChild(child);
     }
 
+    RenderCounter::rendererSubtreeAttached(child);
     child->setNeedsLayoutAndPrefWidthsRecalc();
     if (!owner->normalChildNeedsLayout())
         owner->setChildNeedsLayout(true); // We may supply the static position for an absolute positioned child.

Source/WebCore/rendering/RenderWidget.cpp

@@ -120,15 +120,16 @@ void RenderWidget::destroy()
     if (RenderView* v = view())
         v->removeWidget(this);
 
-    if (m_hasCounterNodeMap)
-        RenderCounter::destroyCounterNodes(this);
 
     if (AXObjectCache::accessibilityEnabled()) {
         document()->axObjectCache()->childrenChanged(this->parent());
         document()->axObjectCache()->remove(this);
     }
     remove();
 
+    if (m_hasCounterNodeMap)
+        RenderCounter::destroyCounterNodes(this);
+
     setWidget(0);
 
     // removes from override size map