1195805 - [Dialer] Phone calls always put the user on Hold; turning off Hold will see it turn back on briefly after

Status: VERIFIED FIXED

(Firefox OS Graveyard :: AudioChannel, defect)

Description

[Oliver Nelson [:oliverthor]]

Attachment: logcat_20150818_0827.txt

Description:
When the user makes a phone call to another device, they will observe the device be put on Hold. When the user attempts to take the device off of Hold, they will observe the phone be put back onto hold briefly after. This issue appears to be a worse regression then this previous issue from last week: bug 1193840 [[Dialer] Hold/pause button is enabled automatically when answering call.], where the phone could be taken off hold and was only a minor inconvenience. The user will always have the phone in hold and thus cannot communicate in a phone call.


Repro Steps:
1) Update a Aries to 20150818012543
2) Call another device
3) Upon answer, observe device be put on hold
4) Tap 'Hold' to take phone off hold

Actual:
Phone will begin on 'Hold' when making a phone call. Taking a phone off hold will return it to 'Hold'.

Expected:
Phone does not begin on hold.

Environmental Variables:
-------------------------------

Device: Aries Master
Build ID: 20150818012543
Gaia: d9d99f32762975a370f1abd34a3512bd6fe29111
Gecko: 90d9b7c391d38ae118865bd87b5d011feee6dded
Gonk: 2916e2368074b5383c80bf5a0fba3fc83ba310bd
Version: 43.0a1 (Master)
Firmware Version: D5803_23.1.A.1.28_NCB.ftf
User Agent: Mozilla/5.0 (Mobile; rv:43.0) Gecko/43.0 Firefox/43.0

Device: Flame Master
BuildID: 20150818030206
Gaia: d9d99f32762975a370f1abd34a3512bd6fe29111
Gecko: 90d9b7c391d38ae118865bd87b5d011feee6dded
Gonk: c4779d6da0f85894b1f78f0351b43f2949e8decd
Version: 43.0a1 (Master) 
Firmware Version: v18D
User Agent: Mozilla/5.0 (Mobile; rv:43.0) Gecko/43.0 Firefox/43.0
-------------------------------


Repro frequency: 5/5
See attached: 
video- https://youtu.be/LciCT45plxE
logcat

Comment 1

[Peter Bylenga [:PBylenga]]

[Blocking Requested - why for this release]:

Comment 2

[Adam Alldredge [:AdamA][Inactive]]

This issue appears to be caused by:
Bug 1195051 -  Creating an AudioContext shows the tab sound indicator even without anything playing

Mozilla-inbound Regression Window

Last Working 
Environmental Variables:
Device: Flame 2.5
Build ID: 20150817051637
Gaia: 47c91ffe7f500ca1aaa60de0aabf4d2429120733
Gecko: d9b9c8130397
Version: 43.0a1 (2.5)
Firmware Version: v18D
User Agent: Mozilla/5.0 (Mobile; rv:43.0) Gecko/43.0 Firefox/43.0

First Broken 
Environmental Variables:
Device: Flame 2.5
BuildID: 20150817055537
Gaia: 47c91ffe7f500ca1aaa60de0aabf4d2429120733
Gecko: 5a66bd178c4c
Version: 43.0a1 (2.5) 
Firmware Version: v18D
User Agent: Mozilla/5.0 (Mobile; rv:43.0) Gecko/43.0 Firefox/43.0

Last Working gaia / First Broken gecko - Issue DOES reproduce
Gaia: 47c91ffe7f500ca1aaa60de0aabf4d2429120733
Gecko: 5a66bd178c4c

First Broken gaia / Last Working gecko - Issue does NOT reproduce
Gaia: 47c91ffe7f500ca1aaa60de0aabf4d2429120733
Gecko: d9b9c8130397

Gecko Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=d9b9c8130397&tochange=5a66bd178c4c

Comment 3

[Jayme Mercado [:JMercado] (Inactive 3/4/2016)]

Ehsan can you take a look at this please?  This seems to be caused by bug 1195051

Comment 4

[(no longer active)]

