Closed Bug 782778 Opened 8 years ago Closed 8 years ago

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

Categories

(Core :: DOM: Device Interfaces, defect)

All
Gonk (Firefox OS)
defect
Not set

Tracking

()

VERIFIED FIXED
mozilla17

People

(Reporter: rwood, Assigned: hsinyi)

Details

Attachments

(3 files)

Attached file 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.
Attached file Marionette B2G test
Tentatively assigning to Hsin-Yi. Can you take a look please?
Assignee: nobody → htsai
Attached patch PatchSplinter Review
'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.
Attachment #652046 - Flags: review?(philipp)
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.
Attachment #652046 - Flags: review?(philipp) → review+
https://hg.mozilla.org/mozilla-central/rev/e4f3ede0569d
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
Verified fixed using new build of the arm-emulator (20-Aug)
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.