Reviewed by Hyatt.
authorbdakin <bdakin@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 16 Mar 2007 00:04:39 +0000 (00:04 +0000)
committerbdakin <bdakin@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 16 Mar 2007 00:04:39 +0000 (00:04 +0000)
        Fix for <rdar://problem/5065396> REGRESSION: leaks in
        RenderBlock::layoutInlineChildren seen on buildbot

        This leak appeared after http://trac.webkit.org/projects/webkit/
        changeset/20188. This change shifted line boxes around in
        removeChild(). But since removeChild() calls
        setNeedsLayoutAndMinMaxRecalc(), all of the line boxes will be
        removed once we actually lay out anyway. So this patch fixes the
        leak by deleting the line boxes instead of shifting them around.

        * editing/IndentOutdentCommand.cpp:
        (WebCore::IndentOutdentCommand::outdentParagraph): Call into
        updateLayout(). This fixes an assertion I got in editing/
        execCommand/4976800.html This is very similar to the line box fix I
        made recently (http://trac.webkit.org/projects/webkit/changeset/
        20177). We need to update layout before relying on VisiblePositions
        after removing a node.
        * rendering/RenderBlock.cpp:
        (WebCore::RenderBlock::deleteLinesForBlock): New helper function
        since this functionality is needed in three places now.
        (WebCore::RenderBlock::makeChildrenNonInline): Call into new
        deleteLinesForBlock().
        (WebCore::RenderBlock::removeChild): Same.
        * rendering/RenderBlock.h:

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

WebCore/ChangeLog
WebCore/editing/IndentOutdentCommand.cpp
WebCore/rendering/RenderBlock.cpp
WebCore/rendering/RenderBlock.h

index 7268fa2a98f50cd4ba5c4b26f67779197efeca73..da7e97fa186ccc87b11524aee14e72a30450d9b8 100644 (file)
@@ -1,3 +1,32 @@
+2007-03-15  Beth Dakin  <bdakin@apple.com>
+
+        Reviewed by Hyatt.
+
+        Fix for <rdar://problem/5065396> REGRESSION: leaks in 
+        RenderBlock::layoutInlineChildren seen on buildbot
+
+        This leak appeared after http://trac.webkit.org/projects/webkit/
+        changeset/20188. This change shifted line boxes around in 
+        removeChild(). But since removeChild() calls 
+        setNeedsLayoutAndMinMaxRecalc(), all of the line boxes will be 
+        removed once we actually lay out anyway. So this patch fixes the 
+        leak by deleting the line boxes instead of shifting them around. 
+
+        * editing/IndentOutdentCommand.cpp:
+        (WebCore::IndentOutdentCommand::outdentParagraph): Call into 
+        updateLayout(). This fixes an assertion I got in editing/
+        execCommand/4976800.html This is very similar to the line box fix I 
+        made recently (http://trac.webkit.org/projects/webkit/changeset/
+        20177). We need to update layout before relying on VisiblePositions 
+        after removing a node.
+        * rendering/RenderBlock.cpp:
+        (WebCore::RenderBlock::deleteLinesForBlock): New helper function 
+        since this functionality is needed in three places now.
+        (WebCore::RenderBlock::makeChildrenNonInline): Call into new 
+        deleteLinesForBlock().
+        (WebCore::RenderBlock::removeChild): Same.
+        * rendering/RenderBlock.h:
+
 2007-03-15  Timothy Hatcher  <timothy@apple.com>
 
         Reviewed by John.
index d9783a78033da6a69f60d1e564267fddda13c6f3..3c51fcfd261ec1ab44dfb61f6ab3173723db918c 100644 (file)
@@ -226,6 +226,7 @@ void IndentOutdentCommand::outdentParagraph()
         visibleEndOfParagraph == endOfEnclosingBlock) {
         // The blockquote doesn't contain anything outside the paragraph, so it can be totally removed.
         removeNodePreservingChildren(enclosingNode);
+        updateLayout();
         if (visibleStartOfParagraph.isNotNull() && !isStartOfParagraph(visibleStartOfParagraph))
             insertNodeAt(createBreakElement(document()).get(), visibleStartOfParagraph.deepEquivalent().node(), visibleStartOfParagraph.deepEquivalent().offset());
         if (visibleEndOfParagraph.isNotNull() && !isEndOfParagraph(visibleEndOfParagraph))
index 86a445944dae23dd8c982b83dd47a07f5233e879..4ba0948a73247e9d60a44672d74f4bfc6d802ab5 100644 (file)
@@ -269,6 +269,18 @@ static void getInlineRun(RenderObject* start, RenderObject* boundary,
     } while (!sawInline);
 }
 
