Bug 137189

Summary: Try to detect no-op innerHTML mutations and avoid repainting.
Product: WebKit Reporter: Andreas Kling <kling>
Component: DOMAssignee: Andreas Kling <kling>
Status: RESOLVED LATER    
Severity: Normal CC: ahmad.saleem792, ap, barraclough, bfulgham, darin, ggaren, kling, koivisto, rniwa
Priority: P2 Keywords: InRadar, Performance
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

Andreas Kling
Reported 2014-09-27 23:57:05 PDT
We should try to avoid repainting when setting an element's innerHTML a HTML string that's equivalent to the existing DOM subtree. <rdar://problem/18241048>
Attachments
Patch (5.16 KB, patch)
2014-09-28 00:11 PDT, Andreas Kling
no flags
Andreas Kling
Comment 1 2014-09-28 00:11:12 PDT
Created attachment 238805 [details] Patch Let's see what EWS thinks of this..
Alexey Proskuryakov
Comment 2 2014-09-28 00:39:00 PDT
A node that does not have a JS wrapper could still be accessible from JS, e.g. if it's part of editing undo stack, XPath node set, and there are more cases like that. So there are many ways in which this change is observable, I think that something very similar has been tried before, but I do not know how to find when. Perhaps it was part of Acid3 performance work? Darin may remember better.
Andreas Kling
Comment 3 2014-09-28 09:46:03 PDT
(In reply to comment #2) > A node that does not have a JS wrapper could still be accessible from JS, e.g. if it's part of editing undo stack, XPath node set, and there are more cases like that. So there are many ways in which this change is observable, Perhaps we could use something like hasOneRef()? > > I think that something very similar has been tried before, but I do not know how to find when. Perhaps it was part of Acid3 performance work? Darin may remember better. This was attempted before, for all tag names, but broke plugin loading IIRC. My approach is to have a small whitelist of simple tags.
Darin Adler
Comment 4 2014-09-28 18:16:35 PDT
(In reply to comment #2) > A node that does not have a JS wrapper could still be accessible from JS, e.g. if it's part of editing undo stack, XPath node set, and there are more cases like that. So there are many ways in which this change is observable. I’d like to understand this better. I agree that a node can be accessible from JavaScript even if it doesn’t have a wrapper. But how would the lack of change in the identity of a node be observable if the node has no properties and does not appear as a property of any JavaScript object (including a collection)?
Alexey Proskuryakov
Comment 5 2014-09-29 00:41:05 PDT
I think that the difference is observable in a scenario like: - we have a node accessible from JavaScript; - JS code performs an assignment to innerHTML that we optimize away; - then JS code accesses the node, and finds one that is in document tree, instead of one outside the tree. In the case of XPath, document mutation should invalidate result sets, but as we don't do the mutation, the sets remain intact.
Andreas Kling
Comment 6 2014-10-01 16:25:09 PDT
It's worth noting that we already short-circuit innerHTML mutations if they are found to only affect the content of a single text node: if (hasOneTextChild(&containerNode.get()) && hasOneTextChild(fragment.get())) { downcast<Text>(*containerNode->firstChild()).setData(downcast<Text>(*fragment->firstChild()).data(), ec); return; } I wonder if there's really any harm in keeping the old DOM subtree alive, even if it's observable from JS.
Ahmad Saleem
Comment 7 2022-08-12 10:57:34 PDT
Patch does not have any test case but I can see this has not landed in Webkit Github and the source does not contain code from the patch: https://github.com/WebKit/WebKit/blob/0eaa48964a00d47f5b1dd9d5fd8b589d5ac74b13/Source/WebCore/editing/markup.cpp rniwa@webkit.org - Is it something needed now or this can be marked as "RESOLVED LATER"? Would appreciate your input?
Ryosuke Niwa
Comment 8 2022-08-12 22:07:22 PDT
Yeah, -> Later.
Note You need to log in before you can comment on or make changes to this bug.