We need the capability to manage the notification audio channel of System app in the AudioChannelManager(Bug 1100822). Current proposals: 1. Add a notification app to play the notification sound. 2. Send the allowed audio channels(sysApp.allowedAudioChannels) to System app with `mozChromeEvent`. 3. The all audio channels of System app could be played by default.

We need to handle the below audio in System app. 1. gaia/apps/system/js/accessibility.js: 266: this.sounds[aSoundKey] = new Audio(this.soundURLs[aSoundKey]); 2. gaia/apps/system/js/carrier_info_notifier.js: 66: var ringtonePlayer = new Audio(); 3. gaia/apps/system/js/dialer_agent.js: 47: var player = new Audio(); 4. gaia/apps/system/js/icc_worker.js: 193: var tonePlayer = new Audio(); /5. gaia/apps/system/js/notifications.js: 549: var ringtonePlayer = new Audio(); 6. gaia/apps/system/lockscreen/js/lockscreen.js: 663: var unlockAudio = new Audio('/resources/sounds/unlock.opus');

Figuring out a good way to manage the audio channels in System app...

Hi Alastor, Alive, Baku, and Dominic. AudioChannelManager[1](the audio channel management module in System app) cannot manage System app's `allowedAudioChannels`. Because we just cannot access the `allowedAudioChannels` directly in System app. In my opinion, we should find a way to access the `allowedAudioChannels` in System app. The below could be a solution for it. Solution: We could add a new mozChromeEvent called `system-audio-channels-initialized` to pass the `allowedAudioChannels` to System app. (or any other way to pass the `allowedAudioChannels` to System app) Then we could just add the below code to manage the `allowedAudioChannels` in System app. Simple and easy. ``` function AudioChannelManager() { this.systemAudioChannels = new Map(); } AudioChannelManager.prototype = { systemAudioChannels: null, handleMozChromeEvent: function(evt) { switch(evt.detail.type) { case 'system-audio-channels-initialized': var audioChannels = evt.detail.allowedAudioChannels; this.systemAudioChannels.forEach((audioChannel) => { this.audioChannels.set( audioChannel.name, // AudioChannelController[2] is a part of our audio channel management module in System app. new AudioChannelController({ instanceID: 'systemAppID' }, audioChannel) ); }); break; } } }; ``` How do you guys think about the above idea? [1]: https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/audio_channel_manager.js [2]: https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/audio_channel_controller.js

:evanxd, can you tell me why the system app cannot access the allowedAudioChannels? In theory if you have access to the mozbrowser iframe, from there you can play with allowedAudioChannels.

Because the mozbrowser iframe of System app is created by shell.js[1]. So we can access the System app's allowedAudioChannels in shell.js, not in the System app. :baku, or do you know any way to access the (System app's) allowedAudioChannels in System app? [1]: https://dxr.mozilla.org/mozilla-central/source/b2g/chrome/content/shell.js#307-313

Our original intention of the new audio channel management is to use the system app to manage all the audio channels under gaia, in theory, system app should manage its own audio channels and those logic should also in system app. So if it's possible for system app to get its allowedAudioChannels then manage them, we can deal with those audio elements/context that directly in system app, without moving them outside of system app and probably wrapped in iframes. But looks like if we want to use the system app's allowedAudioChannels, it can only be in shell.js? if so, I am assuming there will be some extra/duplicate logic in shell.js, which does exactly the same logic that system app's AudioChannelManager does. This should be Evan's question.

I would like to suggest to bypass the mozbrowser events in shell.js instead of exposing the audioChannel object to the app. But if gecko dev has different opinions let's listen.

I'm lost. Why do we want to use the System app's allowedAudioChannels? Does the system app play some audio? Any app, any iframe mozbroser has its own allowedAudioChannels. I guess that the system app has access to each iframe of each app running. If you want to mute the app A, you should mute any iframeAppA.allowedAudioChannels. If you mute the allowedAudioChannels of the system app you are muting just the system app. Is this what you mean? Plus, the system app cannot have access to its own allowedAudioChannels. We have to go up to the shell.js. But really I don't understand why you need this. Can you tell me more?

Hi, Baku, Because of some reasons, Gaia don't want the notification belong to the specific app. Therefore, the notification would be playback by the system app. That is why we need to pass the events of the shell.js to the system app.

