Closed
Bug 1180535
Opened 9 years ago
Closed 9 years ago
Dispatch the media-playback notification when navigating away from a page that has a media element playing
Categories
(Core :: Audio/Video, defect)
Core
Audio/Video
Tracking
()
RESOLVED
FIXED
mozilla42
Tracking | Status | |
---|---|---|
firefox42 | --- | fixed |
People
(Reporter: ehsan.akhgari, Assigned: ehsan.akhgari)
References
Details
Attachments
(1 file)
When navigating away from a document, we mute the playing media elements through the NotifyOwnerDocumentActivityChanged() notification. Sometimes, that function may notify the audio channel agent through its call to AddRemoveSelfReference() which may call UpdateAudioChannelPlayingState() and notify the agent, but when we're navigating away from the page, playingThroughTheAudioChannel will always be equal to mPlayingThroughTheAudioChannel, which causes us to not notify the audio channel agent. This patch fixes this by separating NotifyOwnerDocumentActivityChanged() from its internal consumers, and forcefully notifying the audio channel agent when we navigate away.
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → ehsan
Comment 2•9 years ago
|
||
Comment on attachment 8629719 [details] [diff] [review] Dispatch the media-playback notification when navigating away from a page that has a media element playing Review of attachment 8629719 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/base/test/mochitest.ini @@ +247,5 @@ > [test_audioWindowUtils.html] > [test_audioNotification.html] > skip-if = buildapp == 'mulet' > +[test_audioNotificationStopOnNavigation.html] > +skip-if = buildapp == 'mulet' why no the mulet?
Attachment #8629719 -
Flags: review?(amarchesini) → review+
Assignee | ||
Comment 3•9 years ago
|
||
(In reply to Andrea Marchesini (:baku) from comment #2) > Review of attachment 8629719 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: dom/base/test/mochitest.ini > @@ +247,5 @@ > > [test_audioWindowUtils.html] > > [test_audioNotification.html] > > skip-if = buildapp == 'mulet' > > +[test_audioNotificationStopOnNavigation.html] > > +skip-if = buildapp == 'mulet' > > why no the mulet? test_audioNotification.html was disabled on mulet in bug 1027242 because it failed. I could not care less about mulet so wanted to spend 0 time investigating why that is, and whether my other tests would fail similarly or not, so I just disabled it. :-)
I had to back this out in https://hg.mozilla.org/integration/mozilla-inbound/rev/15a7c2470aaf for mochitest-2 failures: https://treeherder.mozilla.org/logviewer.html#?job_id=11570081&repo=mozilla-inbound
Flags: needinfo?(ehsan)
Assignee | ||
Comment 6•9 years ago
|
||
The cause of the failure is that NotifyOwnerDocumentActivityChanged is a virtual function, and when the internal HTMLMediaElement code called it, the HTMLVideoElement version would be called. But NotifyOwnerDocumentActivityChangedInternal is not virtual, so in that case, we would be skipping HTMLVideoElement::NotifyOwnerDocumentActivityChanged. The solution is to make NotifyOwnerDocumentActivityChangedInternal instead.
Flags: needinfo?(ehsan)
Comment 8•9 years ago
|
||
sorry had to back this out in https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=727b321502a6 since i think the changes broke the cpp tests on windows very frequently with timeouts like https://treeherder.mozilla.org/logviewer.html#?job_id=11595725&repo=mozilla-inbound
Flags: needinfo?(ehsan)
Assignee | ||
Comment 10•9 years ago
|
||
I think bug 1113086 comment 197 explains what happened here!
Flags: needinfo?(ehsan)
Comment 12•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/97161d5cfb8c
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox42:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
You need to log in
before you can comment on or make changes to this bug.
Description
•