Changeset 201672 in webkit


Ignore:
Timestamp:
Jun 3, 2016 5:34:27 PM (8 years ago)
Author:
sbarati@apple.com
Message:

Proxy.ownKeys should no longer throw an exception when duplicate keys are returned and the target is non-extensible
​https://bugs.webkit.org/show_bug.cgi?id=158350
<rdar://problem/26626211>

Reviewed by Michael Saboff.

The spec was recently changes in Proxy OwnPropertyKeys?
to allow for duplicate property names under certain circumstances.
This patch fixes our implementation to match the spec.
See: ​https://github.com/tc39/ecma262/pull/594

  • runtime/ProxyObject.cpp:

(JSC::ProxyObject::performGetOwnPropertyNames):

  • tests/stress/proxy-own-keys.js:

(i.catch):
(ownKeys):
(assert):

Location:
trunk/Source/JavaScriptCore
Files:
3 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/JavaScriptCore/ChangeLog

    r201670 r201672  
     12016-06-03  Saam barati  <sbarati@apple.com>
     2
     3        Proxy.ownKeys should no longer throw an exception when duplicate keys are returned and the target is non-extensible
     4        https://bugs.webkit.org/show_bug.cgi?id=158350
     5        <rdar://problem/26626211>
     6
     7        Reviewed by Michael Saboff.
     8
     9        The spec was recently changes in Proxy [[OwnPropertyKeys]]
     10        to allow for duplicate property names under certain circumstances.
     11        This patch fixes our implementation to match the spec.
     12        See: https://github.com/tc39/ecma262/pull/594
     13
     14        * runtime/ProxyObject.cpp:
     15        (JSC::ProxyObject::performGetOwnPropertyNames):
     16        * tests/stress/proxy-own-keys.js:
     17        (i.catch):
     18        (ownKeys):
     19        (assert):
     20
    1212016-06-03  Saam barati  <sbarati@apple.com>
    222

  • trunk/Source/JavaScriptCore/runtime/ProxyObject.cpp

    r201495 r201672  
    862862    CallType callType;
    863863    JSValue ownKeysMethod = handler->getMethod(exec, callData, callType, makeIdentifier(vm, "ownKeys"), ASCIILiteral("'ownKeys' property of a Proxy's handler should be callable"));
    864     if (exec->hadException())
     864    if (vm.exception())
    865865        return;
    866866    JSObject* target = this->target();
    … …  
    873873    arguments.append(target);
    874874    JSValue arrayLikeObject = call(exec, ownKeysMethod, callType, callData, handler, arguments);
    875     if (exec->hadException())
     875    if (vm.exception())
    876876        return;
    877877
    … …  
    891891    ASSERT(resultFilter);
    892892    RuntimeTypeMask dontThrowAnExceptionTypeFilter = TypeString | TypeSymbol;
    893     HashMap<UniquedStringImpl*, unsigned> uncheckedResultKeys;
    894     unsigned totalSize = 0;
     893    HashSet<UniquedStringImpl*> uncheckedResultKeys;
    895894
    896895    auto addPropName = [&] (JSValue value, RuntimeType type) -> bool {
    … …  
    902901
    903902        Identifier ident = value.toPropertyKey(exec);
    904         if (exec->hadException())
     903        if (vm.exception())
    905904            return doExitEarly;
    906905
    907         ++uncheckedResultKeys.add(ident.impl(), 0).iterator->value;
    908         ++totalSize;
    909 
     906        uncheckedResultKeys.add(ident.impl());
    910907        trapResult.addUnchecked(ident.impl());
    911 
    912908        return dontExitEarly;
    913909    };
    914910
    915911    createListFromArrayLike(exec, arrayLikeObject, dontThrowAnExceptionTypeFilter, ASCIILiteral("Proxy handler's 'ownKeys' method must return an array-like object containing only Strings and Symbols"), addPropName);
    916     if (exec->hadException())
     912    if (vm.exception())
    917913        return;
    918914
    … …  
    921917    PropertyNameArray targetKeys(&vm, propertyNameMode);
    922918    target->methodTable(vm)->getOwnPropertyNames(target, exec, targetKeys, enumerationMode);
    923     if (exec->hadException())
     919    if (vm.exception())
    924920        return;
    925921    Vector<UniquedStringImpl*> targetConfigurableKeys;
    … …  
    928924        PropertyDescriptor descriptor;
    929925        bool isPropertyDefined = target->getOwnPropertyDescriptor(exec, ident.impl(), descriptor);
    930         if (exec->hadException())
     926        if (vm.exception())
    931927            return;
    932928        if (isPropertyDefined && !descriptor.configurable())
    … …  
    936932    }
    937933
    938     auto removeIfContainedInUncheckedResultKeys = [&] (UniquedStringImpl* impl) -> bool {
    939         static const bool isContainedIn = true;
    940         static const bool isNotContainedIn = false;
    941 
     934    enum ContainedIn { IsContainedIn, IsNotContainedIn };
     935    auto removeIfContainedInUncheckedResultKeys = [&] (UniquedStringImpl* impl) -> ContainedIn {
    942936        auto iter = uncheckedResultKeys.find(impl);
    943937        if (iter == uncheckedResultKeys.end())
    944             return isNotContainedIn;
    945 
    946         unsigned& count = iter->value;
    947         if (count == 0)
    948             return isNotContainedIn;
    949 
    950         --count;
    951         --totalSize;
    952         return isContainedIn;
     938            return IsNotContainedIn;
     939
     940        uncheckedResultKeys.remove(iter);
     941        return IsContainedIn;
    953942    };
    954943
    955944    for (UniquedStringImpl* impl : targetNonConfigurableKeys) {
    956         bool contains = removeIfContainedInUncheckedResultKeys(impl);
    957         if (!contains) {
     945        if (removeIfContainedInUncheckedResultKeys(impl) == IsNotContainedIn) {
    958946            throwVMTypeError(exec, makeString("Proxy object's 'target' has the non-configurable property '", String(impl), "' that was not in the result from the 'ownKeys' trap"));
    959947            return;
    … …  
    965953
    966954    for (UniquedStringImpl* impl : targetConfigurableKeys) {
    967         bool contains = removeIfContainedInUncheckedResultKeys(impl);
    968         if (!contains) {
     955        if (removeIfContainedInUncheckedResultKeys(impl) == IsNotContainedIn) {
    969956            throwVMTypeError(exec, makeString("Proxy object's non-extensible 'target' has configurable property '", String(impl), "' that was not in the result from the 'ownKeys' trap"));
    970957            return;
    … …  
    972959    }
    973960
    974 #ifndef NDEBUG
    975     unsigned sum = 0;
    976     for (unsigned keyCount : uncheckedResultKeys.values())
    977         sum += keyCount;
    978     ASSERT(sum == totalSize);
    979 #endif
    980 
    981     if (totalSize) {
    982         throwVMTypeError(exec, ASCIILiteral("Proxy handler's 'ownKeys' method returned a key that was not present in its target or it returned duplicate keys"));
     961    if (uncheckedResultKeys.size()) {
     962        throwVMTypeError(exec, ASCIILiteral("Proxy handler's 'ownKeys' method returned a key that was not present in its non-extensible target"));
    983963        return;
    984964    }

  • trunk/Source/JavaScriptCore/tests/stress/proxy-own-keys.js

    r198813 r201672  
    161161        } catch(e) {
    162162            threw = true;
    163             assert(e.toString() === "TypeError: Proxy handler's 'ownKeys' method returned a key that was not present in its target or it returned duplicate keys");
    164         }
    165         assert(threw);
    166         assert(called);
    167         called = false;
     163            assert(e.toString() === "TypeError: Proxy handler's 'ownKeys' method returned a key that was not present in its non-extensible target");
     164        }
     165        assert(threw);
     166        assert(called);
     167        called = false;
     168    }
     169}
     170
     171{
     172    let target = {};
     173    let called1 = false;
     174    let called2 = false;
     175    Object.defineProperty(target, 'a', { value: 42, configurable: false });
     176    let p1 = new Proxy(target, {
     177        ownKeys() {
     178            called1 = true;
     179            return ['a', 'a'];
     180        }
     181    });
     182    let p2 = new Proxy(p1, {
     183        ownKeys() {
     184            called2 = true;
     185            return ['a'];
     186        }
     187    });
     188
     189    for (let i = 0; i < 500; i++) {
     190        // FIXME: we may update the spec to make this test not throw.
     191        // see: https://github.com/tc39/ecma262/pull/594
     192        let threw = false;
     193        try {
     194            Reflect.ownKeys(p2);
     195        } catch(e) {
     196            assert(e.toString() === "TypeError: Proxy object's 'target' has the non-configurable property 'a' that was not in the result from the 'ownKeys' trap");
     197            threw = true;
     198        }
     199        assert(threw);
     200        assert(called1);
     201        assert(called2);
     202    }
     203}
     204
     205{
     206    let target = {};
     207    let called1 = false;
     208    let called2 = false;
     209    Object.defineProperty(target, 'a', { value: 42, configurable: true });
     210    Object.preventExtensions(target);
     211    let p1 = new Proxy(target, {
     212        ownKeys() {
     213            called1 = true;
     214            return ['a', 'a'];
     215        }
     216    });
     217    let p2 = new Proxy(p1, {
     218        ownKeys() {
     219            called2 = true;
     220            return ['a'];
     221        }
     222    });
     223
     224    for (let i = 0; i < 500; i++) {
     225        // FIXME: we may update the spec to make this test not throw.
     226        // see: https://github.com/tc39/ecma262/pull/594
     227        let threw = false;
     228        try {
     229            Reflect.ownKeys(p2);
     230        } catch(e) {
     231            assert(e.toString() === "TypeError: Proxy object's non-extensible 'target' has configurable property 'a' that was not in the result from the 'ownKeys' trap");
     232            threw = true;
     233        }
     234        assert(threw);
     235        assert(called1);
     236        assert(called2);
    168237    }
    169238}
    … …  
    187256    let proxy = new Proxy(target, handler);
    188257    for (let i = 0; i < 500; i++) {
    189         let threw = false;
    190         try {
    191             Object.keys(proxy);
    192         } catch(e) {
    193             threw = true;
    194             assert(e.toString() === "TypeError: Proxy handler's 'ownKeys' method returned a key that was not present in its target or it returned duplicate keys");
    195         }
    196         assert(threw);
     258        Object.keys(proxy);
    197259        assert(called);
    198260        called = false;
Note: See TracChangeset for help on using the changeset viewer.