Status

()

enhancement
RESOLVED WORKSFORME
2 years ago
Last year

People

(Reporter: njn, Assigned: njn)

Tracking

(Blocks 2 bugs)

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox57 affected)

Details

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.
Depends on: 1392884
Depends on: 1392881
Depends on: 1392989
This is super valuable, I've seen refcounting on atoms in some profiles, FWIW.
Depends on: 1393642
Note to self: bug 1334834 is about converting a bunch of comm-central IDL methods from nsIAtoms to strings.
(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.
> 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.
Well, you got yourself the job to look at three patches in bug 1394215 and pick one of them ;-)
Depends on: 1394694
Depends on: 1396682
Depends on: 1396693
Depends on: 1396694
Neat. Is there more work here for 57?
Flags: needinfo?(n.nethercote)
Depends on: 1400459
Depends on: 1400460
(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
> 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.
Flags: needinfo?(n.nethercote)
Depends on: 1401873
Depends on: 1403430
All blocking bugs have landed.
Blocks: 1257126
Assignee: nobody → n.nethercote
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.