Closed Bug 455595 Opened 16 years ago Closed 6 years ago

deCOMtaminate nsINameSpaceManager


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

Not set



Tracking Status
firefox63 --- fixed


(Reporter: taras.mozilla, Assigned: qdot)


(Blocks 1 open bug)



(1 file, 3 obsolete files)

Attached patch cleanup (obsolete) — Splinter Review
This is one of the tasks listed on

I moved nsNameSpaceManagerImpl into nsINameSpaceManager, got rid of virtual methods. Not sure what else is one is to do. It's pretty trivial and none of these methods are outparamdel-able.

Should change the class to not inherit from nsISupports and s/nsCOMPtr/nsRefPtr/ for pointers containing it?
Attached patch cleanup (obsolete) — Splinter Review
accidentally sent a stale ver
Attachment #338954 - Attachment is obsolete: true
Attachment #338961 - Flags: review?(jst)
Comment on attachment 338961 [details] [diff] [review]

class nsINameSpaceManager : public nsISupports

This could totally be a followup bug, but there's no real need to inherit nsISupports here any longer I don't think. And this class should be nsNameSpaceManager now, no need for ns_I_ here any more IMO.

+  nsresult RegisterNameSpace(const nsAString& aURI,
+                                     PRInt32& aNameSpaceID);

Got some auto-emacs-indentation magic pixie dust that would help here too? If not, fix this manually before landing :)
+nsINameSpaceManager::RegisterNameSpace(const nsAString& aURI, 
                                         PRInt32& aNameSpaceID)


+nsresult nsINameSpaceManager::AddNameSpace(const nsAString& aURI,
                                             const PRInt32 aNameSpaceID)

and here too.

r+sr=jst with that.
Attachment #338961 - Flags: superreview+
Attachment #338961 - Flags: review?(jst)
Attachment #338961 - Flags: review+
Assignee: nobody → tglek
QA Contact: content
Hardware: PC → All
QA Contact: content
QA Contact: content
Blocks: 458300
Closed: 16 years ago
Resolution: --- → FIXED
This broke up the windows and linux leak test builds, so backed out.
Resolution: FIXED → ---
To give you more of a hint than that, it fried everything building both accessibility and -enable-shared: SeaMonkey builds everything shared, so all their Linux and Windows builds, Thunderbird only builds unittests shared, so Windows there (and would have been Linux, except it was already busted). I wondered if it just needed a clobber, so depending on when it started, we may or may not have gotten a clobbered Tb Windows check build before the backout.
Nope, backout made it in before my clobber started.
Don't forget debug builds are also shared by default.
Yeah, so with this patch I get:

Undefined symbols:
  "nsINameSpaceManager::GetNameSpaceURI(int, nsAString_internal&)", referenced from:
      nsDocAccessible::GetNameSpaceURIForID(short, nsAString_internal&)  in nsDocAccessible.o

because that function became non-virtual..
jst, is this still a candidate for decomtamination? Once I rename and remove nsISupports, are the downstream projects expected to adapt?
Component: Content → DOM
QA Contact: content → general
Blocks: deCOM
Is it still possible to deCOM this or should we just close the bug?
Assignee: tglek → Ms2ger
Attached patch WIP (obsolete) — Splinter Review
I don't remember why I stopped working on this bug; this seems to be where I got.
Assignee: Ms2ger → nobody
Assignee: nobody → kyle
Attachment #338961 - Attachment is obsolete: true
Attachment #606884 - Attachment is obsolete: true
Most of this was taken care of in bug 458300. However, nsNameSpaceManager still inherited from nsISupports, only for the refcounting bits. We should be able to use NS_INLINE_DECL_REFCOUNTING and finish this off.
Most of the deCOM work was handled in bug 458300, so we just need to
use the inline refcounting macro to remove nsISupports.

MozReview-Commit-ID: 7npo7RF68Vo
Comment on attachment 8994372 [details]
Bug 455595 - Finish deCOMtaminating nsNameSpaceManager

Andrea Marchesini [:baku] has approved the revision.
Attachment #8994372 - Flags: review+
Pushed by
Finish deCOMtaminating nsNameSpaceManager r=baku
Closed: 16 years ago6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.