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

[bugzilla-daemon at webkit.org]

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 103759: Patch
https://bugs.webkit.org/attachment.cgi?id=103759&action=review

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


R- for the static initializer.	I don't know how I feel about the fragment
identifier hack.  It doesn't seem like the right solution to the problem.  On
the one hand, I don't want to block you from making progress here.  On the
other hand, I don't want to expose these internal hacks to the platform.  :(

> Source/WebCore/dom/ScriptExecutionContext.cpp:390
> +static const String kMediaStreamFragment = "mediastream";

This is a static initializer, which is banned in WebKit.  Instead, use
DEFINE_STATIC_LOCAL in a function.

> Source/WebCore/dom/ScriptExecutionContext.cpp:399
> +    // This makes it faster to detect that this URL points to a MediaStream.

> +    publicURL.setFragmentIdentifier(kMediaStreamFragment);

This is a bit of a hack.  I saw that you started a standards thread about
creating a new scheme.	On the one hand, I don't want to prevent you from
making progress, but on the other hand I'm worried that this sort of thing is
likely to stick around.

> Source/WebCore/platform/MediaStreamRegistry.cpp:39
> +    DEFINE_STATIC_LOCAL(MediaStreamRegistry, instance, ());

Here's an example of how to avoid static initializers.