996421 - [B2G] [RIL] [Tarako] assign options to Buf.newParcel() in sendDialRequest() otherwise options.number is missing in the response handler

Status: RESOLVED FIXED

(Firefox OS Graveyard :: RIL, defect)

Description

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

This is a followup of bug 990467.

Comment 1

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

(In reply to Hsin-Yi Tsai  [:hsinyi] from comment #0)
> This is a followup of bug 990467.

This issue doesn't exist in m-c because the handling has already been there [1].

[1] http://dxr.mozilla.org/mozilla-central/source/dom/system/gonk/ril_worker.js?from=ril_worker.js&case=true#1502

Comment 2

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

Attachment: part 1 - fix

Comment 3

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

Current test cases cannot capture this issue. I am working on a new case.

Comment 4

[Joe Cheng [:jcheng] (please needinfo)]

triage: this is a blocker for Bug 990003. 1.3T+

Comment 5

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

Attachment: part2 - test case

Without part 1 and considering a case that 1st telephony instance makes out a call. Then, 2nd telephony instance is created after the call was made but modem hasn't notified gecko CLCC. 2nd telephony gets a call from enumeration result while the call number is empty.

Part 1 fixes the issue and this test verifies the fix. However, please note that this case is heavily on timing to be able to capture the existing failure. For example, if 2nd telephony isn't created before CLCC, then a correct call number is gotten. But no matter how, this test shouldn't fail!

Comment 6

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

Comment on attachment 8406631 [details] [diff] [review]
part 1 - fix

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

Assign options to Buf.newParcel() in sendDialRequest(), otherwise options.number is missing in the handler of REQUEST_DIAL.

Comment 7

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

Comment on attachment 8406756 [details] [diff] [review]
part2 - test case

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

Hi Vicamo,

Please see comment 5 for explanation. This bug is only for v1.3t. Though I think we should make m-c have this test as well, I am not planning to land this on m-c at this bug. I will file another bug for completing test coverage.

Comment 8

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 8406631 [details] [diff] [review]
part 1 - fix

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

r=me

Comment 9

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

https://tbpl.mozilla.org/?tree=Try&rev=05cc073c2bf9  try green on v1.3t

Comment 10

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

Attachment: part 1 - fix (v2) change commit message, r=vicamo [v1.3t only, no m-c]

Comment 11

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

Attachment: part2 - test case (2), change commit message, r=vicamo [v1.3t only, no m-c]

correct some nits.

Comment 13

[[:fabrice] Fabrice Desré]

https://hg.mozilla.org/releases/mozilla-b2g28_v1_3t/rev/efd5d26acd1d
https://hg.mozilla.org/releases/mozilla-b2g28_v1_3t/rev/a0593a209db7