Which patch in that bug has caused this?  Please bisect to find the actual changeset that has caused this.

Comment 5

[Jayme Mercado [:JMercado] (Inactive 3/4/2016)]

Qanalysts does not currently have the ability to do Gecko bisections.  No-jun can you do this or pass it on to someone who can?

Comment 6

[(no longer active)]

Alastor, since you know what gaia does with audio channels, do you mind taking a look at this to see if anything jumps at you?  Thanks!

Comment 7

[(no longer active)]

(In reply to Jayme Mercado [:JMercado] from comment #5)
> Qanalysts does not currently have the ability to do Gecko bisections. 
> No-jun can you do this or pass it on to someone who can?

I'm specifically interested to know whether part 1 in that bug has caused this, or part 2.  The other two patches cannot be the culprit.  Thanks.

Comment 8

[No-Jun Park [:njpark]]

In the meantime, I'll reapply part 1 and part 2 separately to make a build.  but this might take a while since I need to upload the resulting *.img image. Hopefully alwu would get back in the meantime. I will let you know

Comment 9

[No-Jun Park [:njpark]]

ni?ing Jayme to post the result of the patch that only have Part 1 applied.  If this passes, it's most likely that Part 2 is to blame.

Comment 10

[Jayme Mercado [:JMercado] (Inactive 3/4/2016)]

We have received the part 1 build from No-Jun.  Assigning qawanted to test this now.

Comment 11

[Martin Shuman [:Marty]]

I tested this with the latest build containing part 1 provided by No-Jun. The issue DOES reproduce on this build. The phone will begin on 'Hold' when making a phone call. Attempting to turn off hold will return it to 'Hold' moments later.

Comment 12

[(no longer active)]

Thanks for testing!

I guess what has happened here is that gaia expects a mozinterruptend event when an AudioContext is created for some reason...  I have written a patch that should revert that part of the behavior.  Can you please test it?  It should apply cleanly on trunk.  I'll attach it in a second.

Comment 13

[(no longer active)]

Attachment: Notify the audio channel service when an AudioContext gets created, but wait for the actual audio playback before notifying about playback

Comment 14

[No-Jun Park [:njpark]]

I have made the build, QAnalysts, please ping me when you're in to test this.

Comment 15

[Alastor Wu [:alwu]]

In Gaia call_handler.js, if there gets the moz-interrupt-begin, the call would be suspended [1].
However, these logic would be removed in the bug1185442.

[1] https://github.com/mozilla-b2g/gaia/blob/master/apps/callscreen/js/calls_handler.js#L826

Comment 16

[Johan Lorenzo [:jlorenzo]]

Comms triage: Smoke test blocker

Comment 17

[(no longer active)]

(In reply to Alastor Wu [:alwu] from comment #15)
> In Gaia call_handler.js, if there gets the moz-interrupt-begin, the call
> would be suspended [1].
> However, these logic would be removed in the bug1185442.

That would be super helpful.  Is that something that can be done soon, and also backported to Aurora?  I prefer to avoid the hack in the patch that I have attached here if gaia can be fixed to not require this...

Comment 18

[Oliver Nelson [:oliverthor]]

Issue REPRODUCES with the build provided by njpark in comment 14 (with the patch by Ehsan from comment 13).
Results: Phone will begin on 'Hold' when making a phone call. Taking a phone off hold will return it to 'Hold'.

Device: Aries Master
BuildID: 20150818233934
Gaia: 9bc579e7bcb269077e8116b4031440eb9ebe61ec
Gecko: Unknown -- Due to patch
Gonk: 2916e2368074b5383c80bf5a0fba3fc83ba310bd
Version: 43.0a1 (Master) 
Firmware Version: D5803_23.1.A.1.28_NCB.ftf
User Agent: Mozilla/5.0 (Mobile; rv:43.0) Gecko/43.0 Firefox/43.0
Repro: 5/5

Comment 19

[(no longer active)]

Just to double check another issue that I found yesterday, can you please test the fix to bug 1196062 (which I just landed on inbound) separately from the patch attached here as well?

Comment 20

[Andrea Marchesini [:baku]]

Comment on attachment 8649583 [details] [diff] [review]
Notify the audio channel service when an AudioContext gets created, but wait for the actual audio playback before notifying about playback

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

It seems a bit too complex for what it needs to do. What about this approach:

1. mNotificationAudioChannel => false by default.

2. in InputMutedRunnable call:
  destinationNode->EnableAudioChannelNotification();
  destinationNode->InputMuted(mInputMuted);

3. EnableAudioChannelNotification does:
  MOZ_ASSERT(mAudioChannelAgent);
  mAudioChannelAgent->DestroyAudioChannel();
  mNotificationAudioChannel = true;
  CreateAudioChannelAgent();

::: dom/media/webaudio/AudioDestinationNode.cpp
@@ +738,5 @@
>  {
>    MOZ_ASSERT(Context() && !Context()->IsOffline());
>  
>    if (!mAudioChannelAgent) {
> +    return;

This change seems wrong: we don't continue if we have to mute a null AudioChannelAgent. here you do it.

Comment 21

[(no longer active)]

Comment on attachment 8649583 [details] [diff] [review]
Notify the audio channel service when an AudioContext gets created, but wait for the actual audio playback before notifying about playback

Well, this doesn't fix the bug anyway, and I don't think we should take it any more.  We need to fix gaia.

Comment 22

[Pi Wei Cheng [:piwei] (inactive)]

QAwanted to address comment 19.

Comment 23

[Pi Wei Cheng [:piwei] (inactive)]

Tested on latest mozilla inbound build with fix for bug 1196062 and issue is still occurring. DUT gets put on hold automatically after call connected.

Device: Aries
BuildID: 20150819161815
Gaia: 8f77edf3ac39d36f6df0f5517223d3ed35ed89e0
Gecko: 7936d101d4f593fbceb1ace9498be2a9dfe433e3
Gonk: 2916e2368074b5383c80bf5a0fba3fc83ba310bd
Version: 43.0a1 (2.5 Master) 
Firmware Version: D5803_23.1.A.1.28_NCB.ftf
User Agent: Mozilla/5.0 (Mobile; rv:43.0) Gecko/43.0 Firefox/43.0

Comment 24

[Kevin Grandon :kgrandon]

I have a patch for this over in bug 1185442 I think, so assigning to myself for now.

Comment 25

[Kevin Grandon :kgrandon]

I think this should be fixed now that bug 1185442 is landed.

Comment 26

[Oliver Nelson [:oliverthor]]

I was supplied a build from :njpark that includes the fix mentioned by Kevin Grandon in comment 25
This build does not reproduce this issue; I will return to verify this issue once this lands in nightly.

Results: Phone call does not begin on hold.

Device: Aries 2.5
BuildID: 20150820113949
Gaia: c6705f739fb605031eb2a0b943ba55c64bee5a03
Gecko: Unknown
Gonk: 2916e2368074b5383c80bf5a0fba3fc83ba310bd
Version: 43.0a1 (2.5) 
Firmware Version: D5803_23.1.A.1.28_NCB.ftf
User Agent: Mozilla/5.0 (Mobile; rv:43.0) Gecko/43.0 Firefox/43.0
Repro: 5/5

Comment 27

[John Thomas [:Johnt]]

This issue is now verified fixed for Aries Master.
Result: Call is no longer answered with hold enabled, or becomes on hold after hold setting is disabled.

Environmental Variables:
Device: Aries 2.5 KK
BuildID: 20150820203627
Gaia: c6705f739fb605031eb2a0b943ba55c64bee5a03
Gecko: 7a6cd1616542b9fb22274fb315502fce9bad2a6c
Gonk: 2916e2368074b5383c80bf5a0fba3fc83ba310bd
Version: 43.0a1 (2.5) 
Firmware Version: D5803_23.1.A.1.28_NCB.ftf
User Agent: Mozilla/5.0 (Mobile; rv:43.0) Gecko/43.0 Firefox/43.0