Bug 154768

Summary: ProxyObject.[[GetOwnProperty]] is partially broken because it doesn't propagate information back to the slot
Product: WebKit Reporter: Saam Barati <saam>
Component: JavaScriptCoreAssignee: Saam Barati <saam>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, keith_miller, mark.lam, msaboff
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
patch rniwa: review+

Description Saam Barati 2016-02-26 18:13:49 PST
...
Comment 1 Saam Barati 2016-02-26 20:34:46 PST
Created attachment 272397 [details]
patch
Comment 2 Ryosuke Niwa 2016-02-26 21:53:11 PST
Comment on attachment 272397 [details]
patch

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

> Source/JavaScriptCore/ChangeLog:10
> +        This fixes a big bug with ProxyObject.[[GetOwnProperty]]. We weren't
> +        correctly propagating the result of this operation to the out PropertySlot&
> +        parameter. This patch fixes that and adds tests.

Can we refer to http://www.ecma-international.org/ecma-262/6.0/#sec-proxy-object-internal-methods-and-internal-slots-getownproperty-p
either in the change log or in the code?

> Source/JavaScriptCore/runtime/ObjectConstructor.cpp:236
>      JSObject* description = constructEmptyObject(exec);
> +    if (exec->hadException())
> +        return jsUndefined();

Please mention this change in the change log.

> Source/JavaScriptCore/runtime/ProxyObject.cpp:216
> +        GetterSetter* getterSetter = trapResultAsDescriptor.slowGetterSetter(exec);
> +        if (exec->hadException())
> +            return false;

I guess the only way we can fail here would be memory exhaustion?
Otherwise, we should be careful about the order in which these conditions are checked as they're observable.
Comment 3 Saam Barati 2016-02-28 10:40:53 PST
landed in:
http://trac.webkit.org/changeset/197295