Changeset 274308 in webkit


Ignore:
Timestamp:
Mar 11, 2021 4:08:05 PM (17 months ago)
Author:
Alexey Shvayka
Message:

Align JSGlobalObject::defineOwnProperty() with the spec and other runtimes
​https://bugs.webkit.org/show_bug.cgi?id=203456

Reviewed by Robin Morisset.

JSTests:

  • microbenchmarks/global-var-put-to-scope.js: Added.
  • stress/eval-func-decl-in-frozen-global.js:

Object.freeze() redefines all global variables as ReadOnly, including hoisted var error.
Aligns with V8.

  • stress/global-object-define-own-property-put-to-scope.js: Added.
  • stress/global-object-define-own-property.js: Added.
  • stress/to-this-before-arrow-function-closes-over-this-that-starts-as-lexical-environment.js:

Fix unwanted name conflict, which was an error in the original test, not an intended part of it.
Also, remove misleading comment on defineProperty and assert accessors are created on global object.
Aligns with V8.

LayoutTests/imported/w3c:

  • web-platform-tests/html/browsers/the-windowproxy-exotic-object/windowproxy-define-own-property-unforgeable-same-origin-expected.txt: Added.
  • web-platform-tests/html/browsers/the-windowproxy-exotic-object/windowproxy-define-own-property-unforgeable-same-origin.html: Added.

Source/JavaScriptCore:

Per spec, top-level var bindings are non-configurable properties of the global
object [1], while undefined / NaN / Infinity are also non-writable [2].

Prior to this change, redefining global var binding with accessor descriptor
failed silently (rather than throwing a TypeError); redefining with data or
generic descriptor created a structure property, which took precedence over
symbol table entry in JSGlobalObject::getOwnPropertySlot(), effectively
destroying live binding between global.foo and var foo.

This patch re-engineers JSGlobalObject::defineOwnProperty(), fixing both issues
mentioned above. If defineOwnProperty() override is removed, there is no way
a live binding can be maintained.

In a follow-up change, JSGlobalObject::getOwnPropertySlot() will be updated to
search symbol table first, aligning it with the spec [3], put(), and
defineOwnProperty(). Apart from consistency, this will bring a mild speed-up.

To accomodate global var binding reassignment right after it becomes read-only
(in the same scope), this patch introduces a watchpoint that can be fired by
JSGlobalObject::defineOwnProperty(). put_to_scope performance is neutral.

Also, this patch removes unused symbolTableGet() overload and orphaned
JSGlobalObject::defineGetter() / JSGlobalObject::defineSetter() declarations.

[1]: ​https://tc39.es/ecma262/#sec-object-environment-records-createmutablebinding-n-d
[2]: ​https://tc39.es/ecma262/#sec-value-properties-of-the-global-object
[3]: ​https://tc39.es/ecma262/#sec-global-environment-records-getbindingvalue-n-s

  • dfg/DFGByteCodeParser.cpp:

(JSC::DFG::ByteCodeParser::needsDynamicLookup):
(JSC::DFG::ByteCodeParser::parseBlock):

  • jit/JIT.cpp:

(JSC::JIT::emitVarReadOnlyCheck):

  • jit/JIT.h:
  • jit/JITPropertyAccess.cpp:

(JSC::JIT::emit_op_put_to_scope):

  • jit/JITPropertyAccess32_64.cpp:

(JSC::JIT::emit_op_put_to_scope):

  • llint/LowLevelInterpreter.asm:
  • llint/LowLevelInterpreter32_64.asm:
  • llint/LowLevelInterpreter64.asm:
  • runtime/JSGlobalObject.cpp:

(JSC::JSGlobalObject::JSGlobalObject):
(JSC::JSGlobalObject::defineOwnProperty):

  • runtime/JSGlobalObject.h:

(JSC::JSGlobalObject::varReadOnlyWatchpoint):

  • runtime/JSSymbolTableObject.h:

(JSC::symbolTableGet):

Source/WebCore:

This patch removes location special-casing, which a) incorrectly returned
false if new descriptor was the same as the current one and b) failed
silently otherwise (rather than throwing a TypeError).

