Any media element should be stopped by the AudioChannelService when the window is destroyed

RESOLVED FIXED in Firefox 40

Status

()

Core
DOM
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: darktrojan, Assigned: baku)

Tracking

unspecified
mozilla40
Points:
---

Firefox Tracking Flags

(firefox40 fixed)

Details

(URL)

Attachments

(1 attachment)

(Reporter)

Description

3 years ago
I don't really know much else apart from what's in the bug title.

STR:
Set media.useAudioChannelService to true
Start a YouTube video
Switch tab
(Reporter)

Comment 1

3 years ago
I've found out something else: no messageManager messages make it to message listeners while the tab is not selected.

Also I'm pretty sure this isn't just YouTube, it's any HTML5 media.
(Reporter)

Comment 2

3 years ago
(In reply to Geoff Lankow (:darktrojan) from comment #1)
> I've found out something else: no messageManager messages make it to message
> listeners while the tab is not selected.

I think I was mistaken about this.


@baku: since you're killing this function off in bug 1113086, can I disable it (make visible always true) for desktop only in the meantime? That would make desktop behave as expected with the service enabled.

http://hg.mozilla.org/mozilla-central/file/883e17fc475f/dom/audiochannel/AudioChannelAgent.cpp#l144
Flags: needinfo?(amarchesini)
Summary: With Audio Channel Service enabled, Youtube playback stops on tab switch → With Audio Channel Service enabled, media playback stops on tab switch
(Assignee)

Comment 3

3 years ago
Just set media.defaultAudioChannel to  'content'. This is what I do on desktop when I enable AudioChannelService.

Why do you wantto enable AudioChannelService in Desktop? Some particular addon?
URL:
Flags: needinfo?(amarchesini)
(Reporter)

Comment 4

3 years ago
Okay, I didn't know about that pref, and it fixes the original problem. But now I have another: the audio won't stop playing after I navigate away from the page it was on.

This is what I want it for: https://addons.mozilla.org/en-US/firefox/addon/noise-control/
(Assignee)

Comment 5

3 years ago
Created attachment 8589071 [details] [diff] [review]
audio.patch

The behavior this patch fixes is that, if the AudioChannelService is enabled and a media element is playing when the page is navigated away, that media element has to be muted.
Attachment #8589071 - Flags: review?(ehsan)
(Assignee)

Updated

3 years ago
Assignee: nobody → amarchesini
OS: Linux → All
Hardware: x86_64 → All

Comment 6

3 years ago
Comment on attachment 8589071 [details] [diff] [review]
audio.patch

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

This needs a test!

::: dom/audiochannel/AudioChannelAgent.cpp
@@ +102,5 @@
> +    if (!pWindow->IsInnerWindow()) {
> +      pWindow = pWindow->GetCurrentInnerWindow();
> +    }
> +
> +    mWindow = pWindow;

Nit: pWindow.forget() so that we don't AddRef/Release unnecessarily.

::: dom/audiochannel/AudioChannelService.cpp
@@ +39,5 @@
>  
> +// When a inner-window is destroyed we have to mute all the related
> +// AudioChannelAgents. In order to do this we have to notify them after purging
> +// AudioChannelService::mAgents.
> +struct WindowDestroyedEnumeratorData

MOZ_STACK_CLASS.

@@ +41,5 @@
> +// AudioChannelAgents. In order to do this we have to notify them after purging
> +// AudioChannelService::mAgents.
> +struct WindowDestroyedEnumeratorData
> +{
> +  WindowDestroyedEnumeratorData(uint64_t aInnerID)

explicit
Attachment #8589071 - Flags: review?(ehsan) → review+
https://hg.mozilla.org/mozilla-central/rev/2be1c521ab5f
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox40: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
Depends on: 1153915
(Reporter)

Comment 10

3 years ago
It's getting a bit late, but do you think this could be pushed forward to 39?
Flags: needinfo?(amarchesini)
Summary: With Audio Channel Service enabled, media playback stops on tab switch → Any media element should be stopped by the AudioChannelService when the window is destroyed
(Assignee)

Comment 11

3 years ago
The patch should apply easily and in case, I can rebase it. And I don't think it will introduce regressions. ehsan?
Flags: needinfo?(amarchesini) → needinfo?(ehsan)

Comment 12

3 years ago
As far as I can tell, this shouldn't affect users unless they have manually changed the media.useAudioChannelService pref, so it should be safe to backport, I think.
Flags: needinfo?(ehsan)
(Assignee)

Comment 13

3 years ago
Comment on attachment 8589071 [details] [diff] [review]
audio.patch

Approval Request Comment
[Feature/regressing bug #]: AudioChannelPolicy
[User impact if declined]: nothing. This is useful for users who enabled AudioChannelService in desktop manually. There are 2 addons who will benefice from this patch (one is mine).
[Describe test coverage new/current, TreeHerder]: green on try but no specific tests.
[Risks and why]: All this code is disabled in beta completely.
[String/UUID change made/needed]: none
Attachment #8589071 - Flags: approval-mozilla-beta?
Comment on attachment 8589071 [details] [diff] [review]
audio.patch

I'd like us to let this ride with 40. It doesn't seem urgent or high user impact. If that assessment is wrong let me know or re-nominate this for uplift. Thanks!
Attachment #8589071 - Flags: approval-mozilla-beta? → approval-mozilla-beta-
You need to log in before you can comment on or make changes to this bug.