[Webkit-unassigned] [Bug 46761] Remove calls to Document::updateStyleForAllDocuments()

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=46761





--- Comment #4 from Pratik Solanki <psolanki at apple.com>  2011-01-17 16:15:23 PST ---
I tried this out and I can remove all calls to Document::updateStyleForAllDocuments() except the one in ScriptControllerBase.cpp. Removing that one causes layout test failures. Attached is a test case of code that fails. We get an extra blank line at the end if we don't call updateStyleForAllDocuments.

Debugging the code, I see that the extra whitespace node is created at

#0  WebCore::Text::Text (this=0x10595f410, document=0x1060a3a00, data=@0x7fff5fbfdc70) at Text.h:48
#1  0x0000000101b4f6dc in WebCore::Text::create (document=0x1060a3a00, data=@0x7fff5fbfdc70) at /Volumes/Data/psolanki/sources/external/WebKit.git/Source/WebCore/dom/Text.cpp:46
#2  0x000000010136d3f4 in WebCore::HTMLConstructionSite::insertTextNode (this=0x1059601c8, characters=@0x7fff5fbfdc70) at /Volumes/Data/psolanki/sources/external/WebKit.git/Source/WebCore/html/parser/HTMLConstructionSite.cpp:326
#3  0x00000001013e882c in WebCore::HTMLTreeBuilder::processCharacterBuffer (this=0x105960190, buffer=@0x7fff5fbfdcd0) at /Volumes/Data/psolanki/sources/external/WebKit.git/Source/WebCore/html/parser/HTMLTreeBuilder.cpp:2465
#4  0x00000001013e8ed2 in WebCore::HTMLTreeBuilder::processCharacter (this=0x105960190, token=@0x7fff5fbfdd70) at /Volumes/Data/psolanki/sources/external/WebKit.git/Source/WebCore/html/parser/HTMLTreeBuilder.cpp:2408
#5  0x00000001013e2bea in WebCore::HTMLTreeBuilder::processToken (this=0x105960190, token=@0x7fff5fbfdd70) at /Volumes/Data/psolanki/sources/external/WebKit.git/Source/WebCore/html/parser/HTMLTreeBuilder.cpp:480
#6  0x00000001013ec070 in WebCore::HTMLTreeBuilder::constructTreeFromAtomicToken (this=0x105960190, token=@0x7fff5fbfdd70) at /Volumes/Data/psolanki/sources/external/WebKit.git/Source/WebCore/html/parser/HTMLTreeBuilder.cpp:452
#7  0x00000001013ec14a in WebCore::HTMLTreeBuilder::constructTreeFromToken (this=0x105960190, rawToken=@0x1060a14b0) at /Volumes/Data/psolanki/sources/external/WebKit.git/Source/WebCore/html/parser/HTMLTreeBuilder.cpp:447
#8  0x000000010137231f in WebCore::HTMLDocumentParser::pumpTokenizer (this=0x1060a1400, mode=WebCore::HTMLDocumentParser::AllowYield) at /Volumes/Data/psolanki/sources/external/WebKit.git/Source/WebCore/html/parser/HTMLDocumentParser.cpp:232
#9  0x0000000101372636 in WebCore::HTMLDocumentParser::pumpTokenizerIfPossible (this=0x1060a1400, mode=WebCore::HTMLDocumentParser::AllowYield) at /Volumes/Data/psolanki/sources/external/WebKit.git/Source/WebCore/html/parser/HTMLDocumentParser.cpp:169
#10 0x0000000101372ad8 in WebCore::HTMLDocumentParser::append (this=0x1060a1400, source=@0x7fff5fbfded0) at /Volumes/Data/psolanki/sources/external/WebKit.git/Source/WebCore/html/parser/HTMLDocumentParser.cpp:320
#11 0x00000001010efbb4 in WebCore::DecodedDataDocumentParser::appendBytes (this=0x1060a1400, writer=0x1060848a8, data=0x0, length=0, shouldFlush=true) at /Volumes/Data/psolanki/sources/external/WebKit.git/Source/WebCore/dom/DecodedDataDocumentParser.cpp:54
#12 0x0000000101152190 in WebCore::DocumentWriter::addData (this=0x1060848a8, str=0x0, len=0, flush=true) at /Volumes/Data/psolanki/sources/external/WebKit.git/Source/WebCore/loader/DocumentWriter.cpp:200
#13 0x0000000101152212 in WebCore::DocumentWriter::endIfNotLoadingMainResource (this=0x1060848a8) at /Volumes/Data/psolanki/sources/external/WebKit.git/Source/WebCore/loader/DocumentWriter.cpp:220
#14 0x000000010115225b in WebCore::DocumentWriter::end (this=0x1060848a8) at /Volumes/Data/psolanki/sources/external/WebKit.git/Source/WebCore/loader/DocumentWriter.cpp:206
#15 0x00000001011426b8 in WebCore::DocumentLoader::finishedLoading (this=0x1060ea600) at /Volumes/Data/psolanki/sources/external/WebKit.git/Source/WebCore/loader/DocumentLoader.cpp:279
#16 0x00000001012ac961 in WebCore::FrameLoader::finishedLoading (this=0x1060846b8) at /Volumes/Data/psolanki/sources/external/WebKit.git/Source/WebCore/loader/FrameLoader.cpp:2175
....

