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

RESOLVED FIXED in 2.1 S2 (15aug)

Status

RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: iliu, Assigned: alchen)

Tracking

unspecified
2.1 S2 (15aug)
ARM
Gonk (Firefox OS)

Firefox Tracking Flags

(feature-b2g:2.1)

Details

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

Attachments

(1 attachment, 1 obsolete attachment)

Since working on bug 943825, I find out "storage-state-change" event is triggered twice when there is volume state change.
(Reporter)

Updated

4 years ago
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)
(Assignee)

Updated

4 years ago
Whiteboard: [p=1]
(Assignee)

Comment 3

4 years ago
Created attachment 8470743 [details] [diff] [review]
(0811) Avoid to send the same status in two continuois storage-state-change event

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+
(Assignee)

Comment 5

4 years ago
Created attachment 8471365 [details] [diff] [review]
Avoid to send the same status in two continuois storage-state-change event. r=dhylands

Try server result looks fine.
https://tbpl.mozilla.org/?tree=Try&rev=31112d92b0b1
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=31112d92b0b1
Attachment #8470743 - Attachment is obsolete: true
(Assignee)

Updated

4 years ago
Keywords: checkin-needed

Updated

4 years ago
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
Last Resolved: 4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.