1435766 - make the directory service use GetSpecialSystemDirectory for mac-specific atoms

Status: RESOLVED FIXED

(Core :: XPCOM, enhancement, P3)

Description

[Nathan Froyd [:froydnj]]

This change helps address #4 and #5 in bug 1435745.

Comment 1

[Nathan Froyd [:froydnj]]

Attachment: part 1 - rely on implicit enum numbering for SystemDirectories

The enum values in SystemDirectories are all explicitly assigned
values.  While explicit is normally better than implicit, having
explicit values for everything requires renumbering if you wanted to
insert values somewhere in the middle of the list: a new OS_* constant,
or a Mac_LocalDesktopDirectory.  You could insert them at the end,
always, but perhaps you'd like to keep related things close together for
aesthetics.  Removing values also requires renumbering things, partially
for aesthetics, but partially to not run out of numbers.

To make life for future hackers easier, let's renumber everything using
explicit values for the start of the "sections", and implicit numbering
for everything else.

Comment 2

[Nathan Froyd [:froydnj]]

Attachment: part 2 - reimplement nsDirectoryService Mac getters in terms of GetSpecialSystemDirectory

This change makes the nsDirectoryService implementation for Mac more
consistent with the Windows and generic Unix implementations.  The
change also opens up the possibility of using GetSpecialSystemDirectory
on Mac for Mac-specific things elsewhere in the codebase and not having
things fall over.  We need to add a bunch of SystemDirectories constants
to accommodate all the atoms required by the directory service.

Comment 3

[Nika Layzell [:nika] (ni? for response)]

Comment on attachment 8956095 [details] [diff] [review]
part 1 - rely on implicit enum numbering for SystemDirectories

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

LGTM :-)

Comment 4

[Stephen A Pohl [:spohl]]

Comment on attachment 8956096 [details] [diff] [review]
part 2 - reimplement nsDirectoryService Mac getters in terms of GetSpecialSystemDirectory

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

r+ with the one issue fixed. Thanks!

::: xpcom/io/nsDirectoryService.cpp
@@ +484,2 @@
>    } else if (inAtom == nsDirectoryService::sLocalInternetPlugInDirectory) {
> +    rv = GetSpecialSystemDirectory(Mac_LocalDocumentsDirectory, getter_AddRefs(localFile));

s/Mac_LocalDocumentsDirectory/Mac_LocalInternetPluginDirectory/

Comment 5

[BugBot [:suhaib / :marco/ :calixte]]

There are some r+ patches which didn't land and no activity in this bug for 2 weeks.
:froydnj, could you have a look please?

Comment 6

[Stephen A Pohl [:spohl]]

Setting n-i for myself to come back and see if we can land this.

Comment 7

[Stephen A Pohl [:spohl]]

This has changed substantially due to the changes made in bug 1449686. :froydnj, how should we proceed here?

Comment 8

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee didn't login in Bugzilla in the last 7 months.
:nika, could you have a look please?
For more information, please visit auto_nag documentation.

Comment 9

[Nika Layzell [:nika] (ni? for response)]

I think there's a new patch we could write based on these patches which would help fix the original issues, but the existing patches won't apply anymore as noted due to the changes in 1449686. It looks simple enough to just whip up, so I'll post an updated patch shortly.

Comment 10

[Nika Layzell [:nika] (ni? for response)]

Attachment: Bug 1435766 - Reimplement nsDirectoryService Mac getters in terms of GetSpecialSystemDirectory, r=#xpcom-reviewers

This makes the getters more consistent with getters on other platforms,
and theoretically provides a faster way of getting these directories
from C++ code in the future.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fba6063b1a37
Reimplement nsDirectoryService Mac getters in terms of GetSpecialSystemDirectory, r=xpcom-reviewers,mccr8

Comment 12

[Cristina Cozmuta (:CrissCozmuta)]

https://hg.mozilla.org/mozilla-central/rev/fba6063b1a37