In current code the node has a NULL renderer because of this check in Text::rendererIsNeeded() which returns false.

    RenderObject *prev = previousRenderer();
    if (prev && prev->isBR()) // <span><br/> <br/></span>
        return false;

This works because the renderer for the BR tag is created by the call to updateStyleForAllDocuments(). Here is the stacktrace

#0  WebCore::HTMLBRElement::createRenderer (this=0x111509c50, arena=0x1059438b0, style=0x113555a20) at /Volumes/Data/psolanki/sources/external/WebKit.git/Source/WebCore/html/HTMLBRElement.cpp:79
#1  0x00000001017e94f6 in WebCore::Node::createRendererIfNeeded (this=0x111509c50) at /Volumes/Data/psolanki/sources/external/WebKit.git/Source/WebCore/dom/Node.cpp:1381
#2  0x000000010122c893 in WebCore::Element::attach (this=0x111509c50) at /Volumes/Data/psolanki/sources/external/WebKit.git/Source/WebCore/dom/Element.cpp:918
#3  0x000000010122c0f0 in WebCore::Element::recalcStyle (this=0x111509c50, change=WebCore::Node::NoChange) at /Volumes/Data/psolanki/sources/external/WebKit.git/Source/WebCore/dom/Element.cpp:1008
#4  0x000000010122c6dd in WebCore::Element::recalcStyle (this=0x116921350, change=WebCore::Node::NoChange) at /Volumes/Data/psolanki/sources/external/WebKit.git/Source/WebCore/dom/Element.cpp:1071
#5  0x000000010122c6dd in WebCore::Element::recalcStyle (this=0x105943100, change=WebCore::Node::NoChange) at /Volumes/Data/psolanki/sources/external/WebKit.git/Source/WebCore/dom/Element.cpp:1071
#6  0x0000000101112ad8 in WebCore::Document::recalcStyle (this=0x1060f5800, change=WebCore::Node::NoChange) at /Volumes/Data/psolanki/sources/external/WebKit.git/Source/WebCore/dom/Document.cpp:1599
#7  0x00000001011127ab in WebCore::Document::updateStyleIfNeeded (this=0x1060f5800) at /Volumes/Data/psolanki/sources/external/WebKit.git/Source/WebCore/dom/Document.cpp:1641
#8  0x000000010110b461 in WebCore::Document::updateStyleForAllDocuments () at /Volumes/Data/psolanki/sources/external/WebKit.git/Source/WebCore/dom/Document.cpp:1658
#9  0x00000001019eb50c in WebCore::ScriptController::executeScript (this=0x1060ae7b8, sourceCode=@0x7fff5fbfde70, shouldAllowXSS=WebCore::DoNotAllowXSS) at /Volumes/Data/psolanki/sources/external/WebKit.git/Source/WebCore/bindings/ScriptControllerBase.cpp:64
#10 0x00000001019f6c8a in WebCore::ScriptElement::executeScript (this=0x105d00e70, sourceCode=@0x7fff5fbfde70) at /Volumes/Data/psolanki/sources/external/WebKit.git/Source/WebCore/dom/ScriptElement.cpp:214
#11 0x00000001013c5fa4 in WebCore::HTMLScriptRunner::runScript (this=0x11690e7c0, script=0x105d00df0, scriptStartPosition=@0x7fff5fbfdfb0) at /Volumes/Data/psolanki/sources/external/WebKit.git/Source/WebCore/html/parser/HTMLScriptRunner.cpp:316
...

In my updated code, if I remove the call to updateStyleForAllDocuments from ScriptControllerBase.cpp, I see my empty node's renderer being created first. At that point, BR element renderer is NULL and thus the optimization in Text::rendererIsNeeded() does not work. The BR node renderer is created later as

