Changeset 201703 in webkit for trunk/Source/JavaScriptCore/API/JSCallbackObject.h

Timestamp:

Jun 5, 2016, 6:56:11 PM (9 years ago)

Author:

Yusuke Suzuki

Message:

Change ProxyObject.Get not to use custom accessor
​https://bugs.webkit.org/show_bug.cgi?id=157080

Reviewed by Darin Adler.

Source/JavaScriptCore:

This patch focuses on introducing the second part of the followings.
But to do so, first and third parts are necessary.

1. Insert missing exception checks for getPropertySlot.

While getPropertySlot can perform user-observable behavior if the slot is not VMInquiry,
several places miss exeption checks. For example, ProxyObject's hasProperty already can
throw any errors. Looking through the code, we found several missing error checks after
hasProperty, but this will be fixed in the separated patch[1].

2. Do not use custom accessor to implement ProxyObject's Get.

The caller already allows getOwnPropertySlot to throw an exception if the type
is not VMInquiry. So instead of using custom accessor, we simply implement it
directly in the ProxyObject's method.

3. Strip slotBase from custom accessor.

The custom accessor should not be bound to the specific slot base[2], since it
is just an accessor. There is an alternative design: makeing this custom accessor
to custom value accessor and accept both the slot base and the receiver instead
of allowing throwing an error from getOwnPropertySlot. But we take the first design
that allows getPropertySlot to throw an error, since hasProperty (that does not call
getValue of the custom getters) can already throw any errors.

To query the property with the non-user-observable way, we already provided the way for that:
use VMInquiry and isTaintedByProxy() instead.

Tests just ensure that the current semantics works correctly after this patch.
And this patch is performance neutral.

Later, we will attempt to rename "thisValue" to "receiver"[3].

[1]: ​https://bugs.webkit.org/show_bug.cgi?id=158398
[2]: ​https://bugs.webkit.org/show_bug.cgi?id=157978
[3]: ​https://bugs.webkit.org/show_bug.cgi?id=158397

• API/JSCallbackObject.h:
• API/JSCallbackObjectFunctions.h:

(JSC::JSCallbackObject<Parent>::staticFunctionGetter):
(JSC::JSCallbackObject<Parent>::callbackGetter):

• bytecode/PolymorphicAccess.cpp:

(JSC::AccessCase::generateImpl):

• dfg/DFGOperations.cpp:
• interpreter/Interpreter.cpp:

(JSC::Interpreter::execute):

• jit/JITOperations.cpp:
• jsc.cpp:

(WTF::ImpureGetter::getOwnPropertySlot):
(WTF::CustomGetter::customGetter):
(WTF::RuntimeArray::lengthGetter):
(GlobalObject::finishCreation):
(GlobalObject::moduleLoaderFetch):
(functionGetGetterSetter):
(functionRun):
(functionLoad):
(functionLoadString):
(functionReadFile):
(functionCheckSyntax):
(functionLoadWebAssembly):
(functionLoadModule):
(functionCreateBuiltin):
(functionCheckModuleSyntax):
(dumpException):
(runWithScripts):
(runInteractive):

• llint/LLIntSlowPaths.cpp:

(JSC::LLInt::LLINT_SLOW_PATH_DECL):

• runtime/CommonSlowPaths.cpp:

(JSC::SLOW_PATH_DECL):

• runtime/JSBoundSlotBaseFunction.cpp:

(JSC::boundSlotBaseFunctionCall):

• runtime/JSCJSValue.h:
• runtime/JSCJSValueInlines.h:

(JSC::JSValue::getPropertySlot):

• runtime/JSCellInlines.h:

(JSC::ExecState::vm):
This change is super important for performance. We add several exec->hadException() calls into the super hot path, like JSC::operationGetByIdOptimize.
Without this change, we call ExecState::vm() and it is not inlined. This causes 1 - 2% performance regression in Octane PDFJS.

