Closed Bug 1050720 Opened 6 years ago Closed 6 years ago

[Device Storage] "storage-state-change" event will be triggered twice when there is volume state change

Categories

(Firefox OS Graveyard :: General, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(feature-b2g:2.1)

RESOLVED FIXED
2.1 S2 (15aug)
feature-b2g 2.1

People

(Reporter: iliu, Assigned: alchen)

References

Details

(Whiteboard: [p=1] [ft:devices])

Attachments

(1 file, 1 obsolete file)

Since working on bug 943825, I find out "storage-state-change" event is triggered twice when there is volume state change.
Blocks: 943825
Is it because multiple volume states map onto the same storage state?
Might be not. In my observation, the 'storage-state-change event will be coming as following scenario.

The storage is in Mounted status.

1). do unmount() storage: 
    4 (Mounted) -> 5 (Unmounting)         event: Mounted, Unmounting   (In this case, Mounted event is duplicated)
    5 (Unmounting) -> 1 (Idle-Unmounted)  event: Idle


The storage is in Idle status.

2). manully remove SD card:
    1 (Idle-Unmounted) -> 0 (NoMedia)     event: Idle, NoMedia (In this case, Idle event is duplicated)
Whiteboard: [p=1]
Hi Dave,
In this patch, we will ignore the status if it is the same as last time.
Ian can use this patch to continue his work on setting app.
Attachment #8470743 - Flags: review?(dhylands)
Comment on attachment 8470743 [details] [diff] [review]
(0811) Avoid to send the same status in two continuois storage-state-change event

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

::: dom/devicestorage/nsDeviceStorage.cpp
@@ +4166,5 @@
> +  if (aVolumeStatus == mLastVolumeStatus) {
> +     // We've already sent this status, don't bother sending it again.
> +    return;
> +  }
> +  mLastVolumeStatus = aVolumeStatus;

This looks good.

Since we're calling dsf::GetStorageStatus() (the result of that is whats passed into this function), can we rename aVolumeStatus to be aStorageStatus and rename mLastVolumeStatus to be mLastStorageStatus?

That way the naming is a bit more consistent.
Attachment #8470743 - Flags: review?(dhylands) → review+
Keywords: checkin-needed
feature-b2g: --- → 2.1
Whiteboard: [p=1] → [p=1] [ft:devices]
Target Milestone: --- → 2.1 S2 (15aug)
https://hg.mozilla.org/mozilla-central/rev/112307bd6632
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.