Bug 97829 – REGRESSION (r122168?): External script that includes itself blocks HTML Parser

• Component: DOM
• Status: RESOLVED WONTFIX

[Michael Eastham]

Created attachment 166089 [details]
Test case

Hi there,

The attached pair of scripts cause WebKit's HTML parser to hang. The first defines a function ('callback') which includes the second script using document.write(), and the second script calls this function, thereby inserting itself into the page again. In Chrome 22.0.1229.79 beta opening 'test.html' causes the tab to become unresponsive to clicks, etc. This issue seems to have been introduced in the reworking of how HTMLDocumentParser is paused when waiting for external scripts that was done in r122168 and r122226.

Thanks for taking a look at this.

[Eric Seidel (no email)]

http://trac.webkit.org/changeset/122168

Happy to take a look on Monday, unless Adam beats me to it.

[Eric Seidel (no email)]

So I'm confused.  This is an infinite loop.  Firefox hangs similarly...

[Eric Seidel (no email)]

Or to re-phrase:  I'm trying to understand what the expected behavior is.  Do you want the browser to not hang?  Do you want us to eventually detect the cycle and stop?

[Eric Seidel (no email)]

Michael replied privately that his concern was that the parser never yields (or takes a long time to yield).  Presumably we just need to make sure we're returning to teh scheduling loop so the scheduler can tell us to take a break for a while.

[Eric Seidel (no email)]

Looking at this again now.  It appears from the code it should be yielding, but I'll bring it up in the debugger to confirm.

[Eric Seidel (no email)]

This behavior happens because the PendingScript is now in the ScriptRunner, and we end up looping infinitely here:

void HTMLScriptRunner::executeParsingBlockingScripts()
{
    while (hasParserBlockingScript() && isPendingScriptReady(m_parserBlockingScript))
        executeParsingBlockingScript();
}

But this is only possible if both resources are already in the cache.  Otherwise we'd yield and get a callback from the network when the script was ready.

I think that if a page is doing this infinite loop, we're already going to hang, so hanging here isn't particularly bad.  But it's certainly possible to devise a yield-system for this case.

My first instinct is to just mark this WONTFIX and wait to see if we get any reports of this happening on real websites.

[Eric Seidel (no email)]

Marking this as wont fix.

It's possible we'll want to re-open this bug and change this behavior.  But the complexity (adding an additional timer to handle this yield case) does not seem worth it at this time.


In order to hit this behavior, a page would have to:

1.  Load a set of scripts into the cache (presumably because the page was previously loaded and is being refreshed).
2.  document.write() a script from another script.

We won't yield before executing the document.written script if it's already cached (as we previously would have), even if it takes a long time to execute any of these scripts.

I believe this not to be a problem in practice for sites.

[Eric Seidel (no email)]

(I should note that Michael was hitting this not in a web browser, but rather in one of Google's innumerable server-side forks of WebKit which was hanging in a new case he was not expecting.  Obviously there are lots of ways to hang WebKit, so we're just stressing his "watchdog" infrastructure in a new way.  If actual web users hit this issue, then we should re-visit the idea of making a code change here.)