928558 - AutoMounter mark volumes as being shared even if they have open files

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect)

Description

[Dave Hylands [:dhylands]]

Currently, the AutoMounter waits until it starts the sequence of sharing a volume with the PC before marking a volume as shared.

The AutoMounter detects if there are files open on the volume, and defers initiating the sharing sequence until there are no open files. This is because vold will kill any apps which have open files.

This bug is to have the AutoMounter mark the volumes as being shared as soon as it detects that there are open files. This should then ripple through device storage and cause a volume shared notification to be sent to the apps which have the files open (since the only "normal" way to have files open is through device storage).

This then satifies bug 912609 comment 18.

Marking as koi+ since 912609 is koi+ (and it will require this change).

Comment 1

[Dave Hylands [:dhylands]]

Attachment: Mark volumes shared before checking for open files.

Comment 2

[Dave Hylands [:dhylands]]

Comment on attachment 819257 [details] [diff] [review]
Mark volumes shared before checking for open files.

Ignore the change in VolumeManagerLog.h - I've fixed it already.

The basic gist of this is that we want to mark the volumes as shared when we're about to share them, so that apps will then see that and close any open files they have.

Comment 3

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

Comment on attachment 819257 [details] [diff] [review]
Mark volumes shared before checking for open files.

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

r=me with nits addressed

::: dom/system/gonk/AutoMounter.cpp
@@ +338,5 @@
> +  static bool inUpdateState = false;
> +  if (inUpdateState) {
> +    // When UpdateState calls SetISharing, this causes a volume state
> +    // change to occur, which would normally cause UpdateState to be called
> +    // again. We want the volume state change to go out (so that device

Supernit: No matching paren?

::: dom/system/gonk/Volume.cpp
@@ +67,5 @@
>  
>  void
>  Volume::SetIsSharing(bool aIsSharing)
>  {
> +  if (aIsSharing != mIsSharing) {

Supernit: Flip boolean, return early

::: dom/system/gonk/VolumeManagerLog.h
@@ +20,5 @@
>  #else
>  #define DBG(args...)
>  #endif
>  
> +#endif  // mozilla_system_volAut

Nit: This looks like an accident?

Comment 4

[Dave Hylands [:dhylands]]

https://hg.mozilla.org/integration/b2g-inbound/rev/44b92484bfe6

Comment 5

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

https://hg.mozilla.org/mozilla-central/rev/44b92484bfe6

Comment 6

[Ed Morley [:emorley]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/e346a58d6b6a

Comment 7

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Why not use mozilla::AutoRestore?

Comment 8

[Dave Hylands [:dhylands]]

(In reply to :Ms2ger from comment #7)
> Why not use mozilla::AutoRestore?

Umm - because I didn't know about it...

I filed bug 931424 to fix it.

Comment 9

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Thanks!