[webkit-reviews] review granted: [Bug 74386] Migrate createObjectURL & revokeObjectURL to static (Class) methods : [Attachment 119207] Fixed ChangeLog

[bugzilla-daemon at webkit.org]

Adam Barth <abarth at webkit.org> has granted Kaustubh Atrawalkar
<kaustubh at motorola.com>'s request for review:
Bug 74386: Migrate createObjectURL & revokeObjectURL to static (Class) methods
https://bugs.webkit.org/show_bug.cgi?id=74386

Attachment 119207: Fixed ChangeLog
https://bugs.webkit.org/attachment.cgi?id=119207&action=review

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


I'm going to mark this R+, but there is one subtle issue (mentioned below) that
you might want to consider before landing this patch.

> Source/WebCore/html/DOMURL.cpp:88
> +static PublicURLManagerMap& publicURLManagerMap()

Interesting design choice.  Another option is to store the information in the
ScriptExecutionContext as some sort of a mix-in.  We don't have a facility for
doing that at the moment, but there are a bunch of cases like this where it
would be useful.

> Source/WebCore/html/DOMURL.idl:34
> -	   [ConvertNullStringTo=Undefined] DOMString createObjectURL(in
MediaStream stream);
> +	   static
[ConvertNullStringTo=Undefined,CallWith=ScriptExecutionContext] DOMString
createObjectURL(in MediaStream stream);

So, this is actually slightly different because you'll get a different
ScriptExecution context.  After this change, you get the script execution
context associated with the currently executing script rather than the one
associated with the window object that contains this object.  Is that an
intentional change?  If so, should we add a test that demonstrates it?