Seen during a review, the function is pretty difficult to read but also has some redundant code hidden in it. Patch forthcoming.
Created attachment 128376 [details] Simple cleanup, removed unneeded inline check, while -> for and use moveChildTo.
(In reply to comment #1) > Created an attachment (id=128376) [details] > Simple cleanup, removed unneeded inline check, while -> for and use moveChildTo. Yay! now i can just fix stuff in central RenderBlock::moveChildTo
Comment on attachment 128376 [details] Simple cleanup, removed unneeded inline check, while -> for and use moveChildTo. View in context: https://bugs.webkit.org/attachment.cgi?id=128376&action=review I wonder if this will have a performance effect. > Source/WebCore/rendering/RenderBlock.cpp:992 > + for (RenderObject* child = startChild; child && child != endChild; ) { > + // Save our next sibling as moveChildTo will clear it. > + RenderObject* nextSibling = child->nextSibling(); > + moveChildTo(toBlock, child, beforeChild, fullRemoveInsert); > + child = nextSibling; > } Just to make sure... there are no mutation events (or other JavaScript) run by moveChildTo such that we would need to hold a RefPtr for child?
(In reply to comment #3) > (From update of attachment 128376 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=128376&action=review > > I wonder if this will have a performance effect. > > > Source/WebCore/rendering/RenderBlock.cpp:992 > > + for (RenderObject* child = startChild; child && child != endChild; ) { > > + // Save our next sibling as moveChildTo will clear it. > > + RenderObject* nextSibling = child->nextSibling(); > > + moveChildTo(toBlock, child, beforeChild, fullRemoveInsert); > > + child = nextSibling; > > } > > Just to make sure... there are no mutation events (or other JavaScript) run by moveChildTo such that we would need to hold a RefPtr for child? No Refptrs in the rendering world :(
Comment on attachment 128376 [details] Simple cleanup, removed unneeded inline check, while -> for and use moveChildTo. View in context: https://bugs.webkit.org/attachment.cgi?id=128376&action=review > I wonder if this will have a performance effect. I was wondering that too. Some archaeology did not show the split was done for performance so we should be fine. In case, we can always inline moveChildTo. >>> Source/WebCore/rendering/RenderBlock.cpp:992 >>> } >> >> Just to make sure... there are no mutation events (or other JavaScript) run by moveChildTo such that we would need to hold a RefPtr for child? > > No Refptrs in the rendering world :( Indeed, but more importantly, the operations are atomic from a JS perspective.
Comment on attachment 128376 [details] Simple cleanup, removed unneeded inline check, while -> for and use moveChildTo. Clearing flags on attachment: 128376 Committed r108645: <http://trac.webkit.org/changeset/108645>
All reviewed patches have been landed. Closing bug.