1088668 - Dialer always new AudioContext whenever launching, cause serious power consumption

Status: RESOLVED DUPLICATE of bug 834530

(Firefox OS Graveyard :: Gaia::Dialer, defect)

Description

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

Launching Dialer app, AudioContext will be created. If user dials out numbers and make some calls, the speaker will be turned on forever, it can drain the battery, even user manually kill Dialer app, the speaker still turned on.

STR:
1. Launch Dialer app
2. Silent sound will be played

Comment 1

[Gabriele Svelto [:gsvelto]]

(In reply to Shawn Huang [:shawnjohnjr] from comment #0)
> Launching Dialer app, AudioContext will be created. If user dials out
> numbers and make some calls, the speaker will be turned on forever, it can
> drain the battery, even user manually kill Dialer app, the speaker still
> turned on.

This means that it's not the Dialer that's leaking the context, it must be Gecko as highlighted in bug 1079810 comment 32. The fix for bug 1079810 just postponed the first time the object gets leaked from the callscreen launch (at startup) to the first time an AudioContext is created in the dialer. BTW if the leak is truly in the DevTools then this needs to be moved into another component.

Comment 2

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

(In reply to Gabriele Svelto [:gsvelto] from comment #1)
> (In reply to Shawn Huang [:shawnjohnjr] from comment #0)
> This means that it's not the Dialer that's leaking the context, it must be
> Gecko as highlighted in bug 1079810 comment 32. The fix for bug 1079810 just
> postponed the first time the object gets leaked from the callscreen launch
> (at startup) to the first time an AudioContext is created in the dialer. BTW
> if the leak is truly in the DevTools then this needs to be moved into
> another component.
I'm sorry i did not check this problem in detail and figure out the root cause, so I choose component to dialer first. I also saw sometimes even without playing any DTMF tone, speaker turned on after launching Dialer, maybe something else cause leak.

Comment 3

[Randy Lin [:rlin] - Be Mozillian on 2015/01/26]

I try to create an AudioContext object in js and found it would like to create an audioTrack and keeps it alive.
Do we have better way to avoid this problem? Otherwise we will spend a lot of time to find who mis-use this API and drain battery on b2g device.

Comment 4

[Paul Adenot (:padenot)]

A new API will come up soon that will let people "suspend" an AudioContext. In the meantime, we need to make sure nobody keeps an AudioContext alive.

Comment 5

[Gabriele Svelto [:gsvelto]]

I'm greatly simplifying the audio management in the dialer/callscreen/emergency-call app, you might want to try with my WIP patches there applied to see if this reproduces again. With those patches we should *always* trash the AudioContext correctly when we don't need it. We also instance it lazily so it shouldn't be created unless it's actually needed.

Comment 6

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

[Blocking Requested - why for this release]:
Drain the batter under some situation

Comment 7

[Doug Sherk (:drs) (inactive)]

Attachment: PoC: Trash audio context at the end of every tone playing

(In reply to Shawn Huang [:shawnjohnjr] from comment #0)
> Launching Dialer app, AudioContext will be created. If user dials out
> numbers and make some calls, the speaker will be turned on forever, it can
> drain the battery, even user manually kill Dialer app, the speaker still
> turned on.

I'm unable to repro the exact consequences described here. The dialer is definitely creating an AudioContext and keeping it alive as long as it's foregrounded, even if the screen is off. But killing the Dialer also kills the AudioContext for me.

Shawn, can you still repro this?

(In reply to Gabriele Svelto [:gsvelto] from comment #5)
> I'm greatly simplifying the audio management in the
> dialer/callscreen/emergency-call app, you might want to try with my WIP
> patches there applied to see if this reproduces again. With those patches we
> should *always* trash the AudioContext correctly when we don't need it. We
> also instance it lazily so it shouldn't be created unless it's actually
> needed.

Unfortunately, I don't think that this will solve the problem directly. For starters, the AudioContext only gets destroyed once a GC pass has happened. This could take several seconds for an app that has just been backgrounded. I don't see a reasonable solution here, though.

Also, the Dialer's AudioContext for the keypad is still kept alive if the screen is off, or we're on a tab other than the keypad. I've attached a PoC patch that destroys the AudioContext at the end of each tone play. This runs smoothly on my Flame, but I don't know how it works on other devices. Gabriele, Paul, what do you think of this?

Comment 8

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

Note:
This comment is based on the original code without applying patch as mentioned in Comment 7.
I spent some time to find out the specific reproduce steps, i'm trying to reproduce it from boot-up, so the ref-count will be absolutely 0.

1. Boot into home screen
2. Launch Dialer
3. Check dumpsys, and verify ref-count should be 0:
 Stream volume refCount muteCount
 00     1.000     00       00
 01     0.191     00       00
 02     0.197     00       00
4. Press home button return to Home screen
5. Check dumpsys and verify ref-count should be 0:
 Stream volume refCount muteCount
 00     1.000     00       00
 01     0.501     01       00 <------------Speaker turned on

+++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Yes, you're right. If Dialer launches, and turn speaker on, killing Dialer can release AudioContext.
However, I found I hit another bug make me feel Dialer still keeps speaker on, but actually it's not.

Just simply press 'Power Key' to wake up screen (it doesn't happen if you press Power Key to turn off screen), which make speaker turn on. But I guess this is something related to system app. And this probably another problem.

Comment 9

[Gabriele Svelto [:gsvelto]]

(In reply to Doug Sherk (:drs) (use needinfo?) from comment #7)
> I'm unable to repro the exact consequences described here. The dialer is
> definitely creating an AudioContext and keeping it alive as long as it's
> foregrounded, even if the screen is off. But killing the Dialer also kills
> the AudioContext for me.
> 
> Shawn, can you still repro this?

I'd also like to know if it repros with the patches from bug 834530 applied.

> Unfortunately, I don't think that this will solve the problem directly. For
> starters, the AudioContext only gets destroyed once a GC pass has happened.
> This could take several seconds for an app that has just been backgrounded.
> I don't see a reasonable solution here, though.

Actually as soon as an app goes into the background it's GC'd though at a low priority. I know because hacked that code in gecko :)

> Also, the Dialer's AudioContext for the keypad is still kept alive if the
> screen is off, or we're on a tab other than the keypad.

This shouldn't happen because turning the screen off affects visibility. As per above I'd like to know if this repros after bug 834530 which greatly simplifies visibility handling.

> I've attached a PoC
> patch that destroys the AudioContext at the end of each tone play. This runs
> smoothly on my Flame, but I don't know how it works on other devices.
> Gabriele, Paul, what do you think of this?

I'm not too fond of the idea but if that's what it takes to lower power consumption then I'm OK with it.

Comment 10

[Gabriele Svelto [:gsvelto]]

Comment on attachment 8512220 [details] [diff] [review]
PoC: Trash audio context at the end of every tone playing

This is not going to work as-is because playSequence() invokes _startAt() multiple times. You should handle trashing the pointer in playSequence() and start() rather than in _startAt(). Another quirk is that you want to clear the timer everytime we start a new sound, right now if I play a sound with a duration and then one w/o the context might get trashed before I get a chance to call stop() for the second sound.

Comment 11

[Doug Sherk (:drs) (inactive)]

Comment on attachment 8512220 [details] [diff] [review]
PoC: Trash audio context at the end of every tone playing

I got the feedback that I needed from Gabriele.

Comment 12

[Star Cheng [:scheng]]

(In reply to Shawn Huang [:shawnjohnjr] from comment #8)
> Note:
> This comment is based on the original code without applying patch as
> mentioned in Comment 7.
> I spent some time to find out the specific reproduce steps, i'm trying to
> reproduce it from boot-up, so the ref-count will be absolutely 0.
> 
> 1. Boot into home screen
> 2. Launch Dialer
> 3. Check dumpsys, and verify ref-count should be 0:
>  Stream volume refCount muteCount
>  00     1.000     00       00
>  01     0.191     00       00
>  02     0.197     00       00
> 4. Press home button return to Home screen
> 5. Check dumpsys and verify ref-count should be 0:
>  Stream volume refCount muteCount
>  00     1.000     00       00
>  01     0.501     01       00 <------------Speaker turned on
> 
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> Yes, you're right. If Dialer launches, and turn speaker on, killing Dialer
> can release AudioContext.
> However, I found I hit another bug make me feel Dialer still keeps speaker
> on, but actually it's not.
> 
> Just simply press 'Power Key' to wake up screen (it doesn't happen if you
> press Power Key to turn off screen), which make speaker turn on. But I guess
> this is something related to system app. And this probably another problem.

I can duplicate it with latest SW. 

STR:
1. press power key to lock the screen
2. press power key to turn on the screen(need to slide lock bar to unlock screen )

set break point in the AudioContext constructor [1]. And get the dump info of js stack.

(gdb) printf "%s", PrintJSStack()
0 tp_ensureAudio() ["app://callscreen.gaiamobile.org/shared/js/dialer/tone_player.js":28]
    this = [object Object]
1 visibilitychanged([object Event]) ["app://callscreen.gaiamobile.org/js/index.js":22]
    this = [object HTMLDocument]

While the visibility changed, call screen will use a audio context to do something. 


[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1079810#c12

Comment 13

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

triage: it's blocking as it drains battery and there are STRs.

Comment 14

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

Hey, if this is not GAIA:Dialer, please go ahead and edit the component.

Comment 15

[Gabriele Svelto [:gsvelto]]

Star, could you please try to reproduce with the patches from bug bug 834530 applied? I think the problem should go away with them even with the most recent STR.

Comment 16

[Star Cheng [:scheng]]

(In reply to Gabriele Svelto [:gsvelto] from comment #15)
> Star, could you please try to reproduce with the patches from bug bug 834530
> applied? I think the problem should go away with them even with the most
> recent STR.

Great! It works !!

Comment 17

[Doug Sherk (:drs) (inactive)]

Bug 834530 hasn't landed yet, but once it does, we should verify that it fixes this.