1006834 - [B2G][Dialer] User cannot initiate new outbound call after terminating one line in a conference call

Status: RESOLVED FIXED

(Firefox OS Graveyard :: RIL, defect)

Description

[Brogan Zumwalt [Inactive]]

Attachment: Screenshot

Description:
When user is in a conference call with at least two people, having one member of the conference call hang up will prevent the user who initiated the call from making another outbound call (unable to re-initiate conference call by re-calling party that left call.)

If user attempts to initiate another call while remaining party from conference call is still connected, an error message appears, "Unable to make a phone call now - Cannot make a call. If you are already dialing, please hang up first."

Repro Steps:
1) Updated Open C to Build ID: 20140506000202
2) Launch dialer
3) Initiate outbound call to device 1
4) After call with device 1 is established, initiate call with device 2 then merge calls
5) Have device 2 disconnect from call
6) Attempt to re-initiate call to device 2

Actual:
User cannot reinitiate call to device that has left conference call.

Expected:
User can call back a device that has left existing conference call.

Environmental Variables
Device: Open_C v1.4 Mozilla RIL
Build ID: 20140506000202
Gecko: fe4080728c60
Gaia: b1242f33981024de59b8b4c26bacff8b876211b1
Platform Version: 30.0
Firmware Version: P821A10V1.0.0B06_LOG_DL

Notes:
Repro frequency: 3/3, 100%
See attached: screenshot and logcat

Comment 1

[Brogan Zumwalt [Inactive]]

Attachment: LogCat

Comment 2

[Natalya Kot [:nkot]]

'qawanted' - does it reproduce on Buri?

Comment 3

[Brogan Zumwalt [Inactive]]

Attachment: Logcat - RIL debugging

Ignore first logcat, this one has ril debugging enabled

Comment 4

[Brogan Zumwalt [Inactive]]

Issue does NOT occur on Buri 1.4

Result: User can call back a device that has left existing conference call.

Environmental Variables:
Device: Buri v1.4 Mozilla RIL
BuildID: 20140506040204
Gaia: 4af716f09747edfbea637f5b60f7fd7d0183f19b
Gecko: 81651ad5e43c
Version: 32.0a1
Firmware Version: v1.2-device.cfg


Issue does NOT occur on 2.0 Open C

Result: User can call back a device that has left existing conference call.

Environmental Variables:
Device: Open_C v2.0 Master M-C Mozilla RIL
BuildID: 20140506040204
Gaia: 4af716f09747edfbea637f5b60f7fd7d0183f19b
Gecko: 81651ad5e43c
Version: 32.0a1
Firmware Version: P821A10V1.0.0B06_LOG_DL

Comment 5

[Brogan Zumwalt [Inactive]]

Issue does NOT occur on Open C 1.3 base image P821A10V1.0.0B06_LOG_DL

Comment 6

[Preeti Raghunath(:Preeti)]

Ken

Can we please look into this issue?

Comment 7

[Josh Schmitt [Joshs]]

Issue repros on the first tinderbox 1.4 Open_C build, unable to get a regression window.

1.4 Environmental Variables:
Device: Open_C 1.4
BuildID: 20140424123005
Gaia: fc95009476fac9ce205a59b237d146ca7f6f42e7
Gecko: 37237034e45c
Version: 30.0a2
Firmware Version: P821A10V1.0.0B06_LOG_DL

Comment 8

[Ken Chang[:kenkai]]

Aknow, please help to check this bug first. Thanks.

Comment 9

[Szu-Yu Chen [:aknow] (inactive)]

We cannot find OpenC device here. It seems that the issue only happened on 1.4 Open_C build.

Doug,

Could you get an OpenC device and help us dump some log in gaia? We guess it might hit the condition here [1]. Seems there is something wrong in gecko. We need more information to dig into the problem. We would like to know the following value when condition hit.

  telephony.calls.length
  telephony.conferenceGroup.calls.length

  Also the info of each call in |telephony.calls| and |telephony.conferenceGroup.calls|
  ex: number, state.

[1] https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/dialer/js/telephony_helper.js#L33

Comment 10

[Doug Sherk (:drs) (inactive)]

