Skip to content
Permalink
Browse files
[JSC] Remove defaultValue() from the method table
https://bugs.webkit.org/show_bug.cgi?id=225032

Reviewed by Darin Adler.

Source/JavaScriptCore:

This patch not only removes the unnecessary method table entry, but also makes
the presence of custom ToPrimitive behavior observable to userland code.

To maintain object identity and (possibly) enable caching, Symbol.toPrimitive
method is stored on a structure. To avoid any potential breakage, it's made
replaceable and configurable, covering the case when its holder is a [[ProxyTarget]].

For JSCallbackObject, Symbol.toPrimitive method is created only if ConvertToType
callback is present, before initialization is performed.

Also, this change adds additional ordinaryToPrimitive() cast to fix the invariant
that toPrimitive() returns a primitive value, which was broken if ConvertToType
callback returned an object. The invariant is enforced by the spec [1][2] and is
validated via assertion in JSValue::toStringSlowCase().

[1]: https://tc39.es/ecma262/#sec-toprimitive (step 2.b.vi)
[2]: https://tc39.es/ecma262/#sec-ordinarytoprimitive (step 6)

* API/JSCallbackObject.h:
* API/JSCallbackObjectFunctions.h:
(JSC::JSCallbackObject<Parent>::init):
(JSC::JSCallbackObject<Parent>::customToPrimitive):
(JSC::JSCallbackObject<Parent>::defaultValue): Deleted.
* API/tests/testapiScripts/testapi.js:
* runtime/ClassInfo.h:
* runtime/JSCell.cpp:
(JSC::JSCell::defaultValue): Deleted.
* runtime/JSCell.h:
* runtime/JSObject.cpp:
(JSC::JSObject::toPrimitive const):
(JSC::JSObject::defaultValue): Deleted.
* runtime/JSObject.h:
* runtime/Operations.cpp:
(JSC::jsAddSlowCase):

Source/WebCore:

Test: platform/mac/fast/dom/objc-wrapper-toprimitive.html

* bindings/js/JSPluginElementFunctions.cpp:
(WebCore::pluginElementCustomGetOwnPropertySlot):
* bridge/objc/objc_runtime.h:
* bridge/objc/objc_runtime.mm:
(JSC::Bindings::ObjcFallbackObjectImp::finishCreation):
(JSC::Bindings::ObjcFallbackObjectImp::getOwnPropertySlot):
(JSC::Bindings::JSC_DEFINE_HOST_FUNCTION):
(JSC::Bindings::ObjcFallbackObjectImp::defaultValue): Deleted.
* bridge/runtime_object.cpp:
(JSC::Bindings::RuntimeObject::finishCreation):
(JSC::Bindings::RuntimeObject::getOwnPropertySlot):
(JSC::Bindings::JSC_DEFINE_HOST_FUNCTION):
(JSC::Bindings::RuntimeObject::defaultValue): Deleted.
* bridge/runtime_object.h:

LayoutTests:

* platform/mac/fast/dom/objc-wrapper-toprimitive-expected.txt: Added.
* platform/mac/fast/dom/objc-wrapper-toprimitive.html: Added.
* platform/wk2/TestExpectations:
* plugins/npruntime/tostring-expected.txt:
* plugins/npruntime/tostring.html:
* plugins/npruntime/valueof-expected.txt:
* plugins/npruntime/valueof.html:


Canonical link: https://commits.webkit.org/237087@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@276660 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
shvaikalesh committed Apr 27, 2021
1 parent b91f5a4 commit 32e5915a5be3cdc303859467a1396c09fd0235b7
Showing 24 changed files with 331 additions and 42 deletions.
@@ -1,3 +1,18 @@
2021-04-27 Alexey Shvayka <shvaikalesh@gmail.com>

[JSC] Remove defaultValue() from the method table
https://bugs.webkit.org/show_bug.cgi?id=225032

Reviewed by Darin Adler.

* platform/mac/fast/dom/objc-wrapper-toprimitive-expected.txt: Added.
* platform/mac/fast/dom/objc-wrapper-toprimitive.html: Added.
* platform/wk2/TestExpectations:
* plugins/npruntime/tostring-expected.txt:
* plugins/npruntime/tostring.html:
* plugins/npruntime/valueof-expected.txt:
* plugins/npruntime/valueof.html:

2021-04-27 Aditya Keerthi <akeerthi@apple.com>

[iOS][FCR] Add borders for better control visibility
@@ -0,0 +1,65 @@
This tests ToPrimitive performed on Objective-C wrapper object.

On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".


RuntimeObject

PASS '' + objCController is '<ObjCController>'
PASS +objCController is 0
PASS `${objCController}` is '<ObjCController>'

