Bug 39828

Summary: document.write does not work correctly in the HTML5 parser
Product: WebKit Reporter: Eric Seidel (no email) <eric>
Component: New BugsAssignee: Eric Seidel (no email) <eric>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, commit-queue, hyatt, tonyg
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: OS X 10.5   
Bug Depends on:    
Bug Blocks: 39259    
Attachments:
Description Flags
work in progress
none
Patch
none
Updated to TOT and removed tabs
none
Patch for landing none

Eric Seidel (no email)
Reported 2010-05-27 04:37:35 PDT
document.write does not work correctly in the HTML5 parser
Attachments
work in progress (16.88 KB, patch)
2010-05-27 04:40 PDT, Eric Seidel (no email) 		no flags
DetailsFormatted DiffDiff
Patch (25.16 KB, patch)
2010-05-28 01:25 PDT, Eric Seidel (no email) 		no flags
DetailsFormatted DiffDiff
Updated to TOT and removed tabs (25.18 KB, patch)
2010-05-28 01:52 PDT, Eric Seidel (no email) 		no flags
DetailsFormatted DiffDiff
Patch for landing (24.67 KB, patch)
2010-05-28 02:54 PDT, Adam Barth 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Eric Seidel (no email)
Comment 1 2010-05-27 04:40:37 PDT
Created attachment 57222 [details] work in progress
Eric Seidel (no email)
Comment 2 2010-05-27 04:42:36 PDT
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)
Comment 3 2010-05-28 01:25:43 PDT
Created attachment 57303 [details] Patch
Eric Seidel (no email)
Comment 4 2010-05-28 01:26:47 PDT
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)
Comment 5 2010-05-28 01:33:43 PDT
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)
Comment 6 2010-05-28 01:40:22 PDT
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)
Comment 7 2010-05-28 01:42:14 PDT
fast/dom/document-clear.html still hits an ASSERT after this change, but many many more layout tests pass. :)
Eric Seidel (no email)
Comment 8 2010-05-28 01:52:09 PDT
Created attachment 57305 [details] Updated to TOT and removed tabs
Adam Barth
Comment 9 2010-05-28 02:30:24 PDT
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
Comment 10 2010-05-28 02:54:41 PDT
Created attachment 57308 [details] Patch for landing
WebKit Commit Bot
Comment 11 2010-05-28 03:11:25 PDT
Comment on attachment 57308 [details] Patch for landing Clearing flags on attachment: 57308 Committed r60347: <http://trac.webkit.org/changeset/60347>
WebKit Commit Bot
Comment 12 2010-05-28 03:11:32 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.