GCF TC 31.3.2.1 HOLD button stops responding if hold request fails

RESOLVED DUPLICATE of bug 1161403

Status

defect
RESOLVED DUPLICATE of bug 1161403
4 years ago
4 years ago

People

(Reporter: pgravel, Assigned: gsvelto)

Tracking

unspecified
2.2 S12 (15may)
ARM
Gonk (Firefox OS)
Dependency tree / graph

Firefox Tracking Flags

(blocking-b2g:2.2+)

Details

(Whiteboard: [caf priority: p2][CR 830265])

Attachments

(1 attachment)

2.10 MB, application/x-7z-compressed
Details
(Reporter)

Description

4 years ago
From 3GPP TS 51.010, section 31.3.2.1

GCF TC 31.3.2.1 is to test the correct operation of the HOLD procedure, to test handset reaction to the following messages sent in response to the HOLD message.
1.	HOLD Reject
2.	HOLD ACK 

Scenario for TC 31.3.2.1:
1.	Have a active phone call
2.	Press HOLD button and have network rejects it
3.	Hold button no longer responds to presses


It seems like the UI and telephony helpers assume all hold requests will succeed and so the UI gets in the incorrect state when the hold request fails.
Whiteboard: [CR 830265]
Whiteboard: [CR 830265] → [caf priority: p2][CR 830265]

Comment 1

4 years ago
Hi! Sean,

Have you fixed this before?

--
Keven
Flags: needinfo?(selee)
Hi Keven, Sorry that I have not fixed this before.
Flags: needinfo?(selee)

Comment 3

4 years ago
Hi Ben:
 Could you please help confirm if any error will be returned to Gaia if hold call fail. 

https://github.com/mozilla/gecko-dev/blob/b2g37_v2_2/dom/telephony/TelephonyCall.cpp#L286
Flags: needinfo?(bhsu)

Comment 4

4 years ago
Sure, I am gald to help.

Now, when a call is failed to be held, Gecko will only reject the promise returned by the hold request, and there won't be any error event dispatched to the TelephonyCall. In fact, the error event defined in |TelephonyCall.webidl| is designed to notify an unexpected disconnected call. On the other hand, I think gaia doesn't handle the rejected promise caused by failing to hold a call, as the following link shows. Maybe it's the root cause of this issue.

https://github.com/mozilla-b2g/gaia/blob/master/apps/callscreen/js/calls_handler.js#L601
Flags: needinfo?(bhsu)
Ben, thanks for your information.

Could someone provide some suggestions about the behavior when holding rejected?
(Assignee)

Comment 6

4 years ago
This is a known problem in the callscreen, we had already filed bug 1077073 for this.
See Also: → 1077073
(Assignee)

Comment 7

4 years ago
I've checked our Gaia code and I need more information on this to figure out where the problem is:

- First of all I'd need a RIL log since I can't reproduce the issue locally using the emulator

- I would also need to know what's happening exactly to the button, is it showing the call being held? Or is it disabled?

- Looking at the code I'm not sure what's causing this because while we don't check for error conditions when putting the call on hold we do adjust the status of the button only in response to oncallschanged events. What I would expect is the display to show the call as being on hold even when it's not (which we don't check properly) but the button should be functional nonetheless.
Flags: needinfo?(pgravel)
Assignee: nobody → gsvelto
Target Milestone: --- → 2.2 S12 (15may)
(Reporter)

Comment 8

4 years ago
Posted file 31.3.2.1.7z
Attached zip file containing logs+video

Just to reiterate the expected behavior:
1) try to hold call
2) network rejects it
3) try to hold call again
4) network accepts it
5) UI shows the call being held

What we're currently seeing with some details:
1) try to hold call
2) Speaker and Mute buttons are immediately disabled
3) telephony service gets HoldCall() request
4) network rejects hold request
5) telephonyService send nsITelephonyCallback->NotifyError()
6) Hold button becomes unresponsive (telephonyService no longer gets any HoldCall() requests no matter how often the button is pressed). Mute and Speaker buttons remain disabled.
Flags: needinfo?(pgravel)

Comment 9

4 years ago
(In reply to pgravel from comment #8)
> Created attachment 8601148 [details]
> 31.3.2.1.7z
> 
> Attached zip file containing logs+video
> 
> Just to reiterate the expected behavior:
> 1) try to hold call
> 2) network rejects it
> 3) try to hold call again
> 4) network accepts it
> 5) UI shows the call being held
> 
> What we're currently seeing with some details:
> 1) try to hold call
> 2) Speaker and Mute buttons are immediately disabled
> 3) telephony service gets HoldCall() request
> 4) network rejects hold request
> 5) telephonyService send nsITelephonyCallback->NotifyError()
> 6) Hold button becomes unresponsive (telephonyService no longer gets any
> HoldCall() requests no matter how often the button is pressed). Mute and
> Speaker buttons remain disabled.

The major issue here should be gecko RIL side.
the telephony object stay in HOLDING state (expected ACTIVE) after hold fail, that cause STR#6 in comment 8.