(In reply to Andrea Marchesini (:baku) from comment #8) > I'm lost. Why do we want to use the System app's allowedAudioChannels? > Does the system app play some audio? Yes, system app play some audio. > Any app, any iframe mozbroser has its own allowedAudioChannels. I guess that > the system app has access to each iframe of each app running. If you want to > mute the app A, you should mute any iframeAppA.allowedAudioChannels. > If you mute the allowedAudioChannels of the system app you are muting just > the system app. We're trying to find a good way to manage the audio channels in System app. > Is this what you mean? Plus, the system app cannot have access to its own > allowedAudioChannels. We have to go up to the shell.js. But really I don't > understand why you need this. > Can you tell me more? We just need to manage the audio channels in System app.

We would like to try to use mozChromeEvent and mozContentEvent to control the allowed audio channels in System app, if we can not access its own audio channel in System app yet. How about defining the below mozContentEvent and mozChromeEvent to do that? # mozContentEvent * system-audiochannel-list: Get the list of allowed audio channel name. * system-audiochannel-play: Play the audio channel. * system-audiochannel-pause: Pause the audio channel. * system-audiochannel-volume: Set the volume of the audio channel. # mozChromeEvent * system-audiochannel-list: Get the list of allowed audio channel name. * system-audiochannel-onsuccess: If it is successful, play, pause, set volume for an audio channel. * system-audiochannel-onerror: If it is failed, play, pause, set volume for an audio channel.

(In reply to Evan Tseng [:evanxd][:愛聞插低] from comment #11) > We would like to try to use mozChromeEvent and mozContentEvent to control > the allowed audio channels in System app, if we can not access its own audio > channel in System app yet. > > How about defining the below mozContentEvent and mozChromeEvent to do that? > > # mozContentEvent > * system-audiochannel-list: Get the list of allowed audio channel name. > * system-audiochannel-play: Play the audio channel. > * system-audiochannel-pause: Pause the audio channel. > * system-audiochannel-volume: Set the volume of the audio channel. > > # mozChromeEvent > * system-audiochannel-list: Get the list of allowed audio channel name. All above is fine > * system-audiochannel-onsuccess: If it is successful, play, pause, set > volume for an audio channel. > * system-audiochannel-onerror: If it is failed, play, pause, set volume for > an audio channel. This is confusing.... how do you know what's the success/failed operation?

> > * system-audiochannel-onsuccess: If it is successful, play, pause, set > > volume for an audio channel. > > * system-audiochannel-onerror: If it is failed, play, pause, set volume for > > an audio channel. > > This is confusing.... how do you know what's the success/failed operation? Ah, a mistake. We should do the below things. # mozChromeEvent * system-audiochannel-play-onsuccess: Successful to play. * system-audiochannel-play-onerror: Failed to play. * system-audiochannel-pause-onsuccess: Successful to pause. * system-audiochannel-pause-onerror: Failed to pause. * system-audiochannel-volume-onsuccess: Successful to set volume. * system-audiochannel-volume-onerror: Failed to set volume.

And add this one. # mozChromeEvent * system-audiochannel-state-changed: State of the audio channel is changed.

The Gaia part of managing System's audio channels is done. Adding tests now! For the Gecko part, we need a reviewer.

Comment on attachment 8600828 [details] The Gaia part https://github.com/mozilla-b2g/gaia/pull/29855/files

Comment 12 and comment 13 looks good.

Hi :baku, After discussed in the above, we want to create new MozContentEvent and MozChromeEvent to control the System app's audio channel in the System app. We will have the below events to do that. # mozContentEvent * system-audiochannel-list: Get the list of allowed audio channel name. * system-audiochannel-play: Play the audio channel. * system-audiochannel-pause: Pause the audio channel. * system-audiochannel-volume: Set the volume of the audio channel. # mozChromeEvent * system-audiochannel-list: Get the list of allowed audio channel name. * system-audiochannel-state-changed: State of the audio channel is changed. * system-audiochannel-play-onsuccess: Successful to play. * system-audiochannel-play-onerror: Failed to play. * system-audiochannel-pause-onsuccess: Successful to pause. * system-audiochannel-pause-onerror: Failed to pause. * system-audiochannel-volume-onsuccess: Successful to set volume. * system-audiochannel-volume-onerror: Failed to set volume. How do you think of that? And could you be the reviewer for the Gecko part(shell.js) in the future. (I'm learning to contribute code to Gecko) Thanks. At same time, we're discussing "Exposing the browser api reference to the internal/certified apps(eg. System app) themselves?"[1] in the webapi mailing list. For sure, if we expose the browser APIs, we don't need the above MozContentEvent and MozChromeEvent. [1]: https://groups.google.com/forum/#!topic/mozilla.dev.webapi/Ja6_ZuvPVqw

Added tests[1] for System app's audio channel controller. Continue to add tests for `SystemWindow`, then we could send review request. [1]: https://github.com/evanxd/gaia/commit/ebdbc777d6f31237168c789511de4e6ed2ab7983

Look like the CI result[1] is not good. Fixing it and adding tests. [1]: https://treeherder.mozilla.org/#/jobs?repo=gaia&revision=ee7a52eb0574bcbb7055cf46b571190593b83d62

Added tests for SystemWindow[1] and fixed the failed test. The patch is done. Waiting for CI[2] now. Once CI is good, then we could start to review the patch. [1]: https://github.com/evanxd/gaia/commit/98f0a3d91887aa1c782d88c15bb61af374708ac5 [2]: https://treeherder.mozilla.org/#/jobs?repo=gaia&revision=98f0a3d91887aa1c782d88c15bb61af374708ac5

Sorry, still need info from :baku for Comment 19.

Sorry for the delay, > After discussed in the above, we want to create new MozContentEvent and > MozChromeEvent to control the System app's audio channel in the System app. well, this seems to be the fastest way to implement this feature. I don't know if this is good for a performance point of view, but we can debug it later. > * system-audiochannel-play: Play the audio channel. > * system-audiochannel-pause: Pause the audio channel. Why these 2? Can we have just 'mute' with a boolean? > And could you be the reviewer for the Gecko part(shell.js) in the future. > (I'm learning to contribute code to Gecko) I can review this shell.js code.

(In reply to Andrea Marchesini (:baku) from comment #24) > Sorry for the delay, > > > After discussed in the above, we want to create new MozContentEvent and > > MozChromeEvent to control the System app's audio channel in the System app. > > well, this seems to be the fastest way to implement this feature. > I don't know if this is good for a performance point of view, but we can > debug it later. > Let's try to do it. > > * system-audiochannel-play: Play the audio channel. > > * system-audiochannel-pause: Pause the audio channel. > > Why these 2? Can we have just 'mute' with a boolean? Sure, we could do that(system-audiochannel-mute). > > > And could you be the reviewer for the Gecko part(shell.js) in the future. > > (I'm learning to contribute code to Gecko) > > I can review this shell.js code. Thanks. :)

Updated patch[1] for Comment 24 and fixing bugs. Waiting the CI[2]. [1]: https://github.com/evanxd/gaia/commit/1c4ae743581d9a918b398237892941e45a872a3e [2]: https://treeherder.mozilla.org/#/jobs?repo=gaia&revision=1c4ae743581d9a918b398237892941e45a872a3e

Comment on attachment 8600828 [details] The Gaia part Hi Alive and Dominic, Could you help to review the patch? Thanks.

Continuing to work on the gecko part after fixing Bug 1161621.

Comment on attachment 8600828 [details] The Gaia part >https://github.com/mozilla-b2g/gaia/pull/29855

Hi Alive and Dominic, Could you help to review the patch? Thanks.

Comment on attachment 8603980 [details] Gaia part: https://github.com/mozilla-b2g/gaia/pull/29855 The most concern is the way to instantiate ACC. Please do |new AudioChannelController(this, name);| in SystemWindow then you don't need to have the strange argument assignment in ACC's constructor. One thing else: why need to instantiate SystemWindow in bootstrap instead of Core? Why are we waiting for applicationready here?

Thanks for the review, Alive. Updated for comments[1]. Now waiting for CI[2]. [1]: https://github.com/evanxd/gaia/commit/84d6d10ab80bf1de3d2edbe7d05b6c68f62ea48f [2]: https://treeherder.mozilla.org/#/jobs?repo=gaia&revision=84d6d10ab80bf1de3d2edbe7d05b6c68f62ea48f

If CI is good, we'll start to review the patch again.

Some tests were failed in [1]. Fixing it. [1]: https://treeherder.mozilla.org/#/jobs?repo=gaia&revision=84d6d10ab80bf1de3d2edbe7d05b6c68f62ea48f

Try to run CI[1] again. [1]: https://treeherder.mozilla.org/#/jobs?repo=gaia&revision=38ced2737f068b3ea2e90fc6a7c35265caf033f5

Comment on attachment 8603980 [details] Gaia part: https://github.com/mozilla-b2g/gaia/pull/29855 CI[1] is good now. Alive, could you help to review it again? Thanks. [1]: https://treeherder.mozilla.org/#/jobs?repo=gaia&revision=8661d86d6d45312e8811a3d3c860f2660e2b3e35

Comment on attachment 8603980 [details] Gaia part: https://github.com/mozilla-b2g/gaia/pull/29855 r=me with nits addressed

Comment on attachment 8605762 [details] [diff] [review] Gecko part Hi :baku, Could you help to review the patch for shell.js? Thanks.

:baku, one more question about `shell.sendChromeEvent` method in shell.js. We only can do `shell.sendChromeEvent` after the `MozAfterPaint` event is triggered, right? In my experiment, if I do `shell.sendChromeEvent` too early(before `MozAfterPaint` event is triggered), System app just cannot receive the MozChromeEvent. Is above correct? Or I misunderstood something? Thank.

(In reply to Alive Kuo [:alive][NEEDINFO!] from comment #38) > Comment on attachment 8603980 [details] > Gaia part: https://github.com/mozilla-b2g/gaia/pull/29855 > > r=me with nits addressed Alive, thanks for the review. I'll update the patch.

Comment on attachment 8605762 [details] [diff] [review] Gecko part Let's file a new bug for Gecko part.

Comment on attachment 8603980 [details] Gaia part: https://github.com/mozilla-b2g/gaia/pull/29855 Looks good with Alive's github comments!

Thanks for the review, Dominic. We will land it once CI[1] is good. [1]: https://treeherder.mozilla.org/#/jobs?repo=gaia&revision=b7d6dca09cd386617a1c520ef91a02aef0f4eccc

Run CI one more time: https://treeherder.mozilla.org/#/jobs?repo=gaia&revision=cf374d8cfe1f97bb05b5161d47c2d767401fb5df

master: https://github.com/mozilla-b2g/gaia/commit/117e5c4e22f51c756ae6198dea0d6123a4396cc8