1038606 - implement a nsITelephonyService for Simulator

Status: RESOLVED WONTFIX

(Firefox OS Graveyard :: Simulator, defect)

Description

[Vicamo Yang [:vicamo][:vyang]]

+++ This bug was initially created as a clone of Bug #989926 +++

Please see bug 989926 comment 40 for detailed thoughts.

Comment 1

[Vicamo Yang [:vicamo][:vyang]]

Call holding and resume are working. Conference should be working I guess. Working on CDMA.

Again, working branch is in https://github.com/vicamo/b2g_mozilla-central/tree/experimental/simulator

To have a try, build both b2g-desktop and firefox nightly from the branch HEAD.

  $ cd ${B2G_DESKTOP}
  $ git clone https://git.mozilla.org/releases/gaia.git gaia
  $ ln -s b2g/config/mozconfigs/macosx64_gecko/nightly .mozconfig
  $ nice -n 20 ./mach build

  $ cd ${FIREFOX_NIGHTLY}
  $ ln -s browser/config/mozconfigs/macosx64/debug .mozconfig
  # If you have MacOSX10.5 SDK, use browser/config/mozconfigs/macosx-universal/nightly instead
  $ nice -n 20 ./mach build
  $ ./mach run

Use the just built b2g-desktop as the source of gecko runtime as shown in https://developer.mozilla.org/en-US/Firefox_OS/Running_custom_builds_in_the_App_Manager .

Comment 2

[Vicamo Yang [:vicamo][:vyang]]

Attachment: ScreenShot-AppManager-Simulator.png

Comment 3

[Vicamo Yang [:vicamo][:vyang]]

From bug 989926 comment 51, I should also separate patches by the frontend/backend. Also list necessary/handy elements to be shown.

Comment 4

[Paul Rouget [:paul]]

Vicamo, as mentioned in bug 989926, the directory `browser/devtools/app-manager/` will disappear soon. You want to build a WebIDE panel (or a deck) to control the RIL actor.

If you want, you can take care of the actor, and I will make sure someone in my team builds the UI.

Comment 5

[Vicamo Yang [:vicamo][:vyang]]

