Created attachment 112086 [details] Crash log. r Reproducibility: always Steps: Go to http://dictionary.reference.com/browse/hem What happened: Infinite loop, assert failure and crash. ASSERTION FAILED: row->isAnonymous() /Users/rex/WebKit/Source/WebCore/rendering/RenderTableSection.cpp(123) : virtual void WebCore::RenderTableSection::addChild(WebCore::RenderObject *, WebCore::RenderObject *) 1 0x1054c81e8 WebCore::RenderTableSection::addChild(WebCore::RenderObject*, WebCore::RenderObject*) 2 0x105247e20 WebCore::NodeRendererFactory::createRendererIfNeeded() 3 0x10522cad9 WebCore::Node::createRendererIfNeeded() 4 0x104885a78 WebCore::Element::attach() 5 0x1044867d6 WebCore::Node::reattach() 6 0x1048863f3 WebCore::Element::recalcStyle(WebCore::Node::StyleChange) 7 0x104886baf WebCore::Element::recalcStyle(WebCore::Node::StyleChange) 8 0x104886baf WebCore::Element::recalcStyle(WebCore::Node::StyleChange) 9 0x104886baf WebCore::Element::recalcStyle(WebCore::Node::StyleChange) 10 0x104886baf WebCore::Element::recalcStyle(WebCore::Node::StyleChange) 11 0x104886baf WebCore::Element::recalcStyle(WebCore::Node::StyleChange) 12 0x104886baf WebCore::Element::recalcStyle(WebCore::Node::StyleChange) 13 0x104886baf WebCore::Element::recalcStyle(WebCore::Node::StyleChange) 14 0x104886baf WebCore::Element::recalcStyle(WebCore::Node::StyleChange) 15 0x104886baf WebCore::Element::recalcStyle(WebCore::Node::StyleChange) 16 0x104886baf WebCore::Element::recalcStyle(WebCore::Node::StyleChange) 17 0x104886baf WebCore::Element::recalcStyle(WebCore::Node::StyleChange) 18 0x104886baf WebCore::Element::recalcStyle(WebCore::Node::StyleChange) 19 0x104886baf WebCore::Element::recalcStyle(WebCore::Node::StyleChange) 20 0x104886baf WebCore::Element::recalcStyle(WebCore::Node::StyleChange) 21 0x104886baf WebCore::Element::recalcStyle(WebCore::Node::StyleChange) 22 0x104886baf WebCore::Element::recalcStyle(WebCore::Node::StyleChange) 23 0x104886baf WebCore::Element::recalcStyle(WebCore::Node::StyleChange) 24 0x1046b74e1 WebCore::Document::recalcStyle(WebCore::Node::StyleChange) 25 0x1046b8559 WebCore::Document::updateStyleIfNeeded() 26 0x1046c2cff WebCore::Document::finishedParsing() 27 0x104af8eff WebCore::HTMLTreeBuilder::finished() 28 0x104a449e9 WebCore::HTMLDocumentParser::end() 29 0x104a4398c WebCore::HTMLDocumentParser::attemptToRunDeferredScriptsAndEnd() 30 0x104a43787 WebCore::HTMLDocumentParser::prepareToStopParsing() 31 0x104a441a1 WebCore::HTMLDocumentParser::endIfDelayed() Expected result: No infinite loop, no assert failure, no crash.
r98157
I'm seeing this bug in Chromium on Linux as well.
(In reply to comment #2) > I'm seeing this bug in Chromium on Linux as well. We would need a reduction to see what is going on. But as i understand, we should probably need to change that assert into a hard check, and not adding it as a child when previous sibling is not anonymous. We should just continue forward in that function and let it get shoved into a new anonymous block. note that you would need fixes in rendertable, rendertablesection, rendertablerow and rendertablecell. (same code exists there, check out bug 69536.)
Created attachment 112224 [details] Reduction: Inserting span between two rows triggers an ASSERT failure
Nice, so now you need to see whether the span has to go in its own new anonymous block or it has to go as a last child of row-1. Please compare with firefox, opera renderings.
(In reply to comment #5) > Nice, so now you need to see whether the span has to go in its own new anonymous block or it has to go as a last child of row-1. Please compare with firefox, opera renderings. Add this to testcase. spanElement.appendChild(document.createTextNode("Some text.")); You will see the wrong rendering. You need to change that assert into a hard check. We need to add the child into its own new anonymous row.
(In reply to comment #6) > (In reply to comment #5) > > Nice, so now you need to see whether the span has to go in its own new anonymous block or it has to go as a last child of row-1. Please compare with firefox, opera renderings. > > Add this to testcase. spanElement.appendChild(document.createTextNode("Some text.")); > > You will see the wrong rendering. You need to change that assert into a hard check. We need to add the child into its own new anonymous row. Yup, Firefox inserts the text as a row between row-1, and row-2.
Created attachment 112230 [details] Improved Reduction
(In reply to comment #8) > Created an attachment (id=112230) [details] > Improved Reduction Perfect, do remember to fix all the three places Source/WebCore/rendering/RenderTable.cpp (view diffs) Source/WebCore/rendering/RenderTableRow.cpp (view diffs) Source/WebCore/rendering/RenderTableSection.cpp (view diffs)
(In reply to comment #9) > (In reply to comment #8) > > Created an attachment (id=112230) [details] [details] > > Improved Reduction > > Perfect, do remember to fix all the three places > > Source/WebCore/rendering/RenderTable.cpp (view diffs) > Source/WebCore/rendering/RenderTableRow.cpp (view diffs) > Source/WebCore/rendering/RenderTableSection.cpp (view diffs) Yup, will do, I probably needs tests for all three cases right? Three layout tests.
(In reply to comment #10) > (In reply to comment #9) > > (In reply to comment #8) > > > Created an attachment (id=112230) [details] [details] [details] > > > Improved Reduction > > > > Perfect, do remember to fix all the three places > > > > Source/WebCore/rendering/RenderTable.cpp (view diffs) > > Source/WebCore/rendering/RenderTableRow.cpp (view diffs) > > Source/WebCore/rendering/RenderTableSection.cpp (view diffs) > > Yup, will do, I probably needs tests for all three cases right? Three layout tests. Yeah. You can add all the cases in one test too.
Created attachment 112265 [details] Patch
Comment on attachment 112265 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=112265&action=review > Source/WebCore/ChangeLog:13 > + we need to create a new anonymous Section/Row/Cell respectively, insteading failing an "insteading" -> instead of
Created attachment 112267 [details] Patch
(In reply to comment #13) > (From update of attachment 112265 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=112265&action=review > > > Source/WebCore/ChangeLog:13 > > + we need to create a new anonymous Section/Row/Cell respectively, insteading failing an > > "insteading" -> instead of Oops, fixed.
Comment on attachment 112267 [details] Patch r=me
Comment on attachment 112267 [details] Patch Clearing flags on attachment: 112267 Committed r98372: <http://trac.webkit.org/changeset/98372>
All reviewed patches have been landed. Closing bug.