Closed Bug 707094 Opened 8 years ago Closed 8 years ago

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

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla11

People

(Reporter: philikon, Assigned: fabrice)

References

Details

Attachments

(1 file)

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.
Attached patch patchSplinter Review
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+
https://hg.mozilla.org/mozilla-central/rev/c9eee0711ca6
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla11
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.