Bug 93113

Created attachment 156367 [details] Patch

This patch is also a prerequisite for speculative parsing.

Now HTMLElementStack has a pointer to its owner HTMLConstructionSite. I couldn't come up with a better refactoring. Any idea?

Perhaps HTMLConstructionSite and ElementStack shoudl share some sort of DOMOperations abstraction which they indirect through to make dom modifications. I assume this is done in preparation for making parsing possible on another thread?

(In reply to comment #4) > Perhaps HTMLConstructionSite and ElementStack shoudl share some sort of DOMOperations abstraction which they indirect through to make dom modifications. I assume this is done in preparation for making parsing possible on another thread? It's a good idea. But I chose to move DOM operations to HTMLConstructionSite and also call them construction site tasks in my current implementation of speculative parsing ( https://github.com/kseo/webkit/tree/speculativeparser ) because I need to put beginParsingChildren and finishParsingChildren in the task queue. While speculating, we can't immediately call beginParsingChildren or finishParsingChildren on element because element creation itself is deferred. I identified the list of DOM creations/mutations in the parser: HTMLTreeBuilder - addChildAndAttach - detachNode - moveAllChildrenToNewParent - setAttribute - removeAllChildren HTMLElementStack - beginParsingChildren - finishParsingChildren HTMLConstructionSite - dispatchDocumentElementAvailableIfNeeded - mergeAttributesFromTokenIntoElement - setCompatibilityMode - setCompatibilityModeFromDoctype - setDemoted - insertedByParser - insert - insertOnDocument - insertFosterParentedChild - insertComment - insertCommentOnDocument - insertDoctype - insertText - insertFosterParentedText - createElement - createHTMLElement - createHTMLHtmlElement - createHTMLScriptElement

I think you really want to end up with something like GraphicsContext here. GraphicsContext exists as an (per-platform) abstraction around the drawing context. We pass it around. And it's normally a compile-time defined implementation, but some ports have (in the past) implemented it as a run-time defined implementation. Similarly, it seems like this off-thread parser wants an abstraction through which to talk to the DOM (and possibly a separate one for the reading... although it sounds like you already have the reading part done). I think this abstraction wants to be separate from HTMLConstructionSite, and used by HTMLConstructionSite. Having it be its own separate object, will allow us to separate the mechanics of cross-thread DOM mutation (or queuing of DOM operations) from the logic of what we want to do with the DOM. HTMLConstructionSite remains the HTML parser's "what we want to do with the DOM" logic, and some new class (perhaps DOMModifier or DOMOperations or whatever) is the way that we talk to the DOM. When we're on the main thread, we use a dumb DOMModifier implementation which just calls the DOM operations synchronously. When we're off the main thread, we use a smarter implementation which allocates some sort of objects (similar to how the undo stack, and Editor and EditingCommand work) which are queued up for later execution on the main thread. The code then becomes mostly ignorant of the question of if it's on the main thread or not, and this DOMModifications abstraction handles all the details. I'm not sure if complete ignorance of threaded-ness in HTMLConstructionSite, etc is possible, but were I implementing this, I might start with such as the goal. Hopefully my musings above are helpful.

(In reply to comment #6) > I think this abstraction wants to be separate from HTMLConstructionSite, and used by HTMLConstructionSite. > > Having it be its own separate object, will allow us to separate the mechanics of cross-thread DOM mutation (or queuing of DOM operations) from the logic of what we want to do with the DOM. Sounds reasonable! > HTMLConstructionSite remains the HTML parser's "what we want to do with the DOM" logic, and some new class (perhaps DOMModifier or DOMOperations or whatever) is the way that we talk to the DOM. > > When we're on the main thread, we use a dumb DOMModifier implementation which just calls the DOM operations synchronously. > > When we're off the main thread, we use a smarter implementation which allocates some sort of objects (similar to how the undo stack, and Editor and EditingCommand work) which are queued up for later execution on the main thread. > > The code then becomes mostly ignorant of the question of if it's on the main thread or not, and this DOMModifications abstraction handles all the details. Yes, it's a great idea. My current implementation has m_isSpeculating flag in HTMLConstructionSite. When this flag is true, HTMLConstructionSite just calls the DOM operations synchronously. Otherwise, it queues up for later execution. This certainly works, but makes the code very complicated. You suggestion sounds much better. > I'm not sure if complete ignorance of threaded-ness in HTMLConstructionSite, etc is possible, but were I implementing this, I might start with such as the goal. Okay. I will try! BTW, it's not just about threaded-ness because my first speculative parsing implementation is still on the main thread. But it still needs to queue up DOM operations because element creation and DOM mutations have visible side effects. > Hopefully my musings above are helpful. Thanks for your good advice. It is really helpful.

(In reply to comment #7) > Yes, it's a great idea. My current implementation has m_isSpeculating flag in HTMLConstructionSite. When this flag is true, HTMLConstructionSite just calls the DOM operations synchronously. Otherwise, it queues up for later execution. This certainly works, but makes the code very complicated. You suggestion sounds much better. Oops. "When this flag is true" -> "When this flag is false"

(In reply to comment #6) > Similarly, it seems like this off-thread parser wants an abstraction through which to talk to the DOM (and possibly a separate one for the reading... although it sounds like you already have the reading part done). For the reading part, I think Bug 92057 is the right solution because we can't queue up DOM read operations. So we need only DOMModifier or DOMModifcations abstraction.

Close this bug in favor of Bug 93455.