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

RESOLVED FIXED in Firefox 45

Status

Firefox OS
Simulator
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: jryans, Assigned: janx)

Tracking

unspecified
FxOS-S11 (13Nov)
Dependency tree / graph

Firefox Tracking Flags

(firefox45 fixed)

Details

(Whiteboard: [ft:conndevices])

Attachments

(1 attachment, 1 obsolete attachment)

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

Updated

3 years ago
Depends on: 1090949
(Assignee)

Comment 1

2 years ago
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)
(Assignee)

Updated

2 years ago
Blocks: 1201456, 1201451
(Assignee)

Updated

2 years ago
Assignee: nobody → janx

Updated

2 years ago
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)
(Assignee)

Comment 3

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

Updated

2 years ago
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)

Updated

2 years ago
Whiteboard: [ft:conndevices]
(Assignee)

Comment 7

2 years ago
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. :)

Comment 9

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

Thank you very much! :)
Flags: needinfo?(janx)
(Assignee)

Comment 10

2 years ago
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)
(Assignee)

Comment 11

2 years ago
Created attachment 8682517 [details] [diff] [review]
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 "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+
(Assignee)

Comment 13

2 years ago
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)
(Assignee)

Comment 14

2 years ago
Created attachment 8683074 [details] [diff] [review]
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
Attachment #8683074 - Flags: review+
(Assignee)

Updated

2 years ago
Attachment #8682517 - Attachment is obsolete: true
(Assignee)

Updated

2 years ago
Keywords: checkin-needed

Comment 15

2 years ago
https://hg.mozilla.org/integration/b2g-inbound/rev/a740558de8a6
Keywords: checkin-needed

Comment 16

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/a740558de8a6
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox45: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → FxOS-S11 (13Nov)
You need to log in before you can comment on or make changes to this bug.