<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[197960] trunk/Source/JavaScriptCore</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/197960">197960</a></dd>
<dt>Author</dt> <dd>sbarati@apple.com</dd>
<dt>Date</dt> <dd>2016-03-10 15:09:30 -0800 (Thu, 10 Mar 2016)</dd>
</dl>
<h3>Log Message</h3>
<pre>[ES6] Make ToPropertyDescriptor spec compliant
https://bugs.webkit.org/show_bug.cgi?id=155313
Reviewed by Mark Lam.
We were performing HasProperty(.) and Get(.) in the same operation.
This isn't valid according to the spec and it's user observable
behavior with Proxy. This patch fixes ToPropertyDescriptor to use
two distinct operations for HasProperty(.) and Get(.).
* runtime/ObjectConstructor.cpp:
(JSC::ownEnumerablePropertyKeys):
(JSC::toPropertyDescriptor):
* tests/es6.yaml:
* tests/stress/to-property-key-correctness.js: Added.
(assert):
(test):
(test.let.handler.has):
(arrayEq):
(let.handler.has):
(let.target):
(set get let):</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoreruntimeObjectConstructorcpp">trunk/Source/JavaScriptCore/runtime/ObjectConstructor.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoretestses6yaml">trunk/Source/JavaScriptCore/tests/es6.yaml</a></li>
</ul>
<h3>Added Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoretestsstresstopropertykeycorrectnessjs">trunk/Source/JavaScriptCore/tests/stress/to-property-key-correctness.js</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (197959 => 197960)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog 2016-03-10 23:02:58 UTC (rev 197959)
+++ trunk/Source/JavaScriptCore/ChangeLog 2016-03-10 23:09:30 UTC (rev 197960)
</span><span class="lines">@@ -1,3 +1,28 @@
</span><ins>+2016-03-10 Saam barati <sbarati@apple.com>
+
+ [ES6] Make ToPropertyDescriptor spec compliant
+ https://bugs.webkit.org/show_bug.cgi?id=155313
+
+ Reviewed by Mark Lam.
+
+ We were performing HasProperty(.) and Get(.) in the same operation.
+ This isn't valid according to the spec and it's user observable
+ behavior with Proxy. This patch fixes ToPropertyDescriptor to use
+ two distinct operations for HasProperty(.) and Get(.).
+
+ * runtime/ObjectConstructor.cpp:
+ (JSC::ownEnumerablePropertyKeys):
+ (JSC::toPropertyDescriptor):
+ * tests/es6.yaml:
+ * tests/stress/to-property-key-correctness.js: Added.
+ (assert):
+ (test):
+ (test.let.handler.has):
+ (arrayEq):
+ (let.handler.has):
+ (let.target):
+ (set get let):
+
</ins><span class="cx"> 2016-03-10 Brian Burg <bburg@apple.com>
</span><span class="cx">
</span><span class="cx"> Web Inspector: report the underlying parser error message when JSON parsing fails
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreruntimeObjectConstructorcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/runtime/ObjectConstructor.cpp (197959 => 197960)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/runtime/ObjectConstructor.cpp 2016-03-10 23:02:58 UTC (rev 197959)
+++ trunk/Source/JavaScriptCore/runtime/ObjectConstructor.cpp 2016-03-10 23:09:30 UTC (rev 197960)
</span><span class="lines">@@ -309,84 +309,90 @@
</span><span class="cx"> return JSValue::encode(ownPropertyKeys(exec, object, PropertyNameMode::StringsAndSymbols, DontEnumPropertiesMode::Exclude));
</span><span class="cx"> }
</span><span class="cx">
</span><del>-// ES5 8.10.5 ToPropertyDescriptor
</del><ins>+// ES6 6.2.4.5 ToPropertyDescriptor
+// https://tc39.github.io/ecma262/#sec-topropertydescriptor
</ins><span class="cx"> bool toPropertyDescriptor(ExecState* exec, JSValue in, PropertyDescriptor& desc)
</span><span class="cx"> {
</span><ins>+ VM& vm = exec->vm();
</ins><span class="cx"> if (!in.isObject()) {
</span><del>- exec->vm().throwException(exec, createTypeError(exec, ASCIILiteral("Property description must be an object.")));
</del><ins>+ vm.throwException(exec, createTypeError(exec, ASCIILiteral("Property description must be an object.")));
</ins><span class="cx"> return false;
</span><span class="cx"> }
</span><span class="cx"> JSObject* description = asObject(in);
</span><span class="cx">
</span><del>- PropertySlot enumerableSlot(description, PropertySlot::InternalMethodType::HasProperty);
- if (description->getPropertySlot(exec, exec->propertyNames().enumerable, enumerableSlot)) {
- desc.setEnumerable(enumerableSlot.getValue(exec, exec->propertyNames().enumerable).toBoolean(exec));
- if (exec->hadException())
</del><ins>+ if (description->hasProperty(exec, exec->propertyNames().enumerable)) {
+ JSValue value = description->get(exec, exec->propertyNames().enumerable);
+ if (vm.exception())
</ins><span class="cx"> return false;
</span><del>- }
</del><ins>+ desc.setEnumerable(value.toBoolean(exec));
+ } else if (vm.exception())
+ return false;
</ins><span class="cx">
</span><del>- PropertySlot configurableSlot(description, PropertySlot::InternalMethodType::HasProperty);
- if (description->getPropertySlot(exec, exec->propertyNames().configurable, configurableSlot)) {
- desc.setConfigurable(configurableSlot.getValue(exec, exec->propertyNames().configurable).toBoolean(exec));
- if (exec->hadException())
</del><ins>+ if (description->hasProperty(exec, exec->propertyNames().configurable)) {
+ JSValue value = description->get(exec, exec->propertyNames().configurable);
+ if (vm.exception())
</ins><span class="cx"> return false;
</span><del>- }
</del><ins>+ desc.setConfigurable(value.toBoolean(exec));
+ } else if (vm.exception())
+ return false;
</ins><span class="cx">
</span><span class="cx"> JSValue value;
</span><del>- PropertySlot valueSlot(description, PropertySlot::InternalMethodType::HasProperty);
- if (description->getPropertySlot(exec, exec->propertyNames().value, valueSlot)) {
- desc.setValue(valueSlot.getValue(exec, exec->propertyNames().value));
- if (exec->hadException())
</del><ins>+ if (description->hasProperty(exec, exec->propertyNames().value)) {
+ JSValue value = description->get(exec, exec->propertyNames().value);
+ if (vm.exception())
</ins><span class="cx"> return false;
</span><del>- }
</del><ins>+ desc.setValue(value);
+ } else if (vm.exception())
+ return false;
</ins><span class="cx">
</span><del>- PropertySlot writableSlot(description, PropertySlot::InternalMethodType::HasProperty);
- if (description->getPropertySlot(exec, exec->propertyNames().writable, writableSlot)) {
- desc.setWritable(writableSlot.getValue(exec, exec->propertyNames().writable).toBoolean(exec));
- if (exec->hadException())
</del><ins>+ if (description->hasProperty(exec, exec->propertyNames().writable)) {
+ JSValue value = description->get(exec, exec->propertyNames().writable);
+ if (vm.exception())
</ins><span class="cx"> return false;
</span><del>- }
</del><ins>+ desc.setWritable(value.toBoolean(exec));
+ } else if (vm.exception())
+ return false;
</ins><span class="cx">
</span><del>- PropertySlot getSlot(description, PropertySlot::InternalMethodType::HasProperty);
- if (description->getPropertySlot(exec, exec->propertyNames().get, getSlot)) {
- JSValue get = getSlot.getValue(exec, exec->propertyNames().get);
- if (exec->hadException())
</del><ins>+ if (description->hasProperty(exec, exec->propertyNames().get)) {
+ JSValue get = description->get(exec, exec->propertyNames().get);
+ if (vm.exception())
</ins><span class="cx"> return false;
</span><span class="cx"> if (!get.isUndefined()) {
</span><span class="cx"> CallData callData;
</span><span class="cx"> if (getCallData(get, callData) == CallType::None) {
</span><del>- exec->vm().throwException(exec, createTypeError(exec, ASCIILiteral("Getter must be a function.")));
</del><ins>+ vm.throwException(exec, createTypeError(exec, ASCIILiteral("Getter must be a function.")));
</ins><span class="cx"> return false;
</span><span class="cx"> }
</span><span class="cx"> }
</span><span class="cx"> desc.setGetter(get);
</span><del>- }
</del><ins>+ } else if (vm.exception())
+ return false;
</ins><span class="cx">
</span><del>- PropertySlot setSlot(description, PropertySlot::InternalMethodType::HasProperty);
- if (description->getPropertySlot(exec, exec->propertyNames().set, setSlot)) {
- JSValue set = setSlot.getValue(exec, exec->propertyNames().set);
- if (exec->hadException())
</del><ins>+ if (description->hasProperty(exec, exec->propertyNames().set)) {
+ JSValue set = description->get(exec, exec->propertyNames().set);
+ if (vm.exception())
</ins><span class="cx"> return false;
</span><span class="cx"> if (!set.isUndefined()) {
</span><span class="cx"> CallData callData;
</span><span class="cx"> if (getCallData(set, callData) == CallType::None) {
</span><del>- exec->vm().throwException(exec, createTypeError(exec, ASCIILiteral("Setter must be a function.")));
</del><ins>+ vm.throwException(exec, createTypeError(exec, ASCIILiteral("Setter must be a function.")));
</ins><span class="cx"> return false;
</span><span class="cx"> }
</span><span class="cx"> }
</span><span class="cx"> desc.setSetter(set);
</span><del>- }
</del><ins>+ } else if (vm.exception())
+ return false;
</ins><span class="cx">
</span><span class="cx"> if (!desc.isAccessorDescriptor())
</span><span class="cx"> return true;
</span><span class="cx">
</span><span class="cx"> if (desc.value()) {
</span><del>- exec->vm().throwException(exec, createTypeError(exec, ASCIILiteral("Invalid property. 'value' present on property with getter or setter.")));
</del><ins>+ vm.throwException(exec, createTypeError(exec, ASCIILiteral("Invalid property. 'value' present on property with getter or setter.")));
</ins><span class="cx"> return false;
</span><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> if (desc.writablePresent()) {
</span><del>- exec->vm().throwException(exec, createTypeError(exec, ASCIILiteral("Invalid property. 'writable' present on property with getter or setter.")));
</del><ins>+ vm.throwException(exec, createTypeError(exec, ASCIILiteral("Invalid property. 'writable' present on property with getter or setter.")));
</ins><span class="cx"> return false;
</span><span class="cx"> }
</span><span class="cx"> return true;
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoretestses6yaml"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/tests/es6.yaml (197959 => 197960)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/tests/es6.yaml 2016-03-10 23:02:58 UTC (rev 197959)
+++ trunk/Source/JavaScriptCore/tests/es6.yaml 2016-03-10 23:09:30 UTC (rev 197960)
</span><span class="lines">@@ -1017,7 +1017,7 @@
</span><span class="cx"> - path: es6/Proxy_internal_get_calls_ToPrimitive.js
</span><span class="cx"> cmd: runES6 :normal
</span><span class="cx"> - path: es6/Proxy_internal_get_calls_ToPropertyDescriptor.js
</span><del>- cmd: runES6 :fail
</del><ins>+ cmd: runES6 :normal
</ins><span class="cx"> - path: es6/Proxy_internal_getOwnPropertyDescriptor_calls_[[Set]].js
</span><span class="cx"> cmd: runES6 :fail
</span><span class="cx"> - path: es6/Proxy_internal_getOwnPropertyDescriptor_calls_Function.prototype.bind.js
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoretestsstresstopropertykeycorrectnessjs"></a>
<div class="addfile"><h4>Added: trunk/Source/JavaScriptCore/tests/stress/to-property-key-correctness.js (0 => 197960)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/tests/stress/to-property-key-correctness.js (rev 0)
+++ trunk/Source/JavaScriptCore/tests/stress/to-property-key-correctness.js 2016-03-10 23:09:30 UTC (rev 197960)
</span><span class="lines">@@ -0,0 +1,130 @@
</span><ins>+function assert(b) {
+ if (!b)
+ throw new Error("Bad assertion");
+}
+
+function test(f) {
+ for (let i = 0; i < 100; i++)
+ f();
+}
+
+test(function() {
+ let error = null;
+ let handler = {
+ has: function(theTarget, property) {
+ assert(error === null); // Make sure we don't call into has more than once. Make sure we throw on the fist error.
+ error = new Error;
+ throw error;
+ }
+ };
+ let proxy = new Proxy({}, handler);
+ let foo = {};
+
+ let threw = false;
+ try {
+ Object.defineProperty(foo, "foo", proxy);
+ } catch(e) {
+ threw = true;
+ assert(e === error);
+ }
+ assert(threw);
+});
+
+test(function() {
+ let error = null;
+ let handler = {
+ has: function(theTarget, property) {
+ assert(error === null); // Make sure we don't call into has more than once. Make sure we throw on the fist error.
+ if (property === "set") {
+ error = new Error;
+ throw error;
+ }
+ return Reflect.has(theTarget, property);
+ }
+ };
+ let proxy = new Proxy({}, handler);
+ let foo = {};
+
+ let threw = false;
+ try {
+ Object.defineProperty(foo, "foo", proxy);
+ } catch(e) {
+ threw = true;
+ assert(e === error);
+ }
+ assert(threw);
+});
+
+function arrayEq(a, b) {
+ if (a.length !== b.length)
+ return false;
+ for (let i = 0; i < a.length; i++) {
+ if (a[i] !== b[i])
+ return false;
+ }
+ return true;
+
+}
+test(function() {
+ let error = null;
+ let hasArray = [];
+ let getArray = [];
+ let handler = {
+ has: function(theTarget, property) {
+ hasArray.push(property);
+ return Reflect.has(theTarget, property);
+ },
+ get: function(theTarget, property, receiver) {
+ getArray.push(property);
+ return Reflect.get(theTarget, property, receiver);
+ }
+ };
+
+ let target = {
+ enumerable: true,
+ configurable: true,
+ value: 45
+ };
+ let proxy = new Proxy(target, handler);
+ let foo = {};
+ Object.defineProperty(foo, "foo", proxy);
+ assert(arrayEq(hasArray, ["enumerable", "configurable", "value", "writable", "get", "set"]));
+ assert(arrayEq(getArray, ["enumerable", "configurable", "value"]));
+ assert(foo.foo === 45);
+});
+
+test(function() {
+ let error = null;
+ let hasArray = [];
+ let getArray = [];
+ let handler = {
+ has: function(theTarget, property) {
+ hasArray.push(property);
+ return Reflect.has(theTarget, property);
+ },
+ get: function(theTarget, property, receiver) {
+ getArray.push(property);
+ return Reflect.get(theTarget, property, receiver);
+ }
+ };
+
+ let target = {
+ enumerable: true,
+ configurable: true,
+ value: 45,
+ writable: true,
+ get: function(){},
+ set: function(){}
+ };
+ let proxy = new Proxy(target, handler);
+ let foo = {};
+ let threw = false;
+ try {
+ Object.defineProperty(foo, "foo", proxy);
+ } catch(e) {
+ threw = true;
+ }
+ assert(threw);
+ assert(arrayEq(hasArray, ["enumerable", "configurable", "value", "writable", "get", "set"]));
+ assert(arrayEq(hasArray, getArray));
+});
</ins></span></pre>
</div>
</div>
</body>
</html>