1217285 - [IMS] support IMS fine-grained capabilities, such as wifi-calling or cellular

Status: RESOLVED FIXED

(Firefox OS Graveyard :: RIL, defect)

Description

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

To meet the UI requirements, we need to expose fine-grained capabilities to gaia so that proper UI could be displayed based on the capability support.

My draft idea is that we add |supportedTypes| attribute to ImsRegHandler, just as what we do for mobileConnection.supportedNetworkTypes [1] [2].

In this way, vendor ImsService (the implementation of nsIImsRegHandler) is in charge to retrieving and passing the correct value. I think vendor could refer to [3] to get the idea of the actual implementation.

How do you think?

[1] https://dxr.mozilla.org/mozilla-central/source/dom/webidl/MozMobileConnection.webidl#116
[2] https://dxr.mozilla.org/mozilla-central/source/dom/mobileconnection/interfaces/nsIMobileConnectionService.idl#328
[3] https://dxr.mozilla.org/mozilla-central/source/dom/mobileconnection/gonk/MobileConnectionService.js?from=MobileConnectionService.js#516

Comment 1

[Bevis Tseng[:bevis][:btseng](Exited)]

So far, the UI has to provide additional presentation if multiple bearers for IMS are available.

My idea is to have an attribute called |deviceCapabilities| as a dictionary in ImsRegHandler.webidl to list the optional IMS features built into the device:
> dictionary DeviceCapabilities {
>  boolean overCellular;
>  boolean overWifi;
>  ...
>  // we could add new optional capabilities like video calling, presence, etc in the future.
> };
> interface ImsRegHandler : EventTarget {
>   ...
>   // Contains the supported capabilities built into the device.
>   readonly attribute DeviceCapabilities deviceCapabilities;
>   ...
> }
In internal |nsIImsRegHander|, we can add |overCellularSupported|, |overWifiSupported|, etc.

Then, when the application accesses ImsRegHandler, all these capabilities will be collected during the initialization and will be returned when ImsRegHandler::deviceCapabilities is read.

With this design in mind, the application could display additional options like |preferredProfile| only if both |overCellular| and |overWifi| are supported.

Comment 2

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

(In reply to Bevis Tseng[:bevistseng][:btseng] from comment #1)
> So far, the UI has to provide additional presentation if multiple bearers
> for IMS are available.
> 
> My idea is to have an attribute called |deviceCapabilities| as a dictionary
> in ImsRegHandler.webidl to list the optional IMS features built into the
> device:
> > dictionary DeviceCapabilities {
> >  boolean overCellular;
> >  boolean overWifi;
> >  ...
> >  // we could add new optional capabilities like video calling, presence, etc in the future.
> > };
> > interface ImsRegHandler : EventTarget {
> >   ...
> >   // Contains the supported capabilities built into the device.
> >   readonly attribute DeviceCapabilities deviceCapabilities;
> >   ...
> > }
> In internal |nsIImsRegHander|, we can add |overCellularSupported|,
> |overWifiSupported|, etc.
> 
> Then, when the application accesses ImsRegHandler, all these capabilities
> will be collected during the initialization and will be returned when
> ImsRegHandler::deviceCapabilities is read.
> 
> With this design in mind, the application could display additional options
> like |preferredProfile| only if both |overCellular| and |overWifi| are
> supported.

Oh, yes, this proposal sounds even better! Dictionary usage is easier to use in feature detection :)

Comment 3

[Bevis Tseng[:bevis][:btseng](Exited)]

Hi Carol,

We would like to improve this according the approach in comment 1.
Do you have any concern on this change?

Comment 4

[Carol Yang [:cyang]]

Hi Bevis/Hsinyi,

Since the capability information is static (i.e. display WiFi calling menu, display HD icon, etc), it seems this information does not need to be exposed dynamically via nsIImsRegHandler. Instead, this could be set as a custom prefs.js value. The amount of work in Gaia is the same. Instead of querying via nsIImsRegHandler, it could be a simple call to get the preference value:
> let wfcSupport = Services.prefs.getBoolPref(‘ui.support.wfc’);
> if (wfcSupport) {
>   //Display WiFi Calling menu
> }

OEMs/carriers can easily set this value in the custom prefs.js for the specific UI they expect.

If we were to implement this via nsIImsRegHandler, it would essentially be the same thing where we would report the device capabilities based on some static information for a particular carrier. It seems we could just take out the middleman here.

What do you think?

Comment 5

[Bevis Tseng[:bevis][:btseng](Exited)]

