LayoutTests:
authorantti <antti@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 7 Aug 2007 11:15:44 +0000 (11:15 +0000)
committerantti <antti@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 7 Aug 2007 11:15:44 +0000 (11:15 +0000)
        Reviewed by Hyatt.

        Tests for <rdar://problem/5102553>
        Mail spins trying to display or edit a specific long plain text message in WebCore::TimerBase::...

        - added performance test. With debug build on MBP this takes about 1.5s to run.
        - added test case that shows some additional progression from the patch (less leftover anonymous boxes).

        * fast/block/basic/stress-shallow-nested-expected.txt: Added.
        * fast/block/basic/stress-shallow-nested.html: Added.
        * fast/block/float/nestedAnonymousBlocks2-expected.checksum: Added.
        * fast/block/float/nestedAnonymousBlocks2-expected.png: Added.
        * fast/block/float/nestedAnonymousBlocks2-expected.txt: Added.
        * fast/block/float/nestedAnonymousBlocks2.html: Added.

WebCore:

        Reviewed by Hyatt.

        Fix <rdar://problem/5102553>
        Mail spins trying to display or edit a specific long plain text message in WebCore::TimerBase::...

        Calling removeLeftoverAnonymousBoxes() from RenderBlock::addChildToFlow() made adding children
        O(n^2) in simple cases (repeated <div><div></div></div> for example).

        I couldn't find any limited fix so here is a more complete one. It removes iterating/recursing
        removeLeftoverAnonymousBoxes() method altogether. Instead of hunting around wildly, just get
        rid of anonymous boxes with block children when they occur.

        * rendering/RenderBlock.cpp:
        (WebCore::RenderBlock::addChildToFlow):
        * rendering/RenderButton.h:
        (WebCore::RenderButton::removeLeftoverAnonymousBlock):
        * rendering/RenderContainer.cpp:
        (WebCore::RenderContainer::removeLeftoverAnonymousBlock):
        * rendering/RenderContainer.h:
        * rendering/RenderObject.cpp:
        (WebCore::RenderObject::handleDynamicFloatPositionChange):
        (WebCore::RenderObject::removeLeftoverAnonymousBlock):
        * rendering/RenderObject.h:
        * rendering/RenderTextControl.h:
        (WebCore::RenderTextControl::removeLeftoverAnonymousBlock):

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

15 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/block/basic/stress-shallow-nested-expected.txt [new file with mode: 0644]
LayoutTests/fast/block/basic/stress-shallow-nested.html [new file with mode: 0644]
LayoutTests/fast/block/float/nestedAnonymousBlocks2-expected.checksum [new file with mode: 0644]
LayoutTests/fast/block/float/nestedAnonymousBlocks2-expected.png [new file with mode: 0644]
LayoutTests/fast/block/float/nestedAnonymousBlocks2-expected.txt [new file with mode: 0644]
LayoutTests/fast/block/float/nestedAnonymousBlocks2.html [new file with mode: 0644]
WebCore/ChangeLog
WebCore/rendering/RenderBlock.cpp
WebCore/rendering/RenderButton.h
WebCore/rendering/RenderContainer.cpp
WebCore/rendering/RenderContainer.h
WebCore/rendering/RenderObject.cpp
WebCore/rendering/RenderObject.h
WebCore/rendering/RenderTextControl.h

index 791e231a45f9dbac627de6d36b7cccba7f61f1a8..21a3b2e32957f3f0c4c3e728af1d6f6bf63276c1 100644 (file)
@@ -1,3 +1,20 @@
+2007-08-07  Antti Koivisto  <antti@apple.com>
+
+        Reviewed by Hyatt.
+        
+        Tests for <rdar://problem/5102553>
+        Mail spins trying to display or edit a specific long plain text message in WebCore::TimerBase::...
+        
+        - added performance test. With debug build on MBP this takes about 1.5s to run.
+        - added test case that shows some additional progression from the patch (less leftover anonymous boxes).
+        
+        * fast/block/basic/stress-shallow-nested-expected.txt: Added.
+        * fast/block/basic/stress-shallow-nested.html: Added.
+        * fast/block/float/nestedAnonymousBlocks2-expected.checksum: Added.
+        * fast/block/float/nestedAnonymousBlocks2-expected.png: Added.
+        * fast/block/float/nestedAnonymousBlocks2-expected.txt: Added.
+        * fast/block/float/nestedAnonymousBlocks2.html: Added.
+
 2007-08-06  Sam Weinig  <sam@webkit.org>
 
         Reviewed by Oliver.
