Closed Bug 912457 Opened 11 years ago Closed 11 years ago

[OS.File] OS.File doesn't expose flush on file object from chrome


(Toolkit Graveyard :: OS.File, defect)

Not set


(Not tracked)



(Reporter: ochameau, Assigned: nmaier)


(Keywords: dev-doc-complete, Whiteboard: [Async][mentor=Yoric][lang=js])


(1 file, 1 obsolete file)

The documentation talks about a flush method on file objects, but it isn't being exposed, at least when using OS.File from chrome code:
The change is actually rather simple:
- in osfile_async_worker.jsm, add a method File_prototype_flush that calls File.prototype.flush, much like e.g File_prototype_setPosition calls File.prototype.setPosition);
- in osfile_async_front.jsm, add a method File.prototype.flush that sends message File_prototype_flush, much like e.g. File.prototype.setPosition sends message File_prototype_setPosition.
Whiteboard: [mentor=Yoric][lang=js]
Hi, this could be a first bug to start, can it be assigned to me?
Have fun :)
If you have any questions, don't hesitate to come and chat on #introduction:
Assignee: nobody → tomiberes
Hi Tomiberes, how are things doing on this bug? Do you need any assistance?
Flags: needinfo?(tomiberes)
In the absence of news, de-assigning.
Assignee: tomiberes → nobody
Flags: needinfo?(tomiberes)
Summary: OS.File doesn't expose flush on file object from chrome → [OS.File] OS.File doesn't expose flush on file object from chrome
Whiteboard: [mentor=Yoric][lang=js] → [Async][mentor=Yoric][lang=js]
Note that this patch is on top of some other patches (bug 928239, bug 924858, bug 924916) in my queue and will fail to apply the xpcshell.ini hunk without the other patches.
Assignee: nobody → maierman
Attachment #818844 - Flags: review?(dteller)
In that case, could you add dependencies?
Comment on attachment 818844 [details] [diff] [review]
Expose async |flush()| in OS.File.

Review of attachment 818844 [details] [diff] [review]:

Looks good, thanks.

::: toolkit/components/osfile/modules/osfile_async_front.jsm
@@ +498,5 @@
> +  },
> +
> +  /**
> +   * Flushes the file's buffers and causes all buffered data
> +   * to be written.

Could you add one line to mention that this operation is generally quite expensive, serves essentially to protect against hardware/kernel/power failures taking place immediately after the write and should be reserved for valuable files?
Attachment #818844 - Flags: review?(dteller) → review+
How about:
Disk flushes are very expensive and therefore should be used carefully, sparingly and only in scenarios where it is vital that data survives system crashes. Even though the function will be executed off the main-thread, it might still affect the overall performance of any running application.
Unbitrotted. Carrying over r=yoric.
Part of this green try:
Attachment #823021 - Flags: review+
Attachment #818844 - Attachment is obsolete: true
Keywords: checkin-needed
Flags: in-testsuite+
Keywords: checkin-needed
Whiteboard: [Async][mentor=Yoric][lang=js] → [Async][mentor=Yoric][lang=js][fixed-in-fx-team]
Closed: 11 years ago
Resolution: --- → FIXED
Whiteboard: [Async][mentor=Yoric][lang=js][fixed-in-fx-team] → [Async][mentor=Yoric][lang=js]
Target Milestone: --- → mozilla27
Updated article to state |.flush()| is supported in the async interface since Gecko 27:
Product: Toolkit → Toolkit Graveyard
You need to log in before you can comment on or make changes to this bug.