(In reply to Szu-Yu Chen [:aknow] from comment #9)
> Doug,
> 
> Could you get an OpenC device and help us dump some log in gaia? We guess it
> might hit the condition here [1]. Seems there is something wrong in gecko.
> We need more information to dig into the problem. We would like to know the
> following value when condition hit.

I don't have an Open C handy either. I ordered one just now, but it'll be a while before it gets to me.

(In reply to Josh Schmitt from comment #7)
> Issue repros on the first tinderbox 1.4 Open_C build, unable to get a
> regression window.

Could you explain why you couldn't determine a regression window for this? Did it just occur on every version between 1.3 and 1.4 or something? This is useful information for us.

Comment 11

[Jason Smith [:jsmith]]

(In reply to Doug Sherk (:drs) from comment #10)
> (In reply to Szu-Yu Chen [:aknow] from comment #9)
> > Doug,
> > 
> > Could you get an OpenC device and help us dump some log in gaia? We guess it
> > might hit the condition here [1]. Seems there is something wrong in gecko.
> > We need more information to dig into the problem. We would like to know the
> > following value when condition hit.
> 
> I don't have an Open C handy either. I ordered one just now, but it'll be a
> while before it gets to me.
> 
> (In reply to Josh Schmitt from comment #7)
> > Issue repros on the first tinderbox 1.4 Open_C build, unable to get a
> > regression window.
> 
> Could you explain why you couldn't determine a regression window for this?
> Did it just occur on every version between 1.3 and 1.4 or something? This is
> useful information for us.

The regression likely happened before we had Flame builds on pvtbuilds. We know it doesn't reproduce on 1.3 per comment 7.

Comment 12

[Jason Smith [:jsmith]]

(In reply to Szu-Yu Chen [:aknow] from comment #9)
> We cannot find OpenC device here. It seems that the issue only happened on
> 1.4 Open_C build.

FWIW - William managed to get a hold an Open C in the Taipei office. Can you work with him to debug this?

Comment 13

[Szu-Yu Chen [:aknow] (inactive)]

Good. I got an Open C and and the issue is reproducible. Working on it now.

Comment 14

[Szu-Yu Chen [:aknow] (inactive)]

Attachment: Part 1: DOM: Handle dial error and re-order code to avoid multi-thread issue

Comment 15

[Szu-Yu Chen [:aknow] (inactive)]

Attachment: Part 2: Advance the timing of dial response

Comment 16

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

Comment on attachment 8420887 [details] [diff] [review]
Part 1: DOM: Handle dial error and re-order code to avoid multi-thread issue

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

::: dom/telephony/Telephony.cpp
@@ +713,5 @@
>      NS_ERROR("No existing call!");
>      return NS_ERROR_UNEXPECTED;
>    }
>  
>    nsRefPtr<TelephonyCall> callToNotify = GetCall(aServiceId, aCallIndex);

We can just do this way:

callToNotify = (aCallIndex == -1) ? GetOutgoingCall()
                                  : GetCall(aServiceId, aCallIndex);

Comment 17

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

Comment on attachment 8420888 [details] [diff] [review]
Part 2: Advance the timing of dial response

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

Looks good.

Comment 18

[Szu-Yu Chen [:aknow] (inactive)]

(In reply to Hsin-Yi Tsai  [:hsinyi] from comment #16)
> Comment on attachment 8420887 [details] [diff] [review]
> Part 1: DOM: Handle dial error and re-order code to avoid multi-thread issue
> 
> Review of attachment 8420887 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/telephony/Telephony.cpp
> @@ +713,5 @@
> >      NS_ERROR("No existing call!");
> >      return NS_ERROR_UNEXPECTED;
> >    }
> >  
> >    nsRefPtr<TelephonyCall> callToNotify = GetCall(aServiceId, aCallIndex);
> 
> We can just do this way:
> 
> callToNotify = (aCallIndex == -1) ? GetOutgoingCall()
>                                   : GetCall(aServiceId, aCallIndex);

'callError' is a broadcast message to all telephony objects. Only the telephony which dial out the call could find it from GetOutgoingCall(). For other telephony objects, they will get nothing.

Comment 19

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

Comment on attachment 8420887 [details] [diff] [review]
Part 1: DOM: Handle dial error and re-order code to avoid multi-thread issue

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

r=me with comment addressed.

::: dom/telephony/Telephony.cpp
@@ +698,5 @@
>  Telephony::NotifyError(uint32_t aServiceId,
>                         int32_t aCallIndex,
>                         const nsAString& aError)
>  {
> +  // Special handle for dial failure.

I understood your idea which makes sense! Please describe your thought about "why" do we have this special handler, and leave a comment here.

@@ +702,5 @@
> +  // Special handle for dial failure.
> +  if (aCallIndex == -1) {
> +    nsRefPtr<TelephonyCall> call = GetOutgoingCall();
> +    if (call) {
> +      UpdateActiveCall(call, false);

We don't need this. updateActiveCall will be called in removeCall().

Comment 20

[Szu-Yu Chen [:aknow] (inactive)]

Attachment: Part 1#2: DOM: Handle dial error and re-order code to avoid multi-thread issue

Comment 21

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

Comment on attachment 8421478 [details] [diff] [review]
Part 1#2: DOM: Handle dial error and re-order code to avoid multi-thread issue

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

Looks good, thanks!

::: dom/telephony/Telephony.cpp
@@ +698,5 @@
>  Telephony::NotifyError(uint32_t aServiceId,
>                         int32_t aCallIndex,
>                         const nsAString& aError)
>  {
> +  // Special handle for dial failure.

nit: s/handle/handler ?

Comment 22

[Szu-Yu Chen [:aknow] (inactive)]

Attachment: (1.4) [final] Part 1: DOM: Handle dial error and re-order code to avoid multi-thread issue. r=hsinyi

Comment 23

[Szu-Yu Chen [:aknow] (inactive)]

Attachment: (1.4) [final] Part 2: Advance the timing of dial response. r=hsinyi

Comment 24

[Szu-Yu Chen [:aknow] (inactive)]

https://tbpl.mozilla.org/?tree=Try&rev=9aca6b984c83

Comment 25

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/687708c94064
https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/5dded726837c