[webkit-reviews] review granted: [Bug 193420] JSScript API should only take ascii files. : [Attachment 359101] Patch

[bugzilla-daemon at webkit.org]

Saam Barati <sbarati at apple.com> has granted Keith Miller
<keith_miller at apple.com>'s request for review:
Bug 193420: JSScript API should only take ascii files.
https://bugs.webkit.org/show_bug.cgi?id=193420

Attachment 359101: Patch

https://bugs.webkit.org/attachment.cgi?id=359101&action=review




--- Comment #10 from Saam Barati <sbarati at apple.com> ---
Comment on attachment 359101
  --> https://bugs.webkit.org/attachment.cgi?id=359101
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=359101&action=review

>>> Source/JavaScriptCore/API/JSScript.mm:98
>>> +	 }
>> 
>> Do we really want to pull the entire file into memory? Why can't we just
debug assert this? If we move to properly naming this something like
JSASCIIScript...
> 
> Are you saying that we should rename the class or this constructor? I'm not
sure that we only ever want to support ASCII scripts. We could also have a
scriptFromUTF16 SPI constructor in the future.
> 
> As far as pulling the whole file into memory. There is a FIXME to mmap the
contents of the file so it shouldn't count against us. Also, we are going to
call a code signing function that is going to hash the entire file for signing
anyway.

Good point regarding pulling in all memory.

I think having the name just in this constructor function is fine.