782778 - [WebAPI] WebTelephony: Receive 'connected' call event twice for the same call

Status: VERIFIED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

[Robert Wood [:rwood]]

Attachment: Logcat

Using the B2G WebTelephony API.

Have an existing (outgoing) call connected, then simulate an incoming call. Notice that the 'connected' call event for the original outgoing call is received a second time (when the 'incoming' call event is received for the new/2nd call). Expect to receive the 'connected' call event for the original call only once, at the time of its original connection; shouldn't receive it again when a new/second call is incoming.

To reproduce: Run the attached B2G Marionette WebTelephony test on the arm emulator (test_incoming_already_connected.js).  Expect pass, get failure (received 'connected' event for original call multiple times).

Running the test on arm emulator built Aug-14, Ubuntu 12.04 x64 VM.  See attached logcat.

Comment 1

[Robert Wood [:rwood]]

Attachment: Marionette B2G test

Comment 2

[Philipp von Weitershausen [:philikon]]

Tentatively assigning to Hsin-Yi. Can you take a look please?

Comment 3

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

Attachment: Patch

'this._handleChangedCallState' is called when 1) call state indeed changes or 2) the number of calls changes. Therefore, a 'connected' event for the first call is fired again when there's a newly incoming call.

Case 2 is used to notify that 'active' call changes. However, I realize that 'active' call in ril_worker.js doesn't match mozTelephony.active. According to Telephony API, 'incoming' call and 'waiting' call shouldn't be treated as 'active' in ril_worker.  

This patch corrects "_isActiveCall" such that 'active' call necessarily varies with call state change, not depending on Case 2 anymore. So, this patch deletes case 2, the cause of this bug, as well.

Comment 4

[Philipp von Weitershausen [:philikon]]

Comment on attachment 652046 [details] [diff] [review]
Patch

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

A fix that mostly *removes* code? Too good to be true! :)

::: dom/system/gonk/ril_worker.js
@@ -2323,5 @@
>  
>        if (newCall) {
>          // Call is still valid.
> -        if (newCall.state != currentCall.state ||
> -            this.currentCallsLength != lastCallsLength) {

Ouch. Yeah, I see how that lead to duplicated events.

Comment 5

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/e4f3ede0569d

Comment 6

[Ed Morley [:emorley]]

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

Comment 7

[Robert Wood [:rwood]]

Verified fixed using new build of the arm-emulator (20-Aug)