Closed Bug 1029403 Opened 6 years ago Closed 6 years ago

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

Categories

(Firefox OS Graveyard :: General, defect)

x86_64
Windows 7
defect
Not set

Tracking

(feature-b2g:2.1)

RESOLVED FIXED
2.0 S6 (18july)
feature-b2g 2.1

People

(Reporter: alchen, Assigned: alchen)

References

Details

(Keywords: dev-doc-needed, Whiteboard: [p=3])

Attachments

(1 file, 2 obsolete files)

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.
Assignee: nobody → alchen
Blocks: 943825
Whiteboard: [p=3]
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.
Attachment #8445025 - Flags: feedback?(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.
Attachment #8445025 - Flags: feedback?(dhylands)
In this patch, I rename DispatchMountChangeEvent to be DispatchStatusChangeEvent and create a DispatchStorageStatusChangeEvent function for the storage-status.
Attachment #8445025 - Attachment is obsolete: true
Attachment #8448593 - Flags: review?(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
Attachment #8448593 - Flags: review?(dhylands) → review+
feature-b2g: --- → 2.1
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/98ecfcd9242b
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.0 S6 (18july)
You need to log in before you can comment on or make changes to this bug.