deCOMtaminate nsINameSpaceManager

REOPENED
Unassigned

Status

()

Core
DOM
REOPENED
10 years ago
2 years ago

People

(Reporter: (dormant account), Unassigned)

Tracking

(Blocks: 2 bugs)

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

10 years ago
Created attachment 338954 [details] [diff] [review]
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?
(Reporter)

Comment 1

10 years ago
Created attachment 338961 [details] [diff] [review]
cleanup

accidentally sent a stale ver
Attachment #338954 - Attachment is obsolete: true
Attachment #338961 - Flags: review?(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.
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
(Reporter)

Updated

9 years ago
Blocks: 458300
(Reporter)

Comment 3

9 years ago
pushed http://hg.mozilla.org/mozilla-central/rev/92e24ee719b8
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
This broke up the windows and linux leak test builds, so backed out.
Status: RESOLVED → REOPENED
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.

Comment 7

9 years ago
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..
(Reporter)

Comment 9

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

Updated

9 years ago
Component: Content → DOM
QA Contact: content → general

Updated

8 years ago
Blocks: 105431
Is it still possible to deCOM this or should we just close the bug?

Updated

6 years ago
Assignee: tglek → Ms2ger
Created attachment 606884 [details] [diff] [review]
WIP

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

Updated

6 years ago
Assignee: Ms2ger → nobody
You need to log in before you can comment on or make changes to this bug.