Closed Bug 1139063 Opened 6 years ago Closed 6 years ago

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

Categories

(Firefox OS Graveyard :: RIL, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(firefox39 fixed)

RESOLVED FIXED
2.2 S8 (20mar)
Tracking Status
firefox39 --- fixed

People

(Reporter: tauzen, Assigned: tauzen)

References

Details

Attachments

(1 file, 1 obsolete file)

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.
Assignee: nobody → kmioduszewski
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.
Depends on: 1139835
Summary: [Secure Element] Investigate problems with opening multiple channels on Nexus 5 → [Secure Element] RILContentHelper.handleIccOpenChannel should notify error if channel not available
Hi Edgar, please see comment 1 for more details. Could you review this?
Attachment #8573351 - Flags: review?(echen)
Component: NFC → RIL
OS: Mac OS X → Gonk (Firefox OS)
Hardware: x86 → ARM
See Also: → 1139835
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
Attachment #8573351 - Flags: review?(echen)
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.
Attachment #8573351 - Attachment is obsolete: true
Attachment #8573846 - Flags: review?(echen)
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. :)
Attachment #8573846 - Flags: review?(echen) → review+
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
Keywords: checkin-needed
See Also: → 1140259
https://hg.mozilla.org/mozilla-central/rev/f481ba01e113
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.2 S8 (20mar)
Blocks: 1140259
You need to log in before you can comment on or make changes to this bug.