diff --git a/LayoutTests/fast/block/basic/stress-shallow-nested-expected.txt b/LayoutTests/fast/block/basic/stress-shallow-nested-expected.txt
new file mode 100644 (file)
index 0000000..69cfc5a
--- /dev/null
@@ -0,0 +1,2 @@
+PASS
+
diff --git a/LayoutTests/fast/block/basic/stress-shallow-nested.html b/LayoutTests/fast/block/basic/stress-shallow-nested.html
new file mode 100644 (file)
index 0000000..3814330
--- /dev/null
@@ -0,0 +1,20 @@
+<html>
+<head>
+<script>
+if (window.layoutTestController)
+    layoutTestController.dumpAsText();
+</script>
+</head>
+<body>
+<div id=console></div>
+<script>
+var start = new Date().getTime();
+for (n = 0; n < 30000; n++)
+    document.write('<div><div></div></div>');
+var time = new Date().getTime() - start;
+document.getElementById('console').innerHTML = (time < 8000) 
+    ? "<span style='color:green'>PASS</span>"
+    : "<span style='color:red'>FAIL</span>";
+if (!window.layoutTestController)
+    document.getElementById('console').innerHTML += " time: " + time;
+</script>
\ No newline at end of file
diff --git a/LayoutTests/fast/block/float/nestedAnonymousBlocks2-expected.checksum b/LayoutTests/fast/block/float/nestedAnonymousBlocks2-expected.checksum
new file mode 100644 (file)
index 0000000..b5b989f
--- /dev/null
@@ -0,0 +1 @@
+5ca0d026ec28acd0568648995f8a75aa
\ No newline at end of file
diff --git a/LayoutTests/fast/block/float/nestedAnonymousBlocks2-expected.png b/LayoutTests/fast/block/float/nestedAnonymousBlocks2-expected.png
new file mode 100644 (file)
index 0000000..250f0a2
Binary files /dev/null and b/LayoutTests/fast/block/float/nestedAnonymousBlocks2-expected.png differ
diff --git a/LayoutTests/fast/block/float/nestedAnonymousBlocks2-expected.txt b/LayoutTests/fast/block/float/nestedAnonymousBlocks2-expected.txt
new file mode 100644 (file)
index 0000000..ce5dc85
--- /dev/null
@@ -0,0 +1,12 @@
+layer at (0,0) size 800x600
+  RenderView at (0,0) size 800x600
+layer at (0,0) size 800x600
+  RenderBlock {HTML} at (0,0) size 800x600
+    RenderBody {BODY} at (8,8) size 784x584
+      RenderBlock {DIV} at (0,0) size 784x36
+        RenderBlock (anonymous) at (0,0) size 784x36
+          RenderText {#text} at (0,0) size 719x36
+            text run at (0,0) width 719: "Test anonymous boxes removal on style change. This should not crash or hang. Render tree dump should not gain"
+            text run at (0,18) width 184: "additional anonymous boxes."
+        RenderBlock {DIV} at (0,36) size 784x0
+        RenderBlock {DIV} at (0,36) size 784x0
diff --git a/LayoutTests/fast/block/float/nestedAnonymousBlocks2.html b/LayoutTests/fast/block/float/nestedAnonymousBlocks2.html
new file mode 100644 (file)
index 0000000..12cec3a
--- /dev/null
@@ -0,0 +1,18 @@
+<html>
+<style>
+#floater { float: left }
+</style>
+<body>
+
+<div>
+Test anonymous boxes removal on style change. This should not crash or hang. Render tree dump should not gain additional anonymous boxes.
+<div id=floater></div><div></div></div>
+
+<script>
+var floater = document.getElementById("floater");
+floater.offsetWidth;
+floater.style.float = "none";
+</script>
+
+</body>  
+</html>
index 56bd422502dca326c3f01d58a71997bcc1c708d8..d8fca0aa01bff509e126ffa4ba9b842496161d4f 100644 (file)
@@ -1,3 +1,31 @@
+2007-08-07  Antti Koivisto  <antti@apple.com>
+
+        Reviewed by Hyatt.
+
+        Fix <rdar://problem/5102553>
+        Mail spins trying to display or edit a specific long plain text message in WebCore::TimerBase::...
+
+        Calling removeLeftoverAnonymousBoxes() from RenderBlock::addChildToFlow() made adding children
+        O(n^2) in simple cases (repeated <div><div></div></div> for example).
+        
+        I couldn't find any limited fix so here is a more complete one. It removes iterating/recursing 
+        removeLeftoverAnonymousBoxes() method altogether. Instead of hunting around wildly, just get 
+        rid of anonymous boxes with block children when they occur.
+
+        * rendering/RenderBlock.cpp:
+        (WebCore::RenderBlock::addChildToFlow):
+        * rendering/RenderButton.h:
+        (WebCore::RenderButton::removeLeftoverAnonymousBlock):
+        * rendering/RenderContainer.cpp:
+        (WebCore::RenderContainer::removeLeftoverAnonymousBlock):
+        * rendering/RenderContainer.h:
+        * rendering/RenderObject.cpp:
+        (WebCore::RenderObject::handleDynamicFloatPositionChange):
+        (WebCore::RenderObject::removeLeftoverAnonymousBlock):
+        * rendering/RenderObject.h:
+        * rendering/RenderTextControl.h:
+        (WebCore::RenderTextControl::removeLeftoverAnonymousBlock):
+
 2007-08-06  Sam Weinig  <sam@webkit.org>
 
         Reviewed by Maciej.
index d7f70d92d7bc1629feb14c18f67bb3dcd5f11b81..78dfb3dfa21d2c63e52b5160ebc355faa533ccd4 100644 (file)
@@ -219,8 +219,9 @@ void RenderBlock::addChildToFlow(RenderObject* newChild, RenderObject* beforeChi
     RenderContainer::addChild(newChild,beforeChild);
     // ### care about aligned stuff
 
-    if ( madeBoxesNonInline )
-        removeLeftoverAnonymousBoxes();
+    if (madeBoxesNonInline && parent() && isAnonymousBlock())
+        parent()->removeLeftoverAnonymousBlock(this);
+    // this object may be dead here
 }
 
 static void getInlineRun(RenderObject* start, RenderObject* boundary,
index d72271ec2937e92ecec94845e0e803c0784a52f6..053684c27c2643441887a1dabe32bf8b0edef8ad 100644 (file)
@@ -38,7 +38,7 @@ public:
 
     virtual void addChild(RenderObject* newChild, RenderObject *beforeChild = 0);
     virtual void removeChild(RenderObject*);
-    virtual void removeLeftoverAnonymousBoxes() { }
+    virtual void removeLeftoverAnonymousBlock(RenderBlock*) { }
     virtual bool createsAnonymousWrapper() const { return true; }
 
     virtual void setStyle(RenderStyle*);
index f800f1fc0910e31e3048df57201b1897fe97a6bb..baee5915c51e77c7acce8dacc018af5930e57b1c 100644 (file)
@@ -502,55 +502,47 @@ void RenderContainer::layout()
     setNeedsLayout(false);
 }
 
-void RenderContainer::removeLeftoverAnonymousBoxes()
+void RenderContainer::removeLeftoverAnonymousBlock(RenderBlock* child)
 {
-    // we have to go over all child nodes and remove anonymous boxes, that do _not_
-    // have inline children to keep the tree flat
-    RenderObject* child = m_firstChild;
-    while( child ) {
-        RenderObject* next = child->nextSibling();
-        
-        if ( child->isRenderBlock() && child->isAnonymousBlock() && !child->continuation() && !child->childrenInline() && !child->isTableCell() ) {
-            RenderObject* firstAnChild = child->firstChild();
-            RenderObject* lastAnChild = child->lastChild();
-            if ( firstAnChild ) {
-                RenderObject* o = firstAnChild;
-                while( o ) {
-                    o->setParent( this );
-                    o = o->nextSibling();
-                }
-                firstAnChild->setPreviousSibling( child->previousSibling() );
-                lastAnChild->setNextSibling( child->nextSibling() );
-                if ( child->previousSibling() )
-                    child->previousSibling()->setNextSibling( firstAnChild );
-                if ( child->nextSibling() )
-                    child->nextSibling()->setPreviousSibling( lastAnChild );
-            } else {
-                if ( child->previousSibling() )
-                    child->previousSibling()->setNextSibling( child->nextSibling() );
-                if ( child->nextSibling() )
-                    child->nextSibling()->setPreviousSibling( child->previousSibling() );
-                
-            }
-            if (child == m_firstChild)
-                m_firstChild = firstAnChild;
-            if (child == m_lastChild)
-                m_lastChild = lastAnChild;
-            child->setParent( 0 );
-            child->setPreviousSibling( 0 );
-            child->setNextSibling( 0 );
-            if ( !child->isText() ) {
-                RenderContainer *c = static_cast<RenderContainer*>(child);
-                c->m_firstChild = 0;
-                c->m_next = 0;
-            }
-            child->destroy();
+    ASSERT(child->isAnonymousBlock());
+    ASSERT(!child->childrenInline());
+    
+    if (child->continuation()) 
+        return;
+    
+    RenderObject* firstAnChild = child->firstChild();
+    RenderObject* lastAnChild = child->lastChild();
+    if (firstAnChild) {
+        RenderObject* o = firstAnChild;
+        while(o) {
+            o->setParent(this);
+            o = o->nextSibling();
         }
-        child = next;
+        firstAnChild->setPreviousSibling(child->previousSibling());
+        lastAnChild->setNextSibling(child->nextSibling());
+        if (child->previousSibling())
+            child->previousSibling()->setNextSibling(firstAnChild);
+        if (child->nextSibling())
+            child->nextSibling()->setPreviousSibling(lastAnChild);
+    } else {
+        if (child->previousSibling())
+            child->previousSibling()->setNextSibling(child->nextSibling());
+        if (child->nextSibling())
+            child->nextSibling()->setPreviousSibling(child->previousSibling());
     }
-
-    if (parent()) // && isAnonymousBlock() && !continuation() && !childrenInline() && !isTableCell())
-        parent()->removeLeftoverAnonymousBoxes();
+    if (child == m_firstChild)
+        m_firstChild = firstAnChild;
+    if (child == m_lastChild)
+        m_lastChild = lastAnChild;
+    child->setParent(0);
+    child->setPreviousSibling(0);
+    child->setNextSibling(0);
+    if (!child->isText()) {
+        RenderContainer *c = static_cast<RenderContainer*>(child);
+        c->m_firstChild = 0;
+        c->m_next = 0;
+    }
+    child->destroy();
 }
 
 VisiblePosition RenderContainer::positionForCoordinates(int x, int y)
index 60901512f2afead12f9ef6a81eebfd4555a92aa3..67a248eb3a4c428b2ca3b75a1199dbe825124970 100644 (file)
@@ -52,7 +52,7 @@ public:
     virtual void layout();
     virtual void calcPrefWidths() { setPrefWidthsDirty(false); }
 
-    virtual void removeLeftoverAnonymousBoxes();
+    virtual void removeLeftoverAnonymousBlock(RenderBlock* child);
 
     RenderObject* beforeAfterContainer(RenderStyle::PseudoId);
     virtual void updateBeforeAfterContent(RenderStyle::PseudoId);
index 3364a65e8b34cf2045fdd030a7a5b0cfae921f94..b8bc70ef7137d0cb29ba805892d3fe061dc00538 100644 (file)
@@ -2152,8 +2152,13 @@ void RenderObject::handleDynamicFloatPositionChange()
                 RenderObject* beforeChild = nextSibling();
                 parent()->removeChildNode(this);
                 parentInline->splitFlow(beforeChild, newBox, this, oldContinuation);
-            } else if (parent()->isRenderBlock())
-                static_cast<RenderBlock*>(parent())->makeChildrenNonInline();
+            } else if (parent()->isRenderBlock()) {
+                RenderBlock* o = static_cast<RenderBlock*>(parent());
+                o->makeChildrenNonInline();
+                if (o->isAnonymousBlock() && o->parent())
+                    o->parent()->removeLeftoverAnonymousBlock(o);
+                // o may be dead here
+            }
         } else {
             // An anonymous block must be made to wrap this inline.
             RenderBlock* box = createAnonymousBlock();
@@ -2754,7 +2759,7 @@ void RenderObject::scheduleRelayout()
     }
 }
 