(In reply to Carol Yang [:cyang] from comment #4)
> Hi Bevis/Hsinyi,
> 
> Since the capability information is static (i.e. display WiFi calling menu,
> display HD icon, etc), it seems this information does not need to be exposed
> dynamically via nsIImsRegHandler. Instead, this could be set as a custom
> prefs.js value. The amount of work in Gaia is the same. Instead of querying
> via nsIImsRegHandler, it could be a simple call to get the preference value:
> > let wfcSupport = Services.prefs.getBoolPref(‘ui.support.wfc’);
> > if (wfcSupport) {
> >   //Display WiFi Calling menu
> > }
> 
> OEMs/carriers can easily set this value in the custom prefs.js for the
> specific UI they expect.
> 
> If we were to implement this via nsIImsRegHandler, it would essentially be
> the same thing where we would report the device capabilities based on some
> static information for a particular carrier. It seems we could just take out
> the middleman here.
>
> What do you think?

The preference-style customization was taken into consideration but was not adopted. :(
The reason is that if there is customization related to whether a feature in a Web API set cannot be access or not, there will be more expectation from the app developer that this restriction should be identified from the API itself instead of a separated preference.

So, in addition to the |deviceCapabilities|, the implementation shall reject the request according to the |deviceCapabilities|.
For example, if |overWifi| is false, these request2 shall be rejected: setPreferredProfile("wifi-preferred"), setPreferredProfile("wifi-only")

Comment 6

[Bevis Tseng[:bevis][:btseng](Exited)]

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

Comment 7

[Bevis Tseng[:bevis][:btseng](Exited)]

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

Comment 8

[Bevis Tseng[:bevis][:btseng](Exited)]

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

Comment 9

[Bevis Tseng[:bevis][:btseng](Exited)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=516199071a04

Comment 10

[Bevis Tseng[:bevis][:btseng](Exited)]

Attachment: (v1) Part 1: Web API Change.

Hi Edgar, Hsinyi,

May I have your review for the change in WebIDL part?

Thanks!

Comment 11

[Bevis Tseng[:bevis][:btseng](Exited)]

Attachment: (v1) Part 2: XPCOM Change.

Changes in XPCOM.

Comment 12

[Bevis Tseng[:bevis][:btseng](Exited)]

Attachment: (v1) Part 3: Add Test Coverage.

Add Test case coverage.

Comment 13

[Edgar Chen [:edgar]]

Comment on attachment 8682296 [details] [diff] [review]
(v1) Part 1: Web API Change.

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

Looks good to me, thank you.

::: dom/webidl/ImsRegHandler.webidl
@@ +33,5 @@
> +   *
> +   * This provides the possibility to display the UI options according
> +   * to the capability of the device.
> +   * For example, the |wifi-preferred|, |wifi-only| of the preferred profiles
> +   * will be available in the UI only if |overWifi| is true.

I guess you forget revising the comments.

Comment 14

[Bevis Tseng[:bevis][:btseng](Exited)]

Comment on attachment 8682296 [details] [diff] [review]
(v1) Part 1: Web API Change.

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

::: dom/webidl/ImsRegHandler.webidl
@@ +33,5 @@
> +   *
> +   * This provides the possibility to display the UI options according
> +   * to the capability of the device.
> +   * For example, the |wifi-preferred|, |wifi-only| of the preferred profiles
> +   * will be available in the UI only if |overWifi| is true.

Thanks for catching this!

Comment 15

[Bevis Tseng[:bevis][:btseng](Exited)]

Attachment: Part 1: Web API Change. r=echen,htsai,a=2.2r+

revise API doc.

Comment 16

[Bevis Tseng[:bevis][:btseng](Exited)]

Attachment: Part 2: XPCOM Change. r=echen,a=2.2r+

Comment 17

[Bevis Tseng[:bevis][:btseng](Exited)]

Attachment: Part 3: Add Test Coverage. r=echen,a=2.2r+

Comment 18

[Bevis Tseng[:bevis][:btseng](Exited)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=404cf9b7f59b

Comment 19

[Bevis Tseng[:bevis][:btseng](Exited)]

try server is green in comment 18.

Comment 20

[Bevis Tseng[:bevis][:btseng](Exited)]

Comment on attachment 8682918 [details] [diff] [review]
Part 1: Web API Change. r=echen,htsai,a=2.2r+

[Approval Request Comment]
Bug caused by (feature/regressing bug #): N/A
User impact if declined: N/A
Testing completed: Yes
Risk to taking this patch (and alternatives if risky): No.
String or UUID changes made by this patch:3e39360c-8133-11e5-9ce9-7f3effdb14fe

Comment 21

[Bevis Tseng[:bevis][:btseng](Exited)]

Comment on attachment 8682919 [details] [diff] [review]
Part 2: XPCOM Change. r=echen,a=2.2r+

[Approval Request Comment]
Bug caused by (feature/regressing bug #): N/A
User impact if declined: N/A
Testing completed: Yes
Risk to taking this patch (and alternatives if risky): No.
String or UUID changes made by this patch:3e39360c-8133-11e5-9ce9-7f3effdb14fe

Comment 22

[Bevis Tseng[:bevis][:btseng](Exited)]

Comment on attachment 8682920 [details] [diff] [review]
Part 3: Add Test Coverage. r=echen,a=2.2r+

[Approval Request Comment]
Bug caused by (feature/regressing bug #): N/A
User impact if declined: N/A
Testing completed: Yes
Risk to taking this patch (and alternatives if risky): No.
String or UUID changes made by this patch:3e39360c-8133-11e5-9ce9-7f3effdb14fe

Comment 23

[Carsten Book [:Tomcat]]

landed on 2.2r as:

https://hg.mozilla.org/releases/mozilla-b2g37_v2_2r/rev/21669971b423
https://hg.mozilla.org/releases/mozilla-b2g37_v2_2r/rev/9428af2172c2
https://hg.mozilla.org/releases/mozilla-b2g37_v2_2r/rev/76b4d9614340