489015 - optimize Mac OS X filesystem time methods

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Josh Aas]

We should optimize the code for setting and getting the filesystem modification times on Mac OS X. Right now we are using old catalog info APIs.

Comment 1

[Josh Aas]

Attachment: fix v1.0

This moves us to the same posix/utime APIs used in our unix impl and optimizes the way we get UTF8 path strings.

I got rid of the FNNotify call, it isn't very important that we do it and if we do want to do it then this isn't necessarily the right place. I plan to file another bug on adding filesystem notifications consistently in our implementation.

Comment 2

[Markus Stange [:mstange]]

Comment on attachment 373507 [details] [diff] [review]
fix v1.0

This became so much easier to review when I noticed the similarities to nsLocalFileUnix.cpp...

> NS_IMETHODIMP nsLocalFile::GetLastModifiedTime(PRInt64 *aLastModifiedTime)
...
>-  // Check we are correctly initialized.
>-  CHECK_mBaseURL();

Why can you remove this check? (in SetLastModifiedTime, too)


> nsresult nsLocalFile::GetPathInternal(nsACString& path)
> {
...
>+  if (!whichURLRef)
>+    return NS_ERROR_FAILURE;
...
>-  NS_ENSURE_TRUE(whichURLRef, NS_ERROR_NULL_POINTER);

What's wrong with NS_ENSURE_TRUE?

Comment 3

[Markus Stange [:mstange]]

(In reply to comment #2)
> This became so much easier to review when I noticed the similarities to
> nsLocalFileUnix.cpp...

Oh, you even mentioned it.

Comment 4

[Josh Aas]

(In reply to comment #2)

> >-  CHECK_mBaseURL();
> 
> Why can you remove this check? (in SetLastModifiedTime, too)

Both use "GetPathInternal", the result of which we check and that will fail if we don't have a base URL.

> > nsresult nsLocalFile::GetPathInternal(nsACString& path)
> > {
> ...
> >+  if (!whichURLRef)
> >+    return NS_ERROR_FAILURE;
> ...
> >-  NS_ENSURE_TRUE(whichURLRef, NS_ERROR_NULL_POINTER);
> 
> What's wrong with NS_ENSURE_TRUE?

Nothing really, I just have a tendency to only use that for boolean values not pointer null checks. I'd rather leave it my way if that is OK.

Comment 5

[Markus Stange [:mstange]]

Comment on attachment 373507 [details] [diff] [review]
fix v1.0

OK.

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Can you get rid of the LL_ macro usage in nsLocalFileUnix and then refresh this patch so we're not copying that deprecated goop in here?

Comment 7

[Josh Aas]

Attachment: fix v1.1

Stop using LL_ macros, some other minor compile stuff.

Comment 8

[Josh Aas]

Comment on attachment 374091 [details] [diff] [review]
fix v1.1

wrong (old) patch

Comment 9

[Josh Aas]

Attachment: fix v1.2

don't need to check for zero when multiplying

Comment 10

[Josh Aas]

pushed to mozilla-central

http://hg.mozilla.org/mozilla-central/rev/0c250e6e2062