Closed Bug 923173 Opened 6 years ago Closed 6 years ago

Trying to set audio.volume.master or audio.volume.bt_sco via mozSettings crashes device

Categories

(Firefox OS Graveyard :: AudioChannel, defect, blocker)

ARM
Gonk (Firefox OS)
defect
Not set
blocker

Tracking

(blocking-b2g:koi+, firefox25 wontfix, firefox26 fixed, firefox27 fixed, b2g-v1.2 fixed)

RESOLVED FIXED
blocking-b2g koi+
Tracking Status
firefox25 --- wontfix
firefox26 --- fixed
firefox27 --- fixed
b2g-v1.2 --- fixed

People

(Reporter: bsilverberg, Assigned: mchen)

References

Details

(Keywords: crash, reproducible, Whiteboard: [gaia-ui-test][qa-automation-blocked][b2g-crash])

Crash Data

Attachments

(2 files, 2 obsolete files)

On today's build [1], when running our Python test suite we found that the phone was crashing during our setup routine. 

We narrowed it down to the step where we attempt to silence the phone by setting the volume using mozSettings. We set the following to 0:

audio.volume...

'master', 'content', 'notification', 'alarm', 'telephony', 'bt_sco'

When setting either 'master' or 'bt_sco' the phone crashes.

A crash report can be found at [2], and the logcat from the run is attached.


[1] Gecko  http://hg.mozilla.org/mozilla-central/rev/e3c84e9f2490
Gaia   73a64d360f0ed135fcca205c6292e9219e085413
BuildID 20131002040206
Version 27.0a1

[2] https://crash-stats.mozilla.com/report/index/5a4ef136-b38b-455a-9651-440822131002
Whiteboard: [gaia-ui-test]
The Javascript error we're seeing in the logcat is:

E/GeckoConsole(  460): Content JS LOG at dummy file:499 in GaiaDataLayer.setSetting: setting audio.volume.master to 0
E/GeckoConsole(  500): [JavaScript Error: "NS_ERROR_INVALID_POINTER: Component returned failure code: 0x80004003 (NS_ERROR_INVALID_POINTER) [nsIMessageSender.sendAsyncMessage]" {file: "chrome://global/content/BrowserElementChildPreload.js" line: 41}]
E/GeckoConsole(  500): [JavaScript Error: "NS_ERROR_INVALID_POINTER: Component returned failure code: 0x80004003 (NS_ERROR_INVALID_POINTER) [nsIContentFrameMessageManager.content]" {file: "chrome://global/content/BrowserElementChildPreload.js" line: 718}]

I'm not sure if this is actually related to the attempt to set the audio channel setting, but removing that code removes the error and stops the phone from crashing.