+void RenderBlock::deleteLineBoxTree()
+{
+    InlineFlowBox* line = m_firstLineBox;
+    InlineFlowBox* nextLine;
+    while (line) {
+        nextLine = line->nextFlowBox();
+        line->deleteLine(renderArena());
+        line = nextLine;
+    }
+    m_firstLineBox = m_lastLineBox = 0;
+}
+
 void RenderBlock::makeChildrenNonInline(RenderObject *insertionPoint)
 {    
     // makeChildrenNonInline takes a block whose children are *all* inline and it
@@ -283,14 +295,7 @@ void RenderBlock::makeChildrenNonInline(RenderObject *insertionPoint)
 
     m_childrenInline = false;
 
-    InlineFlowBox* line = m_firstLineBox;
-    InlineFlowBox* nextLine;
-    while (line) {
-        nextLine = line->nextFlowBox();
-        line->deleteLine(renderArena());
-        line = nextLine;
-    }
-    m_firstLineBox = m_lastLineBox = 0;
+    deleteLineBoxTree();
 
     RenderObject *child = firstChild();
 
@@ -343,25 +348,9 @@ void RenderBlock::removeChild(RenderObject *oldChild)
             prev->appendChildNode(next->removeChildNode(no));
             no->setNeedsLayoutAndMinMaxRecalc();
         }
-        
-        // Do the same with line boxes.
-        RenderBlock* prevBlock = static_cast<RenderBlock*>(prev);
         RenderBlock* nextBlock = static_cast<RenderBlock*>(next);
-        if (RootInlineBox* box = nextBlock->firstRootBox()) {
-            if (prevBlock->lastRootBox()) {
-                prevBlock->lastRootBox()->setNextLineBox(box);
-                box->setPreviousLineBox(prevBlock->lastRootBox());
-            } else
-                prevBlock->m_firstLineBox = box;
-
-            while (box) {
-                prevBlock->m_lastLineBox = box;
-                box->m_object = prevBlock;
-                box = box->nextRootBox();
-            }
-            nextBlock->m_firstLineBox = 0;
-            nextBlock->m_lastLineBox = 0;
-        }
+        nextBlock->deleteLineBoxTree();
         
         // Nuke the now-empty block.
         next->destroy();
@@ -385,18 +374,8 @@ void RenderBlock::removeChild(RenderObject *oldChild)
             no->setNeedsLayoutAndMinMaxRecalc();
         }
 
-        // Take over the anonymous child's line boxes.
-        RootInlineBox* box = anonBlock->firstRootBox();
-        m_firstLineBox = box;
-        while (box) {
-            m_lastLineBox = box;
-            box->m_object = this;
-            box = box->nextRootBox();
-        }
-        anonBlock->m_firstLineBox = 0;
-        anonBlock->m_lastLineBox = 0;
-
-        // Nuke the now-empty block.
+        // Delete the now-empty block's lines and nuke it.
+        anonBlock->deleteLineBoxTree();
         anonBlock->destroy();
     }
 }
index a6f1239a204930ba8cd93d6db5f61784512eefde..da71058c3b2fa79a08e1e7f067b640f78332ede2 100644 (file)
@@ -57,6 +57,7 @@ public:
     virtual bool childrenInline() const { return m_childrenInline; }
     virtual void setChildrenInline(bool b) { m_childrenInline = b; }
     void makeChildrenNonInline(RenderObject* insertionPoint = 0);
+    void deleteLineBoxTree();
 
     // The height (and width) of a block when you include overflow spillage out of the bottom
     // of the block (e.g., a <div style="height:25px"> that has a 100px tall image inside