[webkit-reviews] review granted: [Bug 203456] Align JSGlobalObject::defineOwnProperty() with the spec and other runtimes : [Attachment 420024] Patch

[bugzilla-daemon at webkit.org]

Robin Morisset <rmorisset at apple.com> has granted Alexey Shvayka
<shvaikalesh at gmail.com>'s request for review:
Bug 203456: Align JSGlobalObject::defineOwnProperty() with the spec and other
runtimes
https://bugs.webkit.org/show_bug.cgi?id=203456

Attachment 420024: Patch

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




--- Comment #13 from Robin Morisset <rmorisset at apple.com> ---
Comment on attachment 420024
  --> https://bugs.webkit.org/attachment.cgi?id=420024
Patch

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

This looks good to me overall, thanks for this patch!
r=me

> Source/JavaScriptCore/llint/LowLevelInterpreter.asm:1345
> +    loadp CodeBlock[cfr], scratch

It is probably not very important, but the loads of CodeBlock[cfr] and then
CodeBlock::m_globalObject are repeated here and in varInjectionCheck, they
could probably be factored out of these macros.

> Source/JavaScriptCore/runtime/JSGlobalObject.cpp:-1478
> -    slot.disallowVMEntry.reset();

We lose this slot.disallowVMEntry.reset() call. I don't understand why it is no
longer necessary.