Summary: | Threaded HTML parser should pass the remaining fast/tokenizer tests | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Adam Barth <abarth> | ||||
Component: | New Bugs | Assignee: | Adam Barth <abarth> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | eric, ojan.autocc, tonyg, webkit.review.bot | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Bug Depends on: | |||||||
Bug Blocks: | 106127 | ||||||
Attachments: |
|
Description
Adam Barth
2013-02-12 13:12:32 PST
Created attachment 187917 [details]
Patch
Comment on attachment 187917 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=187917&action=review I think we're gonna need to chat/vc about this one. > Source/WebCore/html/parser/HTMLDocumentParser.cpp:317 > + ASSERT(checkpoint->unparsedInput.isSafeToSendToAnotherThread()); Seems we should just call isSafeToSentToAnotherThread on checkpoint itself? (In reply to comment #2) > (From update of attachment 187917 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=187917&action=review > > I think we're gonna need to chat/vc about this one. Ok. Let me know when you're ready. > > Source/WebCore/html/parser/HTMLDocumentParser.cpp:317 > > + ASSERT(checkpoint->unparsedInput.isSafeToSendToAnotherThread()); > > Seems we should just call isSafeToSentToAnotherThread on checkpoint itself? Sure, we could do that. Comment on attachment 187917 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=187917&action=review > Source/WebCore/html/parser/HTMLDocumentParser.cpp:251 > + ASSERT(!m_haveBackgroundParser || mode == ForceSynchronous); These scare me. We should just make sure both of these ASSERT changes are necessary. Comment on attachment 187917 [details]
Patch
LGTM. Please check the ASSERTS before landing.
We talked through the ASSERTs on VC. It's for the case when you have a script-created parser that document.writes something like "<script src=...></script>Foo". We'll parse the script tag synchronously but then wait for the external script to arrive. Once the external script arrives, we'll parse the "Foo" asynchronously. In that case, shouldUseBackgroundParser is true but we don't m_haveBackgroundParser. We discussed several approaches for cleaning up these state variables, and we'll rename or rework them to be clearer in a future patch. Comment on attachment 187917 [details] Patch Clearing flags on attachment: 187917 Committed r142673: <http://trac.webkit.org/changeset/142673> All reviewed patches have been landed. Closing bug. |