Closed
Bug 1004306
Opened 10 years ago
Closed 10 years ago
AudioChannel implementation can cause multiple sounds without any possibility to stop the sound
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla36
People
(Reporter: soeren.hentzschel, Assigned: baku)
Details
Attachments
(1 file, 1 obsolete file)
5.91 KB,
patch
|
mchen
:
review+
rlin
:
review+
|
Details | Diff | Splinter Review |
STR: 1. set media.useAudioChannelService to 'true' 2. set media.defaultAudioChannel to 'content' 3. start playing a video on youtube 4. click on another video in the youtube sidebar before the current video is over Expected: The first video should stop and only the second video should play sound. Actual: You can hear sound from both videos and there is no possibility to stop the sound of the first video, except you restart the browser. It should not be possible to get Firefox into a state in which the user has no longer the control over the browser.
Assignee | ||
Comment 1•10 years ago
|
||
It seems a problem with Cycle Collection or HTMLMediaElements. I say this because opening about:memory and forcing CC and GC, the first audio player stops.
Assignee: nobody → amarchesini
Assignee | ||
Comment 2•10 years ago
|
||
So, the issue here is that AudioChannelService keeps alive the MediaElements. With this patch we removes all the agents when the window is destroyed.
Attachment #8505570 -
Flags: review?(mchen)
Comment 3•10 years ago
|
||
Comment on attachment 8505570 [details] [diff] [review] audio.patch Review of attachment 8505570 [details] [diff] [review]: ----------------------------------------------------------------- Hi Randy, Could you help to check speaker part? thanks. ::: dom/audiochannel/AudioChannelService.cpp @@ +754,5 @@ > + nsAutoPtr<AudioChannelAgentData>& aData, > + void* aPtr) > +{ > + uint64_t* innerID = static_cast<uint64_t*>(aPtr); > + MOZ_ASSERT(*innerID); Do you want to check whether innerID is 0 or not? Or this point is null or not? @@ +766,5 @@ > + MOZ_ASSERT(service); > + > + service->UnregisterAudioChannelAgent(aAgent); > + service->UnregisterType(aData->mChannel, aData->mElementHidden, > + CONTENT_PROCESS_ID_MAIN, aData->mWithVideo); 1. ::UnregisterType() will be performed inside ::UnregisterAudioChannelAgent() already. 2. Inside ::UnregisterAudioChannelAgent(), this aAgent will be ::RemoveAndForget() from hashtable then the return value - PL_DHASH_REMOVE will do the rawRemove again. Is this the right way to do?
Attachment #8505570 -
Flags: review?(mchen) → review?(rlin)
Assignee | ||
Comment 4•10 years ago
|
||
> 1. ::UnregisterType() will be performed inside
> ::UnregisterAudioChannelAgent() already.
Actually you are right and this was not included in my latest version of the patch. I want to do just UnregisterType() and no tUnregisterUserAgent().
Assignee | ||
Comment 5•10 years ago
|
||
Attachment #8505570 -
Attachment is obsolete: true
Attachment #8505570 -
Flags: review?(rlin)
Attachment #8506084 -
Flags: review?(rlin)
Attachment #8506084 -
Flags: review?(mchen)
Updated•10 years ago
|
Attachment #8506084 -
Flags: review?(rlin) → review+
Comment 6•10 years ago
|
||
Comment on attachment 8506084 [details] [diff] [review] audio.patch Review of attachment 8506084 [details] [diff] [review]: ----------------------------------------------------------------- r+ after addressing the only one questions. ::: dom/audiochannel/AudioChannelService.cpp @@ +754,5 @@ > + nsAutoPtr<AudioChannelAgentData>& aData, > + void* aPtr) > +{ > + uint64_t* innerID = static_cast<uint64_t*>(aPtr); > + MOZ_ASSERT(*innerID); Does this check for null pointer? Or does it check for whether innerID is 0?
Attachment #8506084 -
Flags: review?(mchen) → review+
Assignee | ||
Comment 7•10 years ago
|
||
> Does this check for null pointer?
> Or does it check for whether innerID is 0?
The second one. but I can add MOZ_ASSERT for the first condition as well.
Assignee | ||
Comment 8•10 years ago
|
||
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=179c74f84aa5 pushed to try for the last check.
Assignee | ||
Comment 9•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/5bbe8b64b4b7
Comment 10•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/5bbe8b64b4b7
Status: UNCONFIRMED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
Reporter | ||
Comment 11•9 years ago
|
||
I tested with current Nightly and the issue described in comment 0 seems to be fixed. But sometimes the sound is still playing after closing the tab (not always reproducible). I am not sure: is it the same bug ("can cause multiple sounds without any possibility to stop the sound") or should I open a new bug report?
Flags: needinfo?(amarchesini)
Assignee | ||
Comment 12•9 years ago
|
||
Yes, please. file a bug and put me in CC. Thanks.
Flags: needinfo?(amarchesini)
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•