Closed Bug 775833 Opened 12 years ago Closed 12 years ago

VolumeService doesn't work when called OOP

Categories

(Firefox OS Graveyard :: General, defect)

x86_64
Linux
defect
Not set
normal

Tracking

(blocking-basecamp:-)

RESOLVED WONTFIX
blocking-basecamp -

People

(Reporter: dhylands, Unassigned)

Details

(Whiteboard: [blocked-on-input])

When launching the Music Player OOP, it makes calls to DeviceStorage, which in turn makes calls to The VolumeService. However, the VolumeService assumes that it can directly access the VolumeManager data structures, which are in the main process.
At what level are we remoting mediastorage?  I'm surprised the child-process side of things is asking for the volume manager.
when asked script asks for, say, the music directory, we check to see if the /sdcard is backed by a mountable volume.  We do this using the volume service.  The API looks like:

  storage = getDeviceStorage("music");

We want to return null if music doesn't exists (if the sdcard isn't installed.)  I suppose I could proxy this request to the parent and block the child.

thoughts?
Is that using a DOMRequest?  Otherwise it seems like that requires sync disk IO ...
> Is that using a DOMRequest?

No.

> Otherwise it seems like that requires sync disk IO ...

It could, yes.  Most implementations do something like ask the OS for a location on disk.

Jonas, did you have any reasons for not making this async?
This API wasn't originally designed with SD cards in mind.

For non-SD cards, we don't need to do any IO since we know that the directory exists. Aand if it doesn't exist it's clearly a bad bug and we can just make fire an error any time someone tries to interact with the files.

How do SD cards work with someone requesting to get the "music" folder? Do we put a music folder on the SD card? Or do we always return the root of the SD card no matter if someone requests "music" or "pictures"?
Thats a pretty bad API bug. Even local directory existence check is IO and should be async.
Like I said, there is no need to check for directory existence.
All files in the root folder sounds ok to me, at least for v1. I find the directory structure of most sd cards annoying.
> Like I said, there is no need to check for directory existence.

Sort of, yes.

The writers to device storage (the gaia camera application), do not want to write to the pictures directory if it isn't backed back by the SDCard.  Having device storage return early when a request is made for the pictures directory was the cleanest way of doing this.

We could return an error later -- like when the camera applications goes to write to disk, but that means the user will take the picture then fail -- or the camera application will have to write a test file or something.

Maybe we should return a DOMRequest instead of what we are currently doing -- or do what the file system API does:

    http://www.w3.org/TR/file-system-api/#widl-LocalFileSystem-requestFileSystem-void-unsigned-short-type-unsigned-long-long-size-FileSystemCallback-successCallback-ErrorCallback-errorCallback
So, /sdcard exists whether there is a physical SD card present or not.

If the physical SD card is mounted, then writing to /sdcard/foo will write to the sdcard.
If the physical SD card is not mounted (i.e. it may be missing, or it may be unmounted and currently being shared with the PC), then /sdcard points to the root filesystem, which on otoro is the read-only initramfs file system.

You can setup an Observer and look for "volume-state-changed" messages. These are sent out with an nsIVolume subject from which you can query the state. If the state corresponds to "Mounted" then accessing that data on that particular volume is allowed, otherwise it is not.
I'd rather not have to have the device storage code be listening for these events even when it isn't running.  What I'd really like is to be able to ask the volume manager for this information -- and have it return synchronous, and without any IO, the last state it saw.
Right now, you can use sync or async to retrieve the volume state.

If you call GetVolumeByName or GetVolumeByPath, you'll get an nsIVolume. You can query the state from the nsIVolume and it will return its current state.

Right now, it only works in the main process.

So what we really need to decide is whether DeviceStorage should be doing IPCs to get the information from the main process, or whether the VolumeService should be doing IPCs to get the information from the main process.
Do you anticipate other callers in the child process requiring the Volume Service?

If no, I can do the plumbing in device storage to get this information.  I can also plumb the notifications I need.

If yes, then.. well.  maybe the volume service needs to be proxied.
There have been requests so that the status bar can know when USB Mass Storage is active (which is equivalent to getVolumeByName("sdcard").state == Shared)

There have also been requests so that the settings can know how much free space is available or what the total size of a volume is.

It isn't clear to me whether these will be called directly from the UI to the VolumeService or whether it will go through some intermediary like device storage.
it is easy enough to make device storage use a proxied thing.  I'll do that.  If you ever need to make volume service multi process aware... we can remove my stuff.

cjones, does that sound right?
(In reply to Dave Hylands [:dhylands] from comment #14)
> There have been requests so that the status bar can know when USB Mass
> Storage is active (which is equivalent to getVolumeByName("sdcard").state ==
> Shared)
> 

Isn't this the same API mediastorage needs, so that clients know that writes to storage will not succeed?  (The fs isn't mounted locally.)

> There have also been requests so that the settings can know how much free
> space is available or what the total size of a volume is.
> 

Hmm ... this is a trickier one.  Can you link to any of the specific requests?

> It isn't clear to me whether these will be called directly from the UI to
> the VolumeService or whether it will go through some intermediary like
> device storage.

My intuition is that the first case needs to be mediastorage, and that the second case is probably a different API.  But the second case doesn't necessarily require proxied VolumeManager, could be higher level.  So I'd hold off on that for now.
References to stuff related to statusbar
https://github.com/mozilla-b2g/gaia/issues/2333
https://github.com/mozilla-b2g/gaia/issues/2563

References to stuff related to free space
Email sent to dev-gaia list on Jun 22 titled "Device Settings Spec Uploaded" contained a link to this document: http://people.mozilla.com/~lco/Settings_B2G/Release_1_Specs/R1_Device_v1.pdf
See page 3.
All core apps need to run OOP
blocking-basecamp: --- → ?
blocking-basecamp: ? → +
Apparently more discussion is needed between Dave and dougt.
blocking-basecamp: + → ?
Whiteboard: [blocked-on-input]
Doug's landing some stuff that will make this irrelevant.  basecamp-'ing.
blocking-basecamp: ? → -
Since DeviceStorage now only issues requests from the parent process (Bug 780963)
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.