873391 - [regression] Volume state changes get processed out of order in child processes

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect)

Description

[Dave Hylands [:dhylands]]

As part of testing bug 872800 on b2g18 I discovered that the volume status wasn't always being reported correctly. I haven't noticed this problem on master.

I determined the root cause to be that device storage and nsVolumeService both use the NS_VOLUME_STATE_CHANGED message in the child, but device storage may get called before nsVolumeService has updated the volume state, which causes device storage to get bad state information.

I marked this as leo+ since it affects bug 872800 which is leo+

The solution, is to have ContentChild::RecvFileSystemUpdate call some new method in nsVolumeService to update the volume state, and have that method Notify the observers, rather than having nsVolumeService itself be an observer.

This particular regression was introduced as part of bug 838038.

Comment 1

[Dave Hylands [:dhylands]]

Attachment: Process volume change notifications in the volume service (child) first.

This patch causes the NS_VOLUME_STATE_CHANGED messages delivered to the child process to be processed by the volume service first (so it can update it's cache). Then other observers will get notified.

Comment 2

[Dave Hylands [:dhylands]]

Comment on attachment 751049 [details] [diff] [review]
Process volume change notifications in the volume service (child) first.

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

::: dom/devicestorage/nsDeviceStorage.cpp
@@ +20,5 @@
>  #include "mozilla/LazyIdleThread.h"
>  
>  #include "nsAutoPtr.h"
>  #include "nsDOMEvent.h"
> +#include "nsServiceManagerUtils.h"o

That's not supposed to be there....

Comment 3

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Comment on attachment 751049 [details] [diff] [review]
Process volume change notifications in the volume service (child) first.

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

r=me with nit you already found fixed.

Comment 4

[Dave Hylands [:dhylands]]

https://hg.mozilla.org/projects/birch/rev/fe76cf6e4ea4

Comment 5

[Dave Hylands [:dhylands]]

Attachment: b2g18 spefic version of the patch

Carrying r+

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/fe76cf6e4ea4

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18/rev/32ced47e88b5