1052846 - Use enums for MobileNetworkType in nsIMobileConnectionProvider.idl

Status: RESOLVED FIXED

(Firefox OS Graveyard :: RIL, defect)

Description

[Anshul]

The bug 898445 introduced the  MobileNetworkType enum in the MozMobileConnection.webidl but no corresponding constants were added in nsIMobileConnectionProvider.idl. 

Please consider adding enums for network type for getSupportedNetworkTypes API in nsIMobileConnectionProvider.idl to strictly define the IPDL interface.

Comment 1

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

We will need to care "MobilePreferredNetworkType."

Comment 2

[Jessica Jong [:jessica] (inactive)]

Attachment: Part 1: idl changes, v1.

Hsinyi, may I have your review on the interface? Thanks.

Comment 3

[Jessica Jong [:jessica] (inactive)]

Attachment: [final] Part 2: dom/ipc changes. r=echen

Edgar, may I have your review? Thanks.

Comment 4

[Jessica Jong [:jessica] (inactive)]

Attachment: Part 3: gonk changes, v1.

Edgar, may I have your review? Thanks.

Comment 5

[Edgar Chen [:edgar]]

Comment on attachment 8522647 [details] [diff] [review]
[final] Part 2: dom/ipc changes. r=echen

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

Looks good to me, thank you.

Comment 6

[Edgar Chen [:edgar]]

Comment on attachment 8522648 [details] [diff] [review]
Part 3: gonk changes, v1.

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

Thank you.

::: dom/mobileconnection/gonk/MobileConnectionService.js
@@ +406,5 @@
>          if (DEBUG) {
>            this._debug("Unknown network type: " + type);
>          }
> +        RIL.GECKO_SUPPORTED_NETWORK_TYPES_DEFAULT.split(",").forEach(aType => {
> +          enumNetworkTypes.push(RIL.GECKO_SUPPORTED_NETWORK_TYPES.indexOf(aType));

Please help to add comments in ril_consts.js about the items in `GECKO_SUPPORTED_NETWORK_TYPES` should be in sync with nsIMobileConnection.MOBILE_NETWORK_TYPE_*. Thank you.

Comment 7

[Jessica Jong [:jessica] (inactive)]

(In reply to Edgar Chen [:edgar][:echen] from comment #6)
> Comment on attachment 8522648 [details] [diff] [review]
> Part 3: gonk changes, v1.
> 
> Review of attachment 8522648 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Thank you.
> 
> ::: dom/mobileconnection/gonk/MobileConnectionService.js
> @@ +406,5 @@
> >          if (DEBUG) {
> >            this._debug("Unknown network type: " + type);
> >          }
> > +        RIL.GECKO_SUPPORTED_NETWORK_TYPES_DEFAULT.split(",").forEach(aType => {
> > +          enumNetworkTypes.push(RIL.GECKO_SUPPORTED_NETWORK_TYPES.indexOf(aType));
> 
> Please help to add comments in ril_consts.js about the items in
> `GECKO_SUPPORTED_NETWORK_TYPES` should be in sync with
> nsIMobileConnection.MOBILE_NETWORK_TYPE_*. Thank you.

Will do, thanks!

Comment 8

[Jessica Jong [:jessica] (inactive)]

Attachment: [final] Part 1: idl changes. r=hsinyi

aligned consts.

Comment 9

[Jessica Jong [:jessica] (inactive)]

Attachment: [final] Part 3: gonk changes. r=echen

added comments for `GECKO_SUPPORTED_NETWORK_TYPES` in ril_consts.js, see review comment 6.

Comment 10

[Jessica Jong [:jessica] (inactive)]

try looks good!

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=9d751fa665fc

Comment 11

[Jessica Jong [:jessica] (inactive)]

https://hg.mozilla.org/integration/b2g-inbound/rev/2fdfde0fb31a
https://hg.mozilla.org/integration/b2g-inbound/rev/e04c1126aeb4
https://hg.mozilla.org/integration/b2g-inbound/rev/6c0d0fed9ad4

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/2fdfde0fb31a
https://hg.mozilla.org/mozilla-central/rev/e04c1126aeb4
https://hg.mozilla.org/mozilla-central/rev/6c0d0fed9ad4