Closed
Bug 357957
Opened 18 years ago
Closed 18 years ago
nsGeneratedContentIterator looks like dead code
Categories
(Core :: DOM: Selection, defect)
Core
DOM: Selection
Tracking
()
RESOLVED
FIXED
People
(Reporter: sicking, Assigned: sicking)
Details
Attachments
(1 file, 2 obsolete files)
(deleted),
patch
|
dbaron
:
review+
dbaron
:
superreview+
|
Details | Diff | Splinter Review |
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?
Assignee | ||
Comment 1•18 years ago
|
||
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.
Assignee: selection → bugmail
Status: NEW → ASSIGNED
Attachment #243456 -
Flags: superreview?(dbaron)
Attachment #243456 -
Flags: review?(dbaron)
Assignee | ||
Comment 2•18 years ago
|
||
Please ignore the inserted newline at the top of nsSelection.cpp, that won't go in.
Assignee | ||
Comment 3•18 years ago
|
||
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.
Attachment #243456 -
Attachment is obsolete: true
Attachment #243456 -
Flags: superreview?(dbaron)
Attachment #243456 -
Flags: review?(dbaron)
Comment 4•18 years ago
|
||
So we're pretty sure we don't want to fix bug 12460? Or at least that this code is useless for that purpose?
Assignee | ||
Comment 5•18 years ago
|
||
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•18 years ago
|
||
OK, makes sense. :)
Assignee | ||
Comment 7•18 years ago
|
||
Attachment #243503 -
Flags: superreview?(dbaron)
Attachment #243503 -
Flags: review?(dbaron)
Comment 8•18 years ago
|
||
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.
Updated•18 years ago
|
Attachment #243503 -
Flags: superreview?(dbaron)
Attachment #243503 -
Flags: superreview-
Attachment #243503 -
Flags: review?(dbaron)
Attachment #243503 -
Flags: review-
Assignee | ||
Comment 9•18 years ago
|
||
This is what I'm landing. It compiled fine with those changes.
Attachment #243503 -
Attachment is obsolete: true
Assignee | ||
Comment 10•18 years ago
|
||
Checked in, thanks for the quick review. Savings seemed to have been in the 8-9k range.
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Updated•18 years ago
|
Attachment #243508 -
Flags: superreview+
Attachment #243508 -
Flags: review+
You need to log in
before you can comment on or make changes to this bug.
Description
•