PASS objCController[Symbol.toPrimitive].length is 1
PASS objCController[Symbol.toPrimitive]('default') is '<ObjCController>'
PASS objCController[Symbol.toPrimitive]('number') is 0
PASS objCController[Symbol.toPrimitive]('string') is '<ObjCController>'
PASS objCController[Symbol.toPrimitive]('foo') threw exception TypeError: Expected primitive hint to match one of 'default', 'number', 'string'..
PASS objCController[Symbol.toPrimitive].call({}, 'default') threw exception TypeError: RuntimeObject[Symbol.toPrimitive] method called on incompatible |this| value..
PASS (0, objCController[Symbol.toPrimitive])() threw exception TypeError: RuntimeObject[Symbol.toPrimitive] method called on incompatible |this| value..

RuntimeObject (redefined Symbol.toPrimitive)

PASS typeof symbolToPrimitiveDescriptor is 'object'
PASS symbolToPrimitiveDescriptor.value is object[Symbol.toPrimitive]
PASS symbolToPrimitiveDescriptor.writable is true
PASS symbolToPrimitiveDescriptor.enumerable is false
PASS symbolToPrimitiveDescriptor.configurable is true

PASS object[Symbol.toPrimitive]() is 'bar'
PASS typeof symbolToPrimitiveDescriptor is 'object'
PASS symbolToPrimitiveDescriptor.value is newToPrimitive
PASS symbolToPrimitiveDescriptor.writable is false
PASS symbolToPrimitiveDescriptor.enumerable is true
PASS symbolToPrimitiveDescriptor.configurable is true

PASS object[Symbol.toPrimitive] is 123

ObjcFallbackObjectImp

PASS '' + fallbackObject is 'undefined'
PASS +fallbackObject is NaN
PASS `${fallbackObject}` is 'undefined'

PASS fallbackObject[Symbol.toPrimitive].length is 0
PASS fallbackObject[Symbol.toPrimitive]() is undefined
PASS fallbackObject[Symbol.toPrimitive]('foo') is undefined
PASS fallbackObject[Symbol.toPrimitive].call({}) threw exception TypeError: ObjcFallbackObject[Symbol.toPrimitive] method called on incompatible |this| value..
PASS (0, fallbackObject[Symbol.toPrimitive])() threw exception TypeError: ObjcFallbackObject[Symbol.toPrimitive] method called on incompatible |this| value..

ObjcFallbackObjectImp (redefined Symbol.toPrimitive)

PASS typeof symbolToPrimitiveDescriptor is 'object'
PASS symbolToPrimitiveDescriptor.value is object[Symbol.toPrimitive]
PASS symbolToPrimitiveDescriptor.writable is true
PASS symbolToPrimitiveDescriptor.enumerable is false
PASS symbolToPrimitiveDescriptor.configurable is true

PASS object[Symbol.toPrimitive]() is 'bar'
PASS typeof symbolToPrimitiveDescriptor is 'object'
PASS symbolToPrimitiveDescriptor.value is newToPrimitive
PASS symbolToPrimitiveDescriptor.writable is false
PASS symbolToPrimitiveDescriptor.enumerable is true
PASS symbolToPrimitiveDescriptor.configurable is true

PASS object[Symbol.toPrimitive] is 123

@@ -0,0 +1,88 @@
<!DOCTYPE html>
<script src="../../../../resources/js-test-pre.js"></script>
<body>
<p id="description"></p>
<div id="console"></div>
<script>
if (window.testRunner)
testRunner.waitUntilDone();

function runTest()
{
description("This tests ToPrimitive performed on Objective-C wrapper object.");
if (!window.objCController) {
testFailed("window.objCController does not exist. Run with --dump-render-tree.");
return;
}

debug("RuntimeObject\n");

shouldBe("'' + objCController", "'<ObjCController>'");
shouldBe("+objCController", "0");
shouldBe("`${objCController}`", "'<ObjCController>'");

debug("");
shouldBe("objCController[Symbol.toPrimitive].length", "1");
shouldBe("objCController[Symbol.toPrimitive]('default')", "'<ObjCController>'");
shouldBe("objCController[Symbol.toPrimitive]('number')", "0");
shouldBe("objCController[Symbol.toPrimitive]('string')", "'<ObjCController>'");
shouldThrow("objCController[Symbol.toPrimitive]('foo')");
shouldThrow("objCController[Symbol.toPrimitive].call({}, 'default')");
shouldThrow("(0, objCController[Symbol.toPrimitive])()");

debug("\nRuntimeObject (redefined Symbol.toPrimitive)\n");
testDefineOwnProperty(objCController);

debug("\nObjcFallbackObjectImp\n");

fallbackObject = objCController.undefinedKey;

shouldBe("'' + fallbackObject", "'undefined'");
shouldBe("+fallbackObject", "NaN");
shouldBe("`${fallbackObject}`", "'undefined'");

debug("");
shouldBe("fallbackObject[Symbol.toPrimitive].length", "0");
shouldBe("fallbackObject[Symbol.toPrimitive]()", "undefined");
shouldBe("fallbackObject[Symbol.toPrimitive]('foo')", "undefined");
shouldThrow("fallbackObject[Symbol.toPrimitive].call({})");
shouldThrow("(0, fallbackObject[Symbol.toPrimitive])()");

debug("\nObjcFallbackObjectImp (redefined Symbol.toPrimitive)\n");
testDefineOwnProperty(fallbackObject);

if (window.testRunner)
testRunner.notifyDone();
}