Note: You can reference Bug 1106999 (comment 35, 37 mainly). But bug 1106999 is a device branch we handled it as quick fix.
(In reply to shawn ku [:sku] from comment #9)
> (In reply to pgravel from comment #8)
> > Created attachment 8601148 [details]
> > 31.3.2.1.7z
> > 
> > Attached zip file containing logs+video
> > 
> > Just to reiterate the expected behavior:
> > 1) try to hold call
> > 2) network rejects it
> > 3) try to hold call again
> > 4) network accepts it
> > 5) UI shows the call being held
> > 
> > What we're currently seeing with some details:
> > 1) try to hold call
> > 2) Speaker and Mute buttons are immediately disabled
> > 3) telephony service gets HoldCall() request
> > 4) network rejects hold request
> > 5) telephonyService send nsITelephonyCallback->NotifyError()
> > 6) Hold button becomes unresponsive (telephonyService no longer gets any
> > HoldCall() requests no matter how often the button is pressed). Mute and
> > Speaker buttons remain disabled.
> 
> The major issue here should be gecko RIL side.
> the telephony object stay in HOLDING state (expected ACTIVE) after hold
> fail, that cause STR#6 in comment 8.
> 
> Note: You can reference Bug 1106999 (comment 35, 37 mainly). But bug 1106999
> is a device branch we handled it as quick fix.

Let me provide some updates here.
Gecko has changed telephonyCall.hold API into returning a promise in bug 1077075. But corresponding implementation on gaia isn't completed, bug 1077073.
(In reply to Hsin-Yi Tsai [:hsinyi] from comment #10)
> (In reply to shawn ku [:sku] from comment #9)
> > (In reply to pgravel from comment #8)
> > > Created attachment 8601148 [details]
> > > 31.3.2.1.7z
> > > 
> > > Attached zip file containing logs+video
> > > 
> > > Just to reiterate the expected behavior:
> > > 1) try to hold call
> > > 2) network rejects it
> > > 3) try to hold call again
> > > 4) network accepts it
> > > 5) UI shows the call being held
> > > 
> > > What we're currently seeing with some details:
> > > 1) try to hold call
> > > 2) Speaker and Mute buttons are immediately disabled
> > > 3) telephony service gets HoldCall() request
> > > 4) network rejects hold request
> > > 5) telephonyService send nsITelephonyCallback->NotifyError()
> > > 6) Hold button becomes unresponsive (telephonyService no longer gets any
> > > HoldCall() requests no matter how often the button is pressed). Mute and
> > > Speaker buttons remain disabled.
> > 
> > The major issue here should be gecko RIL side.
> > the telephony object stay in HOLDING state (expected ACTIVE) after hold
> > fail, that cause STR#6 in comment 8.
> > 
> > Note: You can reference Bug 1106999 (comment 35, 37 mainly). But bug 1106999
> > is a device branch we handled it as quick fix.
> 
> Let me provide some updates here.
> Gecko has changed telephonyCall.hold API into returning a promise in bug
> 1077075. But corresponding implementation on gaia isn't completed, bug
> 1077073.

Oh, I just saw Gabriele already indicated this.
After more discussion with Ben, comment 9 points to the root cause of this bug. We need to address that in TelephonyCall.cpp.
Comms triage: Certification blocker on a new feature implemented in 2.2.
blocking-b2g: 2.2? → 2.2+
(In reply to Johan Lorenzo [:jlorenzo] (QA) from comment #13)
> Comms triage: Certification blocker on a new feature implemented in 2.2.
See Also: → 1160328
See Also: 11603281161403

Updated

4 years ago
Depends on: 1161403

Comment 15

4 years ago
Hi Ben:
 Could you please help check how much effort it will take to fix in telephonyCall.cpp?

It looks like we need this get fixed on 2.2 after meeting with partner.
Flags: needinfo?(bhsu)
(In reply to shawn ku [:sku] from comment #15)
> Hi Ben:
>  Could you please help check how much effort it will take to fix in
> telephonyCall.cpp?
> 
> It looks like we need this get fixed on 2.2 after meeting with partner.

The gecko patch is on bug 1161403, under review.
Flags: needinfo?(bhsu)
(Assignee)

Comment 17

4 years ago
Alright, I'll put together a gaia patch ASAP on top of bug 1161403.

Comment 18

4 years ago
Gabriele, as I understand there is no Gaia patch required once the fix for bug 1161403 lands. Hsin-Yi, is that correct?
(In reply to Anshul from comment #18)
> Gabriele, as I understand there is no Gaia patch required once the fix for
> bug 1161403 lands. Hsin-Yi, is that correct?
Sorry that my comment 16 sounds confusing. Yes, as Anshul mentioned, I think the gecko patch is enough to fix this GCF failure.
(In reply to Hsin-Yi Tsai [:hsinyi] from comment #19)
> Yes, as Anshul mentioned, I think
> the gecko patch is enough to fix this GCF failure.

Should we move the CAF dependency over to bug 1161403 instead?
Flags: needinfo?(anshulj)
(Assignee)

Comment 21

4 years ago
(In reply to Anshul from comment #18)
> Gabriele, as I understand there is no Gaia patch required once the fix for
> bug 1161403 lands. Hsin-Yi, is that correct?

There should be no need of a gaia patch to fix the basic functionality, but one is required to notify the user of the error.

Comment 22

4 years ago
(In reply to Dylan Oliver [:doliver] from comment #20)
> (In reply to Hsin-Yi Tsai [:hsinyi] from comment #19)
> > Yes, as Anshul mentioned, I think
> > the gecko patch is enough to fix this GCF failure.
> 
> Should we move the CAF dependency over to bug 1161403 instead?
Sure, just marking it dup should take care of that.
Flags: needinfo?(anshulj)
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1161403
No longer blocks: CAF-v2.2-metabug

Updated

4 years ago
Depends on: 1082588
Blocks: 977588
You need to log in before you can comment on or make changes to this bug.