1445892 - Remove nsGkAtoms::Home atom

Status: RESOLVED FIXED

(Core :: XPCOM, enhancement)

Description

[Nicholas Nethercote [inactive]]

nsGkAtoms::Home and DirectoryAtoms::Home are duplicate static atoms. Of the
two, the former seems easier remove, simply by replacing its two uses with
appropriate NS_GetStaticAtom() calls. Those calls should be rare, because they
only occur when pressing the "Home" key on Windows or Linux.

MozReview-Commit-ID: H6PZFNPp6nl

Comment 1

[Nicholas Nethercote [inactive]]

Attachment: Remove nsGkAtoms::Home atom

Comment 2

[Nathan Froyd [:froydnj]]

Comment on attachment 8959079 [details] [diff] [review]
Remove nsGkAtoms::Home atom

Review of attachment 8959079 [details] [diff] [review]:
-----------------------------------------------------------------

Why not move this into some common list like nsGkAtoms?  I mean, if we're going to have this central list in XPCOM, we might as well use the central list, no?

Comment 3

[Nicholas Nethercote [inactive]]

> Why not move this into some common list like nsGkAtoms?  I mean, if we're
> going to have this central list in XPCOM, we might as well use the central
> list, no?

I originally tried that, but there's the weirdness of the directory "Home" atom being defined like this:

https://searchfox.org/mozilla-central/rev/877c99c523a054419ec964d4dfb3f0cadac9d497/xpcom/io/nsDirectoryServiceAtomList.h#14

Its name is "sOS_HomeDirectory" and its value is NS_OS_HOME_DIR. All the other directories are defined via constants, I thought it would be weird to have this one directory handled differently. Especially at usage points like this:

https://searchfox.org/mozilla-central/rev/877c99c523a054419ec964d4dfb3f0cadac9d497/xpcom/io/nsDirectoryService.cpp#601

It seems odd to have nsGkAtoms::Home in among that code.

If you still feel strongly about it, I can do it that way.

Comment 4

[Nicholas Nethercote [inactive]]

Attachment: Bug 1445892 - Remove nsDirectoryService::sOS_HomeDirectory atom.

nsGkAtoms::Home and nsDirectoryAtoms::sOS_HomeDirectory are duplicate static
atoms. This patch comments out the latter to remove the duplication.

Review commit: https://reviewboard.mozilla.org/r/229540/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/229540/

Comment 5

[Nicholas Nethercote [inactive]]

froydnj: I put up a patch that goes in the other direction. I like the first patch better, but I'm happy to defer to your preference.

Comment 6

[Nathan Froyd [:froydnj]]

Comment on attachment 8960809 [details]
Bug 1445892 - Remove nsDirectoryService::sOS_HomeDirectory atom.

https://reviewboard.mozilla.org/r/229540/#review235650

Thanks.

Comment 7

[Nicholas Nethercote [inactive]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/065e8188dc9007ff2e68b00906cc972133a17a43
Bug 1445892 - Remove nsDirectoryService::sOS_HomeDirectory atom. r=froydnj

Comment 8

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/065e8188dc90