856782 - Update mediadb and settings app to use getDeviceStorages

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Gallery, defect)

Description

[Dave Hylands [:dhylands]]

With the gecko changes in bug 838038, navigator.getDeviceStorages was added to return an array of device storage objects.

This implies that some changes will be needed in mediadb.

I couldn't tell if what bug 844207 is supposed to be for (there is no description), so this might be a dup.

This bug is for the changes required in mediadb to switch over to use getDeviceStorages

Comment 1

[Stephany Wilkes]

Dave's gecko work on 838038 is landing TODAY, April 2, 2013. With the goal of UX unblocking 838038 in time for 1.1, then, this is what UX is going to do by Friday, April 5: 

UX (Casey) will create wireframes around the settings apps, the share option, and where they should be placed. 

Something that may help Casey here is updating the patch (that adds a fake volume used for testing) on 838038: Dave used this patch to test and verify his work and that patch is NOT landing, so Casey can put it in his local tree and do an unagi build, and get two storage areas on an unagi to test his work. 

Dave has generously offered to do the gaia side work and pull it all together. Dave will update mediadb and the settings app to use get device storage.

Comment 2

[Wayne Chang [:wchang]]

Dave, is this bug a must to land bug 838038?

Comment 3

[Dave Hylands [:dhylands]]

No - bug 838038 if fully backwards compatible. So technicaly, I guess this bugs depends on the code in 838038, but the feature isn't complete without this bug.

Comment 4

[Lukas Blakk [:lsblakk] use ?needinfo]

OK so we're blocking here based on an understanding that this is required for the feature in bug 838038 to work fully and that feature is a blocker.

Comment 5

[Evelyn Hung]

take this issue to implement the UI changes in Settings app per Casey's UX wireframe:
https://www.dropbox.com/s/lr35b241542fmbu/Storage_v0.1.pdf

Comment 6

[Stephany Wilkes]

+1 Evelyn: Exactly. Thank you.

Comment 7

[Dave Hylands [:dhylands]]

ok - based on Casey's Storage document from bug 838038 comment 69 (page 7), I'm going to propose the following setting names:

A - Enable USB Storage should control ums.mode (as it did previously)

C - Share using USB should control a setting named: ums.volume.NAME.enabled, where NAME comes from the volumeName attribute from the device storage object (i.e. navigator.getDeviceStorages[i].volumeName)

D - Default media location should be called device.storage.writable.name and it should contain NAME as per C. This should then get reflected from a setting into a preference in gecko/b2g/chrome/content/settings.js


For now, we also need something along the following for determining the label to be applied to the volumes:

If navigator.getDeviceStorages returns a single entry, then its volume name will be sdcard, and it should be called "SD Card Storage" (so this covers the unagi).

If navigator.getDeviceStorages returns more than one entry, then the first entry should be sdcard, and it will be called "Internal Storage", and the second one should be called SD Card Storage, and the third one (if it exists) should be called SD Card Storage 2.

On unagi, there is a single physical sdcard, which is called "sdcard".

On leo, there are 2 volumes, sdcard is the internal memory, and extsdcard (or something along those lines) will correspond to the physical sdcard.

On something like a nexus 4, there won't be any sdcard volumes (as far as the volume manager is concerned), but we'll have to create one, and it will correspond to internal memory. By the time we get around to supporting this we can probably figure out how to add an "external" attribute to the device storage object. But we don't have to worry about this particular scenario in this time frame.

Comment 8

[Evelyn Hung]

I need a new icon for display item A on root panel, file bug 866376 for it.

Comment 9

[Evelyn Hung]

