Bug 109764

Summary: Make BackgroundHTMLParser work with doc.writes that enter or leave foreign content
Product: WebKit Reporter: Tony Gentilcore <tonyg>
Component: WebCore Misc.Assignee: Adam Barth <abarth>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, eric, esprehn+autocc, ojan.autocc, syoichi, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 106127    
Attachments:
Description Flags
work in progress
none
Patch
none
Patch
none
Patch for landing
none
Patch for landing none

Tony Gentilcore
Reported 2013-02-13 16:12:25 PST
Currently BackgroundHTMLParser keeps track of whether we are in foreign content mode. However, if a doc.write causes us to enter or leave foreign content mode, we'll miss it because doc.write is processed on the main thread. Adam suggests we can fix this by reading the information from the stack of open elements and sending it to the background HTML parser in the Checkpoint sent in resumeFrom().
Attachments
work in progress (8.94 KB, patch)
2013-03-11 14:41 PDT, Adam Barth 		no flags
DetailsFormatted DiffDiff
Patch (13.87 KB, patch)
2013-03-11 15:29 PDT, Adam Barth 		no flags
DetailsFormatted DiffDiff
Patch (13.81 KB, patch)
2013-03-11 15:34 PDT, Adam Barth 		no flags
DetailsFormatted DiffDiff
Patch for landing (13.67 KB, patch)
2013-03-11 16:01 PDT, Adam Barth 		no flags
DetailsFormatted DiffDiff
Patch for landing (12.61 KB, patch)
2013-03-11 17:02 PDT, Adam Barth 		no flags
DetailsFormatted DiffDiff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.
Adam Barth
Comment 1 2013-03-06 14:57:06 PST
This sounds important to fix.
Adam Barth
Comment 2 2013-03-11 14:41:42 PDT
Created attachment 192563 [details] work in progress
Adam Barth
Comment 3 2013-03-11 15:29:54 PDT
Created attachment 192576 [details] Patch
Adam Barth
Comment 4 2013-03-11 15:31:58 PDT
Comment on attachment 192576 [details] Patch Tweaks coming.
Adam Barth
Comment 5 2013-03-11 15:34:50 PDT
Created attachment 192578 [details] Patch
Eric Seidel (no email)
Comment 6 2013-03-11 15:55:07 PDT
Comment on attachment 192578 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=192578&action=review LGTM. Please return the "simulator" prefix. :) > Source/WebCore/html/parser/BackgroundHTMLParser.cpp:155 > + if (!m_treeBuilder.simulate(m_pendingTokens->last(), m_tokenizer.get()) || m_pendingTokens->size() >= pendingTokenLimit) This is a little confusing. It's not really a tree-builder, and if it was you wouldn't tell it to "simulate". :) > Source/WebCore/html/parser/HTMLTreeBuilder.h:71 > + HTMLElementStack* openElements() const { return m_tree.openElements(); } Should this be const? We don't want folks modifying this. :) > Source/WebCore/html/parser/HTMLTreeBuilderSimulator.cpp:136 > + return namespaceStack; You're going to end up copying the vector a couple times here.
Adam Barth
Comment 7 2013-03-11 16:01:30 PDT
(In reply to comment #6) > (From update of attachment 192578 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=192578&action=review > > LGTM. Please return the "simulator" prefix. :) Done. > > Source/WebCore/html/parser/BackgroundHTMLParser.cpp:155 > > + if (!m_treeBuilder.simulate(m_pendingTokens->last(), m_tokenizer.get()) || m_pendingTokens->size() >= pendingTokenLimit) > > This is a little confusing. It's not really a tree-builder, and if it was you wouldn't tell it to "simulate". :) Fixed. > > Source/WebCore/html/parser/HTMLTreeBuilder.h:71 > > + HTMLElementStack* openElements() const { return m_tree.openElements(); } > > Should this be const? We don't want folks modifying this. :) Fixed. > > Source/WebCore/html/parser/HTMLTreeBuilderSimulator.cpp:136 > > + return namespaceStack; > > You're going to end up copying the vector a couple times here. Yeah, I'd be surprised if that was a big problem. We only call this function after executing script. I'm inclined to iterate in a future patch if we see this on a profile.
Adam Barth
Comment 8 2013-03-11 16:01:48 PDT
Created attachment 192583 [details] Patch for landing
Adam Barth
Comment 9 2013-03-11 17:02:04 PDT
Created attachment 192598 [details] Patch for landing
WebKit Review Bot
Comment 10 2013-03-11 19:12:00 PDT
Comment on attachment 192598 [details] Patch for landing Clearing flags on attachment: 192598 Committed r145464: <http://trac.webkit.org/changeset/145464>
WebKit Review Bot
Comment 11 2013-03-11 19:12:05 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.