-void RenderObject::removeLeftoverAnonymousBoxes()
+void RenderObject::removeLeftoverAnonymousBlock(RenderBlock*)
 {
 }
 
index 5cf1610c9873302d9b6b507f2a5b4a6178379b65..bfb07d7a9683e0b22f340f077f564ba114f58e0e 100644 (file)
@@ -854,6 +854,8 @@ public:
     void remove() { if (parent()) parent()->removeChild(this); }
 
     void invalidateVerticalPosition() { m_verticalPosition = PositionUndefined; }
+    
+    virtual void removeLeftoverAnonymousBlock(RenderBlock* child);
 
 protected:
     virtual void printBoxDecorations(GraphicsContext*, int /*x*/, int /*y*/, int /*w*/, int /*h*/, int /*tx*/, int /*ty*/) { }
@@ -862,8 +864,6 @@ protected:
 
     short getVerticalPosition(bool firstLine) const;
 
-    virtual void removeLeftoverAnonymousBoxes();
-
     void adjustRectForOutlineAndShadow(IntRect&) const;
 
     void arenaDelete(RenderArena*, void* objectBase);
index b26100159ba7fb4ea663e41edf75a3b36ca670f9..567db74254d9a9008ff196dd090095a4516d84a5 100644 (file)
@@ -45,7 +45,7 @@ public:
     virtual IntRect controlClipRect(int tx, int ty) const;
     virtual void calcHeight();
     virtual void calcPrefWidths();
-    virtual void removeLeftoverAnonymousBoxes() { }
+    virtual void removeLeftoverAnonymousBlock(RenderBlock*) { }
     virtual void setStyle(RenderStyle*);
     virtual void updateFromElement();
     virtual bool canHaveChildren() const { return false; }