Testing on Lion. Regression occurred http://trac.webkit.org/projects/webkit/changeset/110064. I believe this change has a typo. I think the ! needs to be eliminated. - if (!static_cast<HTMLAudioElement*>(node)->paused()) + if (!static_cast<HTMLAudioElement*>(node)->hasPendingActivity()) As it stands an HTMLAudioElement is observable when it is not waiting for any events. This means that the DOM can never be cleaned up when a user navigates away from the page. Observed on http://twitter.com. <rdar://problem/11135186> REGRESSION: audio elements with pending activity progressively slow down the PLT3.
Created attachment 134210 [details] patch Uploaded a patch to fix the if statement, but I'm not sure if it is possible to write a test to show the bug.
*** Bug 81781 has been marked as a duplicate of this bug. ***
(In reply to comment #1) > Created an attachment (id=134210) [details] > patch > > Uploaded a patch to fix the if statement, but I'm not sure if it is possible to write a test to show the bug. A nice list of media tests are flaky on GTK because of this issue, thanks for looking into it and sorry, I'm a bit embarassed :)
I've just checked that with your patch those media tests are no longer flaky on GTK: media/W3C/audio/networkState/networkState_during_loadstart.html media/audio-delete-while-step-button-clicked.html media/audio-mpeg-supported.html media/audio-repaint.html media/media-controls-clone-crash.html media/audio-repaint.html If you could unskip them as part of your patch it'd be great. media/audio-garbage-collect.html is now failing though. I'll have a look at this one again soon.
I don't think that this kind of change requires a regression test. Please upload an updated patch and mark it for review.
I think it would be even better just to revert http://trac.webkit.org/projects/webkit/changeset/110064 and consult Eric and Jer about what the best fix is. It's not clear that HTMLAudioElement::hasPendingActivity is the right test for GC.
Jer wasn't convinced that the original fix was correct, but said I should talk to Eric. Since it is such a bad performance bug I'm rolling out until I can catch up with Eric and find the right fix.
In the end, what we need is a DOM API that is true when the audio node is waiting for an event to fire, for a load to complete, or for playing to end, and is *guaranteed* to become false if the node is removed from the document or the window is closed or navigated away from.
(In reply to comment #6) > I think it would be even better just to revert http://trac.webkit.org/projects/webkit/changeset/110064 and consult Eric and Jer about what the best fix is. It's not clear that HTMLAudioElement::hasPendingActivity is the right test for GC. HTMLAudioElement::hasPendingActivity is definitely NOT the correct test. The reason that we care about this is that the spec says when a media element can be collected: Media elements that are potentially playing while not in a Document must not play any video, but should play any audio component. Media elements must not stop playing just because all references to them have been removed; only once a media element is in a state where no further audio could ever be played by that element may the element be garbage collected. and "potentially playing" is defined as: A media element is said to be potentially playing when its paused attribute is false, the element has not ended playback, playback has not stopped due to errors, the element either has no current media controller or has a current media controller but is not blocked on its media controller, and the element is not a blocked media element. As it happens, we have HTMLMediaElement::potentiallyPlaying because "potentially playing" is used in other parts of the media element spec.
Rolled out in http://trac.webkit.org/projects/webkit/changeset/112483 I reopened the original bug for doing the correct fix. (https://bugs.webkit.org/show_bug.cgi?id=72698)