586298 - Move nsRadioVisitor and its children to nsRadioVisitor.{h,cpp} and remove radio visitor factories

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Mounir Lamouri (:mounir)]

For the moment, only nsIRadioVisitor is declared in public/. Implementations (nsRadioVisitor and all other radio visitors) are declared and defined in nsHTMLInputElement. However, there are some factories declared in public/ used in nsHTMLInputElement.

So, if these factories are only needed for nsHTMLInputElement (it looks like they are) we should not declare them in public/ and define them in nsHTMLInputElement. We should not have factories considering the nsIRadioVisitor implementations are not visible outside nsHTMLInputElement.cpp.

Even better IMO, we can create nsRadioVisitor.{h,cpp} in content/html/content/src.

Comment 1

[Mounir Lamouri (:mounir)]

Attachment: Patch v1

Man, it feels so good to clean that up...

Comment 2

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

Comment on attachment 518744 [details] [diff] [review]
Patch v1

Is there a reason these are refcounted? Couldn't we just change nsIRadioVisitor to RadioVisitor and new/delete them, or even allocate them on the stack?

r=me either way. But if you do do the above please do it as a second patch on top of this one.

Comment 3

[Mounir Lamouri (:mounir)]

(In reply to comment #2)
> Comment on attachment 518744 [details] [diff] [review]
> Patch v1
> 
> Is there a reason these are refcounted? Couldn't we just change nsIRadioVisitor
> to RadioVisitor and new/delete them, or even allocate them on the stack?
> 
> r=me either way. But if you do do the above please do it as a second patch on
> top of this one.

FWIW, I'm thinking of removing ns{I,}RadioVisitor given two visitors might be removed by bug 641001 and the last one by bug 636774 (or one of its follow-up).

Anyway, to keep that in my radar, I've open bug 641349.

Comment 4

[Mounir Lamouri (:mounir)]

Pushed:
http://hg.mozilla.org/mozilla-central/rev/55a070932e02