Bug 39903

Summary: HTML5 parser should block script execution until stylesheets load
Product: WebKit Reporter: Eric Seidel (no email) <eric>
Component: New BugsAssignee: Eric Seidel (no email) <eric>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, commit-queue, hyatt
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: OS X 10.5   
Attachments:
Description Flags
work in progress
none
I believe this patch works, still testing
none
patch for review
none
Added fix for fast/parser/tokenizer-close-during-document-write.html none

Description Eric Seidel (no email) 2010-05-28 14:34:40 PDT
HTML5 parser should block script execution while stylesheets load
Comment 1 Eric Seidel (no email) 2010-05-28 14:37:59 PDT
Created attachment 57382 [details]
work in progress
Comment 2 Eric Seidel (no email) 2010-05-28 14:42:11 PDT
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.
Comment 3 Eric Seidel (no email) 2010-05-28 15:52:08 PDT
Created attachment 57392 [details]
I believe this patch works, still testing
Comment 4 Eric Seidel (no email) 2010-05-29 00:22:45 PDT
Created attachment 57406 [details]
patch for review
Comment 5 Eric Seidel (no email) 2010-05-29 01:24:45 PDT
Created attachment 57407 [details]
Added fix for fast/parser/tokenizer-close-during-document-write.html
Comment 6 Adam Barth 2010-05-29 08:57:37 PDT
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().
Comment 7 WebKit Commit Bot 2010-05-29 09:12:17 PDT
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>
Comment 8 WebKit Commit Bot 2010-05-29 09:12:23 PDT
All reviewed patches have been landed.  Closing bug.
Comment 9 Eric Seidel (no email) 2010-05-29 10:53:09 PDT
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.