455595 - deCOMtaminate nsINameSpaceManager

Status: RESOLVED FIXED

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

Description

[(dormant account)]

Attachment: cleanup

This is one of the tasks listed on https://wiki.mozilla.org/Gecko:DeCOMtamination

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?

Comment 1

[(dormant account)]

Attachment: cleanup

accidentally sent a stale ver

Comment 2

[Johnny Stenback (:jst)]

Comment on attachment 338961 [details] [diff] [review]
cleanup

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)

Here,

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

and here too.

r+sr=jst with that.

Comment 3

[(dormant account)]

pushed http://hg.mozilla.org/mozilla-central/rev/92e24ee719b8

Comment 4

[Ben Turner (not reading bugmail, use the needinfo flag!)]

This broke up the windows and linux leak test builds, so backed out.

Comment 5

[Phil Ringnalda (:philor)]

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.

Comment 6

[Phil Ringnalda (:philor)]

Nope, backout made it in before my clobber started.

Comment 7

[[e-mail]]

Don't forget debug builds are also shared by default.

Comment 8

[Boris Zbarsky [:bzbarsky]]

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..

Comment 9

[(dormant account)]

jst, is this still a candidate for decomtamination? Once I rename and remove nsISupports, are the downstream projects expected to adapt?

Comment 10

[Zack Weinberg (:zwol)]

Is it still possible to deCOM this or should we just close the bug?

Comment 11

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: WIP

I don't remember why I stopped working on this bug; this seems to be where I got.

Comment 12

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

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.

Comment 13

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Attachment: Bug 455595 - Finish deCOMtaminating nsNameSpaceManager

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 14

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=7c381d15e95c384e1b250f789c04cb7f5e2cf0c4

Comment 15

[Phabricator Automation]

Comment on attachment 8994372 [details]
Bug 455595 - Finish deCOMtaminating nsNameSpaceManager

Andrea Marchesini [:baku] has approved the revision.

https://phabricator.services.mozilla.com/D2309

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cf462cc971f8
Finish deCOMtaminating nsNameSpaceManager r=baku

Comment 17

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/cf462cc971f8