Last Comment Bug 782778 - [WebAPI] WebTelephony: Receive 'connected' call event twice for the same call
: [WebAPI] WebTelephony: Receive 'connected' call event twice for the same call
Product: Core
Classification: Components
Component: DOM: Device Interfaces (show other bugs)
: unspecified
: All Gonk (Firefox OS)
-- normal (vote)
: mozilla17
Assigned To: Hsin-Yi Tsai [:hsinyi]
: Andrew Overholt [:overholt]
Depends on:
  Show dependency treegraph
Reported: 2012-08-14 14:06 PDT by Robert Wood [:rwood]
Modified: 2012-08-20 12:20 PDT (History)
2 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Logcat (21.89 KB, text/plain)
2012-08-14 14:06 PDT, Robert Wood [:rwood]
no flags Details
Marionette B2G test (5.97 KB, application/x-javascript)
2012-08-14 14:07 PDT, Robert Wood [:rwood]
no flags Details
Patch (3.92 KB, patch)
2012-08-15 02:35 PDT, Hsin-Yi Tsai [:hsinyi]
philipp: review+
Details | Diff | Splinter Review

Description User image Robert Wood [:rwood] 2012-08-14 14:06:20 PDT
Created attachment 651876 [details]

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 User image Robert Wood [:rwood] 2012-08-14 14:07:19 PDT
Created attachment 651878 [details]
Marionette B2G test
Comment 2 User image Philipp von Weitershausen [:philikon] 2012-08-14 14:32:58 PDT
Tentatively assigning to Hsin-Yi. Can you take a look please?
Comment 3 User image Hsin-Yi Tsai [:hsinyi] 2012-08-15 02:35:11 PDT
Created attachment 652046 [details] [diff] [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 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 User image Philipp von Weitershausen [:philikon] 2012-08-16 15:20:09 PDT
Comment on attachment 652046 [details] [diff] [review]

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 6 User image Ed Morley [:emorley] 2012-08-17 05:27:33 PDT
Comment 7 User image Robert Wood [:rwood] 2012-08-20 12:20:34 PDT
Verified fixed using new build of the arm-emulator (20-Aug)

Note You need to log in before you can comment on or make changes to this bug.