• runtime/JSFunction.cpp:

(JSC::JSFunction::argumentsGetter):
(JSC::JSFunction::callerGetter):

• runtime/JSFunction.h:
• runtime/JSGenericTypedArrayViewConstructorInlines.h:

(JSC::constructGenericTypedArrayViewWithArguments):

• runtime/JSModuleNamespaceObject.cpp:

(JSC::callbackGetter):

• runtime/JSONObject.cpp:

(JSC::Stringifier::Holder::appendNextProperty):
Here's UNLIKELY is important for Kraken's json-stringify-tinderbox. Without it, we can observe 0.5% regression.

(JSC::Walker::walk):

• runtime/JSObject.h:

(JSC::JSObject::getPropertySlot):

• runtime/ObjectPrototype.cpp:

(JSC::objectProtoFuncToString):

• runtime/PropertySlot.cpp:

(JSC::PropertySlot::customGetter):

• runtime/PropertySlot.h:

(JSC::PropertySlot::thisValue):

• runtime/ProxyObject.cpp:

(JSC::performProxyGet):
(JSC::ProxyObject::performGet):
(JSC::ProxyObject::getOwnPropertySlotCommon):

• runtime/ProxyObject.h:
• runtime/RegExpConstructor.cpp:

(JSC::regExpConstructorDollar):
(JSC::regExpConstructorInput):
(JSC::regExpConstructorMultiline):
(JSC::regExpConstructorLastMatch):
(JSC::regExpConstructorLastParen):
(JSC::regExpConstructorLeftContext):
(JSC::regExpConstructorRightContext):

• tests/stress/get-from-scope-dynamic-onto-proxy.js: Added.

(shouldBe):
(shouldThrow.handler.has):
(handler.has):
(try.handler.has):

• tests/stress/operation-in-throw-error.js: Added.

(testCase.handler.has):
(testCase):

• tests/stress/proxy-and-json-stringify.js: Added.

(shouldThrow):

• tests/stress/proxy-and-typed-array.js: Added.
• tests/stress/proxy-json-path.js: Added.
• tests/stress/proxy-with-statement.js: Added.

Source/WebCore:

• bindings/js/JSCryptoAlgorithmDictionary.cpp:

(WebCore::getProperty):

• bindings/js/JSDOMBinding.h:

(WebCore::nonCachingStaticFunctionGetter):

• bindings/js/JSDOMWindowCustom.cpp:

(WebCore::jsDOMWindowWebKit):

• bindings/js/JSDictionary.cpp:

(WebCore::JSDictionary::tryGetProperty):

• bindings/js/JSPluginElementFunctions.cpp:

(WebCore::pluginElementPropertyGetter):

• bindings/js/JSPluginElementFunctions.h:
• bindings/scripts/CodeGeneratorJS.pm:

(GenerateHeader):
(GenerateImplementation):

• bindings/scripts/test/JS/JSInterfaceName.cpp:

(WebCore::jsInterfaceNameConstructor):

• bindings/scripts/test/JS/JSTestActiveDOMObject.cpp:

(WebCore::jsTestActiveDOMObjectExcitingAttr):
(WebCore::jsTestActiveDOMObjectConstructor):

• bindings/scripts/test/JS/JSTestClassWithJSBuiltinConstructor.cpp:

(WebCore::jsTestClassWithJSBuiltinConstructorConstructor):

• bindings/scripts/test/JS/JSTestCustomConstructorWithNoInterfaceObject.cpp:

(WebCore::jsTestCustomConstructorWithNoInterfaceObjectConstructor):

• bindings/scripts/test/JS/JSTestCustomNamedGetter.cpp:

(WebCore::jsTestCustomNamedGetterConstructor):

• bindings/scripts/test/JS/JSTestEventConstructor.cpp:

(WebCore::jsTestEventConstructorAttr1):
(WebCore::jsTestEventConstructorAttr2):
(WebCore::jsTestEventConstructorAttr3):
(WebCore::jsTestEventConstructorConstructor):