(In reply to Paul Rouget [:paul] (slow to respond. Ping me on IRC) from comment #4)
> Vicamo, as mentioned in bug 989926, the directory
> `browser/devtools/app-manager/` will disappear soon. You want to build a
> WebIDE panel (or a deck) to control the RIL actor.
> 
> If you want, you can take care of the actor, and I will make sure someone in
> my team builds the UI.

Thank you for the great support. I had a quick scan in WebIDE UI bits. Really not as simple as what AppManager has.

I'm having a review of the call management in the back-end side. Found a few more bugs in B2G emulator. After this, I'll have a detailed description about the elements in need, and action flows.

Comment 6

[Paul Rouget [:paul]]

Attachment: WebIDE UI Placeholder

Here is a patch that provides a UI placeholder for the telephony tools.

Just add your code to telephony.(xhtml|js).

Let me know if that works for you.

Comment 7

[Vicamo Yang [:vicamo][:vyang]]

Attachment: bug-1038606-1-simulator-voice-calls.patch

This patch sets _PLATFORM_WITH_RIL when building with FXOS_SIMULATOR turned on. However, FXOS_SIMULATOR itself is an environment variable, therefore I'm not really sure if this is a good idea.  And what's its relation to MOZ_MULET? Should I use MOZ_MULET instead?

In the following patches I created dom/telephony/simulator and dom/system/simulator. Shall I use dom/telephony/mulet, dom/system/mulet instead?

Comment 8

[Vicamo Yang [:vicamo][:vyang]]

Attachment: part 2/5: FxOS simulator icc/mobileconnection/cellbroadcast/voicemail backends

In this patch I copied dom/system/gonk/{RILContentHelper.js,RadioInterfaceLayer.manifest} to dom/system/simulator.  We may still need a fake RadioInterfaceLayer to simulate radio on/off though.

Most bits relying on RadioInterfaceLayer has been removed.  ICC is always the one we have on emulator, so applies to mobileconnection.

Comment 9

[Vicamo Yang [:vicamo][:vyang]]

(In reply to Vicamo Yang [:vicamo][:vyang] from comment #8)
> Created attachment 8456951 [details] [diff] [review]
> part 2/5: FxOS simulator icc/mobileconnection/cellbroadcast/voicemail
> backends

When MOZ_B2G_RIL is enabled on FxOS simulator build, we need this patch so that we can boot to homescreen without being stopped by various exceptions.

Comment 10

[Vicamo Yang [:vicamo][:vyang]]

Attachment: part 3/5: implement TelephonyService for FxOS simulator

After discussion with Hsin-Yi this afternoon, I removed those incomplete CDMA lines. Conference/hold/resume for GSM calls are supported at the time being.

Comment 11

[Vicamo Yang [:vicamo][:vyang]]

Attachment: part 4/5: Add devtools RDP support for telephony service

In this patch I try to hook up that nsISimulatorTelephonyService and RDP.

So far I defined a dictionary type "voicecall" to represent each voice call, two events "voicecall-state-changed" and "voicecall-error" to notify client side voice call state changes. Would you give me some opinions?

Comment 12

[Vicamo Yang [:vicamo][:vyang]]

Attachment: [Deprecated] AppManager UI

This patch add a "Voice Calls" panel to AppManager. However that's going to be replaced by WebIDE so I'm not going to request reviews for it. Just for further reference and a handy tool before we really have something landed in WebIDE.

Comment 13

[Paul Rouget [:paul]]

Comment on attachment 8456964 [details] [diff] [review]
part 4/5: Add devtools RDP support for telephony service

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

Can you add a comment at the top of the file explaining what RIL means and what this file does? If you're not familiar with B2G, this file is a bit cryptic.

::: toolkit/devtools/server/actors/ril.js
@@ +8,5 @@
> +
> +Cu.import("resource://gre/modules/XPCOMUtils.jsm");
> +
> +const events = require("sdk/event/core");
> +const {on, once, off, emit} = events;

I don't think any of these are used.

@@ +45,5 @@
> +  QueryInterface: XPCOMUtils.generateQI([Ci.nsITelephonyListener]),
> +
> +  _actor: null,
> +  _callsEnumerating: null,
> +  calls: null,

Maybe you want to make `.calls` a readonly getter.

Comment 14

[Vicamo Yang [:vicamo][:vyang]]

(In reply to Vicamo Yang [:vicamo][:vyang] from comment #1)
> Again, working branch is in
> https://github.com/vicamo/b2g_mozilla-central/tree/experimental/simulator

Renamed working branch to https://github.com/vicamo/b2g_mozilla-central/tree/bugzilla/1038606/simulator-telephony

Comment 15

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 8456949 [details] [diff] [review]
bug-1038606-1-simulator-voice-calls.patch

Please help clarify what should I do for a new simulator back-end.

We have MOZ_B2G, FXOS_SIMULATOR, MOZ_MULET. What's relationship between them and which is more appropriate for guarding simulator components?

Comment 16

[Paul Rouget [:paul]]

Alexandre will know.

Comment 17

[:gerard-majax]

(In reply to Vicamo Yang [:vicamo][:vyang] from comment #15)
> Comment on attachment 8456949 [details] [diff] [review]
> bug-1038606-1-simulator-voice-calls.patch
> 
> Please help clarify what should I do for a new simulator back-end.
> 
> We have MOZ_B2G, FXOS_SIMULATOR, MOZ_MULET. What's relationship between them
> and which is more appropriate for guarding simulator components?

We tend to use MOZ_MULET for stuff that is really problematic between B2G Desktop and Browser, as far as I can remember. I think (but not 100% sure) we can define this ordering: MOZ_B2G < MOZ_MULET < FXOS_SIMULATOR.

Comment 18

[:gerard-majax]

Comment on attachment 8456949 [details] [diff] [review]
bug-1038606-1-simulator-voice-calls.patch

I would say that any flag that targets B2G Desktop would be fine :)

Comment 19

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Comment on attachment 8456955 [details] [diff] [review]
part 3/5: implement TelephonyService for FxOS simulator

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

Looks really good! I think we still need to invite build peers for package-manifest.in and moz.build.

Comment 20

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Comment on attachment 8456951 [details] [diff] [review]
part 2/5: FxOS simulator icc/mobileconnection/cellbroadcast/voicemail backends

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

As offline discussion, all cpmm-related code should be removed. Thank you!

::: dom/system/simulator/RILContentHelper.js
@@ +930,4 @@
>        return request;
>      }
>  
>      cpmm.sendAsyncMessage("RIL:GetCallForwardingOptions", {

There's no corresponding 'ppmm'' to handle this. We should clean the cpmm part.

Comment 21

[Vicamo Yang [:vicamo][:vyang]]

Attachment: part 1/4: Build MOZ_B2G_RIL on FxOS simullator : v2

We don't need ipc/unixsocket for b2g simulator. Actually, that doesn't compile on desktop.

Rebase only.

Comment 22

[Vicamo Yang [:vicamo][:vyang]]

Attachment: part 2/4: FxOS simulator icc/mobileconnection/cellbroadcast/voicemail backends : v2

Address comment 20. All cpmm/ppmm related parts are removed. Use dispatchFireRequestSuccess/Error instead.

Comment 23

[Vicamo Yang [:vicamo][:vyang]]

Attachment: part 3/4: implement TelephonyService for FxOS simulator : v2

1. Rebase
2. Document nsISimulatorTelephonyService even more.
3. handle empty string failcause

Comment 24

[Vicamo Yang [:vicamo][:vyang]]

Attachment: part 4/4: Add devtools RDP support for telephony service : v2

Address comment 13. Document the new ril actors module more. Note that 'emit' is referenced because we do emit two events "voicecall-state-changed" and "voicecall-error".

Comment 25

[Vicamo Yang [:vicamo][:vyang]]

Attachment: [Deprecated] AppManager UI : v2

Add 'voicecalls-store.js' to EXTRA_JS_MODULES after bug 1043957.

Comment 26

[Vicamo Yang [:vicamo][:vyang]]

Full try in https://tbpl.mozilla.org/?tree=Try&rev=1c21745d03e7

Comment 27

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Comment on attachment 8465991 [details] [diff] [review]
part 2/4: FxOS simulator icc/mobileconnection/cellbroadcast/voicemail backends : v2

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

Hi Vicamo,

Bug 843452 is review granted and should be landed in a day or two. Can you provide a patch rebased on that and with "mobileconnection" part addressed accordingly? Other than that, the whole structure looks good to me. Thank you, and sorry for the inconvenience.

Comment 28

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 8465994 [details] [diff] [review]
part 3/4: implement TelephonyService for FxOS simulator : v2

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

r=me on the build and xpcom glue.

Somebody else needs to look at the telephony service.

::: dom/telephony/simulator/TelephonyService.js
@@ +1,4 @@
> +/* -*- indent-tabs-mode: nil; js-indent-level: 2 -*- */
> +/* This Source Code Form is subject to the terms of the Mozilla Public
> + * License, v. 2.0. If a copy of the MPL was not distributed with this file,
> + * You can obtain one at http://mozilla.org/MPL/2.0/. */

I'm not qualified to review this file.

::: dom/telephony/simulator/nsISimulatorTelephonyService.idl
@@ +5,5 @@
> +
> +#include "nsITelephonyService.idl"
> +
> +[scriptable, uuid(08dc6bb6-cd78-433c-8530-7c0eb8783b7f)]
> +interface nsISimulatorTelephonyService : nsITelephonyService

Whoever is reviewing the implementation (hsinyi?) should also review this interface.

Comment 29

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(In reply to Kyle Huey [:khuey] ([e-mail]) from comment #28)
> Comment on attachment 8465994 [details] [diff] [review]
> part 3/4: implement TelephonyService for FxOS simulator : v2
> 
> Review of attachment 8465994 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> r=me on the build and xpcom glue.
> 
> Somebody else needs to look at the telephony service.
> 
> ::: dom/telephony/simulator/TelephonyService.js
> @@ +1,4 @@
> > +/* -*- indent-tabs-mode: nil; js-indent-level: 2 -*- */
> > +/* This Source Code Form is subject to the terms of the Mozilla Public
> > + * License, v. 2.0. If a copy of the MPL was not distributed with this file,
> > + * You can obtain one at http://mozilla.org/MPL/2.0/. */
> 
> I'm not qualified to review this file.
> 
> ::: dom/telephony/simulator/nsISimulatorTelephonyService.idl
> @@ +5,5 @@
> > +
> > +#include "nsITelephonyService.idl"
> > +
> > +[scriptable, uuid(08dc6bb6-cd78-433c-8530-7c0eb8783b7f)]
> > +interface nsISimulatorTelephonyService : nsITelephonyService
> 
> Whoever is reviewing the implementation (hsinyi?) should also review this
> interface.

Yup, it's review granted from me, see comment 19.

Comment 30

[Alexandre Poirot [:ochameau]]

(In reply to Vicamo Yang [:vicamo][:vyang] from comment #15)
> Comment on attachment 8456949 [details] [diff] [review]
> bug-1038606-1-simulator-voice-calls.patch
> 
> Please help clarify what should I do for a new simulator back-end.
> 
> We have MOZ_B2G, FXOS_SIMULATOR, MOZ_MULET. What's relationship between them
> and which is more appropriate for guarding simulator components?

Just keep using FXOS_SIMULATOR, I'll figure out later what to do on mulet.
(Mulet may end up implying FXOS_SIMULATOR)

Comment 31

[Vicamo Yang [:vicamo][:vyang]]

Waiting for bug 843452 being landed to m-c.

Comment 32

[Wander Lairson Costa]

Hi,

To which repo/branch these patches apply? I tried m-c and b2g-inbound but none applies. I also tried the github repository [1] but I could not build it (seems old code?).

[1] https://github.com/vicamo/b2g_mozilla-central/tree/experimental/simulator

Comment 33

[Vicamo Yang [:vicamo][:vyang]]

(In reply to Wander Lairson Costa [:wcosta] from comment #32)
> Hi,
> 
> To which repo/branch these patches apply? I tried m-c and b2g-inbound but
> none applies. I also tried the github repository [1] but I could not build
> it (seems old code?).
> 
> [1] https://github.com/vicamo/b2g_mozilla-central/tree/experimental/simulator

Yes, as you have seen we're re-organizing RIL interfaces a lot in order to meet 2.2FL interface freezing requirement and that breaks the patches here. Am I blocking any urgent task here?

Comment 34

[Vicamo Yang [:vicamo][:vyang]]

Bug 889737 is going to unify sendMMI() and dial().

Comment 35

[Wander Lairson Costa]

(In reply to Vicamo Yang [:vicamo][:vyang] from comment #33)
> (In reply to Wander Lairson Costa [:wcosta] from comment #32)
> > Hi,
> > 
> > To which repo/branch these patches apply? I tried m-c and b2g-inbound but
> > none applies. I also tried the github repository [1] but I could not build
> > it (seems old code?).
> > 
> > [1] https://github.com/vicamo/b2g_mozilla-central/tree/experimental/simulator
> 
> Yes, as you have seen we're re-organizing RIL interfaces a lot in order to
> meet 2.2FL interface freezing requirement and that breaks the patches here.
> Am I blocking any urgent task here?

No worries, nothing urgent here. Is there a (meta) bug/wiki page/repo for these changes?

Comment 36

[Wander Lairson Costa]

Hi,

I updated the patches to build on top of b2g-inbound again [1], but still struggling to make it run on desktop.

[1] https://github.com/walac/gecko-dev/tree/bugz/1038606

Comment 37

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

Hi Vivien,
Looks like Vicamo is no longer actively working on this.
Do you have mitigation plan in mind?

Comment 38

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

[Tracking Requested - why for this release]:

Comment 39

[Alexandre Poirot [:ochameau]]

Wesley, Would your patch still "work" on top of current master https://github.com/walac/gecko-dev/tree/bugz/1038606 ?
I'm willing to help if there is anything wrong on the devtools/RDP/simulator side.
But I'm afraid I won't be of much help if there is something wrong on the RIL/telephony side.

What were your issues?

Comment 40

[Wander Lairson Costa]

(In reply to Alexandre Poirot [:ochameau] from comment #39)
> Wesley, Would your patch still "work" on top of current master
> https://github.com/walac/gecko-dev/tree/bugz/1038606 ?
> I'm willing to help if there is anything wrong on the devtools/RDP/simulator
> side.
> But I'm afraid I won't be of much help if there is something wrong on the
> RIL/telephony side.
> 
> What were your issues?

I am not working on this anymore (at least for now). But basically simulator was not even starting, I just got to the point to build the code, nothing more.

Comment 41

[:gerard-majax]

Can we get this prioritized? Who should take this decision?

Comment 42

[Gregor Wagner [:gwagner]]

Wilfred, Peter, how should we proceed with developer tools improvement? Should we add this to the product roadmap or should engineering prioritize this internally?

Comment 43

[Wander Lairson Costa]

(In reply to Alexandre LISSY :gerard-majax from comment #41)
> Can we get this prioritized? Who should take this decision?

afaik, nobody is leading this currently. I began to work on it, but had to move to other stuff.

Comment 44

[Wilfred Mathanaraj [:WDM]]

i would like to make it part of planning  - it is critical part.

Comment 45

[Paul Rouget [:paul]]

We can work on the integration in the devtools/WebIDE (see that with :jryans and :ednapiranha). But we need to understand what the roadmap is for the platform bit.

Comment 46

[Peter Dolanjski [:pdol]]

(In reply to Gregor Wagner [:gwagner] from comment #42)
> Wilfred, Peter, how should we proceed with developer tools improvement?
> Should we add this to the product roadmap or should engineering prioritize
> this internally?

What area of the product will need work to support this?

Comment 47

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Per comment 37, and hope you don't mind, Vicamo ;)

Comment 48

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

Is there are rough time frame for the platform work here?  WebIDE team is happy to help on the UI side once the platform work moves along.

Comment 49

[Wander Lairson Costa]

Hi,

In Portland we came to an agreement to use the approach implemented at Bug 989926 (maybe we should reopen that bug). I started to work on it this week by updating :gerard-majax's patches [1], but I am still working on TC stuff [2]. Sorry it is taking so much time to get it done :(

[1] https://github.com/walac/gecko-dev/tree/b2g-desktop-ril
[2] https://bugzilla.mozilla.org/show_bug.cgi?id=1080265

Comment 50

[Gregor Wagner [:gwagner]]

Any update here?

Comment 51

[James Lal [:lightsofapollo]]

This has stalled a bit (my fault here) I have asked wcosta to wrap up this prior to finalizing work here https://bugzilla.mozilla.org/show_bug.cgi?id=1119387

Comment 52

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

ni myself to triage it in telephony platform team.

Comment 53

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

triage: this is more like a feature so let's not block on it.
I redirect it to feature-b2g:3.0? for future planning. 
Maybe James' team will look into this.

Comment 54

[:gerard-majax]

(In reply to Wander Lairson Costa [:wcosta] from comment #49)
> Hi,
> 
> In Portland we came to an agreement to use the approach implemented at Bug
> 989926 (maybe we should reopen that bug). I started to work on it this week
> by updating :gerard-majax's patches [1], but I am still working on TC stuff
> [2]. Sorry it is taking so much time to get it done :(
> 
> [1] https://github.com/walac/gecko-dev/tree/b2g-desktop-ril
> [2] https://bugzilla.mozilla.org/show_bug.cgi?id=1080265

Rebased at https://github.com/lissyx/mozilla-central/commits/libril-bug989926-split_rebase_gecko38

Comment 55

[:gerard-majax]

(In reply to Alexandre LISSY :gerard-majax from comment #54)
> (In reply to Wander Lairson Costa [:wcosta] from comment #49)
> > Hi,
> > 
> > In Portland we came to an agreement to use the approach implemented at Bug
> > 989926 (maybe we should reopen that bug). I started to work on it this week
> > by updating :gerard-majax's patches [1], but I am still working on TC stuff
> > [2]. Sorry it is taking so much time to get it done :(
> > 
> > [1] https://github.com/walac/gecko-dev/tree/b2g-desktop-ril
> > [2] https://bugzilla.mozilla.org/show_bug.cgi?id=1080265
> 
> Rebased at
> https://github.com/lissyx/mozilla-central/commits/libril-bug989926-
> split_rebase_gecko38

It's building and mostly starting, but someone will have to see the RIL-related changes that landed in between, I'm getting errors related to gMobileConnectionService for example ...

Comment 56

[:gerard-majax]

Should we RESOLVED:WONTFIX this?

Comment 57

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(In reply to Alexandre LISSY :gerard-majax from comment #56)
> Should we RESOLVED:WONTFIX this
My team don't have a plan for this, and we'd want to make emu-x86 feasible. So, WONTFIX makes sense to me.