function testDefineOwnProperty(_object)
{
object = _object;

symbolToPrimitiveDescriptor = Object.getOwnPropertyDescriptor(object, Symbol.toPrimitive);
shouldBe("typeof symbolToPrimitiveDescriptor", "'object'");
shouldBe("symbolToPrimitiveDescriptor.value", "object[Symbol.toPrimitive]");
shouldBe("symbolToPrimitiveDescriptor.writable", "true");
shouldBe("symbolToPrimitiveDescriptor.enumerable", "false");
shouldBe("symbolToPrimitiveDescriptor.configurable", "true");

debug("");
newToPrimitive = () => "bar";
Object.defineProperty(object, Symbol.toPrimitive, { value: newToPrimitive, writable: false, enumerable: true });
shouldBe("object[Symbol.toPrimitive]()", "'bar'");

symbolToPrimitiveDescriptor = Object.getOwnPropertyDescriptor(object, Symbol.toPrimitive);
shouldBe("typeof symbolToPrimitiveDescriptor", "'object'");
shouldBe("symbolToPrimitiveDescriptor.value", "newToPrimitive");
shouldBe("symbolToPrimitiveDescriptor.writable", "false");
shouldBe("symbolToPrimitiveDescriptor.enumerable", "true");
shouldBe("symbolToPrimitiveDescriptor.configurable", "true");

debug("");
Object.defineProperty(object, Symbol.toPrimitive, { get: () => 123 });
shouldBe("object[Symbol.toPrimitive]", "123");
}

window.onload = runTest;
</script>
</body>
@@ -557,6 +557,7 @@ loader/navigation-while-deferring-loads.html [ Skip ]

# WebKitTestRunner doesn't have objCController
platform/mac/fast/dom/objc-wrapper-identity.html
platform/mac/fast/dom/objc-wrapper-toprimitive.html
platform/mac/fast/dom/wrapper-classes-objc.html
platform/mac/fast/dom/wrapper-round-tripping.html
platform/mac/fast/objc/dom-html-select-activate.html
@@ -3,6 +3,7 @@ This tests that toString() works correctly on NPObject.
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".


PASS plugin[Symbol.toPrimitive] is undefined
PASS plugin.toString() is "TestObject"
PASS [plugin, ''].join('') is "TestObject"
PASS successfullyParsed is true
@@ -14,6 +14,7 @@
plugin.setAttribute('test', 'to-string-and-value-of-object');
document.body.appendChild(plugin);

shouldBe("plugin[Symbol.toPrimitive]", "undefined");
shouldBeEqualToString("plugin.toString()", "TestObject");
// Normal plugin.testObject + "" will call valueOf,
// do some tricks to make a call to implicit toString.
@@ -3,6 +3,7 @@ This tests that valueOf() works correctly on NPObject.
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".


PASS plugin[Symbol.toPrimitive] is undefined
PASS plugin.valueOf() is 123456789
PASS plugin == 123456789 is true
PASS successfullyParsed is true
@@ -14,6 +14,7 @@
plugin.setAttribute('test', 'to-string-and-value-of-object');
document.body.appendChild(plugin);

