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

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Simulator, defect)

Description

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

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?

Comment 1

[Jan Keromnes [:janx]]

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 "[e-mail]").

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?

Comment 2

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

(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 "[e-mail]").
> 
> 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?

Comment 3

[Jan Keromnes [:janx]]

(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?

Comment 4

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

(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.

Comment 5

[Evelyn Hung]

(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. :)

Comment 6

[Evelyn Hung]

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!

Comment 7

[Jan Keromnes [:janx]]

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.

Comment 8

[Evelyn Hung]

Great! Hope we could have it fixed soon. :)

Comment 9

[Josh Cheng [:josh]]

Hi Jan, 
May I have your updated status for this bug?
Could you kindly provide an ETA?

Thank you very much! :)

Comment 10

[Jan Keromnes [: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.

Comment 11

[Jan Keromnes [:janx]]

Attachment: Add a "type" parameter to Simulator configurations in WebIDE.

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 "[e-mail]" 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.

Comment 12

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

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.

Comment 13

[Jan Keromnes [:janx]]

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>[e-mail]</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)

Comment 14

[Jan Keromnes [:janx]]

Attachment: Add a "type" parameter to Simulator configurations in WebIDE.

Silly `s/contains/includes/g` messed up `element.classList.contains()`.

Fixed: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f99e1deca3f9

Comment 15

[Pulsebot]

https://hg.mozilla.org/integration/b2g-inbound/rev/a740558de8a6

Comment 16

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/a740558de8a6