However, this change introduces window / document special-casing because
they exist on the structure and as symbol table entries (for performance reasons).
Aligns WebKit with Blink and partly with Gecko.

Test: imported/w3c/web-platform-tests/html/browsers/the-windowproxy-exotic-object/windowproxy-define-own-property-unforgeable-same-origin.html

  • bindings/js/JSDOMWindowCustom.cpp:

(WebCore::JSDOMWindow::defineOwnProperty):

LayoutTests:

  • fast/dom/Window/Location/window-override-location-using-defineGetter-expected.txt:
  • fast/dom/Window/Location/window-override-location-using-defineGetter.html:
  • fast/dom/Window/Location/window-override-window-using-defineGetter-expected.txt:
  • fast/dom/Window/Location/window-override-window-using-defineGetter.html:
  • fast/dom/getter-on-window-object2-expected.txt:
  • fast/dom/getter-on-window-object2.html:
Location:
trunk
Files:
5 added
25 edited

Legend:

Unmodified
Added
Removed

  • trunk/JSTests/ChangeLog

    r274274 r274308  
     12021-03-11  Alexey Shvayka  <shvaikalesh@gmail.com>
     2
     3        Align JSGlobalObject::defineOwnProperty() with the spec and other runtimes
     4        https://bugs.webkit.org/show_bug.cgi?id=203456
     5
     6        Reviewed by Robin Morisset.
     7
     8        * microbenchmarks/global-var-put-to-scope.js: Added.
     9        * stress/eval-func-decl-in-frozen-global.js:
     10        Object.freeze() redefines all global variables as ReadOnly, including hoisted `var error`.
     11        Aligns with V8.
     12
     13        * stress/global-object-define-own-property-put-to-scope.js: Added.
     14        * stress/global-object-define-own-property.js: Added.
     15        * stress/to-this-before-arrow-function-closes-over-this-that-starts-as-lexical-environment.js:
     16        Fix unwanted name conflict, which was an error in the original test, not an intended part of it.
     17        Also, remove misleading comment on `defineProperty` and assert accessors are created on global object.
     18        Aligns with V8.
     19
    1202021-03-11  Commit Queue  <commit-queue@webkit.org>
    221

  • trunk/JSTests/stress/eval-func-decl-in-frozen-global.js

    r215984 r274308  
    3737Object.freeze(this);
    3838{
    39   var error = false;
     39  let error = false;
    4040  try {
    4141    eval('{ function boo() {} }');

  • trunk/JSTests/stress/to-this-before-arrow-function-closes-over-this-that-starts-as-lexical-environment.js

    r202693 r274308  
    1818    function wrapper() {
    1919        let x = () => {
    20             // This should not defineProperty on a JSLexicalEnvironment! That's a huge bug.
    21             Object.defineProperty(this, "foo", {
     20            Object.defineProperty(this, "baz", {
    2221                get: function() { },
    2322                set: function() { }
    … …  
    3837    function wrapper() {
    3938        let x = () => {
    40             // This should not defineProperty on a JSLexicalEnvironment! That's a huge bug.
    41             Object.defineProperty(this, "foo", {
     39            Object.defineProperty(this, "baz2", {
    4240                get: function() { },
    4341                set: function() { }
    4442            });
     43            assert(this === globalThis);
    4544        }
    4645
    … …  
    5756}
    5857foo2();
     58
     59assert(this.hasOwnProperty("baz"));
     60assert(this.hasOwnProperty("baz2"));

  • trunk/LayoutTests/ChangeLog

    r274301 r274308  
     12021-03-11  Alexey Shvayka  <shvaikalesh@gmail.com>
     2
     3        Align JSGlobalObject::defineOwnProperty() with the spec and other runtimes
     4        https://bugs.webkit.org/show_bug.cgi?id=203456
     5
     6        Reviewed by Robin Morisset.
     7
     8        * fast/dom/Window/Location/window-override-location-using-defineGetter-expected.txt:
     9        * fast/dom/Window/Location/window-override-location-using-defineGetter.html:
     10        * fast/dom/Window/Location/window-override-window-using-defineGetter-expected.txt:
     11        * fast/dom/Window/Location/window-override-window-using-defineGetter.html:
     12        * fast/dom/getter-on-window-object2-expected.txt:
     13        * fast/dom/getter-on-window-object2.html:
     14
    1152021-03-11  Aditya Keerthi  <akeerthi@apple.com>
    216

  • trunk/LayoutTests/fast/dom/Window/Location/window-override-location-using-defineGetter-expected.txt

    r42218 r274308  
     1PASS function () {
     2        window.__defineGetter__("location", () => "haxored");
     3    } threw exception TypeError: Attempting to change configurable attribute of unconfigurable property..
    14PASS result is correctValue
    25PASS successfullyParsed is true

  • trunk/LayoutTests/fast/dom/Window/Location/window-override-location-using-defineGetter.html

    r155265 r274308  
    66<body>
    77<script>
    8     window.__defineGetter__("location", function() { return "haxored"; });
     8    shouldThrowErrorName(function() {
     9        window.__defineGetter__("location", () => "haxored");
     10    }, "TypeError");
    911
    1012    var result = normalizeURL(String(window.location));

  • trunk/LayoutTests/fast/dom/Window/Location/window-override-window-using-defineGetter-expected.txt

    r42218 r274308  
     1PASS function () {
     2        window.__defineGetter__("window", () => ({ location: "haxored" }));
     3    } threw exception TypeError: Attempting to change configurable attribute of unconfigurable property..
    14PASS result is correctValue
    25PASS successfullyParsed is true

  • trunk/LayoutTests/fast/dom/Window/Location/window-override-window-using-defineGetter.html

    r155265 r274308  
    66<body>
    77<script>
    8     window.__defineGetter__("window", function() {
    9         return { location: "haxored" };
    10     });
     8    shouldThrowErrorName(function() {
     9        window.__defineGetter__("window", () => ({ location: "haxored" }));
     10    }, "TypeError");
    1111
    1212    var result = normalizeURL(String(window.location));

  • trunk/LayoutTests/fast/dom/getter-on-window-object2-expected.txt

    r99136 r274308  
    44
    55
     6PASS function () {
     7    window.__defineGetter__("x", function() { return "window.x __getter__"; });
     8} threw exception TypeError: Attempting to change configurable attribute of unconfigurable property..
    69PASS window.x is 1
    710PASS typeof window.__lookupGetter__('x') is 'undefined'
    811PASS typeof Object.getOwnPropertyDescriptor(window, 'x').get is 'undefined'
    912
     13PASS function () {
     14window.__defineSetter__("x", function() { debug("window.x __setter__ called"); });
     15} threw exception TypeError: Attempting to change configurable attribute of unconfigurable property..
    1016PASS window.x is 2
    1117PASS typeof window.__lookupGetter__('x') is 'undefined'

  • trunk/LayoutTests/fast/dom/getter-on-window-object2.html

    r155265 r274308  
    55
    66var x = 1;
    7 try {
     7shouldThrowErrorName(function() {
    88    window.__defineGetter__("x", function() { return "window.x __getter__"; });
    9 } catch(e) { debug(e); }
     9}, "TypeError");
    1010
    1111shouldBe("window.x", "1");
    … …  
    1515
    1616
    17 try {
     17shouldThrowErrorName(function() {
    1818window.__defineSetter__("x", function() { debug("window.x __setter__ called"); });
    19 } catch(e) { debug(e); }
     19}, "TypeError");
    2020x = 2;
    2121

  • trunk/LayoutTests/imported/w3c/ChangeLog

    r274235 r274308  
     12021-03-11  Alexey Shvayka  <shvaikalesh@gmail.com>
     2
     3        Align JSGlobalObject::defineOwnProperty() with the spec and other runtimes
     4        https://bugs.webkit.org/show_bug.cgi?id=203456
     5
     6        Reviewed by Robin Morisset.
     7
     8        * web-platform-tests/html/browsers/the-windowproxy-exotic-object/windowproxy-define-own-property-unforgeable-same-origin-expected.txt: Added.
     9        * web-platform-tests/html/browsers/the-windowproxy-exotic-object/windowproxy-define-own-property-unforgeable-same-origin.html: Added.
     10
    1112021-03-10  Antoine Quint  <graouts@webkit.org>
    212

  • trunk/Source/JavaScriptCore/ChangeLog

    r274288 r274308  
     12021-03-11  Alexey Shvayka  <shvaikalesh@gmail.com>
     2
     3        Align JSGlobalObject::defineOwnProperty() with the spec and other runtimes
     4        https://bugs.webkit.org/show_bug.cgi?id=203456
     5
     6        Reviewed by Robin Morisset.
     7
     8        Per spec, top-level `var` bindings are non-configurable properties of the global
     9        object [1], while `undefined` / `NaN` / `Infinity` are also non-writable [2].
     10
     11        Prior to this change, redefining global `var` binding with accessor descriptor
     12        failed silently (rather than throwing a TypeError); redefining with data or
     13        generic descriptor created a structure property, which took precedence over
     14        symbol table entry in JSGlobalObject::getOwnPropertySlot(), effectively
     15        destroying live binding between `global.foo` and `var foo`.
     16
     17        This patch re-engineers JSGlobalObject::defineOwnProperty(), fixing both issues
     18        mentioned above. If defineOwnProperty() override is removed, there is no way
     19        a live binding can be maintained.
     20
     21        In a follow-up change, JSGlobalObject::getOwnPropertySlot() will be updated to
     22        search symbol table first, aligning it with the spec [3], put(), and
     23        defineOwnProperty(). Apart from consistency, this will bring a mild speed-up.
     24
     25        To accomodate global `var` binding reassignment right after it becomes read-only
     26        (in the same scope), this patch introduces a watchpoint that can be fired by
     27        JSGlobalObject::defineOwnProperty(). put_to_scope performance is neutral.
     28
     29        Also, this patch removes unused symbolTableGet() overload and orphaned
     30        JSGlobalObject::defineGetter() / JSGlobalObject::defineSetter() declarations.
     31
     32        [1]: https://tc39.es/ecma262/#sec-object-environment-records-createmutablebinding-n-d
     33        [2]: https://tc39.es/ecma262/#sec-value-properties-of-the-global-object
     34        [3]: https://tc39.es/ecma262/#sec-global-environment-records-getbindingvalue-n-s
     35
     36        * dfg/DFGByteCodeParser.cpp:
     37        (JSC::DFG::ByteCodeParser::needsDynamicLookup):
     38        (JSC::DFG::ByteCodeParser::parseBlock):
     39        * jit/JIT.cpp:
     40        (JSC::JIT::emitVarReadOnlyCheck):
     41        * jit/JIT.h:
     42        * jit/JITPropertyAccess.cpp:
     43        (JSC::JIT::emit_op_put_to_scope):
     44        * jit/JITPropertyAccess32_64.cpp:
     45        (JSC::JIT::emit_op_put_to_scope):
     46        * llint/LowLevelInterpreter.asm:
     47        * llint/LowLevelInterpreter32_64.asm:
     48        * llint/LowLevelInterpreter64.asm:
     49        * runtime/JSGlobalObject.cpp:
     50        (JSC::JSGlobalObject::JSGlobalObject):
     51        (JSC::JSGlobalObject::defineOwnProperty):
     52        * runtime/JSGlobalObject.h:
     53        (JSC::JSGlobalObject::varReadOnlyWatchpoint):
     54        * runtime/JSSymbolTableObject.h:
     55        (JSC::symbolTableGet):
     56
    1572021-03-11  BJ Burg  <bburg@apple.com>
    258

  • trunk/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp

    r274037 r274308  
    42164216
    42174217    switch (type) {
    4218     case GlobalProperty:
    42194218    case GlobalVar:
     4219    case GlobalVarWithVarInjectionChecks: {
     4220        if (opcode == op_put_to_scope && globalObject->varReadOnlyWatchpoint()->hasBeenInvalidated())
     4221            return true;
     4222
     4223        return false;
     4224    }
     4225
     4226    case GlobalProperty:   
    42204227    case GlobalLexicalVar:
    42214228    case ClosureVar:
    … …  
    42524259
    42534260    case GlobalPropertyWithVarInjectionChecks:
    4254     case GlobalVarWithVarInjectionChecks:
    42554261    case GlobalLexicalVarWithVarInjectionChecks:
    42564262    case ClosureVarWithVarInjectionChecks:
    … …  
    79267932                    addToGraph(CheckNotEmpty, value);
    79277933                }
     7934                if (resolveType == GlobalVar || resolveType == GlobalVarWithVarInjectionChecks)
     7935                    m_graph.watchpoints().addLazily(globalObject->varReadOnlyWatchpoint());
    79287936
    79297937                JSSegmentedVariableObject* scopeObject = jsCast<JSSegmentedVariableObject*>(JSScope::constantScopeForCodeBlock(resolveType, m_inlineStackTop->m_codeBlock));

  • trunk/Source/JavaScriptCore/jit/JIT.cpp

    r274024 r274308  
    116116{
    117117    addSlowCase(branch8(NotEqual, Address(pointerToSet, WatchpointSet::offsetOfState()), TrustedImm32(IsInvalidated)));
     118}
     119
     120void JIT::emitVarReadOnlyCheck(ResolveType resolveType)
     121{
     122    if (resolveType == GlobalVar || resolveType == GlobalVarWithVarInjectionChecks)
     123        addSlowCase(branch8(Equal, AbsoluteAddress(m_codeBlock->globalObject()->varReadOnlyWatchpoint()->addressOfState()), TrustedImm32(IsInvalidated)));
    118124}
    119125

  • trunk/Source/JavaScriptCore/jit/JIT.h

    r272580 r274308  
    753753        void emitNewFuncExprCommon(const Instruction*);
    754754        void emitVarInjectionCheck(bool needsVarInjectionChecks);
     755        void emitVarReadOnlyCheck(ResolveType);
    755756        void emitResolveClosure(VirtualRegister dst, VirtualRegister scope, bool needsVarInjectionChecks, unsigned depth);
    756757        void emitLoadWithStructureCheck(VirtualRegister scope, Structure** structureSlot);

  • trunk/Source/JavaScriptCore/jit/JITPropertyAccess.cpp

    r272580 r274308  
    12281228            RELEASE_ASSERT(constantScope);
    12291229            emitVarInjectionCheck(needsVarInjectionChecks(resolveType));
     1230            emitVarReadOnlyCheck(resolveType);
    12301231            if (!isInitialization(getPutInfo.initializationMode()) && (resolveType == GlobalLexicalVar || resolveType == GlobalLexicalVarWithVarInjectionChecks)) {
    12311232                // We need to do a TDZ check here because we can't always prove we need to emit TDZ checks statically.

  • trunk/Source/JavaScriptCore/jit/JITPropertyAccess32_64.cpp

    r272580 r274308  
    12441244            emitWriteBarrier(constantScope, value, ShouldFilterValue);
    12451245            emitVarInjectionCheck(needsVarInjectionChecks(resolveType));
     1246            emitVarReadOnlyCheck(resolveType);
    12461247            if (!isInitialization(getPutInfo.initializationMode()) && (resolveType == GlobalLexicalVar || resolveType == GlobalLexicalVarWithVarInjectionChecks)) {
    12471248                // We need to do a TDZ check here because we can't always prove we need to emit TDZ checks statically.

  • trunk/Source/JavaScriptCore/llint/LowLevelInterpreter.asm

    r272170 r274308  
    13421342end
    13431343
     1344macro varReadOnlyCheck(slowPath, scratch)
     1345    loadp CodeBlock[cfr], scratch
     1346    loadp CodeBlock::m_globalObject[scratch], scratch
     1347    loadp JSGlobalObject::m_varReadOnlyWatchpoint[scratch], scratch
     1348    bbeq WatchpointSet::m_state[scratch], IsInvalidated, slowPath
     1349end
     1350
    13441351macro checkSwitchToJIT(increment, action)
    13451352    loadp CodeBlock[cfr], t0

  • trunk/Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm

    r273104 r274308  
    26262626.pGlobalVar:
    26272627    bineq t0, GlobalVar, .pGlobalLexicalVar
     2628    varReadOnlyCheck(.pDynamic, t2)
    26282629    putGlobalVariable()
    26292630    writeBarrierOnGlobalObject(size, get, m_value)
    … …  
    26532654.pGlobalVarWithVarInjectionChecks:
    26542655    bineq t0, GlobalVarWithVarInjectionChecks, .pGlobalLexicalVarWithVarInjectionChecks
     2656    # FIXME: Avoid loading m_globalObject twice
     2657    # https://bugs.webkit.org/show_bug.cgi?id=223097
    26552658    varInjectionCheck(.pDynamic)
     2659    varReadOnlyCheck(.pDynamic, t2)
    26562660    putGlobalVariable()
    26572661    writeBarrierOnGlobalObject(size, get, m_value)

  • trunk/Source/JavaScriptCore/llint/LowLevelInterpreter64.asm

    r272580 r274308  
    27282728.pGlobalVar:
    27292729    bineq t0, GlobalVar, .pGlobalLexicalVar
     2730    varReadOnlyCheck(.pDynamic, t2)
    27302731    putGlobalVariable()
    27312732    writeBarrierOnGlobalObject(size, get, m_value)
    … …  
    27552756.pGlobalVarWithVarInjectionChecks:
    27562757    bineq t0, GlobalVarWithVarInjectionChecks, .pGlobalLexicalVarWithVarInjectionChecks
     2758    # FIXME: Avoid loading m_globalObject twice
     2759    # https://bugs.webkit.org/show_bug.cgi?id=223097
    27572760    varInjectionCheck(.pDynamic, t2)
     2761    varReadOnlyCheck(.pDynamic, t2)
    27582762    putGlobalVariable()
    27592763    writeBarrierOnGlobalObject(size, get, m_value)

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

    r273777 r274308  
    496496    , m_havingABadTimeWatchpoint(WatchpointSet::create(IsWatched))
    497497    , m_varInjectionWatchpoint(WatchpointSet::create(IsWatched))
     498    , m_varReadOnlyWatchpoint(WatchpointSet::create(IsWatched))
    498499    , m_weakRandom(Options::forceWeakRandomSeed() ? Options::forcedWeakRandomSeed() : static_cast<unsigned>(randomNumber() * (std::numeric_limits<unsigned>::max() + 1.0)))
    499500    , m_arrayIteratorProtocolWatchpointSet(IsWatched)
    … …  
    14811482{
    14821483    VM& vm = globalObject->vm();
     1484    auto scope = DECLARE_THROW_SCOPE(vm);
    14831485    JSGlobalObject* thisObject = jsCast<JSGlobalObject*>(object);
    1484     PropertySlot slot(thisObject, PropertySlot::InternalMethodType::VMInquiry, &vm);
    1485     // silently ignore attempts to add accessors aliasing vars.
    1486     if (descriptor.isAccessorDescriptor() && symbolTableGet(thisObject, propertyName, slot))
    1487         return false;
    1488     slot.disallowVMEntry.reset();
    1489     return Base::defineOwnProperty(thisObject, globalObject, propertyName, descriptor, shouldThrow);
     1486
     1487    SymbolTableEntry entry;
     1488    PropertyDescriptor currentDescriptor;
     1489    if (symbolTableGet(thisObject, propertyName, entry, currentDescriptor)) {
     1490        bool isExtensible = false; // ignored since current descriptor is present
     1491        bool isCurrentDefined = true;
     1492        bool isCompatibleDescriptor = validateAndApplyPropertyDescriptor(globalObject, nullptr, propertyName, isExtensible, descriptor, isCurrentDefined, currentDescriptor, shouldThrow);
     1493        EXCEPTION_ASSERT(!!scope.exception() == !isCompatibleDescriptor);
     1494        if (!isCompatibleDescriptor)
     1495            return false;
     1496
     1497        if (descriptor.value()) {
     1498            bool ignoreReadOnlyErrors = true;
     1499            bool putResult = false;
     1500            if (symbolTablePutTouchWatchpointSet(thisObject, globalObject, propertyName, descriptor.value(), shouldThrow, ignoreReadOnlyErrors, putResult))
     1501                ASSERT(putResult);
     1502            scope.assertNoException();
     1503        }
     1504        if (descriptor.writablePresent() && !descriptor.writable() && !entry.isReadOnly()) {
     1505            entry.setAttributes(static_cast<unsigned>(PropertyAttribute::ReadOnly));
     1506            thisObject->symbolTable()->set(propertyName.uid(), entry);
     1507            thisObject->varReadOnlyWatchpoint()->fireAll(vm, "GlobalVar was redefined as ReadOnly");
     1508        }
     1509        return true;
     1510    }
     1511
     1512    RELEASE_AND_RETURN(scope, Base::defineOwnProperty(thisObject, globalObject, propertyName, descriptor, shouldThrow));
    14901513}
    14911514

  • trunk/Source/JavaScriptCore/runtime/JSGlobalObject.h

    r273661 r274308  
    475475    RefPtr<WatchpointSet> m_havingABadTimeWatchpoint;
    476476    RefPtr<WatchpointSet> m_varInjectionWatchpoint;
     477    RefPtr<WatchpointSet> m_varReadOnlyWatchpoint;
    477478
    478479    std::unique_ptr<JSGlobalObjectRareData> m_rareData;
    … …  
    624625    JS_EXPORT_PRIVATE static bool getOwnPropertySlot(JSObject*, JSGlobalObject*, PropertyName, PropertySlot&);
    625626    JS_EXPORT_PRIVATE static bool put(JSCell*, JSGlobalObject*, PropertyName, JSValue, PutPropertySlot&);
    626 
    627     JS_EXPORT_PRIVATE static void defineGetter(JSObject*, JSGlobalObject*, PropertyName, JSObject* getterFunc, unsigned attributes);
    628     JS_EXPORT_PRIVATE static void defineSetter(JSObject*, JSGlobalObject*, PropertyName, JSObject* setterFunc, unsigned attributes);
    629627    JS_EXPORT_PRIVATE static bool defineOwnProperty(JSObject*, JSGlobalObject*, PropertyName, const PropertyDescriptor&, bool shouldThrow);
    630628
    … …  
    999997    WatchpointSet* havingABadTimeWatchpoint() { return m_havingABadTimeWatchpoint.get(); }
    1000998    WatchpointSet* varInjectionWatchpoint() { return m_varInjectionWatchpoint.get(); }
     999    WatchpointSet* varReadOnlyWatchpoint() { return m_varReadOnlyWatchpoint.get(); }
    10011000       
    10021001    bool isHavingABadTime() const

  • trunk/Source/JavaScriptCore/runtime/JSSymbolTableObject.h

    r273138 r274308  
    100100template<typename SymbolTableObjectType>
    101101inline bool symbolTableGet(
    102     SymbolTableObjectType* object, PropertyName propertyName, PropertyDescriptor& descriptor)
     102    SymbolTableObjectType* object, PropertyName propertyName, SymbolTableEntry& entry, PropertyDescriptor& descriptor)
    103103{
    104104    SymbolTable& symbolTable = *object->symbolTable();
    … …  
    107107    if (iter == symbolTable.end(locker))
    108108        return false;
    109     SymbolTableEntry::Fast entry = iter->value;
     109    entry = iter->value;
    110110    ASSERT(!entry.isNull());
    111111
    … …  
    116116
    117117    descriptor.setDescriptor(object->variableAt(offset).get(), entry.getAttributes() | PropertyAttribute::DontDelete);
    118     return true;
    119 }
    120 
    121 template<typename SymbolTableObjectType>
    122 inline bool symbolTableGet(
    123     SymbolTableObjectType* object, PropertyName propertyName, PropertySlot& slot,
    124     bool& slotIsWriteable)
    125 {
    126     SymbolTable& symbolTable = *object->symbolTable();
    127     ConcurrentJSLocker locker(symbolTable.m_lock);
    128     SymbolTable::Map::iterator iter = symbolTable.find(locker, propertyName.uid());
    129     if (iter == symbolTable.end(locker))
    130         return false;
    131     SymbolTableEntry::Fast entry = iter->value;
    132     ASSERT(!entry.isNull());
    133 
    134     ScopeOffset offset = entry.scopeOffset();
    135     // Defend against the inspector asking for a var after it has been optimized out.
    136     if (!object->isValidScopeOffset(offset))
    137         return false;
    138 
    139     slot.setValue(object, entry.getAttributes() | PropertyAttribute::DontDelete, object->variableAt(offset).get());
    140     slotIsWriteable = !entry.isReadOnly();
    141118    return true;
    142119}

  • trunk/Source/WebCore/ChangeLog

    r274307 r274308  
     12021-03-11  Alexey Shvayka  <shvaikalesh@gmail.com>
     2
     3        Align JSGlobalObject::defineOwnProperty() with the spec and other runtimes
     4        https://bugs.webkit.org/show_bug.cgi?id=203456
     5
     6        Reviewed by Robin Morisset.
     7
     8        This patch removes `location` special-casing, which a) incorrectly returned
     9        `false` if new descriptor was the same as the current one and b) failed
     10        silently otherwise (rather than throwing a TypeError).
     11
     12        However, this change introduces `window` / `document` special-casing because
     13        they exist on the structure and as symbol table entries (for performance reasons).
     14        Aligns WebKit with Blink and partly with Gecko.
     15
     16        Test: imported/w3c/web-platform-tests/html/browsers/the-windowproxy-exotic-object/windowproxy-define-own-property-unforgeable-same-origin.html
     17
     18        * bindings/js/JSDOMWindowCustom.cpp:
     19        (WebCore::JSDOMWindow::defineOwnProperty):
     20
    1212021-03-11  Chris Dumez  <cdumez@apple.com>
    222

  • trunk/Source/WebCore/bindings/js/JSDOMWindowCustom.cpp

    r273901 r274308  
    476476bool JSDOMWindow::defineOwnProperty(JSC::JSObject* object, JSC::JSGlobalObject* lexicalGlobalObject, JSC::PropertyName propertyName, const JSC::PropertyDescriptor& descriptor, bool shouldThrow)
    477477{
    478     JSC::VM& vm = lexicalGlobalObject->vm();
    479     auto scope = DECLARE_THROW_SCOPE(vm);
    480 
    481478    JSDOMWindow* thisObject = jsCast<JSDOMWindow*>(object);
    482479    // Only allow defining properties in this way by frames in the same origin, as it allows setters to be introduced.
    483480    if (!BindingSecurity::shouldAllowAccessToDOMWindow(lexicalGlobalObject, thisObject->wrapped(), ThrowSecurityError))
    484         RELEASE_AND_RETURN(scope, false);
    485 
    486     EXCEPTION_ASSERT(!scope.exception());
    487     // Don't allow shadowing location using accessor properties.
    488     if (descriptor.isAccessorDescriptor() && propertyName == Identifier::fromString(vm, "location"))
    489481        return false;
    490482
    491     RELEASE_AND_RETURN(scope, Base::defineOwnProperty(thisObject, lexicalGlobalObject, propertyName, descriptor, shouldThrow));
     483    auto& builtinNames = static_cast<JSVMClientData*>(lexicalGlobalObject->vm().clientData)->builtinNames();
     484    if (propertyName == builtinNames.documentPublicName() || propertyName == builtinNames.windowPublicName())
     485        return JSObject::defineOwnProperty(thisObject, lexicalGlobalObject, propertyName, descriptor, shouldThrow);
     486
     487    return Base::defineOwnProperty(thisObject, lexicalGlobalObject, propertyName, descriptor, shouldThrow);
    492488}
    493489
Note: See TracChangeset for help on using the changeset viewer.