1170296 - GCF test case 31.4.2.1.1.2 failure: HOLD button stops responding if hold fails for a conference call

Status: RESOLVED FIXED

(Firefox OS Graveyard :: RIL, defect)

Description

[pgravel]

This is very similar to bug 1160325, which was fixed with bug 1161403, but that fix only resolved the single-call case.

Steps to reproduce:
* Have a conference call up
* Try to hold the conference call and have the network reject the hold request
* Now hold button is no longer responsive


One observable difference between the single-call and the conference call case is that with the latter isn't a promise.

Comment 1

[Dylan Oliver [:doliver]]

Hsin-Yi, can your team take a look at this one?

Comment 2

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

Ben, please help this, thanks!

Comment 3

[Johan Lorenzo [:jlorenzo]]

Comms triage: Certification blocker.

Comment 4

[Ben Hsu [:HoPang]]

Attachment: Take |holding| as one of the active states

Comment 5

[Ben Hsu [:HoPang]]

Hi Hsinyi and Phil,

After reproducing this isuue on the emulator, I think the root cause might be the way we determine whether a |conferenceGroup| is active, which makes the dialer send wrong requests after failing to hold the call. Here is a more wordy explaination, after recieving and processing a hold request, the internal state of the |conferenceGroup| becomes |holding| and be taken as inactive, and then when we press the |hold button| on the dialer, according to the inactiveness, it sends out a resume request insteads of a hold request, which makes |hold button| not work as we expected.

Comment 6

[Ben Hsu [:HoPang]]

Hi Phil,

Since I can only try this patch on the emulator, I'm not sure whether the patch works for you. Can you try this patch out first? I'll request a review for it once recieving your confirmation. Thanks.

Comment 7

[Ben Hsu [:HoPang]]

Comment on attachment 8614491 [details] [diff] [review]
Take |holding| as one of the active states

Hi Aknow,

The details are described in comment 5. Can you review this patch?

Comment 8

[Ben Hsu [:HoPang]]

https://treeherder.mozilla.org/#/jobs?repo=mozilla-b2g37_v2_2&exclusion_profile=false

Comment 9

[pgravel]

Thanks, that seems to do the trick.

Comment 10

[Ben Hsu [:HoPang]]

Comment on attachment 8614491 [details] [diff] [review]
Take |holding| as one of the active states

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 1170296
User impact if declined: 
Once we fail to hold a conference call, we can no longer hold the conference call.

Testing completed:
Functionality is manually tested with emulator,
and regression is automatically tested with try-server.

Risk to taking this patch (and alternatives if risky): NA
String or UUID changes made by this patch: NA

Comment 11

[Josh Cheng [:josh]]

Hi Ben,
Thanks for the help.
I am waiting for master landed before 2.2 approval.

Comment 12

[Ben Hsu [:HoPang]]

Hi Josh,

Sorry for such a late response and not making things clear. In fact, the |holding| state is entirely deprecated in mozilla-central since  bug 1131403, so we cannot land this patch to mozilla-central. In other words, this patch is dedicated to branch 2.2.

Comment 13

[Josh Cheng [:josh]]

Comment on attachment 8614491 [details] [diff] [review]
Take |holding| as one of the active states

Thanks Ben!

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/74b9b62fc0cd