| Summary: | Invalidate outstanding checkpoints for the background input stream and preload scanner | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Tony Gentilcore <tonyg> | ||||
| Component: | New Bugs | Assignee: | Tony Gentilcore <tonyg> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | abarth, eric, esprehn+autocc, ojan.autocc, webkit.review.bot | ||||
| Priority: | P2 | ||||||
| Version: | 528+ (Nightly build) | ||||||
| Hardware: | Unspecified | ||||||
| OS: | Unspecified | ||||||
| Bug Depends on: | |||||||
| Bug Blocks: | 106127 | ||||||
| Attachments: |
|
||||||
|
Description
Tony Gentilcore
2013-02-21 17:37:58 PST
Created attachment 189648 [details]
Patch
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. 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. > 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? > 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).
(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. (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. Comment on attachment 189648 [details] Patch Clearing flags on attachment: 189648 Committed r143685: <http://trac.webkit.org/changeset/143685> All reviewed patches have been landed. Closing bug. |