• bindings/scripts/test/JS/JSTestEventTarget.cpp:

(WebCore::jsTestEventTargetConstructor):

• bindings/scripts/test/JS/JSTestException.cpp:

(WebCore::jsTestExceptionName):
(WebCore::jsTestExceptionConstructor):

• bindings/scripts/test/JS/JSTestGenerateIsReachable.cpp:

(WebCore::jsTestGenerateIsReachableConstructor):

• bindings/scripts/test/JS/JSTestGlobalObject.cpp:

(WebCore::jsTestGlobalObjectRegularAttribute):
(WebCore::jsTestGlobalObjectEnabledAtRuntimeAttribute):
(WebCore::jsTestGlobalObjectConstructor):

• bindings/scripts/test/JS/JSTestInterface.cpp:

(WebCore::jsTestInterfaceConstructorImplementsStaticReadOnlyAttr):
(WebCore::jsTestInterfaceConstructorImplementsStaticAttr):
(WebCore::jsTestInterfaceImplementsStr1):
(WebCore::jsTestInterfaceImplementsStr2):
(WebCore::jsTestInterfaceImplementsStr3):
(WebCore::jsTestInterfaceImplementsNode):
(WebCore::jsTestInterfaceConstructorSupplementalStaticReadOnlyAttr):
(WebCore::jsTestInterfaceConstructorSupplementalStaticAttr):
(WebCore::jsTestInterfaceSupplementalStr1):
(WebCore::jsTestInterfaceSupplementalStr2):
(WebCore::jsTestInterfaceSupplementalStr3):
(WebCore::jsTestInterfaceSupplementalNode):
(WebCore::jsTestInterfaceConstructor):

• bindings/scripts/test/JS/JSTestJSBuiltinConstructor.cpp:

(WebCore::jsTestJSBuiltinConstructorTestAttributeCustom):
(WebCore::jsTestJSBuiltinConstructorTestAttributeRWCustom):
(WebCore::jsTestJSBuiltinConstructorConstructor):

• bindings/scripts/test/JS/JSTestMediaQueryListListener.cpp:

(WebCore::jsTestMediaQueryListListenerConstructor):

• bindings/scripts/test/JS/JSTestNamedConstructor.cpp:

(WebCore::jsTestNamedConstructorConstructor):

• bindings/scripts/test/JS/JSTestNode.cpp:

(WebCore::jsTestNodeName):
(WebCore::jsTestNodeConstructor):

• bindings/scripts/test/JS/JSTestNondeterministic.cpp:

(WebCore::jsTestNondeterministicNondeterministicReadonlyAttr):
(WebCore::jsTestNondeterministicNondeterministicWriteableAttr):
(WebCore::jsTestNondeterministicNondeterministicExceptionAttr):
(WebCore::jsTestNondeterministicNondeterministicGetterExceptionAttr):
(WebCore::jsTestNondeterministicNondeterministicSetterExceptionAttr):
(WebCore::jsTestNondeterministicConstructor):

• bindings/scripts/test/JS/JSTestObj.cpp:

