357957 - nsGeneratedContentIterator looks like dead code

Status: RESOLVED FIXED

(Core :: DOM: Selection, defect)

Description

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

I can't find any code that instantiates any nsGeneratedContentIterator or  objects. The only place that uses its CID is
http://lxr.mozilla.org/mozilla/source/layout/generic/nsSelection.cpp#5066
and that lives in an #ifdef USE_SELECTION_GENERATED_CONTENT_ITERATOR_CODE, which is never defined. Its ContractID is never used.

Seems like nsGeneratedIterator.cpp can die along with the nsIGeneratedContentIterator interface and all code living inside #ifdef USE_SELECTION_GENERATED_CONTENT_ITERATOR_CODE

Looking at blame for nsSelection.cpp it seems like it was turned off in bug 49772 in october 2000, so I'd say it's pretty safe to assume that the code is pretty rotted at this point.

Does anyone know of a reason for this code to stay around?

Comment 1

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Attachment: Patch to kill

I'm going to go out on a limb here and guess we're not going to turn this code on anytime soon. And if/when we do we probably want to rewrite this given that our code has changed a tad the last 6 years.

Comment 2

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Please ignore the inserted newline at the top of nsSelection.cpp, that won't go in.

Comment 3

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 243456 [details] [diff] [review]
Patch to kill

Hey, there's even more code that can be killed. nsFrameContentIterator is only used by nsGeneratedContentIterator so that can go too. New patch in a bit.

Comment 4

[Boris Zbarsky [:bzbarsky]]

So we're pretty sure we don't want to fix bug 12460?  Or at least that this code is useless for that purpose?

Comment 5

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

I'm not at all sure of either of those things. However I really dislike having dead code in the tree and even more so in shipped binaries.

I do think that we'll probably want to fix bug 12460 at some point, when we do we can evaluate if it's worth trying to revive this code, or if new should be written. Honestly, I suspect that this stuff can be written much cleaner these days given nsINode and other goodies that has happened in the last 6 years.

Comment 6

[Boris Zbarsky [:bzbarsky]]

OK, makes sense.  :)

Comment 7

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Attachment: Patch to kill, v2

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

You need to remove the definitions of NS_GENERATEDCONTENTITERATOR_CID and NS_GENERATEDSUBTREEITERATOR_CID, and then make sure the whole thing still compiles.  You also need to rev the IID on nsIPresShell.  If it still compiles after doing that without further changes, then r+sr=dbaron.

Comment 9

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Attachment: Patch to kill, v2.1

This is what I'm landing. It compiled fine with those changes.

Comment 10

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Checked in, thanks for the quick review. Savings seemed to have been in the 8-9k range.