1162464 - [B2G][ICC] Move LogicalChannel access from IccProvider to IccService.

Status: RESOLVED FIXED

(Firefox OS Graveyard :: RIL, defect)

Description

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

To completely deprecate IccProvider, we also have to move the APIs of logical channel from IccProvider to IccService.

I planned to fix this in bug 1114938.
Since logical channel access is not part of STK feature,
it will be more clear to separate this task from bug 1114938.

Comment 1

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

Attachment: Part 1: IDL Changes. r=echen

This is the idl part to move the ICC Logical Channel API from IccProvider to IccService.

Hi Edgar,

May I have your review for this change?

Thanks!

Comment 2

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

Attachment: Part 2: Implementation Changes.

Implementation part.

Comment 3

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

Attachment: Part 3: Refactor UiccConnector due to interface change. r=allstars.chh

Hi Yoshi,

May I have your review in UiccConnector due to the API changes in this bug?

Thanks!

Comment 4

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

[Tracking Requested - why for this release]:

Comment 5

[Edgar Chen [:edgar]]

Comment on attachment 8603150 [details] [diff] [review]
Part 2: Implementation Changes.

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

Thank you.

::: dom/icc/gonk/IccService.js
@@ +487,5 @@
> +  iccOpenChannel: function(aAid, aCallback) {
> +    this._radioInterface.sendWorkerMessage("iccOpenChannel",
> +                                           { aid: aAid },
> +                                           (aResponse) => {
> +      if (!aCallback) {

Other function doesn't have such check. Just curious, is there any special reason to have |aCallback| check for logicalChannel API?

@@ +491,5 @@
> +      if (!aCallback) {
> +        return;
> +      }
> +
> +      return !aResponse.errorMsg

Don't need 'return'.

@@ +518,5 @@
> +      if (!aCallback) {
> +        return;
> +      }
> +
> +      return !aResponse.errorMsg

Ditto.

@@ +534,5 @@
> +      if (!aCallback) {
> +        return;
> +      }
> +
> +      return !aResponse.errorMsg

Ditto.

Comment 6

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

Comment on attachment 8603150 [details] [diff] [review]
Part 2: Implementation Changes.

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

::: dom/icc/gonk/IccService.js
@@ +487,5 @@
> +  iccOpenChannel: function(aAid, aCallback) {
> +    this._radioInterface.sendWorkerMessage("iccOpenChannel",
> +                                           { aid: aAid },
> +                                           (aResponse) => {
> +      if (!aCallback) {

No, I also have the same question on this.
I just keep the same logic done in RILContentHelper.
I should revise more aggressively instead.

I'll address this in next update. :)

@@ +491,5 @@
> +      if (!aCallback) {
> +        return;
> +      }
> +
> +      return !aResponse.errorMsg

Ditto.

@@ +518,5 @@
> +      if (!aCallback) {
> +        return;
> +      }
> +
> +      return !aResponse.errorMsg

Ditto.

Comment 7

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

Attachment: Part 2 v2: Implementation Changes. r=echen

address comment 5.

Comment 8

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

update try server result:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=21719408d352

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/b2g-inbound/rev/579df6b63b9e
https://hg.mozilla.org/integration/b2g-inbound/rev/9f0d8d7bc7a1
https://hg.mozilla.org/integration/b2g-inbound/rev/4d1576db931f

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/579df6b63b9e
https://hg.mozilla.org/mozilla-central/rev/9f0d8d7bc7a1
https://hg.mozilla.org/mozilla-central/rev/4d1576db931f