862409 - [Audio] Audio_Data API is broken When dialer app tried to send a keytone

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Marco Chen [:mchen]]

Reproduce steps:

Gecko version: changeset e22145c3b33e in m-c.
Gaia version: no limited.

1. Goto dialer app.
2. Press any key in dialer app.

Expect result: Key tone is fired.
Actual result: no sound appeared.

Log: No any related logs appeared on adb logcat or console.

Note: It may be a regression by Bug 858211.

Comment 1

[Boris Zbarsky [:bzbarsky]]

So just to check, do we know whether the call is failing or whether it's just ending up with the wrong data or something else?

Comment 2

[Marco Chen [:mchen]]

I added debug messages before & after the point as below and I can just the message before mozSetup().

https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/dialer/js/keypad.js#L101

Comment 3

[Boris Zbarsky [:bzbarsky]]

OK.  If you try/catch around that mozSetup call, and log the exception in the catch, what do you get?

Comment 4

[Marco Chen [:mchen]]

E/GeckoConsole(  611): Content JS ERROR at app://communications.gaiamobile.org/dialer/js/keypad.js:104 in tp_start: mozSetup: TypeError: this._audio.mozSetup is not a function tp_start@app://communications.gaiamobile.org/dialer/js/keypad.js:101
E/GeckoConsole(  611): kh_keyHandler@app://communications.gaiamobile.org/dialer/js/keypad.js:501
E/GeckoConsole(  611): emitEvent@app://communications.gaiamobile.org/shared/js/mouse_event_shim.js:239
E/GeckoConsole(  611): handleTouchStart@app://communications.gaiamobile.org/shared/js/mouse_event_shim.js:93

Comment 5

[Boris Zbarsky [:bzbarsky]]

Thanks, that helps a lot.  This is a bug the patch for bug 858211 for sure.

It made the existence of these properties pref-controlled, via the "media.audio_data.enabled" preference.  Which they sort of already were, like so:

23 static bool
24 IsAudioAPIEnabled()
25 {
26   return mozilla::Preferences::GetBool("media.audio_data.enabled", true);
27 }

but note the default value being passed in there.  So if the preference is not set, IsAudioAPIEnabled() returns true, but the bindings assume that not set means false.  And the preference is not set anywhere.

It looks like the only test we have for this stuff has this right at the beginning:

23   try {
24     a1.mozSetup(channels, rate);
25   } catch (ex) {
26     todo(false, "Audio hardware is disabled, can't test audio write API");
27     SimpleTest.finish();
28     return;
29   }

which means that just disabling audio API completely still passes tests.

What we should probably do is add the pref to all.js and modify the test to check that the API is there at all (using "in" or something) before doing that bailout.

Comment 6

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: Patch v1

Comment 7

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 738527 [details] [diff] [review]
Patch v1

r=me

Comment 8

[Marco Chen [:mchen]]

Hi Boris & Ms2ger,

Thanks for your help here.

Comment 9

[Boris Zbarsky [:bzbarsky]]

Thank you for your help getting this debugged!

Comment 10

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

https://hg.mozilla.org/mozilla-central/rev/ffd56766e73d