1177399 - Enable AudioChannelService in desktop by default

Status: RESOLVED FIXED

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

Description

[Andrea Marchesini [:baku]]

We need this component for a couple of new features. Let's use this bug as a meta bug for any possible dependence.

Comment 1

[Andrea Marchesini [:baku]]

Attachment: default.patch

Comment 2

[(no longer active)]

Comment on attachment 8632694 [details] [diff] [review]
default.patch

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

::: mobile/android/app/mobile.js
@@ +795,5 @@
>  
> +// Make <audio>, <video> and webAudio talk to the AudioChannelService.
> +pref("media.useAudioChannelService", true);
> +// Add Mozilla AudioChannel APIs.
> +pref("media.useAudioChannelAPI", false);

You should be able to remove these.

Comment 3

[Andrea Marchesini [:baku]]

Attachment: 252588.diff

Comment 4

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/cfa13a24045b

Comment 5

[Carsten Book [:Tomcat]]

sorry had to back this out for test failures like https://treeherder.mozilla.org/logviewer.html#?job_id=11651363&repo=mozilla-inbound

Comment 6

[Pulsebot]

Backout:
https://hg.mozilla.org/integration/mozilla-inbound/rev/67208447a3d6

Comment 7

[Carsten Book [:Tomcat]]

also seems https://treeherder.mozilla.org/logviewer.html#?job_id=11651196&repo=mozilla-inbound is related

Comment 8

[Andrea Marchesini [:baku]]

Attachment: debug.patch

It took a while to debug it properly. This patch does:

1. Unregister the actor when unlinked by CC
2. Removed part of a test because enabling the ACS, we enable the AudioChannel Policy and we check if the page has the right permission to change the audiochannel. The permission check has covered by other tests.

Comment 9

[(no longer active)]

Comment on attachment 8635275 [details] [diff] [review]
debug.patch

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

r=me with the below fixed.

::: dom/audiochannel/AudioChannelAgent.cpp
@@ +15,5 @@
> +NS_IMPL_CYCLE_COLLECTION_CLASS(AudioChannelAgent)
> +
> +NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(AudioChannelAgent)
> +  if (tmp->mIsRegToService) {
> +    tmp->NotifyStoppedPlaying();

Do you mind refactoring this logic into a helper method and call it both from the Unlink function and the destructor, so that in the future we won't forget to change one but not the other?

Comment 10

[Andrea Marchesini [:baku]]

Attachment: debug.patch

just because we have an intermittent failure for this CC thing, I split the patch in 2 parts. Point 1 of the previous comment is not in this new patch.

Comment 11

[Andrea Marchesini [:baku]]

Attachment: debug.patch

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a459b176febd

Comment 12

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0184189bf1c5
https://hg.mozilla.org/integration/mozilla-inbound/rev/b1b8616162b4

Comment 13

[Andrea Marchesini [:baku]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/37a7604e5e1e

Comment 14

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

https://hg.mozilla.org/mozilla-central/rev/0184189bf1c5
https://hg.mozilla.org/mozilla-central/rev/b1b8616162b4
https://hg.mozilla.org/mozilla-central/rev/37a7604e5e1e