910885 - Improve FileUtils.getDir(…, …, true) performance

Status: RESOLVED FIXED

(Toolkit :: General, defect)

Description

[Marco Castelluccio [:marco]]

Attachment: improve_getdir_true

nsIFile already creates any path segments that don't exist, so we can avoid to call |exists| and |create| for every path segment.
We can also avoid calling |exists| and put the |create| call in a try block.

I know the function will go away eventually, but in the meantime this small fix is really simple.

Comment 1

[David Teller [:Yoric] - still alive but not very active]

Comment on attachment 797471 [details] [diff] [review]
improve_getdir_true

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

::: toolkit/modules/FileUtils.jsm
@@ +67,5 @@
> +
> +    if (shouldCreate) {
> +      try {
> +        dir.create(Ci.nsIFile.DIRECTORY_TYPE, this.PERMS_DIRECTORY);
> +      } catch (ex if ex.result == Cr.NS_ERROR_FILE_ALREADY_EXISTS) {}

Nit: Could you add a (trivial) comment explaining the empty brackets?

Comment 2

[Marco Castelluccio [:marco]]

Attachment: improve_getdir_true

Carrying r+.

Comment 3

[Marco Castelluccio [:marco]]

Attachment: improve_getdir_true

Looks like ellipsis characters cause build problems.

https://tbpl.mozilla.org/?tree=Try&rev=43a19f905b68

https://tbpl.mozilla.org/?tree=Try&rev=3edc89cf937e

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6bd785bca4ff

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/6bd785bca4ff

Comment 6

[Alexandre Poirot [:ochameau]]

This optimization introduces some unexpected errors on Windows, for example on these FileUtils.getFile usages:
http://mxr.mozilla.org/mozilla-central/search?string=DefRt&find=dom&findi=&filter=^[^\0]*%24&hitlimit=&tree=mozilla-central
  FileUtils.getFile("DefRt", ["settings.json"], false);

"DefRt" is c:\program files\Firefox\defaults
And by default program files can only be read by a regular windows user.
So FileUtils.getFile calls FileUtils.getDir with shouldCreate=true,
to it tries to create the "defaults" directory and throws with a file access denied exception:
  NS_ERROR_FILE_ACCESS_DENIED: Component returned failure code: 0x80520015 (NS_ERROR_FILE_ACCESS_DENIED) [nsIFile.create]

Right now I'm only seing this exception when trying to run gaia in firefox on Windows.
But I wouldn't be surprised other usecases fail with the same exception.

I don't really know how to address that without getting back to the original code, may be that?
   } catch (ex if ex.result == Cr.NS_ERROR_FILE_ALREADY_EXISTS) {
     //...
+  } catch (ex if ex.result == Cr.NS_ERROR_FILE_ACCESS_DENIED) {
+    if (!dir.exists())
+      throw ex;
+  }