Bug 39891 - HTML5ScriptRunner can re-enter from event dispatch
Summary: HTML5ScriptRunner can re-enter from event dispatch
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 39259
  Show dependency treegraph
 
Reported: 2010-05-28 11:37 PDT by Eric Seidel (no email)
Modified: 2010-06-22 01:45 PDT (History)
2 users (show)

See Also:


Attachments
Cleaned up test case which reveals at least one ASSERT in ToT (300 bytes, text/html)
2010-06-18 10:53 PDT, Eric Seidel (no email)
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Seidel (no email) 2010-05-28 11:37:15 PDT
HTML5ScriptRunner can re-enter from event dispatch

HTML5ScriptRunner tries to only enter scripting when calling "executeScript" on the HTML5SriptRunnerHost, but I realize after writing it that it will also re-enter from events which will cause us to hit m_scriptNestingLevel ASSERTs and do the wrong thing. :)

I tried to write a test for this, but it doesn't quite work yet, and I have more important bugs to fix in the code before I get back to this one so recording it for posterity:

1
<script>
function doubleWrite(number)
{
    document.write("<script>document.write(" + number ")</scr" + "ipt><script>document.write(" + (number+1) ")</scr" + "ipt>")
}
</script>
2
<script
onbeforeload="doubleWrite(3)"
onload="doubleWrite(5)"
src="data:text/plain,doubleWrite(7)"
></script>
9
Comment 1 Eric Seidel (no email) 2010-06-18 10:53:39 PDT
Created attachment 59134 [details]
Cleaned up test case which reveals at least one ASSERT in ToT

The fix for the first assert:

ASSERTION FAILED: !haveParsingBlockingScript()
(/Projects/WebKit/WebCore/html/HTML5ScriptRunner.cpp:262 void WebCore::HTML5ScriptRunner::runScript(WebCore::Element*, int))

is to just re-order the setting of m_parsingBlockingScript until after the beforeLoad check, since the before load might cancel the script anyway!

The next assertions you hit, of close m_source, relate to the insertion point never getting set for some of these calls.  Those need a bit more thought.
Comment 2 Eric Seidel (no email) 2010-06-21 14:17:50 PDT
http://code.google.com/p/chromium/issues/detail?id=47080
is probably caused by this issue.
Comment 3 Eric Seidel (no email) 2010-06-22 01:44:49 PDT
As of http://trac.webkit.org/changeset/61610 we believe this issue to be completely resolved.  Adam is also mailing the W3c about Minefield's behavior discrepancy.
Comment 4 Eric Seidel (no email) 2010-06-22 01:45:29 PDT
http://trac.webkit.org/changeset/61606
http://trac.webkit.org/changeset/61607
http://trac.webkit.org/changeset/61608
were all related to this fix, btw.