[webkit-reviews] review denied: [Bug 112369] Assertion failure !m_lastChunkBeforeScript in HTMLDocumentParser during inspector/debugger/pause-in-inline-script.html : [Attachment 195664] Patch

[bugzilla-daemon at webkit.org]

Adam Barth <abarth at webkit.org> has denied  review:
Bug 112369: Assertion failure !m_lastChunkBeforeScript in HTMLDocumentParser
during inspector/debugger/pause-in-inline-script.html
https://bugs.webkit.org/show_bug.cgi?id=112369

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

------- Additional Comments from Adam Barth <abarth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=195664&action=review


> Source/WebCore/html/parser/HTMLDocumentParser.cpp:320
> +    bool calledFromNestedEventLoop = (document()->activeParserCount() > 0);

The problem with using activeParserCount is that you'll get cross-talk from
other parsers associated with the current document.  Maybe that's ok because
only the first parser can be a threaded parser (since all other parsers will be
script-created)?

It seems better to use state that's associated with this this specific parser
rather than using state associated wit the document.

> Source/WebCore/html/parser/HTMLDocumentParser.cpp:346
> +    if (!m_speculations.isEmpty() && !isWaitingForScripts() && !isStopped())

> +	   pumpPendingSpeculations();

This is just duplicating logic from pumpPendingSpeculations.  IMHO, we should
call pumpPendingSpeculations instead of processParsedChunkFromBackgroundParser
so that we measure the parserTimeLimit accurately.