831183 - B2G STK: callMessage of STK_CMD_SET_UP_CALL doesn't work properly

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect)

Description

[Edgar Chen [:edgar]]

Found this bug when I wrote test cases for bug 830696.

Comment 1

[Edgar Chen [:edgar]]

Attachment: Part 1: Fix callMessage, v1

Comment 2

[Edgar Chen [:edgar]]

Attachment: Part 2: xpcshell tests for SET_UP_CALL, v1

xpcshell tests

Comment 3

[Edgar Chen [:edgar]]

Attachment: Part 3: marionette tests for SET_UP_CALL, v1

marionette tests

Comment 4

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Comment on attachment 702711 [details] [diff] [review]
Part 2: xpcshell tests for SET_UP_CALL, v1

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

::: dom/system/gonk/tests/test_ril_worker_icc.js
@@ +976,5 @@
> +  let worker = newUint8Worker();
> +  let pduHelper = worker.GsmPDUHelper;
> +  let berHelper = worker.BerTlvHelper;
> +  let stkHelper = worker.StkProactiveCmdHelper;
> +  let stkCmdHelper = worker.StkCommandParamsFactory;

Use a better naming, otherwise it's easy to confuse with stkHelper above

@@ +987,5 @@
> +    0x05, 0x0A, 0x44, 0x69, 0x73, 0x63, 0x6F, 0x6E, 0x6E, 0x65, 0x63, 0x74,
> +    0x86, 0x09, 0x81, 0x10, 0x32, 0x04, 0x21, 0x43, 0x65, 0x1C, 0x2C,
> +    0x05, 0x07, 0x4D, 0x65, 0x73, 0x73, 0x61, 0x67, 0x65];
> +
> +  for(let i = 0 ; i < set_up_call_1.length; i++) {

nit, space after for.

Comment 5

[Edgar Chen [:edgar]]

Attachment: Part 2: xpcshell tests for SET_UP_CALL, v2, r=allstars.chh

Address review comment #4.

Comment 6

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d36cbf792480
https://hg.mozilla.org/integration/mozilla-inbound/rev/5eaefcb4a441
https://hg.mozilla.org/integration/mozilla-inbound/rev/9797a7529ae7

Comment 7

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Comment on attachment 702709 [details] [diff] [review]
Part 1: Fix callMessage, v1

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

Request to check in this patch to b2g18.

Comment 8

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Comment on attachment 702724 [details] [diff] [review]
Part 2: xpcshell tests for SET_UP_CALL, v2, r=allstars.chh

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

Request to check in this patch to b2g18.

Comment 9

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Comment on attachment 702712 [details] [diff] [review]
Part 3: marionette tests for SET_UP_CALL, v1

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

Request to check in this patch to b2g18.

Comment 10

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Comment on attachment 702724 [details] [diff] [review]
Part 2: xpcshell tests for SET_UP_CALL, v2, r=allstars.chh

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

Request to check in this patch to b2g18.

Comment 11

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Edgar, can you try if these patches can be pushed to b2g18 ?
I think the test case would have conflict.

Comment 12

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/d36cbf792480
https://hg.mozilla.org/mozilla-central/rev/5eaefcb4a441
https://hg.mozilla.org/mozilla-central/rev/9797a7529ae7

Comment 13

[Edgar Chen [:edgar]]

Attachment: (b2g18) Part 2: xpcshell tests for SET_UP_CALL, v2, r=allstars.chh

(for b2g18)

Comment 14

[Edgar Chen [:edgar]]

Attachment: (b2g18) Part 3: marionette tests for SET_UP_CALL, v1, r=allstars.chh

(for b2g18)

Comment 15

[Edgar Chen [:edgar]]

Comment on attachment 702709 [details] [diff] [review]
Part 1: Fix callMessage, v1

[Approval Request Comment]
Bug caused by (feature/regressing bug #):
User impact if declined: STK proactive command, SET_UP_CALL, doesn't work properly.
Testing completed: xpcshell/marionette tests
Risk to taking this patch (and alternatives if risky): No
String or UUID changes made by this patch: No

Comment 16

[Edgar Chen [:edgar]]

Comment on attachment 703159 [details] [diff] [review]
(b2g18) Part 2: xpcshell tests for SET_UP_CALL, v2, r=allstars.chh

[Approval Request Comment]
Bug caused by (feature/regressing bug #):
User impact if declined: STK proactive command, SET_UP_CALL, doesn't work properly.
Testing completed: xpcshell/marionette tests
Risk to taking this patch (and alternatives if risky): No
String or UUID changes made by this patch: No

Comment 17

[Edgar Chen [:edgar]]

Comment on attachment 703160 [details] [diff] [review]
(b2g18) Part 3: marionette tests for SET_UP_CALL, v1, r=allstars.chh

[Approval Request Comment]
Bug caused by (feature/regressing bug #):
User impact if declined: STK proactive command, SET_UP_CALL, doesn't work properly.
Testing completed: xpcshell/marionette tests
Risk to taking this patch (and alternatives if risky): No
String or UUID changes made by this patch: No

Comment 18

[Andrew Overholt [:overholt]]

Edgar, can you provide more justification for taking these patches on mozilla-b2g18?  It's unclear how important it is since we're not blocking on this.

Comment 19

[Edgar Chen [:edgar]]

Hi Andrew,

Sorry for the unclear.
Let me explain more detailed about this bug.

In STK SET_UP_CALL command, sim card may pass a call message to STK application, this call message will be showed on UI in call set up phase (After got user confirmation).
But this mechanism has a potential issue: gecko can not pass the call message to gaia correctly.
Although gaia does not implement this mechanism yet, but I think we may still need to fix this issue in gecko. So request to check in this patch to b2g18.

Thanks.

Comment 20

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18/rev/718fe51bc8f7
https://hg.mozilla.org/releases/mozilla-b2g18/rev/4facc2ed0e1b
https://hg.mozilla.org/releases/mozilla-b2g18/rev/096b4f3a085d