Closed Bug 1007053 Opened 6 years ago Closed 6 years ago

Implement "canBeFormatted", "canBeMounted" and "canBeShared" API for device storage

Categories

(Firefox OS Graveyard :: General, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
2.0 S2 (23may)

People

(Reporter: alan.yenlin.huang, Assigned: dhylands)

References

Details

(Keywords: dev-doc-needed)

Attachments

(4 files)

According to bug 943825 comment 9, we need these APIs to expose more volume configurations. Then, Settings App can decide how to deal with it.
Summary: Implement "CanBeFormatted", "CanBeMounted" and "CanBeShared" API for device storage → Implement "canBeFormatted", "canBeMounted" and "canBeShared" API for device storage
Bug 1006328 implements isShareable, which implies canBeMounted and canBeFormatted.
Duplicate of this bug: 1006328
Alan - I have a patch that implements isShareable from bug 1006328. Is it ok for me to take ownership of this?
Sorry my last 2 comments were totally in the wrong bug
While trying to implement this, I noticed that the isFake attribute in the Volume wasn't being propagated to the child processes.
Attachment #8422007 - Flags: review?(kyle)
Attachment #8422007 - Attachment description: Make isFake be propagated properly to child processes → Pt 1 - Make isFake be propagated properly to child processes
Assignee: ahuang → dhylands
Comment on attachment 8422008 [details] [diff] [review]
Pt 2 - Add canBeShared, canBeMounted and canBeFormatted attributes to devicestororage webidl

r=me
Attachment #8422008 - Flags: review?(bzbarsky) → review+
Attachment #8422007 - Flags: review?(kyle) → review+
Marking as r+ since this is a trivial change, and it only impacts a test program.
Attachment #8422011 - Flags: review+
Attachment #8422009 - Flags: review?(kyle) → review+
feature-b2g: --- → 2.0
feature-b2g: 2.0 → ---
Hi Dave, I'm working on bug 943825. And I try to use the three attributes since API is landed and supported already. The attributes provide some limitation for specific devices(Nexus 4/5). But I cannot reach guidance from MDN(https://developer.mozilla.org/en-US/docs/Web/API/DeviceStorage). Just Q&A Gecko devs from the code base. Shell we add some document for the new attributes? That would be more convenient for developments. Thanks.
Flags: needinfo?(dhylands)
Yeah - I should have flagged this as dev_doc_needed. Now that I think of it, there is probably some others that need this too.
Flags: needinfo?(dhylands)
Keywords: dev-doc-needed
canBeFormatted/Shared/Mounted should return false on Nexus 4/5.
Hi Dave, I have a question about the definition of 'canBeMounted' attribute. There is an internal storage in device 'flame'. And, the 'canBeMounted' attribute will return true. But, this internal storage is not removable form the hardware. Could we use the attribute to identify a removable/unremovable storage? Per spec. of SDCard management, we won't provide mount/unmount functionality for an unremovable storage. Do we have some way to identify the 'removable' attribute? Thanks.
Flags: needinfo?(dhylands)
Internal storage on the flame is formattable, but not removable.

Unfortunately, we don't have a way right now to determine if a media is removable or not, and I haven't figured out a way to ddo it programatically.

I think we would need to add something into /system/etc/volume.cfg to indicate if a given volume is removable or not.

For the internal storage, you still need to unmount before you can format don't you?
Flags: needinfo?(dhylands)
So, shell we provide 'canBeRemovable' or 'canBePlugged' attribute for decision functionality mount/unmount for internal storage? Otherwise, we still need some 'heuristic' judgement to make sure internal/external(https://github.com/mozilla-b2g/gaia/blob/master/apps/settings/js/media_storage.js#L345-L353). Then, we always disable mount/unmount for the internal storage. This is a little be tricky and nontrivial for implementation. I'm wondering some of special case we don't take care.

Since platform have supported format() API via bug 841660, I don't need to unmount it before format. The API will help to unmount storage before format. This is a friendly design for Gaia developer.:)
BTW, some of the devices, which removable external storage is under the battery. It means a user have to shut down the device. Then, he/she removes the SDCard from the SD-Slot after unplugged the battery. I would like to suggest to considerate this case in UX. +Jenny for discussion.

This kinds of hardware limitation might be hard to expose to Gaia/Gecko side? Dave, do you have any idea for this section?
What's the use case for knowing that the media is removable or not?

If I were going to create a new API, I would use isRemovable (not canBeRemovable). (isRemovable describes an attribute, canBe describes the ability to perform an action - a subtle distinction)

Whether you consider an sdcard under the battery as removable or not really depends on the use case.

If its just to expose mount/unmount, then I would consider an sdcard under the battery as non-removable (i.e. can't be removed while the phone is powered on).

The other option is to not add anything new to the API and have gaia use canBeMounted to determine if the storage area should expose mount/unmount.

Currently, canBeFormatted/Mounted/Shared all have the same value, but we could add an entry to /system/etc/volume.cfg that causes canBeMounted to return false for sdcards which can't be removed while the phone is powered on.
(In reply to Dave Hylands [:dhylands] from comment #22)
> What's the use case for knowing that the media is removable or not?
The use case is defined in SD Card spec., page 9, 'Note' in right corner.(https://bugzilla.mozilla.org/show_bug.cgi?id=921105#c22)

Note:
Mount SD Card/Unmount SD Card/Format SD Card may not show if the device or storage doesn’t support these functions.

Per discussion with UX designer, they will not provide mount/unmount functionality for non-removable media.
> 
> If I were going to create a new API, I would use isRemovable (not
> canBeRemovable). (isRemovable describes an attribute, canBe describes the
> ability to perform an action - a subtle distinction)
'isRemovable' is pretty clear for me.:)

> 
> Whether you consider an sdcard under the battery as removable or not really
> depends on the use case.
> 
> If its just to expose mount/unmount, then I would consider an sdcard under
> the battery as non-removable (i.e. can't be removed while the phone is
> powered on).
> 
> The other option is to not add anything new to the API and have gaia use
> canBeMounted to determine if the storage area should expose mount/unmount.
> 
> Currently, canBeFormatted/Mounted/Shared all have the same value, but we
> could add an entry to /system/etc/volume.cfg that causes canBeMounted to
> return false for sdcards which can't be removed while the phone is powered
> on.

Both of above options are okay for me. But, if we configure 'canBeMounted' attribute for 'isRemovable' use case, the storage will not be able to mount/unmount one day.(Might the user story of settings::media storage be reorganised one day, we could need 'canBeMounted' attribute. Because the attribute is corresponding to storage mount/unmount ability, not on use case.) Base on my view, I would like the new attribute 'isRemovable'. This is more accurate for me.
I'm fine with adding isRemovable.

File a new bug, cc me, and add the dev-doc-needed keyword to the bug.
Thanks for your suggestion. Let's track via bug 1033952.
You need to log in before you can comment on or make changes to this bug.