156733 - nsDirectoryService needs to rid its reliance on nsSpecialSystemDirectory

Status: RESOLVED INVALID

(Core :: XPCOM, defect)

Description

[Conrad Carlen (not reading bugmail)]

nsSpecialSystemDirectory is based on nsFileSpec, which in itself is evil, but is
especially a problem for XP_MACOSX builds. In that world, the XP_UNIX version of
nsFileSpec is used because it uses '/' delimited POSIX paths which we want. But,
the nature of the nsFileSpec is very different between XP_MAC and XP_UNIX.
nsSpecialSystemDirectory, under XP_MACOSX, is caught in an odd place in between.

What I want to do is eliminate the use of nsSpecialSystemDirectory and then this
can be cleaned up properly.

Comment 1

[Conrad Carlen (not reading bugmail)]

Fixing summary.

Comment 2

[Conrad Carlen (not reading bugmail)]

Attachment: phase #1 patch

This should be done first. It removes all consumers of nsSpecialSystemDirectory
(outside xpcom) from the tree and no longer exports the file.

Comment 3

[Steve Dagley]

Comment on attachment 97338 [details] [diff] [review]
phase #1 patch

r=sdagley w/a few comments:

1) Nice to see how many times nsSpecialSystemDirectory.h was gratuitously
included :-)
2) whitespace seems off on several files

3) In DataStruct::GetFileSpec() there's a comment that implies aFileName may be
NULL but the new code is testing aFileName.IsEmpty() which isn't the same and
may not be safe.

Comment 4

[Conrad Carlen (not reading bugmail)]

> In DataStruct::GetFileSpec() there's a comment that implies aFileName may be
NULL but the new code is testing aFileName.IsEmpty

that's because the old "aFileName" was a const char*. Now it's a const
nsACString&. The comment could be changed to say "empty" instead of "NULL."

Comment 5

[Conrad Carlen (not reading bugmail)]

cc'ing Alec for sr=.

BTW, phase #1 is to get rid of all consumers of this object and limit its use to
the internals of directory service. Once that is done, the next phase is
refactoring the code so that it's not nsFileSpec-based but is nsILocalFile-based
(either as a class with a GetFile() method or an nsIDirectoryServiceProvider
impl which is created and registered by directory service when it is created).

Comment 6

[Alec Flett]

Comment on attachment 97338 [details] [diff] [review]
phase #1 patch

sr=alecf
wow, mail uses way too many temp files!

Comment 7

[Conrad Carlen (not reading bugmail)]

Minimo landing did this.