shouldBe("plugin[Symbol.toPrimitive]", "undefined");
shouldBe("plugin.valueOf()", "123456789");
shouldBeTrue("plugin == 123456789");
var successfullyParsed = true;
@@ -199,7 +199,7 @@ class JSCallbackObject final : public Parent {
void finishCreation(VM&);

static IsoSubspace* subspaceForImpl(VM&, SubspaceAccess);
static JSValue defaultValue(const JSObject*, JSGlobalObject*, PreferredPrimitiveType);
static JSC_HOST_CALL_ATTRIBUTES EncodedJSValue customToPrimitive(JSGlobalObject*, CallFrame*);

static bool getOwnPropertySlot(JSObject*, JSGlobalObject*, PropertyName, PropertySlot&);
static bool getOwnPropertySlotByIndex(JSObject*, JSGlobalObject*, unsigned propertyName, PropertySlot&);
@@ -112,13 +112,23 @@ template <class Parent>
void JSCallbackObject<Parent>::init(JSGlobalObject* globalObject)
{
ASSERT(globalObject);
VM& vm = getVM(globalObject);

bool hasConvertToType = false;
Vector<JSObjectInitializeCallback, 16> initRoutines;
JSClassRef jsClass = classRef();
do {
if (jsClass->convertToType)
hasConvertToType = true;
if (JSObjectInitializeCallback initialize = jsClass->initialize)
initRoutines.append(initialize);
} while ((jsClass = jsClass->parentClass));

if (hasConvertToType) {
this->putDirect(vm, vm.propertyNames->toPrimitiveSymbol,
JSFunction::create(vm, globalObject, 1, "[Symbol.toPrimitive]"_s, customToPrimitive),
static_cast<unsigned>(PropertyAttribute::DontEnum));
}

// initialize from base to derived
for (int i = static_cast<int>(initRoutines.size()) - 1; i >= 0; i--) {
@@ -217,12 +227,17 @@ bool JSCallbackObject<Parent>::getOwnPropertySlotByIndex(JSObject* object, JSGlo
}

template <class Parent>
JSValue JSCallbackObject<Parent>::defaultValue(const JSObject* object, JSGlobalObject* globalObject, PreferredPrimitiveType hint)
EncodedJSValue JSCallbackObject<Parent>::customToPrimitive(JSGlobalObject* globalObject, CallFrame* callFrame)
{
VM& vm = getVM(globalObject);
auto scope = DECLARE_THROW_SCOPE(vm);

const JSCallbackObject* thisObject = jsCast<const JSCallbackObject*>(object);
JSCallbackObject* thisObject = jsDynamicCast<JSCallbackObject*>(vm, callFrame->thisValue());
if (!thisObject)
return throwVMTypeError(globalObject, scope, "JSCallbackObject[Symbol.toPrimitive] method called on incompatible |this| value."_s);
PreferredPrimitiveType hint = toPreferredPrimitiveType(globalObject, callFrame->argument(0));
RETURN_IF_EXCEPTION(scope, { });

JSContextRef ctx = toRef(globalObject);
JSObjectRef thisRef = toRef(jsCast<const JSObject*>(thisObject));
::JSType jsHint = hint == PreferString ? kJSTypeString : kJSTypeNumber;
@@ -231,16 +246,18 @@ JSValue JSCallbackObject<Parent>::defaultValue(const JSObject* object, JSGlobalO
if (JSObjectConvertToTypeCallback convertToType = jsClass->convertToType) {
JSValueRef exception = nullptr;
JSValueRef result = convertToType(ctx, thisRef, jsHint, &exception);
if (exception) {
throwException(globalObject, scope, toJS(globalObject, exception));
return jsUndefined();
if (exception)
return throwVMError(globalObject, scope, toJS(globalObject, exception));
if (result) {
JSValue jsResult = toJS(globalObject, result);
if (UNLIKELY(jsResult.isObject()))
return JSValue::encode(asObject(jsResult)->ordinaryToPrimitive(globalObject, hint));
return JSValue::encode(jsResult);
}
if (result)
return toJS(globalObject, result);
}
}

RELEASE_AND_RETURN(scope, Parent::defaultValue(object, globalObject, hint));
RELEASE_AND_RETURN(scope, JSValue::encode(thisObject->ordinaryToPrimitive(globalObject, hint)));
}

template <class Parent>
@@ -228,6 +228,24 @@ shouldBe("globalObjectHeirGlobalStaticValue2Descriptor.enumerable", true);
shouldBe("globalObjectHeirGlobalStaticValue2Descriptor.configurable", true);
shouldBe("this.globalStaticValue2", 3);

var symbolToPrimitiveDescriptor = Object.getOwnPropertyDescriptor(MyObject, Symbol.toPrimitive);
shouldBe("typeof symbolToPrimitiveDescriptor", "object");
shouldBe("symbolToPrimitiveDescriptor.value", MyObject[Symbol.toPrimitive]);
shouldBe("symbolToPrimitiveDescriptor.writable", true);
shouldBe("symbolToPrimitiveDescriptor.enumerable", false);
shouldBe("symbolToPrimitiveDescriptor.configurable", true);

shouldBe("MyObject[Symbol.toPrimitive]('default')", 1);
shouldBe("MyObject[Symbol.toPrimitive]('number')", 1);
shouldBe("MyObject[Symbol.toPrimitive]('string')", "MyObjectAsString");

shouldThrow("MyObject[Symbol.toPrimitive]('foo')");
shouldThrow("MyObject[Symbol.toPrimitive].call({}, 'default')");
shouldThrow("(0, MyObject[Symbol.toPrimitive])('default')");

MyObject[Symbol.toPrimitive] = () => null;
shouldBe("MyObject[Symbol.toPrimitive]('bar')", null);

derived = new Derived();

shouldBe("derived instanceof Derived", true);
@@ -307,6 +325,11 @@ shouldThrow("EvilExceptionObject*5");
EvilExceptionObject.toStringExplicit = function f() { return f(); }
shouldThrow("String(EvilExceptionObject)");

EvilExceptionObject.toNumber = () => ({ valueOf: () => 4815 });
shouldBe("Number(EvilExceptionObject)", 4815);
EvilExceptionObject.toStringExplicit = () => ({ toString: () => "foobar" });
shouldBe("`${EvilExceptionObject}`", "foobar");

shouldBe("console", "[object console]");
shouldBe("typeof console.log", "function");

@@ -1,3 +1,45 @@
2021-04-27 Alexey Shvayka <shvaikalesh@gmail.com>

[JSC] Remove defaultValue() from the method table
https://bugs.webkit.org/show_bug.cgi?id=225032

Reviewed by Darin Adler.

This patch not only removes the unnecessary method table entry, but also makes
the presence of custom ToPrimitive behavior observable to userland code.

To maintain object identity and (possibly) enable caching, Symbol.toPrimitive
method is stored on a structure. To avoid any potential breakage, it's made
replaceable and configurable, covering the case when its holder is a [[ProxyTarget]].

For JSCallbackObject, Symbol.toPrimitive method is created only if ConvertToType
callback is present, before initialization is performed.

Also, this change adds additional ordinaryToPrimitive() cast to fix the invariant
that toPrimitive() returns a primitive value, which was broken if ConvertToType
callback returned an object. The invariant is enforced by the spec [1][2] and is
validated via assertion in JSValue::toStringSlowCase().

[1]: https://tc39.es/ecma262/#sec-toprimitive (step 2.b.vi)
[2]: https://tc39.es/ecma262/#sec-ordinarytoprimitive (step 6)

* API/JSCallbackObject.h:
* API/JSCallbackObjectFunctions.h:
(JSC::JSCallbackObject<Parent>::init):
(JSC::JSCallbackObject<Parent>::customToPrimitive):
(JSC::JSCallbackObject<Parent>::defaultValue): Deleted.
* API/tests/testapiScripts/testapi.js:
* runtime/ClassInfo.h:
* runtime/JSCell.cpp:
(JSC::JSCell::defaultValue): Deleted.
* runtime/JSCell.h:
* runtime/JSObject.cpp:
(JSC::JSObject::toPrimitive const):
(JSC::JSObject::defaultValue): Deleted.
* runtime/JSObject.h:
* runtime/Operations.cpp:
(JSC::jsAddSlowCase):

2021-04-27 Keith Miller <keith_miller@apple.com>

StructureStubInfo and PolymorphicAccess should account for their non-GC memory
@@ -71,9 +71,6 @@ struct MethodTable {
using ToThisFunctionPtr = JSValue (*)(JSCell*, JSGlobalObject*, ECMAMode);
ToThisFunctionPtr METHOD_TABLE_ENTRY(toThis);

using DefaultValueFunctionPtr = JSValue (*)(const JSObject*, JSGlobalObject*, PreferredPrimitiveType);
DefaultValueFunctionPtr METHOD_TABLE_ENTRY(defaultValue);

using GetOwnPropertyNamesFunctionPtr = void (*)(JSObject*, JSGlobalObject*, PropertyNameArray&, DontEnumPropertiesMode);
GetOwnPropertyNamesFunctionPtr METHOD_TABLE_ENTRY(getOwnPropertyNames);
GetOwnPropertyNamesFunctionPtr METHOD_TABLE_ENTRY(getOwnSpecialPropertyNames);
@@ -156,7 +153,6 @@ struct MethodTable {
&ClassName::getOwnPropertySlot, \
&ClassName::getOwnPropertySlotByIndex, \
&ClassName::toThis, \
&ClassName::defaultValue, \
&ClassName::getOwnPropertyNames, \
&ClassName::getOwnSpecialPropertyNames, \
&ClassName::customHasInstance, \

0 comments on commit 32e5915

Please sign in to comment.