Closed Bug 996619 Opened 6 years ago Closed 5 years ago

SD card emulation with the simulator

Categories

(Firefox OS Graveyard :: Simulator, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
2.2 S2 (19dec)

People

(Reporter: fharper, Assigned: ochameau)

References

Details

(Keywords: dev-doc-complete)

Attachments

(1 file, 5 obsolete files)

It would be nice to add emulation of an SD card with the simulator. It would be useful for developers who want to tests their application using the DeviceStorage with an SD card.

Maybe the opportunity to simulate this by selecting a folder on their computer.
This does seem like a good idea.  Today the simulator maps the host's Pictures, Music, etc. folders into the Camera / Music, etc. apps, but IIRC there's no such thing yet for general storage.
Component: Developer Tools: App Manager → Simulator
Product: Firefox → Firefox OS
This was in the older versions of the Firefox OS Simulator, but must have been broken or removed when the App Manager was introduced.

Anyway, the lack of support for the full DeviceStorage API is what prevents me from using the simulator to develop the app Firetext (https://marketplace.firefox.com/app/firetext).
So, it turns out there is a version of this today, but it just maps to a hard to find directory at the moment.

The getDeviceStorage("sdcard") will map to
Keywords: dev-doc-needed
... the folder "fake-sdcard" inside the *simulator's* Gaia profile, which is inside your own Firefox profile.

For example:

FIREFOX_PROFILE_DIR/extensions/fxos_1_4_simulator@mozilla.org/profile/fake-sdcard

This directory won't exist unless you've already written something to the SD card.

There's also a pref "device.storage.overrideRootDir" which we could use in the Simulator to set this to a more normal location.
Depends on: 959591
Also, with overrideRootDir, the pictures, music, videos, and sdcard types all use the same tree (which is what happens on the device).

Without overrideRootDir, on desktop each of the types winds up being in a separate tree.
What about create this directory by default, and mention somewhere in documentation?
Maybe also a symlinked folder for all the versions of the simulator?
No longer blocks: 1059897
Duplicate of this bug: 1059897
Assignee: nobody → poirot.alex
Attached patch patch v1 (obsolete) — Splinter Review
WIP patch, support --sd-card argument on the command line,
or default to fake-sdcard folder within b2g profile.
It should allow the simulator addon to customize the storage folder
to a shared directory between all addons, may be within firefox profile?
Or user defined one? Or integrate within device profiles?

This patch also introduce a "SD" button that opens the directory.
(I think we need it somewhere, it may be too visible here,
but it ease discovering this feature!)
Attached patch patch v2 (obsolete) — Splinter Review
Attachment #8521524 - Attachment is obsolete: true
Jan, What do you think about handling storage path in profiles?
I wish we could share the same storage path between all simulators.
In order to do that, WebIDE should create a unique folder somewhere and pass it to b2g-bin as a command line argument. Then expose it to the user so that he can put medias in it and inspect it with his OS file explorer.
So I was wondering if that could be part of profiles, as it seems quite similar to custom screen size support??
Flags: needinfo?(janx)
Fake-SD card path definitely sounds like a good "simulator option" (item that goes into a profile), similar to screen size, so partly blocking on bug 1090949:

- Once we're able to pass "simulator options" as start arguments to a simulator / custom runtime, we'll map the profile's SD path to the --sd-card command line.

- To address "all simulators use the same SD folder [by default]", I suggest that WebIDE creates one "fake-sdcard" folder in the firefox profile, which will be the default SD path for all new simulators.
Flags: needinfo?(janx)
And this comment should probably be updated: http://dxr.mozilla.org/mozilla-central/source/dom/devicestorage/nsDeviceStorage.cpp#917

I don't think we want to make the simulator switch to computer SD card when there is one.
Personally, I think that we shouldn't use the "fake-sd" path.

Device storage on desktop has 2 modes of operation:

1 - Use the users directories for photos, music, and videos. This is, unfortunately, the default mode, and the one where the fake-sd card directory comes from the profile directory.

2 - Use the device.storage.overrideRootDir preference. This puts video, music, pictures and sdcard all under the same directory tree, and can be from an arbitrary location. If you want something that is relative to the profile, then I would allow something in the preference (like %PROFILE%) to mean use the profile directory.

Approach 2 is what happens on a real device. It allows you save a file to sdcard (say an email attachment) and if it happens to be a photo, then you can see it in the gallery.

If you go with approach 1, then files stored in fake-sdcard will not be visible to the gallery app, which will only look in the pictures storage area which is ~/Pictures or something.

Using approach 2 will make the apps running on the simulator behave almost identically to the apps running on a real phone. Going with approach 1 means that any app which saves a file to figure out which storage area to use, and doesn't allow for mixed media (like having album art in your music collection).
Dave, even if me mention "Fake sdcard" here and there, in this patch we always end up using device.storage.overrideRootDir. This is how we are going to explain this features to app developers. Here is the SD card folder where all storage APIs map to.
Attached patch patch v3 (obsolete) — Splinter Review
To restate what this patch does:
It starts setting "device.storage.overrideRootDir" pref to some value.
By default, it sets it to %PROFILE%/sdcard.
Otherwise, it also accepts a --sd-card command line option to define it to a custom value.

This patch also introduce a "SD" button within b2g desktop UI,
I'm not sure it is really worth, it may be better to keep it in WebIDE interface.

We are planning to pass --sd-card argument from webIDE,
in order to always use the same storage path for all simulator versions!
Attachment #8525297 - Attachment is obsolete: true
Attachment #8529140 - Flags: review?(fabrice)
(In reply to Alexandre Poirot [:ochameau] from comment #15)
> Dave, even if me mention "Fake sdcard" here and there, in this patch we
> always end up using device.storage.overrideRootDir. This is how we are going
> to explain this features to app developers. Here is the SD card folder where
> all storage APIs map to.

Excellent. Thanks for clearing that up (and my apologies for stirring anything up). I didn't read the patch, I was just skimming comments.
Comment on attachment 8529140 [details] [diff] [review]
patch v3

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

::: b2g/chrome/content/desktop.js
@@ +57,5 @@
> +  let sdButton = document.getElementById('sd-button');
> +  if (!sdButton) {
> +    // The toolbar only exists in b2g desktop build with
> +    // FXOS_SIMULATOR turned on.
> +    return;

so we won't process the --sd-card argument on b2g desktop without FXOS_SIMULATOR turned on? Is that what we want?

@@ +86,5 @@
> +  }
> +  dump("Set storage path to: " + directory.path + "\n");
> +
> +  // This is the magic, where we override the default location for the storages
> +  Services.prefs.setCharPref('device.storage.overrideRootDir', directory.path);

So we don't set the "sd-card" path, but the "device-storage" path. Can we align the command line parameter name?
Attachment #8529140 - Flags: review?(fabrice)
Attached patch patch v4 (obsolete) — Splinter Review
Removed the SD button.
Renamed command line to --storage-path.

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=89a48eca4cc8
Attachment #8529140 - Attachment is obsolete: true
Attachment #8532652 - Flags: review?(fabrice)
Comment on attachment 8532652 [details] [diff] [review]
patch v4

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

r=me if you triple check the octal notation.

::: b2g/chrome/content/desktop.js
@@ +55,5 @@
>  
> +function setupStorage() {
> +  let directory = null;
> +
> +  // Get the --storage-path argument from the command line

nit: full stop at the end of comments.

@@ +73,5 @@
> +  if (!directory) {
> +    directory = Services.dirsvc.get('ProfD', Ci.nsIFile);
> +    directory.append('storage');
> +    if (!directory.exists()) {
> +      directory.create(Ci.nsIFile.DIRECTORY_TYPE, 0o755);

0o755? not o0755? or parseInt(octalStr, 8)
Attachment #8532652 - Flags: review?(fabrice) → review+
Attached patch patch v5 (obsolete) — Splinter Review
(In reply to Fabrice Desré [:fabrice] from comment #20)
> @@ +73,5 @@
> > +  if (!directory) {
> > +    directory = Services.dirsvc.get('ProfD', Ci.nsIFile);
> > +    directory.append('storage');
> > +    if (!directory.exists()) {
> > +      directory.create(Ci.nsIFile.DIRECTORY_TYPE, 0o755);
> 
> 0o755? not o0755? or parseInt(octalStr, 8)

0o755 === parseInt("755", 8), but I switched to parseInt to make it clear for old and new guards ;)

drwxr-xr-x 3 alex alex   4096 Dec 11 22:32 storage
Attachment #8532652 - Attachment is obsolete: true
Attachment #8535205 - Flags: review+
Blocks: 1020668
Keywords: checkin-needed
No longer blocks: 1020668
seems this patch don't apply cleanly:

patching file b2g/chrome/content/desktop.js
Hunk #1 FAILED at 47
1 out of 2 hunks FAILED -- saving rejects to file b2g/chrome/content/desktop.js.rej
patch failed, unable to continue (try -v)
patch failed, rejects left in working dir
errors during apply, please fix and refresh Bug-996619---Implement-SD-card-support-in-simulato.patch

could you take a look thanks!
Flags: needinfo?(poirot.alex)
Keywords: checkin-needed
Attached patch patch v6Splinter Review
Rebased against last m-c.
Attachment #8535205 - Attachment is obsolete: true
Attachment #8536477 - Flags: review+
Flags: needinfo?(poirot.alex)
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/e2d2d5bafe94
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
I've added a section here: https://developer.mozilla.org/en-US/docs/Tools/Firefox_OS_Simulator#SD_card_emulation - does that cover it? I guess there's another piece here to configure the location from WebIDE, that hasn't landed yet?
Flags: needinfo?(jryans)
Uhm the fakesdcard is not a new feature of firefox os simulator 2.2 because this exist from 1.3.
Thanks Will, yes, we hope to share the same folder between all simulators from WebIDE.
fake-sdcard directory used to be supported before this landing, *but* you had to create it in the profile folder, or it won't work. Also this is slightly different from just create this folder in profile directory. See comment 5 and 14. It ensures mapping everything to a single directory.
OK, I've updated the page with (hopefully) correct info on what exactly is new in 2.2. Please let me know if this is all right.
Flags: needinfo?(jryans) → needinfo?(poirot.alex)
Looks great, thanks Will!
Flags: needinfo?(poirot.alex)
The choice of 'storage' subdir here is a bit unfortunate as that is the directory where we store all our IndexedDB stuff (also all future pluggable storage engines)...
(In reply to Ben Turner [:bent] (use the needinfo flag!) from comment #31)
> The choice of 'storage' subdir here is a bit unfortunate as that is the
> directory where we store all our IndexedDB stuff (also all future pluggable
> storage engines)...

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