#0  WebCore::HTMLBRElement::createRenderer (this=0x113d917c0, arena=0x105978140, style=0x113dbc2f0) at /Volumes/Data/psolanki/sources/external/WebKit.git/Source/WebCore/html/HTMLBRElement.cpp:79
#1  0x00000001017e94fe in WebCore::Node::createRendererIfNeeded (this=0x113d917c0) at /Volumes/Data/psolanki/sources/external/WebKit.git/Source/WebCore/dom/Node.cpp:1381
#2  0x000000010122c89b in WebCore::Element::attach (this=0x113d917c0) at /Volumes/Data/psolanki/sources/external/WebKit.git/Source/WebCore/dom/Element.cpp:918
#3  0x000000010122c0f8 in WebCore::Element::recalcStyle (this=0x113d917c0, change=WebCore::Node::NoChange) at /Volumes/Data/psolanki/sources/external/WebKit.git/Source/WebCore/dom/Element.cpp:1008
#4  0x000000010122c6e5 in WebCore::Element::recalcStyle (this=0x113dc3ae0, change=WebCore::Node::NoChange) at /Volumes/Data/psolanki/sources/external/WebKit.git/Source/WebCore/dom/Element.cpp:1071
#5  0x000000010122c6e5 in WebCore::Element::recalcStyle (this=0x1059da420, change=WebCore::Node::NoChange) at /Volumes/Data/psolanki/sources/external/WebKit.git/Source/WebCore/dom/Element.cpp:1071
#6  0x0000000101112ae0 in WebCore::Document::recalcStyle (this=0x1060b1200, change=WebCore::Node::NoChange) at /Volumes/Data/psolanki/sources/external/WebKit.git/Source/WebCore/dom/Document.cpp:1599
#7  0x00000001011127b3 in WebCore::Document::updateStyleIfNeeded (this=0x1060b1200) at /Volumes/Data/psolanki/sources/external/WebKit.git/Source/WebCore/dom/Document.cpp:1641
#8  0x0000000101110c06 in WebCore::Document::finishedParsing (this=0x1060b1200) at /Volumes/Data/psolanki/sources/external/WebKit.git/Source/WebCore/dom/Document.cpp:4258
#9  0x00000001013ec23c in WebCore::HTMLTreeBuilder::finished (this=0x113d475a0) at /Volumes/Data/psolanki/sources/external/WebKit.git/Source/WebCore/html/parser/HTMLTreeBuilder.cpp:2805
#10 0x0000000101371fca in WebCore::HTMLDocumentParser::end (this=0x1060bdc00) at /Volumes/Data/psolanki/sources/external/WebKit.git/Source/WebCore/html/parser/HTMLDocumentParser.cpp:332
#11 0x00000001013720b5 in WebCore::HTMLDocumentParser::attemptToRunDeferredScriptsAndEnd (this=0x1060bdc00) at /Volumes/Data/psolanki/sources/external/WebKit.git/Source/WebCore/html/parser/HTMLDocumentParser.cpp:341
#12 0x0000000101372cce in WebCore::HTMLDocumentParser::prepareToStopParsing (this=0x1060bdc00) at /Volumes/Data/psolanki/sources/external/WebKit.git/Source/WebCore/html/parser/HTMLDocumentParser.cpp:150
#13 0x0000000101371ef0 in WebCore::HTMLDocumentParser::attemptToEnd (this=0x1060bdc00) at /Volumes/Data/psolanki/sources/external/WebKit.git/Source/WebCore/html/parser/HTMLDocumentParser.cpp:353
#14 0x0000000101371f28 in WebCore::HTMLDocumentParser::finish (this=0x1060bdc00) at /Volumes/Data/psolanki/sources/external/WebKit.git/Source/WebCore/html/parser/HTMLDocumentParser.cpp:381
#15 0x0000000101109b78 in WebCore::Document::finishParsing (this=0x1060b1200) at /Volumes/Data/psolanki/sources/external/WebKit.git/Source/WebCore/dom/Document.cpp:2290
#16 0x000000010115222e in WebCore::DocumentWriter::endIfNotLoadingMainResource (this=0x10608daa8) at /Volumes/Data/psolanki/sources/external/WebKit.git/Source/WebCore/loader/DocumentWriter.cpp:221
#17 0x0000000101152263 in WebCore::DocumentWriter::end (this=0x10608daa8) at /Volumes/Data/psolanki/sources/external/WebKit.git/Source/WebCore/loader/DocumentWriter.cpp:206
#18 0x00000001011426c0 in WebCore::DocumentLoader::finishedLoading (this=0x106104000) at /Volumes/Data/psolanki/sources/external/WebKit.git/Source/WebCore/loader/DocumentLoader.cpp:279
#19 0x00000001012ac969 in WebCore::FrameLoader::finishedLoading (this=0x10608d8b8) at /Volumes/Data/psolanki/sources/external/WebKit.git/Source/WebCore/loader/FrameLoader.cpp:2175
....

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.