[webkit-reviews] review granted: [Bug 99868] webkitsourceopen event doesn't always fire : [Attachment 169737] Added LayoutTest and stop() method.

[bugzilla-daemon at webkit.org]

Adam Barth <abarth at webkit.org> has granted Aaron Colwell
<acolwell at chromium.org>'s request for review:
Bug 99868: webkitsourceopen event doesn't always fire
https://bugs.webkit.org/show_bug.cgi?id=99868

Attachment 169737: Added LayoutTest and stop() method.
https://bugs.webkit.org/attachment.cgi?id=169737&action=review

------- Additional Comments from Adam Barth <abarth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=169737&action=review


> Source/WebCore/Modules/mediasource/MediaSourceRegistry.cpp:53
> +    // Add a pending activity to the source so Javascript knows this object
is still active.

I'd remove this comment.  It's just saying what the code is doing.

> Source/WebCore/Modules/mediasource/MediaSourceRegistry.cpp:69
> +    // Remove the pending activity added in registerMediaSourceURL().

By contrast, this comment is helpful.  :)

>
LayoutTests/http/tests/media/media-source/video-media-source-garbage-collection
-before-sourceopen.html:14
> +		   if (window.GCController)
> +		       GCController.collect();

Generally we prefer to use gc.js so that we can run the test outside of
DumpRenderTree.

>
LayoutTests/http/tests/media/media-source/video-media-source-garbage-collection
-before-sourceopen.html:40
> +		   setTimeout(runGC, 0);

Is there a race here between onSourceOpen and the setTimeout?  Maybe we can
keep the URL created by createObjectURL around and only set video.src after
running GC?