[webkit-reviews] review denied: [Bug 40934] Support <script defer> as specified by HTML5 : [Attachment 60078] Patch

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has denied Tony Gentilcore <tonyg at chromium.org>'s
request for review:
Bug 40934: Support <script defer> as specified by HTML5
https://bugs.webkit.org/show_bug.cgi?id=40934

Attachment 60078: Patch
https://bugs.webkit.org/attachment.cgi?id=60078&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
WebCore/html/HTMLDocumentParser.cpp:292
 +	if (m_scriptRunner &&
!m_scriptRunner->executeScriptsWaitingForParsing()) {
This is kinda subtle.  I might have broken the running of
m_scriptRunner->executeScriptsWaitingForParsing into a separate inner if:

if (m_scriptRunner) {
    bool continueParsing = m_scriptRunner->executeScriptsWaitingForParsing();
    if (!continueParsing) {


WebCore/html/HTMLScriptRunner.cpp:237
 +		watchForLoad(m_parsingBlockingScript);
Is this always right?  What about the case where parsing finshed, but a
stylesheet hasn't loaded.  That seems like it will end up recursing under
watchForLoad() because the m_parsingBlockingScript will have already loaded but
can't run because we're blocked on CSS.  Or?

WebCore/html/HTMLScriptRunner.cpp:245
 +  void HTMLScriptRunner::requestScript(PendingScript& pendingScript, Element*
scriptElement)
I still think this is a fantastic change to this function to make it not affect
the instance, only its args.


otherwise it looks fine.  r- for the possible slow CSS problem.