E.g. the SetXBLInsertionPoint.

A few probably dead code lines can be found by comparing the coverage between these two revisions:
https://coverage.moz.tools/#revision=563f437f24b9e12495b0f2c364538a2b8d8a0ca7&path=dom%2Fbase%2FFragmentOrElement.cpp&view=file&line=91
https://coverage.moz.tools/#revision=8e8e5bb63105fd26d10f7cba6adf85a606bceaf0&path=dom%2Fbase%2FFragmentOrElement.cpp&view=file&line=96

Thanks Marco. By the way, Bug 1593119 is going to unifdef the MOZ_XBL preprocessor instruction so I suspect many lines that became unconvered will go away there. Though it won't remove functions like SetXBLInsertionPoint so we should still track this separately (and any other changes ccov detects that aren't inside MOZ_XBL).

(In reply to Brian Grinstead [:bgrins] from comment #1)

Thanks Marco. By the way, Bug 1593119 is going to unifdef the MOZ_XBL preprocessor instruction so I suspect many lines that became unconvered will go away there. Though it won't remove functions like SetXBLInsertionPoint so we should still track this separately (and any other changes ccov detects that aren't inside MOZ_XBL).

Yeah, I did see some code which was behind MOZ_XBL turn red, but I ignored that as it will automatically be skipped (it would still be useful to get rid of that at some point, but less useful than removing dead code which is actually built).

Other than whole functions, there are also some branches that are no longer covered after the change (e.g. https://coverage.moz.tools/#revision=563f437f24b9e12495b0f2c364538a2b8d8a0ca7&path=dom%2Fbase%2FFragmentOrElement.cpp&view=file&line=1650 is covered, https://coverage.moz.tools/#revision=8e8e5bb63105fd26d10f7cba6adf85a606bceaf0&path=dom%2Fbase%2FFragmentOrElement.cpp&view=file&line=1650 is uncovered).

(In reply to Marco Castelluccio [:marco] from comment #2)

(In reply to Brian Grinstead [:bgrins] from comment #1)

Thanks Marco. By the way, Bug 1593119 is going to unifdef the MOZ_XBL preprocessor instruction so I suspect many lines that became unconvered will go away there. Though it won't remove functions like SetXBLInsertionPoint so we should still track this separately (and any other changes ccov detects that aren't inside MOZ_XBL).

Yeah, I did see some code which was behind MOZ_XBL turn red, but I ignored that as it will automatically be skipped (it would still be useful to get rid of that at some point, but less useful than removing dead code which is actually built).

Other than whole functions, there are also some branches that are no longer covered after the change (e.g. https://coverage.moz.tools/#revision=563f437f24b9e12495b0f2c364538a2b8d8a0ca7&path=dom%2Fbase%2FFragmentOrElement.cpp&view=file&line=1650 is covered, https://coverage.moz.tools/#revision=8e8e5bb63105fd26d10f7cba6adf85a606bceaf0&path=dom%2Fbase%2FFragmentOrElement.cpp&view=file&line=1650 is uncovered).

That's interesting. Boris, do you expect those two if (!root) branches to be unreachable now:

• https://searchfox.org/mozilla-central/rev/8b7aa8af652f87d39349067a5bc9c0256bf6dedc/dom/base/FragmentOrElement.cpp#1477-1479
• https://searchfox.org/mozilla-central/rev/8b7aa8af652f87d39349067a5bc9c0256bf6dedc/dom/base/FragmentOrElement.cpp#1650-1652

Looking at ccov it appears this is due to the two aNode->UnoptimizableCCNode conditions not entering in FindOptimizableSubtreeRoot at https://searchfox.org/mozilla-central/rev/8b7aa8af652f87d39349067a5bc9c0256bf6dedc/dom/base/FragmentOrElement.cpp#1410,1416.

(In reply to Marco Castelluccio [:marco] from comment #2)

Other than whole functions, there are also some branches that are no longer covered after the change (e.g. https://coverage.moz.tools/#revision=563f437f24b9e12495b0f2c364538a2b8d8a0ca7&path=dom%2Fbase%2FFragmentOrElement.cpp&view=file&line=1650 is covered, https://coverage.moz.tools/#revision=8e8e5bb63105fd26d10f7cba6adf85a606bceaf0&path=dom%2Fbase%2FFragmentOrElement.cpp&view=file&line=1650 is uncovered).

Marco, I don't know if it's just me or if this is a known issue, but when I load those URLs I'm sure it's supposed to scroll me to line 1650 but I get scrolled to around line 1760.

I see some non-XBL cases in nsINode::UnoptimizableCCNode (https://searchfox.org/mozilla-central/rev/8b7aa8af652f87d39349067a5bc9c0256bf6dedc/dom/base/nsINode.cpp#1099-1101) so I wonder if this is signalling a lack of test coverage for those other cases.

We should definitely at least be able to remove https://searchfox.org/mozilla-central/rev/8b7aa8af652f87d39349067a5bc9c0256bf6dedc/dom/base/nsINode.cpp#1103-1104.

Yeah, so the only way to reach those early returns is to have a node that is not in the document and that has a node returning true from UnoptimizableCCNode() on its parent chain. With XBL this is pretty easy: any ancestor in the XBL namespace will do it, as will an ancestor with an XBL binding applied to it. Without XBL, the NodeType() == ATTRIBUTE_NODE case in UnoptimizableCCNode() can never get hit when called from FragmentOrElement methods (a FragmentOrElement is not an attribute node and can't have an attribute node ancestor). So that leaves only the native-anonymous cases. And I'm not sure whether nodes with those flags can really be hanging out with a null GetComposedDoc(). I am pretty sure we don't create them that way initially; the question is whether they can get that way when the non-anonymous node they are attached to is removed from the DOM, and whether that can happen with script holding on to them... Olli, do you know offhand?

NAC nodes stay NAC after being removed from document.
https://searchfox.org/mozilla-central/rev/8b7aa8af652f87d39349067a5bc9c0256bf6dedc/dom/base/nsINode.h#120-121,123
I wouldn't trust script not being able to have a pointer to NAC, especially chrome JS.

So one thing we could try is to use document.insertAnonymousContent followed by document.removeAnonymousContent. After that, as long as we keep the AnonymousContent alive from JS we should have a live NAC node that is not bound to a document. If we CC at that point, I'd think we might hit the early returns linked above?

I'm guessing bug 1596391 will remove most of the code detected here.

(Note: we can probably use the coverage difference report to find even more things which we could remove)

(In reply to Marco Castelluccio [:marco] from comment #10)

(Note: we can probably use the coverage difference report to find even more things which we could remove)

How can we generate a coverage report between two revisions?

The feature is not yet available on https://coverage.moz.tools unfortunately, so the best I can offer you is a small archive with the two downloaded reports and a script (which you can modify according to your needs) to find the files with the highest number of differing covered lines. Then you will have to manually go and look at them on the website (but the script offers you links, so that's quick).
There is noise of course, as there were other patches landing between the two reports, plus the usual amount of randomness.