(WebCore::jsTestObjReadOnlyLongAttr):
(WebCore::jsTestObjReadOnlyStringAttr):
(WebCore::jsTestObjReadOnlyTestObjAttr):
(WebCore::jsTestObjConstructorStaticReadOnlyLongAttr):
(WebCore::jsTestObjConstructorStaticStringAttr):
(WebCore::jsTestObjConstructorTestSubObj):
(WebCore::jsTestObjTestSubObjEnabledBySettingConstructor):
(WebCore::jsTestObjEnumAttr):
(WebCore::jsTestObjByteAttr):
(WebCore::jsTestObjOctetAttr):
(WebCore::jsTestObjShortAttr):
(WebCore::jsTestObjClampedShortAttr):
(WebCore::jsTestObjEnforceRangeShortAttr):
(WebCore::jsTestObjUnsignedShortAttr):
(WebCore::jsTestObjLongAttr):
(WebCore::jsTestObjLongLongAttr):
(WebCore::jsTestObjUnsignedLongLongAttr):
(WebCore::jsTestObjStringAttr):
(WebCore::jsTestObjTestObjAttr):
(WebCore::jsTestObjTestNullableObjAttr):
(WebCore::jsTestObjLenientTestObjAttr):
(WebCore::jsTestObjUnforgeableAttr):
(WebCore::jsTestObjStringAttrTreatingNullAsEmptyString):
(WebCore::jsTestObjXMLObjAttr):
(WebCore::jsTestObjCreate):
(WebCore::jsTestObjReflectedStringAttr):
(WebCore::jsTestObjReflectedIntegralAttr):
(WebCore::jsTestObjReflectedUnsignedIntegralAttr):
(WebCore::jsTestObjReflectedBooleanAttr):
(WebCore::jsTestObjReflectedURLAttr):
(WebCore::jsTestObjReflectedCustomIntegralAttr):
(WebCore::jsTestObjReflectedCustomBooleanAttr):
(WebCore::jsTestObjReflectedCustomURLAttr):
(WebCore::jsTestObjEnabledAtRuntimeAttribute):
(WebCore::jsTestObjTypedArrayAttr):
(WebCore::jsTestObjAttrWithGetterException):
(WebCore::jsTestObjAttrWithGetterExceptionWithMessage):
(WebCore::jsTestObjAttrWithSetterException):
(WebCore::jsTestObjAttrWithSetterExceptionWithMessage):
(WebCore::jsTestObjStringAttrWithGetterException):
(WebCore::jsTestObjStringAttrWithSetterException):
(WebCore::jsTestObjStrictTypeCheckingAttribute):
(WebCore::jsTestObjCustomAttr):
(WebCore::jsTestObjOnfoo):
(WebCore::jsTestObjOnwebkitfoo):
(WebCore::jsTestObjWithScriptStateAttribute):
(WebCore::jsTestObjWithCallWithAndSetterCallWithAttribute):
(WebCore::jsTestObjWithScriptExecutionContextAttribute):
(WebCore::jsTestObjWithScriptStateAttributeRaises):
(WebCore::jsTestObjWithScriptExecutionContextAttributeRaises):
(WebCore::jsTestObjWithScriptExecutionContextAndScriptStateAttribute):
(WebCore::jsTestObjWithScriptExecutionContextAndScriptStateAttributeRaises):
(WebCore::jsTestObjWithScriptExecutionContextAndScriptStateWithSpacesAttribute):
(WebCore::jsTestObjWithScriptArgumentsAndCallStackAttribute):
(WebCore::jsTestObjConditionalAttr1):
(WebCore::jsTestObjConditionalAttr2):
(WebCore::jsTestObjConditionalAttr3):
(WebCore::jsTestObjConditionalAttr4Constructor):
(WebCore::jsTestObjConditionalAttr5Constructor):
(WebCore::jsTestObjConditionalAttr6Constructor):
(WebCore::jsTestObjCachedAttribute1):
(WebCore::jsTestObjCachedAttribute2):
(WebCore::jsTestObjAnyAttribute):
(WebCore::jsTestObjContentDocument):
(WebCore::jsTestObjMutablePoint):
(WebCore::jsTestObjImmutablePoint):
(WebCore::jsTestObjStrawberry):
(WebCore::jsTestObjStrictFloat):
(WebCore::jsTestObjDescription):
(WebCore::jsTestObjId):
(WebCore::jsTestObjHash):
(WebCore::jsTestObjReplaceableAttribute):
(WebCore::jsTestObjNullableDoubleAttribute):
(WebCore::jsTestObjNullableLongAttribute):
(WebCore::jsTestObjNullableBooleanAttribute):
(WebCore::jsTestObjNullableStringAttribute):
(WebCore::jsTestObjNullableLongSettableAttribute):
(WebCore::jsTestObjNullableStringSettableAttribute):
(WebCore::jsTestObjNullableStringValue):
(WebCore::jsTestObjAttribute):
(WebCore::jsTestObjAttributeWithReservedEnumType):
(WebCore::jsTestObjPutForwardsAttribute):
(WebCore::jsTestObjPutForwardsNullableAttribute):
(WebCore::jsTestObjConstructor):

