Closed Bug 1049704 Opened 8 years ago Closed 6 years ago

Add a "type" parameter to the Simulator configuration so it supports for multiple Gaia device types

Categories

(Firefox OS Graveyard :: Simulator, defect)

defect
Not set
normal

Tracking

(firefox45 fixed)

RESOLVED FIXED
FxOS-S11 (13Nov)
Tracking Status
firefox45 --- fixed

People

(Reporter: jryans, Assigned: janx)

References

Details

(Whiteboard: [ft:conndevices])

Attachments

(1 file, 1 obsolete file)

As noted in part of bug 1020668, Gaia profiles can be built for different device types, currently:

* phone
* tablet
* tv

To simulate real devices accurately, we'd need to get these different modes into the simulator.

However, would we really have to ship 3 independent profiles (~50 MB each), or can we create some kind of uber-profile that contains all of them without lots of duplication?

At the moment, Gaia appears to use the device type to:

* Include a CSS file specific to type
* Set the list of default apps
* Change a few settings

There is a related problem of GAIA_DEV_PIXELS_PER_PX, which swaps out the images most apps uses for higher quality versions as needed.  I would guess this actually contributes to the largest file size difference.  However, do we even care about this for the simulator?  Maybe there is one value here that makes sense for the simulator?
As mentioned in bug 1201451 comment 8, we could add the "Simulator type" notion to its configuration, and ship dedicated simulator add-ons for some types (e.g. a TV Simulator called "fxos_3_0_tv_simulator@mozilla.org").

Ryan, would that work for you? Or should we do something smarter like packaging the different Gaia profile types separately from the simulator Gecko? Or create an "uber-profile" as you suggested?
Flags: needinfo?(jryans)
Blocks: 1201456, 1201451
Assignee: nobody → janx
Blocks: 1212352
(In reply to Jan Keromnes [:janx] from comment #1)
> As mentioned in bug 1201451 comment 8, we could add the "Simulator type"
> notion to its configuration, and ship dedicated simulator add-ons for some
> types (e.g. a TV Simulator called "fxos_3_0_tv_simulator@mozilla.org").
> 
> Ryan, would that work for you? Or should we do something smarter like
> packaging the different Gaia profile types separately from the simulator
> Gecko? Or create an "uber-profile" as you suggested?

I think your plan in bug 1201451 comment 8 sounds fine to me.

It would still be nice to not duplicate resources and download time for the different kinds of Gaia profiles, but it seems unlikely we would pursue it any time soon because (a) it sounds complex and hard to do correctly and (b) I don't think DevTools has the time to do it now.  So, it would really be up to the Gaia team, and they seem exceptionally busy with "real" feature work. :)

So, the most practical thing sounds like your plan.  Maybe we WONTFIX this then?
Flags: needinfo?(jryans)
(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #2)
> I think your plan in bug 1201451 comment 8 sounds fine to me.

Great!

> It would still be nice to not duplicate resources and download time for the
> different kinds of Gaia profiles, but it seems unlikely we would pursue it
> any time soon because (a) it sounds complex and hard to do correctly and (b)
> I don't think DevTools has the time to do it now.  So, it would really be up
> to the Gaia team, and they seem exceptionally busy with "real" feature work.
> :)

Right, I agree this would be a lot better, but it's probably too involved and out-of-scope for now.

> So, the most practical thing sounds like your plan.  Maybe we WONTFIX this
> then?

Actually, I was planning to hijack this bug to add a "type" parameter to the Simulator configuration. Instead, should I WONTFIX this and create a separate bug for the Simulator types?
Flags: needinfo?(jryans)
(In reply to Jan Keromnes [:janx] from comment #3)
> Actually, I was planning to hijack this bug to add a "type" parameter to the
> Simulator configuration. Instead, should I WONTFIX this and create a
> separate bug for the Simulator types?

Ah, makes sense, I wasn't sure if Evelyn had already filed another for this work.  If not, then it makes sense to use this one.
Flags: needinfo?(jryans)
(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #4)
> (In reply to Jan Keromnes [:janx] from comment #3)
> > Actually, I was planning to hijack this bug to add a "type" parameter to the
> > Simulator configuration. Instead, should I WONTFIX this and create a
> > separate bug for the Simulator types?
> 
> Ah, makes sense, I wasn't sure if Evelyn had already filed another for this
> work.  If not, then it makes sense to use this one.

No I didn't, let's use this one. Thank you so much, Ryan and Jan. :)
Summary: Simulator support for multiple Gaia device types → Add a "type" parameter to the Simulator configuration so it supports for multiple Gaia device types
Hi :janx, are you still working on this bug? I guess it's a blocker of my bug 1201451 so I'd like to make sure we are synced. Thanks!
Flags: needinfo?(janx)
Whiteboard: [ft:conndevices]
Hi Evelyn, yes I'm still working on this bug. I have a local, work-in-progress patch that adds a Simulator type, and I'm hoping to finish it soon.
Flags: needinfo?(janx)
Great! Hope we could have it fixed soon. :)
Hi Jan, 
May I have your updated status for this bug?
Could you kindly provide an ETA?

Thank you very much! :)
Flags: needinfo?(janx)
Hi Josh, I give this bug a high priority, and have started implementing it locally.

I was hoping to complete this work last week, but unfortunately I got sidetracked with higher priority things (Firefox OS blockers and Push Service Worker / about:debugging bugs). If I don't get sidetracked again, I'm hoping to get to this bug tomorrow or Wednesday.

Sorry for the multiple delays, I'm currently very busy.
Flags: needinfo?(janx) → needinfo?(jocheng)
https://treeherder.mozilla.org/#/jobs?repo=try&revision=ce4caada8bd4

Ryan, this patch introduces a `simulator.type` parameter, which defaults to "phone".

That type controls the default simulation options used:
- at the creation of a Simulator (but this doesn't existing parameters in the `options` constructor parameter)
- when a Simulator is "reset to defaults"

Most/all simulators will have the default type "phone", but installing a simulator addon with an ID like "fxos_2_5_tv_simulator@mozilla.org" will cause its Simulator object to be of type "television".

A couple drive-bys: [].map.call() changed to Array.map(), str.contains() changed to str.includes().

It's not possible to change the simulator type from the UI yet, but maybe in the future we'll want to enable that (e.g. if you switch it from a phone simulator addon to a TV simulator addon, or if you have a custom build and switch to TV simulation parameters). Anyway, I'm speculating, let's see about that if we ever file a bug for it.
Attachment #8682517 - Flags: review?(jryans)
Comment on attachment 8682517 [details] [diff] [review]
Add a "type" parameter to Simulator configurations in WebIDE.

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

These changes look good to me, thanks!

We should ensure that the real XPI builds make all the necessary metadata changes, like add-on ID of course, but also the add-on name should include "TV" etc.
Attachment #8682517 - Flags: review?(jryans) → review+
Thanks Ryan! The 4 new test add-ons have the right XPI name, add-on ID and their add-on name includes "TV":

> <em:id>fxos_3_0_tv_simulator@mozilla.org</em:id>
> <em:name>Firefox OS 3.0 TV Simulator</em:name>
> <em:description>a Firefox OS 3.0 TV simulator</em:description>

(from fxos_3_0_tv_simulator-linux.xpi / install.rdf)
Flags: needinfo?(jocheng)
Silly `s/contains/includes/g` messed up `element.classList.contains()`.

Fixed: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f99e1deca3f9
Attachment #8683074 - Flags: review+
Attachment #8682517 - Attachment is obsolete: true
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/a740558de8a6
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → FxOS-S11 (13Nov)
You need to log in before you can comment on or make changes to this bug.