[Webkit-unassigned] [Bug 66878] HTMLAudioElement can be garbage collected while it playing

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Oct 13 09:03:29 PDT 2011


https://bugs.webkit.org/show_bug.cgi?id=66878





--- Comment #38 from Eric Carlson <eric.carlson at apple.com>  2011-10-13 09:03:29 PST ---
(From update of attachment 110845)
View in context: https://bugs.webkit.org/attachment.cgi?id=110845&action=review

> Source/WebCore/ChangeLog:16
> +        Fix consists of 2 parts:
> +        (a) make HTMLAudioElement an 'active' one, meaning that it cannot be
> +        garbage collected if it has panding activity
> +        (b) workaround for v8 bindings bug that was in the code forever, event
> +        listener could be garbage collected even if object it attached to is
> +        still alive, resulting in the loss of events. Workaround is to make
> +        HTMLAudioElement to implement EventTarget interface, and modify code
> +        generator to create hidden dependency between object and event listener
> +        even for types that inherit from Node (but not for Node itself).

Can't this be broken into two patches? While part (a) depends on part (b), the later is not specific to this bug and seems worthy of its own bug/patch.

> LayoutTests/media/audio-garbage-collect.html:7
> +<html>
> +<body>
> +
> +<p>Tests that we don't garbage collect playing audio object or event listener.</p>
> +<p>According to http://www.whatwg.org/specs/web-apps/current-work/multipage/the-video-element.html,<br />

Nit: Indentation would make this file much more readable, IMO.

> LayoutTests/media/audio-garbage-collect.html:18
> +</p>
> +
> +<script src=media-file.js></script>
> +<script src=video-test.js></script>

Nit: Is there any reason to not have the script nodes in <head>?

> LayoutTests/media/audio-garbage-collect.html:26
> +var a;
> +var num_played;
> +var total_played = 0;

Nit: is there any reason to have these in the global scope?

> LayoutTests/media/audio-garbage-collect.html:46
> +  var audioFile = findMediaFile("audio", "content/click2");

1. Why do you need a new audio file, can't you use silence.wav (it and all of the existing files with audio tracks purposely have encoded silence so a computer running layout tests doesn't make noise)? If it is too long, just seek to near the end before starting playback. 
2. findMediaFile() is only necessary when you have a multiple versions of a media file with different encodings.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.



More information about the webkit-unassigned mailing list