• bindings/scripts/test/JS/JSTestOverloadedConstructors.cpp:

(WebCore::jsTestOverloadedConstructorsConstructor):

• bindings/scripts/test/JS/JSTestOverrideBuiltins.cpp:

(WebCore::jsTestOverrideBuiltinsConstructor):

• bindings/scripts/test/JS/JSTestSerializedScriptValueInterface.cpp:

(WebCore::jsTestSerializedScriptValueInterfaceValue):
(WebCore::jsTestSerializedScriptValueInterfaceReadonlyValue):
(WebCore::jsTestSerializedScriptValueInterfaceCachedValue):
(WebCore::jsTestSerializedScriptValueInterfacePorts):
(WebCore::jsTestSerializedScriptValueInterfaceCachedReadonlyValue):
(WebCore::jsTestSerializedScriptValueInterfaceConstructor):

• bindings/scripts/test/JS/JSTestTypedefs.cpp:

(WebCore::jsTestTypedefsUnsignedLongLongAttr):
(WebCore::jsTestTypedefsImmutableSerializedScriptValue):
(WebCore::jsTestTypedefsConstructorTestSubObj):
(WebCore::jsTestTypedefsAttrWithGetterException):
(WebCore::jsTestTypedefsAttrWithSetterException):
(WebCore::jsTestTypedefsStringAttrWithGetterException):
(WebCore::jsTestTypedefsStringAttrWithSetterException):
(WebCore::jsTestTypedefsConstructor):

• bindings/scripts/test/JS/JSattribute.cpp:

(WebCore::jsattributeReadonly):
(WebCore::jsattributeConstructor):

• bindings/scripts/test/JS/JSreadonly.cpp:

(WebCore::jsreadonlyConstructor):

• bridge/runtime_array.cpp:

(JSC::RuntimeArray::lengthGetter):

• bridge/runtime_array.h:
• bridge/runtime_method.cpp:

(JSC::RuntimeMethod::lengthGetter):

• bridge/runtime_method.h:
• bridge/runtime_object.cpp:

(JSC::Bindings::RuntimeObject::fallbackObjectGetter):
(JSC::Bindings::RuntimeObject::fieldGetter):
(JSC::Bindings::RuntimeObject::methodGetter):

• bridge/runtime_object.h:

Source/WebKit2:

• WebProcess/Plugins/Netscape/JSNPObject.cpp:

(WebKit::JSNPObject::propertyGetter):
(WebKit::JSNPObject::methodGetter):

• WebProcess/Plugins/Netscape/JSNPObject.h:

File:

• trunk/Source/JavaScriptCore/API/JSCallbackObject.h (modified) (1 diff)

trunk/Source/JavaScriptCore/API/JSCallbackObject.h

@@ -212 +212 @@
    
     JSValue getStaticValue(ExecState*, PropertyName);
-    static EncodedJSValue staticFunctionGetter(ExecState*, EncodedJSValue, PropertyName, JSObject*);
-    static EncodedJSValue callbackGetter(ExecState*, EncodedJSValue, PropertyName, JSObject*);
+    static EncodedJSValue staticFunctionGetter(ExecState*, EncodedJSValue, PropertyName);
+    static EncodedJSValue callbackGetter(ExecState*, EncodedJSValue, PropertyName);
 
     std::unique_ptr<JSCallbackObjectData> m_callbackObjectData;