[webkit-reviews] review granted: [Bug 197324] JSWrapperMap should check if existing prototype properties are wrappers when copying exported methods. : [Attachment 368569] 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 197324: JSWrapperMap should check if existing prototype properties are
wrappers when copying exported methods.
https://bugs.webkit.org/show_bug.cgi?id=197324

Attachment 368569: Patch

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




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

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

r=me

> Source/JavaScriptCore/ChangeLog:12
> +	   property. Normally this would only happen if a Objective-C super

a => an

> Source/JavaScriptCore/ChangeLog:15
> +	   Object.protype the overriden method won't be exported.

protype => prototype

> Source/JavaScriptCore/API/JSWrapperMap.mm:274
> +	       bool hasProperty = jsObject->getPropertySlot(exec,
JSC::Identifier::fromString(exec, String(name)), slot);

what about exceptions? Maybe you wanted VMInquiry above?

> Source/JavaScriptCore/API/tests/testapi.mm:2541
> +- (BOOL)other;

is this needed?