1029403 - [Device Storage] Trigger a storage-state-change event when there is volume state change

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect)

Description

[Alphan Chen [:alchen]]

Based on the discussion of bug 943825, we need to trigger a storage-state-change event with actual volume state when there is volume state change. By listening these event changes, GAIA could know a clear timing to do corresponding behavior.

Comment 1

[Alphan Chen [:alchen]]

Attachment: Trigger a storage-state-change event when there is volume state change

Hi Dave,
I don't know if it is a suitable place to accomplish this goal.
please have a look about this patch and give some suggestion about it.
Thanks a lot.

Comment 2

[Dave Hylands [:dhylands]]

Comment on attachment 8445025 [details] [diff] [review]
Trigger a storage-state-change event when there is volume state change

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

Hmm. I think we need to do some cleanup here. And we need to move where you're sending things.

DispatchMountChangeEvent is called from here: http://dxr.mozilla.org/mozilla-central/source/dom/devicestorage/nsDeviceStorage.cpp#4261
and the status passed in is determined here:
http://dxr.mozilla.org/mozilla-central/source/dom/devicestorage/nsDeviceStorage.cpp#1541

So the status passed to DispatchMountChangeEvent is one of (available, unavailable, shared).

For consistency, we should call (available, unavailable, shared) a status. We should call the statuses listed here:
http://dxr.mozilla.org/mozilla-central/source/dom/system/gonk/nsIVolume.idl?from=nsIVolume.idl#12 storage statuses (or possible volume statuses). I've been trying to use the word storage for the API facing stuff, and only using volume on the internal side of things.

You'll need to send your storage-status-changed from here:
http://dxr.mozilla.org/mozilla-central/source/dom/devicestorage/nsDeviceStorage.cpp#4257
and call DeviceStorageFile::GetStorageStatus to get the string.

In DispatchMountChangeEvent, please rename aVolumeStatus to be aStatus.

In fact, we should probably rename DispatchMountChangeEvent to be DispatchStatusChangeEvent and create a DispatchStorageStatusChangeEvent function for the storage-status.

::: dom/devicestorage/nsDeviceStorage.cpp
@@ +4193,5 @@
> +                                                 mStorageName,
> +                                                 aStorageStatus);
> +  bool ignore;
> +  if (NS_SUCCEEDED(rv))
> +    DispatchEvent(ce, &ignore);

nit: Add curly braces, even for single line statements.

Move declaration of ignore inside the curly braces.

Comment 3

[Alphan Chen [:alchen]]

Attachment: (0701) Trigger a storage-state-change event when there is volume state change

In this patch, I rename DispatchMountChangeEvent to be DispatchStatusChangeEvent and create a DispatchStorageStatusChangeEvent function for the storage-status.

Comment 4

[Dave Hylands [:dhylands]]

Comment on attachment 8448593 [details] [diff] [review]
(0701) Trigger a storage-state-change event when there is volume state change

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

Just one nit on one of the variable names. Otherwise this looks good.

Thanks

::: dom/devicestorage/nsDeviceStorage.cpp
@@ +4285,4 @@
>      }
>  
>      DeviceStorageFile dsf(mStorageType, mStorageName);
> +    nsString status, volumeStatus;

nit: Let's call this storageStatus instead of volumeStatus

Comment 5

[Alphan Chen [:alchen]]

Attachment: Trigger a storage-state-change event when there is volume state change. r=dhylands

Here is the try server result. It looks fine.
https://tbpl.mozilla.org/?tree=Try&rev=906d34133161

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/b2g-inbound/rev/98ecfcd9242b

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/98ecfcd9242b