Closed Bug 1204618 Opened 5 years ago Closed 5 years ago
Storage] Add a new API to query the current low-disk-space status
For 2.5 we want to enable the messages and dialer apps to stop storing any user data when the available storage space is running out and inform the user of what's happening. The only way we currently have to figure out if we're in a low-disk space scenario is via the 'change' event, however that prevents us from checking what's the status of the storage at app startup time. For example, if we're already in a low-space condition when the apps are launched the event handler won't be triggered until a certain amount of time has elapsed. To work around this issue we'd like to have an interface to retrieve the low-disk-space status by querying it directly.
This is a very crude patch that adds a lowDiskSpace field to the DeviceStorage object. I'm not sure if this is the right way to go but at least it seems to work.
Attachment #8660903 - Flags: feedback?(dhylands)
Comment on attachment 8660903 [details] [diff] [review] bug-1204618-low-disk-space-query.patch Review of attachment 8660903 [details] [diff] [review]: ----------------------------------------------------------------- Looks good to me, although I think you need to get a DOM Peer to review the webidl change.
Attachment #8660903 - Flags: feedback?(dhylands) → feedback+
Assignee: nobody → gsvelto
Status: NEW → ASSIGNED
I ended up using a slightly different approach that relies on the DiskSpaceWatcher event; this is reliable as we always trigger it at least once at startup and makes it so that the flag is always in-sync with the contents of the event delivered to the application. r? dhlyands for the device storage code and bzbarsky for the webidl changes.
Comment on attachment 8661200 [details] [diff] [review] [PATCH] Add a field to the DeviceStorage object holding the low-disk-space status "Indicates whether" or "True if", please. r=me
Attachment #8661200 - Flags: review?(bzbarsky) → review+
Attachment #8661200 - Flags: review?(dhylands) → review+
Thanks to both for the quick reviews! Updated patch with nits addressed, the try run is here: https://treeherder.mozilla.org/#/jobs?repo=try&revision=8ad083b4a61a
After retriggering a lot of jobs with intermittent failures I've finally got a green try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=8ad083b4a61a Time to land
Comment on attachment 8662017 [details] [diff] [review] [PATCH v2] Add a field to the DeviceStorage object holding the low-disk-space status. r=dhylands, r=bzbarsky Review of attachment 8662017 [details] [diff] [review]: ----------------------------------------------------------------- A last-minute comment, sorry that I look at this only after it landed... ::: dom/webidl/DeviceStorage.webidl @@ +86,5 @@ > // Indicates if the storage area denoted by storageName is removable > readonly attribute boolean isRemovable; > > + // True if the storage area is close to being full > + readonly attribute boolean lowDiskSpace; Mmm I would have used a String actually; because in the future we'll have a 3-state value: "normal situation", "close to low-storage condition", "low-storage condition". But if we use "" (or null), "near-low", "low" it could be backward compatible, assuming developers don't check for the actual boolean value.
If we think we'll want a multi-state thing here in the future, we should just go ahead and use an IDL enum. But we'd also want a different name for it, then, yes?
Ah yeah, right, enum is the right approach here. About the name, I'm bad at naming, but I think "lowDiskSpace" still makes sense, since it would hold the status of being near or in low storage condition.
I'd think "diskSpaceStatus" would make more sense there.
why not !
OK, well this mirrors the internal implementation for now (yeah, I know, not nice for an API) but we're free to change it when we also change the event since they are coupled.
I've added the new property to this page: https://developer.mozilla.org/en-US/docs/Web/API/DeviceStorage a dedicated page here: https://developer.mozilla.org/en-US/docs/Web/API/DeviceStorage/lowDiskSpace and a note to the relevant release notes: https://developer.mozilla.org/en-US/docs/Mozilla/Firefox_OS/Releases/2.5 Let me know if these look ok. Thanks!
I'm working on Bug 1208892, which should display warning messages in settings when app storage is almost full. I filled up my device to < 5MB left and found a strange issue: When I tried to inspect navigator.getDeviceStorage('apps').lowDiskSpace from WebIDE console, in Settings it shows false (incorrectly), but in System it shows true (correctly). Wonder if it's a bug in the API?
Oops I meant Bug 1208893
Sorry for the delay, it was due to the branch date for 2.5. I'm now looking into this.
OK, I've figured out where the problem is and will open a new bug to fix it: I'm not doing IPC correctly so the low-space notification event I'm listening to is handled only in the main process. In content processes the message is never sent (there's no disk space watcher!) and thus the field is never set to true.
You need to log in before you can comment on or make changes to this bug.