HTMLConstructionSite has DOM operations methods required for tree construction. But some DOM operations are still performed directly in the tree builder. Extracted such DOM operations and moved them to HTMLConstructionSite for consistency. This change removes code duplication and improves the readability of HTMLTreeBuilder, esepcially HTMLTreeBuilder::callTheAdoptionAgency(AtomicHTMLToken*).
Created attachment 156266 [details] Patch
This patch is a prerequisite for speculative parsing. Because we can't create nodes while in speculation, all DOM operations must be queued until we know if the speculation is successful or not. I want to ensure that all tree construction and DOM manipulations are performed only in HTMLConstructionSite.
Comment on attachment 156266 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=156266&action=review > Source/WebCore/html/parser/HTMLTreeBuilder.cpp:-1612 > - newItem->element()->attach(); Can we use lazyAttach here?
Comment on attachment 156266 [details] Patch This is probably right, but I'm too tired to review it well right now. I'll give it a shot tomorrow.
(In reply to comment #3) > (From update of attachment 156266 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=156266&action=review > > > Source/WebCore/html/parser/HTMLTreeBuilder.cpp:-1612 > > - newItem->element()->attach(); > > Can we use lazyAttach here? I am not sure. I don't understand the difference between lazyAttach and attach here. I just copied the original code verbatim to make sure not to change the behavior.
(In reply to comment #4) > (From update of attachment 156266 [details]) > This is probably right, but I'm too tired to review it well right now. I'll give it a shot tomorrow. Okay. Thanks!
Created attachment 156288 [details] Patch
(In reply to comment #7) > Created an attachment (id=156288) [details] > Patch I missed removeAllChildren in the previous patch.
See also Bug 93113
@skyul: I wonder if we should keep these patches in your git branch for the moment. If we don't end up with speculative parsing, then we probably won't want to add this layer of indirection...
(In reply to comment #10) > @skyul: I wonder if we should keep these patches in your git branch for the moment. If we don't end up with speculative parsing, then we probably won't want to add this layer of indirection... Okay. I won't land any patch on speculative parsing until we reach the final decision. BTW, I though that this patch has its own benefit because it removes code duplication in the adoption agency algorithm. But I agree some new methods in HTMLConstructionSite just add a layer of indirection.
> BTW, I though that this patch has its own benefit because it removes code duplication in the adoption agency algorithm. But I agree some new methods in HTMLConstructionSite just add a layer of indirection. Yeah, that part of the patch is valuable independent of speculative parsing. My guess is that we can come to a decision about speculative parsing relatively quickly, so this won't cause a long delay.
(In reply to comment #12) > Yeah, that part of the patch is valuable independent of speculative parsing. > > My guess is that we can come to a decision about speculative parsing relatively quickly, so this won't cause a long delay. I think I can complete the initial implementation within a week. I will push my work-in-progress patches into my git branch next Monday for interim review. Thank you!
Comment on attachment 156288 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=156288&action=review > Source/WebCore/html/parser/HTMLTreeBuilder.cpp:771 > + m_tree.detachNode(m_tree.openElements()->bodyElement()); I don't think detachNode is a good name. It reminds me of detach(), which has a completely different semantic.
(In reply to comment #14) > I don't think detachNode is a good name. It reminds me of detach(), which has a completely different semantic. Okay. What about removeChild? Does it sound better?
(In reply to comment #15) > (In reply to comment #14) > > I don't think detachNode is a good name. It reminds me of detach(), which has a completely different semantic. > > Okay. What about removeChild? Does it sound better? Oops. I was confused. It should be something like removeFromItsParent.
Close this bug in favor of Bug 93455.