1139063 - [Secure Element] RILContentHelper.handleIccOpenChannel should notify error if channel not available

Status: RESOLVED FIXED

(Firefox OS Graveyard :: RIL, defect)

Description

[Krzysztof Mioduszewski[:tauzen]]

Test cases and complicated scenarios which involve opening more than one logical channel at the same time are failing. These problems weren't present on Flame 2.1 with QCRIL.

Comment 1

[Krzysztof Mioduszewski[:tauzen]]

I've raised a separate bug for dealing with the errors from RIL Bug 1139835.

RILContentHelper in |handleIccOpenChannel| does check only the |message.errorMsg| from ril_worker. If it is not present it calls |callback.notifyOpenChannelSuccess(message.channel)|. In the problematic scenario, errorMsg is not present but channel is undefined. In such situation |callback.notifyError| should be called.

Comment 2

[Krzysztof Mioduszewski[:tauzen]]

Attachment: 0001-Bug-1139063-Secure-Element-RILContentHelper.handleIc.patch

Hi Edgar, please see comment 1 for more details. Could you review this?

Comment 3

[Edgar Chen [:edgar]]

Comment on attachment 8573351 [details] [diff] [review]
0001-Bug-1139063-Secure-Element-RILContentHelper.handleIc.patch

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

From the log provided in bug 1139835, modem/rild did report error code 16 [1] to notify error, but ril_consts.js leaks the define for error code 16 [2]. :(
So what we should do is defining the proper errorMsg in ril_consts.js for error code 16, then RILContentHelper could notify error correctly.
Thank you. :)

[1] https://github.com/android/platform_hardware_ril/blob/lollipop-release/include/telephony/ril.h#L101
[2] https://dxr.mozilla.org/mozilla-central/source/dom/system/gonk/ril_consts.js#216-302

Comment 4

[Krzysztof Mioduszewski[:tauzen]]

Attachment: 0001-Bug-1139063-RIL-Adding-MissingResource-error-msg-no-.patch

As discussed on IRC, since the missing error message for error code 16 is breaking the SE API tests, this is a quick patch adding "MissingResource" error message.

Comment 5

[Edgar Chen [:edgar]]

Comment on attachment 8573846 [details] [diff] [review]
0001-Bug-1139063-RIL-Adding-MissingResource-error-msg-no-.patch

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

Thank you. :)

Comment 6

[Krzysztof Mioduszewski[:tauzen]]

Had to retrigger some of the tests. Still some are failing, but I assume that they are not related to this patch: https://treeherder.mozilla.org/#/jobs?repo=try&revision=504f8aac5aa2

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/b2g-inbound/rev/f481ba01e113

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/f481ba01e113