Hi Dave, 
I need to provide a media type when acquiring deviceStorage objects, like:
var storages = navigator.getDeviceStorages(storageType);
However, per spec design, the settings are listed by volumes, but I have no idea which media type I should use to get all volumes. Current implementation in Settings app uses 'picture' (I believe it's for no reason). I was wondering is it possible that different media type will get different number of volumes?

Comment 10

[Dave Hylands [:dhylands]]

Hi Evelyn,

(In reply to Evelyn Hung [:evelyn] from comment #9)
> Hi Dave, 
> I need to provide a media type when acquiring deviceStorage objects, like:
> var storages = navigator.getDeviceStorages(storageType);
> However, per spec design, the settings are listed by volumes, but I have no
> idea which media type I should use to get all volumes. Current
> implementation in Settings app uses 'picture' (I believe it's for no
> reason). I was wondering is it possible that different media type will get
> different number of volumes?

pictures, music, video, and sdcard will all return the same volumes. apps will only ever return one (different) volume from the above.

The used space and files returned by pictures, music, video and sdcard will be different, but the volumes will be the same.

Comment 11

[Evelyn Hung]

(In reply to Dave Hylands [:dhylands] from comment #10)
> The used space and files returned by pictures, music, video and sdcard will
> be different, but the volumes will be the same.
Thanks for the quick response. :)
One more question, how about free space? I guess there is only one free space value per volume and it's not related to media type? i.e. I can pass any media type to get the free space of a volume?

Comment 12

[Dave Hylands [:dhylands]]

(In reply to Evelyn Hung [:evelyn] from comment #11)
> (In reply to Dave Hylands [:dhylands] from comment #10)
> > The used space and files returned by pictures, music, video and sdcard will
> > be different, but the volumes will be the same.
> Thanks for the quick response. :)
> One more question, how about free space? I guess there is only one free
> space value per volume and it's not related to media type? i.e. I can pass
> any media type to get the free space of a volume?

Right - for a given volume, the free space returned from pictures, video, music and sdcard should all be the same.

Comment 13

[Evelyn Hung]

(In reply to Dave Hylands [:dhylands] from comment #12)
> (In reply to Evelyn Hung [:evelyn] from comment #11)
> > (In reply to Dave Hylands [:dhylands] from comment #10)
> > > The used space and files returned by pictures, music, video and sdcard will
> > > be different, but the volumes will be the same.
> > Thanks for the quick response. :)
> > One more question, how about free space? I guess there is only one free
> > space value per volume and it's not related to media type? i.e. I can pass
> > any media type to get the free space of a volume?
> 
> Right - for a given volume, the free space returned from pictures, video,
> music and sdcard should all be the same.
Got it, thank you.
To be honest, the API design is not intuitive to me. I will file another bug for discussion if it's really an issue. :)

Comment 14

[Kai-Chih Hu [:khu]]

Talked with Evelyn and so far, we have confidence that this bug can be resolved before May 3rd. Thanks!

Comment 15

[Evelyn Hung]

Attachment: point to https://github.com/mozilla-b2g/gaia/pull/9546

It's a 1000-line huge patch, I will describe it later.

Comment 16

[Arthur Chen [:arthurcc][inactive after 6/30]]

Comment on attachment 745261 [details]
point to https://github.com/mozilla-b2g/gaia/pull/9546

Thank you for the patch, Evelyn! The major issue I've seen is the selector of default location. But I believe you've started working on it. Other minor issues please refer to github.

Comment 17

[Evelyn Hung]

Comment on attachment 745261 [details]
point to https://github.com/mozilla-b2g/gaia/pull/9546

comment addressed. Please take a look, thanks!

Comment 18

[Evelyn Hung]

Hi Dave, two question:
1. On leo device, no matter I inserted an external sd card or not, the navigator.getDeviceStorages will returns two volumes and both are available (storage.available() will be 'available'), and I will get the same usedSpace() value.

2. I can't separately enable either one volume's ums via settings, is 'ums.volume.NAME.enabled' implemented?

Comment 19

[Dave Hylands [:dhylands]]

1. That's expected until bug 858416 lands
2. The implementation for that will come with bug 852768

I'm very close to landing bug 858416, and bug 852768 should follow shortly afterwards

Comment 20

[Evelyn Hung]

Okay, then I will land UI part first if Arthur r+.

Comment 21

[Arthur Chen [:arthurcc][inactive after 6/30]]

Comment on attachment 745261 [details]
point to https://github.com/mozilla-b2g/gaia/pull/9546

Please handle the UI in the root panel when disabling UMS per our discussion.

Comment 22

[Evelyn Hung]

Comment on attachment 745261 [details]
point to https://github.com/mozilla-b2g/gaia/pull/9546

comment addressed. Thanks!

Comment 23

[Arthur Chen [:arthurcc][inactive after 6/30]]

Comment on attachment 745261 [details]
point to https://github.com/mozilla-b2g/gaia/pull/9546

We are almost there. Please fix a few nit picks and squash commits into one before landing the patch. r=me. Thanks!

Comment 24

[Evelyn Hung]

gaia-master
https://github.com/mozilla-b2g/gaia/commit/fef913839cc45039116a066784cc9c1d4a4e581b

v1-train
https://github.com/mozilla-b2g/gaia/commit/8907ac1953a770ef90c3d2a8719c8a8f5acc2bc4

Comment 25

[Darren]

Looks like there is a test case that more or less, covers the issue reported in this bug. Let me know if you agree, or if you think a more specific test case would be required.

https://moztrap.mozilla.org/manage/cases/?filter-id=3887

Comment 26

[Tony Chung [:tchung]]

(In reply to Darren from comment #25)
> Looks like there is a test case that more or less, covers the issue reported
> in this bug. Let me know if you agree, or if you think a more specific test
> case would be required.
> 
> https://moztrap.mozilla.org/manage/cases/?filter-id=3887

Deferring your question to Jason.  He owns system front end QA,and its possible the test are covered in bug 838038 already.  but will let him reply.

Comment 27

[Jason Smith [:jsmith]]

(In reply to Tony Chung [:tchung] from comment #26)
> (In reply to Darren from comment #25)
> > Looks like there is a test case that more or less, covers the issue reported
> > in this bug. Let me know if you agree, or if you think a more specific test
> > case would be required.
> > 
> > https://moztrap.mozilla.org/manage/cases/?filter-id=3887
> 
> Deferring your question to Jason.  He owns system front end QA,and its
> possible the test are covered in bug 838038 already.  but will let him reply.

I didn't work on this feature, so I don't know the answer to this question. I think John might have, but I'm not 100% sure.