Closed Bug 784429 Opened 7 years ago Closed 5 years ago

[WebAPI] WebTelephony: Hang-up while connecting, call is not terminated

Categories

(Firefox OS Graveyard :: RIL, defect)

All
Gonk (Firefox OS)
defect
Not set

Tracking

(tracking-b2g:backlog)

RESOLVED INVALID
tracking-b2g backlog

People

(Reporter: rwood, Assigned: aknow)

References

Details

(Whiteboard: [p=2])

Attachments

(4 files)

Attached file Logcat
Using the B2G WebTelephony WebAPI.

Receive an incoming call and answer it - but immediately after receiving the 'connecting' call event, and BEFORE receiving the 'connected' event, hangUp the call (via the API). Notice that a 'disconnecting' call event is then received as expected, but a 'disconnected' call event never follows - and actually a 'connected' call event is then received (after the 'disconnecting' event).

At this point, the emulator shows that the call is active (issue the 'gsm list' command to the emulator and the call status is active).

Had expected the call to be terminated - according to the B2G Telephony Call States diagram, if a hang-up occurs during the 'connecting' state (but before the actual 'connected' state), both the 'disconnecting' and 'disconnected' events should be received: https://wiki.mozilla.org/File:B2GTelephonyCallStates.png

To reproduce: Run the attached B2G WebAPI Marionette test (test_incoming_connecting_hangup.js). Expect the test to pass, instead it times out waiting for the 'disconnected' call event after the hang-up. See logcat attached.
Attached file Marionette B2G test
Thanks for the investigation. This is quite the edge case, unlikely to be encountered on the device. Definitely not a blocker. If we can find an extremely easy fix, we'll take it, but we've got bigger fish to fry.
Assignee: philipp → nobody
This is a race, I'd expect that we could hit it in the real world.
(In reply to ben turner [:bent] from comment #3)
> This is a race, I'd expect that we could hit it in the real world.

I didn't say we couldn't, just said that it was unlikely given that the 'connecting' and 'connected' events are something like O(100ms) apart (and that's probably pretty generous). Definition for a blocker is something we would hold the release for. Personally I would totally ship the phone with this bug not fixed.
Component: DOM: Device Interfaces → RIL
Product: Core → Firefox OS
Assignee: nobody → szchen
Put this bug into backlog.
blocking-b2g: --- → backlog
Attachment #8408038 - Flags: review?(htsai)
Whiteboard: [p=2]
Target Milestone: --- → 1.4 S6 (25apr)
Comment on attachment 8408036 [details] [diff] [review]
Part 1: Handle disconnecting state properly

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

::: dom/telephony/TelephonyCall.cpp
@@ +70,5 @@
>  
> +  // Rule for state transition.
> +  // Only accept 'disconnected' after 'disconnecting'.
> +  if (mCallState == nsITelephonyProvider::CALL_STATE_DISCONNECTING &&
> +      aCallState != nsITelephonyProvider::CALL_STATE_DISCONNECTED) {

This is not right. When the remote party releases the call, we will meet a case that aCallState is DISCONNECTED but mCallState isn't DISCONNECTING.

Another example is, if App_1 is the caller to trigger hang up, then App_2 is never received DISCONNECTED call event even that call is indeed released.
Attachment #8408036 - Flags: review?(htsai) → review-
(In reply to Hsin-Yi Tsai  [:hsinyi] from comment #8)
> Comment on attachment 8408036 [details] [diff] [review]
> Part 1: Handle disconnecting state properly
> 
> Review of attachment 8408036 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/telephony/TelephonyCall.cpp
> @@ +70,5 @@
> >  
> > +  // Rule for state transition.
> > +  // Only accept 'disconnected' after 'disconnecting'.
> > +  if (mCallState == nsITelephonyProvider::CALL_STATE_DISCONNECTING &&
> > +      aCallState != nsITelephonyProvider::CALL_STATE_DISCONNECTED) {
> 
> This is not right. When the remote party releases the call, we will meet a
> case that aCallState is DISCONNECTED but mCallState isn't DISCONNECTING.
> 
> Another example is, if App_1 is the caller to trigger hang up, then App_2 is
> never received DISCONNECTED call event even that call is indeed released.

Oops, sorry... I was dazzled... Please ignore this :(
Comment on attachment 8408036 [details] [diff] [review]
Part 1: Handle disconnecting state properly

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

::: dom/telephony/TelephonyCall.cpp
@@ +70,5 @@
>  
> +  // Rule for state transition.
> +  // Only accept 'disconnected' after 'disconnecting'.
> +  if (mCallState == nsITelephonyProvider::CALL_STATE_DISCONNECTING &&
> +      aCallState != nsITelephonyProvider::CALL_STATE_DISCONNECTED) {

Please ignore my previous comment :(

But I am still concerned about the patch:
App_1 is the caller to trigger call.hangUp(). App_2 is simply a monitor. What if 'connected' event really comes, and the call state is indeed connected? Then with this patch, App_1 doesn't get any chance to get updated but App_2 does... We now have some problems with call state synchronization between objects. And this solution seems make things worse...
Attachment #8408036 - Flags: review-
(In reply to Hsin-Yi Tsai  [:hsinyi] from comment #10)
> Comment on attachment 8408036 [details] [diff] [review]
> Part 1: Handle disconnecting state properly
> 
> Review of attachment 8408036 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/telephony/TelephonyCall.cpp
> @@ +70,5 @@
> >  
> > +  // Rule for state transition.
> > +  // Only accept 'disconnected' after 'disconnecting'.
> > +  if (mCallState == nsITelephonyProvider::CALL_STATE_DISCONNECTING &&
> > +      aCallState != nsITelephonyProvider::CALL_STATE_DISCONNECTED) {
> 
> Please ignore my previous comment :(
> 
> But I am still concerned about the patch:
> App_1 is the caller to trigger call.hangUp(). App_2 is simply a monitor.
> What if 'connected' event really comes, and the call state is indeed
> connected? Then with this patch, App_1 doesn't get any chance to get updated
> but App_2 does... We now have some problems with call state synchronization
> between objects. And this solution seems make things worse...

Call state is not synchronized because we introduce +ing state in our system. IMO, if we also broadcast and sync the +ing state between telephony objects, the problem could be solved.
After discussing with Hsinyi, we decided to postpone this bug. I will work on synchronizing the +ing state first. That is, all the other telephony objects will also receive 'connecting', 'holding', 'resuming', 'disconnecting' ... state change.
Target Milestone: 1.4 S6 (25apr) → ---
Attachment #8408038 - Flags: review?(htsai)
See Also: → 1005816
"connecting" state is going to be removed in Bug 1095366.
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → INVALID
blocking-b2g: backlog → ---
You need to log in before you can comment on or make changes to this bug.