James, can you cc in whomever would be the proper person to look into this?
Flags: needinfo?(jlal)
Blocks: 801898
Severity: normal → blocker
Whiteboard: [gaia-ui-test] → [gaia-ui-test][qa-automation-blocked]
We might be affected by bug 874508, and bug 923776; I've commented in the latter, asking if it could be causing/related to this bug, just FYI.
Blocks: 924412
Blocks: 924414
What's the need to set master volume to 0?
Also, bt_sco is written by BT earphone, overwrite the value using mozSettings may not be meaningful.
(In reply to Alive Kuo [:alive] from comment #3)
> What's the need to set master volume to 0?

The people in MV office where our phones are located were annoyed by the dialer and camera tests playing noises throughout the office :)
(In reply to Zac C (:zac) from comment #4)
> (In reply to Alive Kuo [:alive] from comment #3)
> > What's the need to set master volume to 0?
> 
> The people in MV office where our phones are located were annoyed by the
> dialer and camera tests playing noises throughout the office :)

Let's try to set audio.volume.notifications and audio.volume.publicnotification to 0?
I'm not sure why it crashes but maybe relevant to BT.
I believe the channels we use came from https://wiki.mozilla.org/WebAPI/AudioChannels#Volume_control, but I'm not sure where 'master' came from, tbh. We can certainly change the list of channels we are silencing to be a more appropriate list, but the fact remains that this worked a few days ago and now causes a crash, which seems to suggest that there is a problem somewhere that is worth investigating.

:Alive: Do you have a recommendation for a different set of channels that would ensure that the phones don't make any noise during device testing?
Currently silent: alarm, notification, publicnotification, content should be O.K.
Master Volume is to control all the channels together by a factor.
Set BT_SCO is meaningless because it's for earphone; that means it doesn't make noise.

If this still crashes then we should find some audio guys to see what happens.

BTW I'm not sure about publicnotificaion channel is implemented or not now.
Attempting to set publicnotification also causes the phone to crash, but using the other three mentioned above seems to do the trick. This unblocks the gaia-ui-tests (once bug 924412 is fixed), but the error is probably still worth investigating.
(In reply to Bob Silverberg [:bsilverberg] from comment #8)
> Attempting to set publicnotification also causes the phone to crash, but
> using the other three mentioned above seems to do the trick. This unblocks
> the gaia-ui-tests (once bug 924412 is fixed), but the error is probably
> still worth investigating.

Yes, the phone shouldn't just crash when we change a settings...
^Marco?
I think alive means Marco Chen... setting needinfo.
Flags: needinfo?(jlal) → needinfo?(mchen)
Given tomorrow's build, this /should/ be resolved by these two landings:

bug 923776
bug 923704
Marco, given the following commit in a local tree, is it correct?

https://github.com/stephendonner/gaia/commit/2eaf0b2876c49241b6f84ac116a14d068c7469e0

Which channels should we be setting?
(In reply to Stephen Donner [:stephend] from comment #12)
> Marco, given the following commit in a local tree, is it correct?
> 
> https://github.com/stephendonner/gaia/commit/
> 2eaf0b2876c49241b6f84ac116a14d068c7469e0
> 
> Which channels should we be setting?

Yes, these 4 channels are the ones we handled in AudioChannelService.cpp.
For crash issue by others name, I will check it. Thanks for reporting the issue.
Flags: needinfo?(mchen)
Duplicate of this bug: 923511
The crash point is 
http://hg.mozilla.org/mozilla-central/file/64b497e6f593/dom/audiochannel/AudioChannelService.cpp#l712

I think that web content can set any name of key through mozSettings web api then the code there cause web content easy to crash the B2G.
Hi,

Please refer to comment 15 to the reason for this patch. Thanks.
Assignee: nobody → mchen
Attachment #814781 - Flags: review?(ehsan)
Attachment #814781 - Attachment is obsolete: true
Attachment #814781 - Flags: review?(ehsan)
Attachment #814783 - Flags: review?(ehsan)
Comment on attachment 814783 [details] [diff] [review]
bug-923173-audio-volume-unreachPatch v1: Change MOZ_ASSUME_UNREACHABLE to MOZ_WARNING

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

This looks good.  Sorry for stepping on your toes, but this shows that the original MOZ_ASSERT there was semantically incorrect!  :-)
Attachment #814783 - Flags: review?(ehsan) → review+
Whiteboard: [gaia-ui-test][qa-automation-blocked] → [gaia-ui-test][qa-automation-blocked][b2g-crash]
Crash Signature: bp-5a4ef136-b38b-455a-9651-440822131002 → [@ mozalloc_abort(char const*) | abort | arena_dalloc | free | moz_free | nsTArray_Impl<ObserverRef, nsTArrayInfallibleAllocator>::RemoveElementsAt(unsigned int, unsigned int)]
(In reply to :Ehsan Akhgari (needinfo? me!) from comment #18)
> Comment on attachment 814783 [details] [diff] [review]
> bug-923173-audio-volume-unreachPatch v1: Change MOZ_ASSUME_UNREACHABLE to
> MOZ_WARNING
> 
> Review of attachment 814783 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> This looks good.  Sorry for stepping on your toes, but this shows that the
> original MOZ_ASSERT there was semantically incorrect!  :-)

Yes, you are right. The original MOZ_ASSERT was still a wrong thing there.
Thanks for your help on it.
Keywords: checkin-needed
Without this patch, any web content with mozsettings permission can be easy to crash the b2g by adding setting name.
blocking-b2g: --- → koi?
(In reply to Marco Chen [:mchen] (Summit 10/03 ~10/08) from comment #21)
> Without this patch, any web content with mozsettings permission can be easy
> to crash the b2g by adding setting name.

Have we confirmed this bug would reproduce on 1.2? That will help figure out if we need this on 1.2.
This replicates on 1.2 aswell.
We were just seeing it on today's Inari/mozilla-aurora nightly build. I can check for Hamachi too but I think I know the answer already :)
https://hg.mozilla.org/mozilla-central/rev/4a5442007b43
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
(In reply to Zac C (:zac) from comment #24)
> This replicates on 1.2 aswell.
> We were just seeing it on today's Inari/mozilla-aurora nightly build. I can
> check for Hamachi too but I think I know the answer already :)

Okay. Blocking+ then.
blocking-b2g: koi? → koi+
You need to log in before you can comment on or make changes to this bug.