Closed Bug 1388208 Opened 3 years ago Closed 3 years ago

Stop using FileUtils.getFile

Categories

(WebExtensions :: General, enhancement)

enhancement
Not set
normal

Tracking

(firefox57 fixed)

RESOLVED FIXED
mozilla57
Tracking Status
firefox57 --- fixed

People

(Reporter: kmag, Assigned: kmag)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

This performs main thread IO to make sure that directories exist, which is not something we should be doing on the startup path.
Blocks: webext-perf
Comment on attachment 8894697 [details]
Bug 1388208: Stop using FileUtils.getFile.

Kris "asked" me to steal the review.
Attachment #8894697 - Flags: review?(mixedpuppy) → review?(tomica)
Comment on attachment 8894697 [details]
Bug 1388208: Stop using FileUtils.getFile.

https://reviewboard.mozilla.org/r/165858/#review171174

::: toolkit/components/extensions/ExtensionParent.jsm:1414
(Diff revision 1)
>    async _readData() {
>      let result = new Map();
>      try {
>        let data = await promiseFileContents(this.file);
>  
>        result = aomStartup.decodeBlob(data);

Now that we switched to `OS.File.read()` which supports compression directly, can we eliminate this dependency on `aomStartup`?
Attachment #8894697 - Flags: review?(tomica) → review+
Comment on attachment 8894697 [details]
Bug 1388208: Stop using FileUtils.getFile.

https://reviewboard.mozilla.org/r/165858/#review171174

> Now that we switched to `OS.File.read()` which supports compression directly, can we eliminate this dependency on `aomStartup`?

This isn't compression. It's structured clone deserialization.
https://hg.mozilla.org/mozilla-central/rev/903db0ac80c6
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Would it make sense to add an eslint rule to ban further usage of FileUtils.getFile()?
Blocks: 1391576
(In reply to :Ehsan Akhgari (needinfo please, extremely long backlog, Away 8/18) from comment #7)
> Would it make sense to add an eslint rule to ban further usage of
> FileUtils.getFile()?

We should just migrate the callers and remove the function. Bug 920187 was already on file to do that, but in the meantime I filed bug 1391576 for the low-hanging fruit.
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.