Closed Bug 987101 Opened 6 years ago Closed 6 years ago

Switch the CrashMonitor to native OS.File.read()

Categories

(Toolkit :: Async Tooling, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla31

People

(Reporter: Yoric, Assigned: marco)

Details

(Whiteboard: [Async:ready][mentor=Yoric][lang=js])

Attachments

(1 file, 1 obsolete file)

No description provided.
This should be pretty simple. There is a call to NetUtil.asyncFetch used to load a file. Instead of that call, use OS.File.read.
Whiteboard: [Async:ready][mentor=Yoric][lang=js]
Attached patch Patch (obsolete) — Splinter Review
Assignee: nobody → mar.castelluccio
Status: NEW → ASSIGNED
Attachment #8395957 - Flags: review?(dteller)
Comment on attachment 8395957 [details] [diff] [review]
Patch

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

::: toolkit/components/crashmonitor/CrashMonitor.jsm
@@ +96,5 @@
>     * Load checkpoints from previous session asynchronously.
>     *
>     * @return {Promise} A promise that resolves/rejects once loading is complete
>     */
>    loadPreviousCheckpoints: function () {

Could you use a Task instead?
Attachment #8395957 - Flags: review?(dteller) → feedback+
Attached patch PatchSplinter Review
Looks much clearer now ;)
Attachment #8395957 - Attachment is obsolete: true
Attachment #8396688 - Flags: review?(dteller)
Comment on attachment 8396688 [details] [diff] [review]
Patch

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

Looks good, thanks.
Attachment #8396688 - Flags: review?(dteller) → review+
https://hg.mozilla.org/mozilla-central/rev/ab98dbf842aa
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
We now need to keep watch on the startup indicators to confirm that we are not regressing.
You need to log in before you can comment on or make changes to this bug.