1217303 - Remove mozContentEvent and mozChromeEvent about controlling System's audio channel in shell.js

Status: RESOLVED FIXED

(Firefox OS Graveyard :: AudioChannel, defect)

Description

[Evan Tseng [:evanxd]]

Remove mozContentEvent and mozChromeEvent about controlling System's audio channel in shell.js. Please refer to https://hg.mozilla.org/mozilla-central/diff/ef1d868c5a85/b2g/chrome/content/shell.js.

Comment 1

[Evan Tseng [:evanxd]]

Attachment: bug-1217303.patch

Comment 2

[Evan Tseng [:evanxd]]

Attachment: bug-1217303.patch

Comment 3

[Evan Tseng [:evanxd]]

Comment on attachment 8677351 [details] [diff] [review]
bug-1217303.patch

Hi baku,

Could you review the patch?

Because we already exported System app's allowed audio channels in `navigator.mozAudioChannelManager.allowedAudioChannels`, we don't need the mozChromeEvent and mozContentEvent to control the audio channels anymore. I remove the useless code in the patch.

Thanks.

Comment 4

[Evan Tseng [:evanxd]]

Comment on attachment 8677351 [details] [diff] [review]
bug-1217303.patch

This patch is conflicted with current code base. Will update it.

Comment 5

[Evan Tseng [:evanxd]]

Attachment: bug-1217303.patch

Comment 6

[Evan Tseng [:evanxd]]

Comment on attachment 8677931 [details] [diff] [review]
bug-1217303.patch

Hi baku,

Could you review the patch?

Thanks.

Comment 7

[Alastor Wu [:alwu]]

Try-server result.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=ffcb9452d41d

Comment 8

[Evan Tseng [:evanxd]]

Thanks Alastor.

Comment 9

[Andrea Marchesini [:baku]]

Comment on attachment 8677931 [details] [diff] [review]
bug-1217303.patch

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

::: b2g/chrome/content/shell.js
@@ +406,2 @@
>      let audioChannels = systemAppFrame.allowedAudioChannels;
>      audioChannels && audioChannels.forEach(function(audioChannel) {

do we still need this?

Comment 10

[Evan Tseng [:evanxd]]

(In reply to Andrea Marchesini (:baku) from comment #9)
> Comment on attachment 8677931 [details] [diff] [review]
> bug-1217303.patch
> 
> Review of attachment 8677931 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: b2g/chrome/content/shell.js
> @@ +406,2 @@
> >      let audioChannels = systemAppFrame.allowedAudioChannels;
> >      audioChannels && audioChannels.forEach(function(audioChannel) {
> 
> do we still need this?

Yes, we still need this to set System app's audio channels as unmuted by default. Because some audio in System app will be played, before AudioChannelService[1] in Gaia is loaded.

Comment 11

[Evan Tseng [:evanxd]]

And thanks for the review.

Comment 12

[Evan Tseng [:evanxd]]

Retrigger treeherder tasks: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ffcb9452d41d

Once all tasks are good, I'll do `checkin-needed`.

Comment 13

[Evan Tseng [:evanxd]]

Treeherder tasks are all passed. Do checkin-needed!

Comment 14

[Pulsebot]

https://hg.mozilla.org/integration/b2g-inbound/rev/279b06d77e81

Comment 15

[Tim Guan-tin Chien [:timdream] (please needinfo)]

(In reply to Evan Tseng [:evanxd][:愛聞插低] from comment #10)
> Yes, we still need this to set System app's audio channels as unmuted by
> default. Because some audio in System app will be played, before
> AudioChannelService[1] in Gaia is loaded.

Can we initialize AudioChannelService before the first boot sound?

Comment 16

[Tim Guan-tin Chien [:timdream] (please needinfo)]

If so please file another bug and fix it.

Comment 17

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/279b06d77e81

Comment 18

[Evan Tseng [:evanxd]]

(In reply to Tim Guan-tin Chien [:timdream] (please needinfo) from comment #15)
> (In reply to Evan Tseng [:evanxd][:愛聞插低] from comment #10)
> > Yes, we still need this to set System app's audio channels as unmuted by
> > default. Because some audio in System app will be played, before
> > AudioChannelService[1] in Gaia is loaded.
> 
> Can we initialize AudioChannelService before the first boot sound?

Yes, I think we can initialize AudioChannelService before the first boot sound. Already filed the Bug 1221022 to fix it.