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
+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.
--- /dev/null
+<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
--- /dev/null
+5ca0d026ec28acd0568648995f8a75aa
\ No newline at end of file
--- /dev/null
+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
--- /dev/null
+<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>
+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.
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,
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*);
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)
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);
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();
}
}
-void RenderObject::removeLeftoverAnonymousBoxes()
+void RenderObject::removeLeftoverAnonymousBlock(RenderBlock*)
{
}
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*/) { }
short getVerticalPosition(bool firstLine) const;
- virtual void removeLeftoverAnonymousBoxes();
-
void adjustRectForOutlineAndShadow(IntRect&) const;
void arenaDelete(RenderArena*, void* objectBase);
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; }