1449686 - Remove most of XPCOM's special directories

Status: RESOLVED FIXED

(Core :: XPCOM, enhancement)

Description

[Nicholas Nethercote [inactive]]

XPCOM has a ton of special directories, many of which map to OS-specific special directories. The handling of these directories has multiple components.

- xpcom/io/nsDirectoryServiceDefs.h has lots of #defines, e.g. NS_OS_TEMP_DIR is defined as "TmpD".

- xpcom/io/nsDirectoryServiceAtomList.h specifies lots of atoms, e.g. sOS_TemporaryDirectory is the atom for NS_OS_TEMP_DIR ("TmpD").

- xpcom/io/nsDirectoryService.cpp has the function nsDirectoryService::GetFile(), which basically maps atoms to constants, e.g. sOS_TemporaryDirectory maps to SystemDirectory::OS_TemporaryDirectory.

- That function then passes these constants to another function that actually obtains the relevant directory, mostly GetSpecialSystemDirectory(), which does OS-specific things for each case.

Most of these special directories are unused. For example, nsDirectoryService()::GetFile() handles 86 different directories. But to start the browser on Linux, only 4 of them are used. (See also bug 1449491, which removed four of them.) 

(There are other special directories that are defined in other places, such as "ProfD".)

Having all these directories made sense when XPCOM was intended to be part of a generic application platform. But it's probably safe to remove all the unused ones now.

Comment 1

[Nicholas Nethercote [inactive]]

These are the atoms that are used by nsDirectoryService()::GetFile() during startup on Linux:

> nsDirectoryService::sCurrentProcess
> nsDirectoryService::sOS_TemporaryDirectory
> nsDirectoryService::sOS_CurrentWorkingDirectory
> nsGkAtoms::Home

These (and possibly more) are used during startup on Mac:

> nsDirectoryService::sUserLibDirectory
> nsGkAtoms::Home

Comment 2

[Josh Aas]

Attachment: Fix v1.0

I think all this stuff can go. This builds on Linux, haven't done any other testing yet.

Comment 3

[Josh Aas]

Attachment: Fix v1.1

Comment 4

[Nathan Froyd [:froydnj]]

Comment on attachment 8982065 [details] [diff] [review]
Fix v1.1

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

Thank you!  Will be interesting to see if these are used by some system addon somewhere...please post on dev-platform about this, so Thunderbird is aware of potential breakage, and maybe some system addon people can be made aware as well?

Comment 5

[Josh Aas]

Posted to m.d.platform:

https://groups.google.com/forum/#!topic/mozilla.dev.platform/oDzcra6j3hg

If I don't hear about anything problematic soon I'll get this landed.

Comment 6

[Josh Aas]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=c31d765b5401fe228ed58b461ddfebc748749f4a

Comment 7

[Josh Aas]

Attachment: Fix v1.1 w/commit message

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/221a0435d2dd
Remove most XPCOM special directories as they are unused. r=froydnj

Comment 9

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/221a0435d2dd

Comment 10

[Jorg K (CEST = GMT+2)]

Hmm, we used this to retrieve the Windows Documents directory. Any chance to restore this? Otherwise we'd have to fork that stuff starting with static nsresult GetLibrarySaveToPath() whose call was removed here:
https://hg.mozilla.org/mozilla-central/rev/221a0435d2dd#l1.139
-    case Win_Documents: {
-      return GetLibrarySaveToPath(CSIDL_MYDOCUMENTS,
-                                  FOLDERID_DocumentsLibrary,
-                                  aFile);
-    }

Or is there a better way to implement this?

Comment 11

[Jorg K (CEST = GMT+2)]

See our band-aid in:
https://hg.mozilla.org/comm-central/rev/298f45b92eeac214d20bf31f974fae2cc9483e64

Comment 12

[Josh Aas]

I would recommend just getting the directory you need in a simpler way without this xpcom stuff, probably specific to TB. Better to have this service just provide what Gecko needs instead of being a general platform service provider.

I am not a regular contributor any more though, so my advice here may not match current guidelines. Maybe froydnj is a better person to get feedback from.

Comment 13

[Nathan Froyd [:froydnj]]

I don't particularly mind having extra directories, but given that having extra atoms has startup implications (among other things), I'd prefer that if there are extra things Firefox doesn't need, those extra directories be exclusively accessible through GetSpecialSystemDirectory, and *not* through the directory service generally.