DOMApplicationRegistry (Webapps.jsm): Use FileUtils.jsm for simpler and more robust nsIFile handling

RESOLVED FIXED in mozilla11

Status

()

Core
DOM
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: philikon, Assigned: fabrice)

Tracking

unspecified
mozilla11
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

In DOMApplicationRegistry, which resides in Webapps.jsm:

>    let file =  Cc["@mozilla.org/file/directory_service;1"].getService(Ci.nsIProperties).get("ProfD", Ci.nsIFile);
>    file.append("webapps");
>    if (!file.exists() || !file.isDirectory()) {
>      file.create(Ci.nsIFile.DIRECTORY_TYPE, 0700);
>    }
>    this.appsDir = file;
>    this.appsFile = file.clone();
>    this.appsFile.append("webapps.json");

First off, tsk tsk for not using Services.dirSvc. But, this whole thing can be written in just one line:

  let appsFile = FileUtils.getFile("ProfD", ["webapps", "webapps.json"], true);

The only difference is that the "webapps" directory may not be created with the desired 0700 permissions. Is this *that* important? The code gives no clue as to why you would want to protect that directory specially. (Also, magic constants should be const'ed!)


>  _writeFile: function ss_writeFile(aFile, aData, aCallbak) {
>    // Initialize the file output stream.
>    let ostream = Cc["@mozilla.org/network/safe-file-output-stream;1"].createInstance(Ci.nsIFileOutputStream);
>    ostream.init(aFile, 0x02 | 0x08 | 0x20, 0600, ostream.DEFER_OPEN);

Again, tsk tsk for not const'ing the magic constants here. And, as usual, FileUtils will reduce this to one line:

  let ostream = FileUtils.openSafeFileOutputStream(aFile);

Once again, it gives no clue as to why the 0600 permission is requested here. FileUtils.openSafeFileOutputStream() will happily take different mode flags, but not different permissions yet. Though that could easily be added, and I would definitely welcome ways that would make FileUtils more useful.
(Assignee)

Comment 1

6 years ago
Created attachment 579204 [details] [diff] [review]
patch

Cleaned up with FileUtils. Yay for less code!
Assignee: nobody → fabrice
Attachment #579204 - Flags: review?(philipp)
Comment on attachment 579204 [details] [diff] [review]
patch

\o/
Attachment #579204 - Flags: review?(philipp) → review+
(Assignee)

Comment 3

6 years ago
pushed:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c9eee0711ca6

Comment 4

6 years ago
https://hg.mozilla.org/mozilla-central/rev/c9eee0711ca6
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla11
You need to log in before you can comment on or make changes to this bug.