[webkit-reviews] review denied: [Bug 65925] MediaStream API: add createObjectURL functionality : [Attachment 103375] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Aug 9 11:30:49 PDT 2011


Adam Barth <abarth at webkit.org> has denied Tommy Widenflycht
<tommyw at google.com>'s request for review:
Bug 65925: MediaStream API: add createObjectURL functionality
https://bugs.webkit.org/show_bug.cgi?id=65925

Attachment 103375: Patch
https://bugs.webkit.org/attachment.cgi?id=103375&action=review

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


> LayoutTests/fast/files/create-blob-url-crash-expected.txt:2
> -PASS: Not enough arguments
> +PASS: Type error

Why is this exception changing type?  I suspect we need to change the code
generator to check the number of arguments before it checks the type of the
arguments.

> Source/WebCore/dom/ScriptExecutionContext.cpp:396
> +    publicURL.setProtocol("x-raw-media");

Where does this URI scheme come from?  It is registered?

> Source/WebCore/dom/ScriptExecutionContext.cpp:397
> +    // Since WebWorkers cannot obtain Stream objects, we should be on the
main thread.

Maybe add an ASSERT(isMainThread()) ?

> Source/WebCore/dom/ScriptExecutionContext.cpp:424
> +	   // Since WebWorkers cannot obtain Stream objects, we should be on
the main thread.

Same here.

> Source/WebCore/platform/MediaStreamRegistry.cpp:55
> +PassRefPtr<MediaStream> MediaStreamRegistry::getStream(const KURL& url)
const

getStream -> WebKit doesn't use a leading "get" in function names.


More information about the webkit-reviews mailing list