document.write does not work correctly in the HTML5 parser

• Component: New Bugs
• Status: RESOLVED FIXED

[Eric Seidel (no email)]

document.write does not work correctly in the HTML5 parser

[Eric Seidel (no email)]

Created attachment 57222 [details]
work in progress

[Eric Seidel (no email)]

Comment on attachment 57222 [details]
work in progress

Sorry, forgot the --no-review flag.

This patch has some extra junk in it and is not ready for review.

It passes the simple document.write cases, but hits the ASSERT(scriptElement) in bool HTML5ScriptRunner::execute(PassRefPtr<Element> scriptElement) on more complicated ones.  I think that's expected, I just need to sort through the exact conditions under which that ASSERT is bogus.

[Eric Seidel (no email)]

Created attachment 57303 [details]
Patch

[Eric Seidel (no email)]

This implementation is WAY simpler than the current document.write implementation in the old parser.  Either I did something wrong, or this is validation of our parser design. :)  I'm hopeful for the latter.

[Eric Seidel (no email)]

This appears to have fixed our major crash-on-navigation issue which I believe was caused by failing to unregister as a client from a CachedResource and then having it notify a dead pointer. :)

[Eric Seidel (no email)]

Actually, looking at this more, I think this will be an incomplete solution.  There are other codepaths through which we could re-enter scripting.  The various event dispatches being some.

We may need to move the event dispatch code onto the HTML5ScriptRunnerHost.

[Eric Seidel (no email)]

fast/dom/document-clear.html still hits an ASSERT after this change, but many many more layout tests pass. :)

[Eric Seidel (no email)]

Created attachment 57305 [details]
Updated to TOT and removed tabs

[Adam Barth]

Comment on attachment 57305 [details]
Updated to TOT and removed tabs

Wow, that's very pretty.  :)  Some comments below.

WebCore/ChangeLog:12
 +          before calling ScriptControlle::executeScript to allow safe
ScriptControlle -> ScriptController

WebCore/html/HTML5Tokenizer.cpp:42
 +      , m_docuemnt(document)
m_docuemnt -> m_document

WebCore/html/HTML5Tokenizer.cpp:120
 +      // not an already-loaded CachedResource.
There's some grammar problems here.

WebCore/html/HTML5Tokenizer.cpp:131
 +      // Not currently possible to execute scripts on frameless documents.
It's not so much that it's not possible.  We explicitly don't want to.

WebCore/html/HTML5Tokenizer.cpp:135
 +      SegmentedString oldInsertionPoint = m_source;
How expensive is this copy?  Maybe we should use OwnPtr and swap?

[Adam Barth]

Created attachment 57308 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 57308 [details]
Patch for landing

Clearing flags on attachment: 57308

Committed r60347: <http://trac.webkit.org/changeset/60347>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.