1392883 - DeCOMtaminate nsIAtom

Status: RESOLVED WORKSFORME

(Core :: XPCOM, enhancement)

Description

[Nicholas Nethercote [inactive]]

xpcom/ds/nsIAtom.idl has this disconcerting commment about nsIAtom:

> /*
>  * Should this really be scriptable?  Using atoms from script or proxies
>  * could be dangerous since double-wrapping could lead to loss of
>  * pointer identity.
>  */

And nsIAtom isn't used much from script. Barely at all in Firefox, and a small amount in Thunderbird; in both cases via nsIAtomService.

So it would be nice to convert nsIAtom to a vanilla C++ class, and it seems doable. I think this would allow all nsIAtom methods (including refcounting) to be devirtualized, too.

Comment 1

[(no longer active)]

This is super valuable, I've seen refcounting on atoms in some profiles, FWIW.

Comment 2

[Nicholas Nethercote [inactive]]

Note to self: bug 1334834 is about converting a bunch of comm-central IDL methods from nsIAtoms to strings.

Comment 3

[Jorg K (CEST = GMT+2)]

(In reply to Nicholas Nethercote [:njn] from comment #2)
> Note to self: bug 1334834 is about converting a bunch of comm-central IDL
> methods from nsIAtoms to strings.
We removed use of nsIAtoms from mailnews/nsIFolder*.idl and we'll remove remaining usage of nsIAtom in bug 1394215.

Comment 4

[Nicholas Nethercote [inactive]]

> We removed use of nsIAtoms from mailnews/nsIFolder*.idl and we'll remove
> remaining usage of nsIAtom in bug 1394215.

Thank you.

I realized there are some more uses in ChatZilla and Inspector. I didn't see those at first because I didn't realize they were put in the mozilla/ directory when you check out comm-central with the client.py script.

Comment 5

[Jorg K (CEST = GMT+2)]

Well, you got yourself the job to look at three patches in bug 1394215 and pick one of them ;-)

Comment 6

[David Bolter [:davidb] (NeedInfo me for attention)]

Neat. Is there more work here for 57?

Comment 7

[Nicholas Nethercote [inactive]]

(In reply to David Bolter [:davidb] (NeedInfo me for attention) from comment #6)
> Neat. Is there more work here for 57?

There are two more steps, though they be done in time for 57:
- Bug 1400459: devirtualizing
- Bug 1400460: renaming

Comment 8

[Nicholas Nethercote [inactive]]

> There are two more steps, though they be done in time for 57:

I meant to say: they *won't* be done in time for 57.

Comment 9

[Nicholas Nethercote [inactive]]

All blocking bugs have landed.