Last Comment Bug 765444 - Update device storage paths for Gonk
: Update device storage paths for Gonk
Product: Core
Classification: Components
Component: DOM: Device Interfaces (show other bugs)
: Trunk
: All Gonk (Firefox OS)
: -- normal (vote)
: mozilla16
Assigned To: Doug Turner (:dougt)
: Andrew Overholt [:overholt]
Depends on:
Blocks: 765498
  Show dependency treegraph
Reported: 2012-06-15 20:21 PDT by Doug Turner (:dougt)
Modified: 2012-06-18 07:28 PDT (History)
4 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

patch v.1 (2.46 KB, patch)
2012-06-15 20:24 PDT, Doug Turner (:dougt)
fabrice: review+
Details | Diff | Splinter Review

Description Doug Turner (:dougt) 2012-06-15 20:21:19 PDT

Comment 1 Doug Turner (:dougt) 2012-06-15 20:24:36 PDT
Created attachment 633763 [details] [diff] [review]
patch v.1

This code is GAIA specific.  Asking fabrice to review.

This was tested on a Nexus S running B2G from Jun 15.  Verified that Gallery worked as expected.  This file paths mapped what was on /sdcard/
Comment 2 David Flanagan [:djf] 2012-06-15 21:05:58 PDT
Did you just change lowercase /sdcard/{pictures,movies,videos} to uppercase /sdcard/{Pictures,Movies,Videos}?  Or has it always been that way?  I like initial caps better. (But in a gaia code review just made the author of the Music app change from /sdcard/Music to /sdcard/music...)

Will the test for existance of /sdcard work in the case where /sdcard is a symbolic link to /mnt/sdcard?  I.e if /sdcard points to /mnt/sdcard, but nothing is mounted there, does the test do what you want?
Comment 3 David Flanagan [:djf] 2012-06-15 21:17:12 PDT

If you can, please test this on an otoro as part of your review.  As similar patch from Doug was not working for me on that device. But perhaps something was misconfigured on mine.  

Also, note that the changing paths in this patch will require changes to the install-media-samples target in the Gaia Makefile
Comment 4 [:fabrice] Fabrice Desré 2012-06-15 21:43:24 PDT
Comment on attachment 633763 [details] [diff] [review]
patch v.1

Review of attachment 633763 [details] [diff] [review]:

::: dom/devicestorage/nsDeviceStorage.cpp
@@ +138,5 @@
>    nsCOMPtr<nsIProperties> dirService = do_GetService(NS_DIRECTORY_SERVICE_CONTRACTID);
>    NS_ASSERTION(dirService, "Must have directory service");
> +  // check that /sdcard exists, if it doesn't go no further.

On an otoro without sdcard, /sdcard exists because it is just a symlink to /mnt/sdcard

So this test will pass, but any subsequent write will fail. Can we be smarter here?
Comment 5 Doug Turner (:dougt) 2012-06-15 23:48:25 PDT
> Can we be smarter here?

Suggest something.
Comment 6 Doug Turner (:dougt) 2012-06-15 23:51:23 PDT
there is a gonk volume manager - maybe there could be something I could query off that?
Comment 7 [:fabrice] Fabrice Desré 2012-06-16 09:35:49 PDT
(In reply to Doug Turner (:dougt) from comment #6)
> there is a gonk volume manager - maybe there could be something I could
> query off that?

Dave, any hint on how to do that?
Comment 8 Dave Hylands [:dhylands] 2012-06-16 09:58:06 PDT
I just finished coding a patch (awaiting some review). Its attached to bug 764228

Basically, you would #include "VolumeManager.h" (Hmm. I may need to export some headers). Then you would RegisterVolumeEventObserver and pass in the string 'sdcard' and an observer object.

The Notify method of the observer will be called immediately and each time that the state of the named volume changes.

You can look at Volume.h to see the various states.

The MOUNTED state means that the volume has been mounted on the phone. You can use the Volume class method MountPoint to get the filesystem location where the volume is mounted. Ideally, you should query the mount point each time the volume enters the MOUNTED state, since technically the mount point could change when the volume is not mounted.

In any state other than MOUNTED, the phone is unable to view the contents of the volume. SHARED means that volume is being shared with the PC. I've never seen the state SHARED_MOUNTED, I think it was going to be used for MTP, but that wound up getting implemented differently.

The SHARED state means that the volume is being shared with the PC.
Comment 9 Doug Turner (:dougt) 2012-06-16 10:01:18 PDT
sounds like a perfect thing for a follow up bug!  Filed bug 765498.  Fabrice, would you review as-is?
Comment 10 Doug Turner (:dougt) 2012-06-16 18:50:13 PDT

I'll look at bug 765498 this coming week.
Comment 11 Ed Morley [:emorley] 2012-06-18 07:28:51 PDT

Note You need to log in before you can comment on or make changes to this bug.