RESOLVED FIXED 110537
Invalidate outstanding checkpoints for the background input stream and preload scanner 
https://bugs.webkit.org/show_bug.cgi?id=110537
Summary Invalidate outstanding checkpoints for the background input stream and preloa...
Tony Gentilcore
Reported 2013-02-21 17:37:58 PST
Invalidate outstanding checkpoints for the background input stream and preload scanner
Attachments
Patch (4.18 KB, patch)
2013-02-21 17:40 PST, Tony Gentilcore 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Tony Gentilcore
Comment 1 2013-02-21 17:40:59 PST
Created attachment 189648 [details] Patch
Adam Barth
Comment 2 2013-02-21 17:46:48 PST
This patch is probably right, but we should add some tests to ensure that document.writes work correctly in the case of a beforeload event handler on a script tag. For example, what if the beforeload event handler document.writes a partial entity that's completed by the actual script? I guess that shouldn't block this patch, but we should at least have a bug on file so we remember to test that case.
Adam Barth
Comment 3 2013-02-21 17:48:31 PST
By the way, the HTMLDocumentParser should send a message to the BackgroundHTMLParser periodically saying that it will never rewind to a given checkpoint so that we can free up the memory that we're keeping around for those earlier checkpoints.
Tony Gentilcore
Comment 4 2013-02-21 17:53:35 PST
> For example, what if the beforeload event handler document.writes a partial entity that's completed by the actual script? I'll add that test case to this patch tomorrow before landing. > By the way, the HTMLDocumentParser should send a message to the BackgroundHTMLParser periodically saying that it will never rewind to a given checkpoint so that we can free up the memory that we're keeping around for those earlier checkpoints. Isn't that what my new calls to m_checkpoints.clear() do? Maybe I'm missing something else?
Adam Barth
Comment 5 2013-02-21 18:02:25 PST
> Isn't that what my new calls to m_checkpoints.clear() do? Maybe I'm missing something else? Yes, but that only happens if someone calls document.write. We should free up the memory even if the page doesn't call document.write (i.e., when the speculation succeeds).
Tony Gentilcore
Comment 6 2013-02-21 19:44:09 PST
(In reply to comment #4) > > For example, what if the beforeload event handler document.writes a partial entity that's completed by the actual script? > > I'll add that test case to this patch tomorrow before landing. Actually, it turns out that test case crashes the main thread parser. So it needs to be investigated separately. I filed bug 110546.
Tony Gentilcore
Comment 7 2013-02-21 19:57:04 PST
(In reply to comment #5) > > Isn't that what my new calls to m_checkpoints.clear() do? Maybe I'm missing something else? > > Yes, but that only happens if someone calls document.write. We should free up the memory even if the page doesn't call document.write (i.e., when the speculation succeeds). Filed as bug 110547.
WebKit Review Bot
Comment 8 2013-02-21 20:05:26 PST
Comment on attachment 189648 [details] Patch Clearing flags on attachment: 189648 Committed r143685: <http://trac.webkit.org/changeset/143685>
WebKit Review Bot
Comment 9 2013-02-21 20:05:30 PST
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.