Bug 163160 - Object.freeze() and seal() should throw if [[PreventExtensions]]() fails.
Summary: Object.freeze() and seal() should throw if [[PreventExtensions]]() fails.
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Local Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Lam
Depends on:
Reported: 2016-10-07 17:24 PDT by Mark Lam
Modified: 2016-10-07 20:24 PDT (History)
7 users (show)

See Also:

repro test case. (696 bytes, application/x-javascript)
2016-10-07 17:26 PDT, Mark Lam
no flags Details
proposed patch. (9.42 KB, patch)
2016-10-07 18:18 PDT, Mark Lam
mark.lam: review-
Details | Formatted Diff | Diff
proposed patch. (9.37 KB, patch)
2016-10-07 18:20 PDT, Mark Lam
saam: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Lam 2016-10-07 17:24:52 PDT
See https://tc39.github.io/ecma262/#sec-object.freeze and https://tc39.github.io/ecma262/#sec-setintegritylevel.  If O.[[PreventExtensions]]() returns false, we should throw a TypeError but currently does not.
Comment 1 Mark Lam 2016-10-07 17:26:58 PDT
Created attachment 290987 [details]
repro test case.
Comment 2 Mark Lam 2016-10-07 18:18:19 PDT
Created attachment 290994 [details]
proposed patch.
Comment 3 Mark Lam 2016-10-07 18:19:24 PDT
Comment on attachment 290994 [details]
proposed patch.

Bad patch.  That's what I get for making last minute changes.
Comment 4 Mark Lam 2016-10-07 18:20:11 PDT
Created attachment 290996 [details]
proposed patch.
Comment 5 Saam Barati 2016-10-07 18:24:54 PDT
Comment on attachment 290996 [details]
proposed patch.

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


> Source/JavaScriptCore/runtime/ObjectConstructor.cpp:480
> +template<IntegrityLevel level>
> +bool setIntegrityLevel(ExecState* exec, VM& vm, JSObject* object)

Maybe it's worth including a link to the spec here.

> Source/JavaScriptCore/runtime/ObjectConstructor.cpp:496
> +        if (exec->propertyNames().isPrivateName(propertyName))

You should use vm to get the PropertyNames.
Comment 6 Mark Lam 2016-10-07 20:24:14 PDT
Thanks for the review.  I've made the suggested changes.

Landed in r206948: <http://trac.webkit.org/r206948>.