Bug 39903 – HTML5 parser should block script execution until stylesheets load

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Eric Seidel (no email)]

HTML5 parser should block script execution while stylesheets load

[Eric Seidel (no email)]

Created attachment 57382 [details]
work in progress

[Eric Seidel (no email)]

This patch doesn't work yet because it triggers parser reentry on any <style> tag. :(


#4	0x0440bf46 in WebCore::HTML5Tokenizer::resumeParsingAfterScriptExecution at HTML5Tokenizer.cpp:110
#5	0x0440bff3 in WebCore::HTML5Tokenizer::executeScriptsWaitingForStylesheets at HTML5Tokenizer.cpp:155
#6	0x04246567 in WebCore::Document::removePendingSheet at Document.cpp:2567
#7	0x0447e0ac in WebCore::HTMLStyleElement::sheetLoaded at HTMLStyleElement.cpp:107
#8	0x041f0305 in WebCore::CSSStyleSheet::checkLoaded at CSSStyleSheet.cpp:213
#9	0x04aa3412 in WebCore::StyleElement::createSheet at StyleElement.cpp:115
#10	0x04aa3671 in WebCore::StyleElement::process at StyleElement.cpp:85
#11	0x0447e25d in WebCore::HTMLStyleElement::finishParsingChildren at HTMLStyleElement.cpp:61
#12	0x04476c30 in WebCore::HTMLParser::popOneBlockCommon at HTMLParser.cpp:1480
#13	0x0446fabe in WebCore::HTMLParser::popOneBlock at HTMLParser.cpp:1508
#14	0x04471756 in WebCore::HTMLParser::popBlock at HTMLParser.cpp:1423
#15	0x044762fb in WebCore::HTMLParser::processCloseTag at HTMLParser.cpp:1023
#16	0x04475c0b in WebCore::HTMLParser::parseToken at HTMLParser.cpp:271
#17	0x0440d763 in WebCore::HTML5TreeBuilder::passTokenToLegacyParser at HTML5TreeBuilder.cpp:140
#18	0x0440dcaf in WebCore::HTML5TreeBuilder::constructTreeFromToken at HTML5TreeBuilder.cpp:173
#19	0x0440bdd9 in WebCore::HTML5Tokenizer::pumpLexer at HTML5Tokenizer.cpp:62
#20	0x0440bf46 in WebCore::HTML5Tokenizer::resumeParsingAfterScriptExecution at HTML5Tokenizer.cpp:110
#21	0x0440c066 in WebCore::HTML5Tokenizer::notifyFinished at HTML5Tokenizer.cpp:147
#22	0x040d78af in WebCore::CachedScript::checkNotify at CachedScript.cpp:106
#23	0x040d7993 in WebCore::CachedScript::data at CachedScript.cpp:96
#24	0x048092ba in WebCore::Loader::Host::didFinishLoading at loader.cpp:406
#25	0x04aa97f6 in WebCore::SubresourceLoader::didFinishLoading at SubresourceLoader.cpp:194
#26	0x04a01b9c in WebCore::ResourceLoader::didFinishLoading at ResourceLoader.cpp:443
#27	0x049fdff1 in -[WebCoreResourceHandleAsDelegate connectionDidFinishLoading:] at ResourceHandleMac.mm:862

I'm not sure yet what the proper solution is.  We may have to not do anything in executeScriptsWaitingForStylesheets until after executeScriptsWaitingForLoad has executed once and failed to due to unloaded style sheets.

The old HTMLTokenizer has something similar to that:

    // Make external scripts wait for external stylesheets.
    // FIXME: This needs to be done for inline scripts too.
    m_hasScriptsWaitingForStylesheets = !m_doc->haveStylesheetsLoaded();
    if (m_hasScriptsWaitingForStylesheets)
        return;

We probably want that exact check, but we need to explain better in our comments why it does what it does.

[Eric Seidel (no email)]

Created attachment 57392 [details]
I believe this patch works, still testing

[Eric Seidel (no email)]

Created attachment 57406 [details]
patch for review

[Eric Seidel (no email)]

Created attachment 57407 [details]
Added fix for fast/parser/tokenizer-close-during-document-write.html

[Adam Barth]

Comment on attachment 57407 [details]
Added fix for fast/parser/tokenizer-close-during-document-write.html

Makes sense to me.  Can we add a test that we block script in this way?  It's not clear to me exactly how this is observable without CSS expressions().

[WebKit Commit Bot]

Comment on attachment 57407 [details]
Added fix for fast/parser/tokenizer-close-during-document-write.html

Clearing flags on attachment: 57407

Committed r60409: <http://trac.webkit.org/changeset/60409>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Eric Seidel (no email)]

Yes, we can detect this.  Start a timer at the top of the page, then load a slow style sheet, then check the time in a script.  I'm not sure if we already have tests for htat since we have no clue how many tests we pass/fail yet. :)  However this fixed a bunch of test cases.  Not sure if that aspect of the change is tested or not.  I can certainly write more tests.