Update method creation for operations #155
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Filed web-platform-tests/wpt#3585 to add tests for the built-in method behavior |
|
@bzbarsky ping |
| class='esvalue'>null</span> or <span class='esvalue'>undefined</span>, | ||
| set <var>O</var> to <var>realm</var>'s <a class='dfnref external' | ||
| href='https://html.spec.whatwg.org/multipage/webappapis.html#concept-realm-global'>global | ||
| object</a>. (This will subsequently cause a <span |
There was a problem hiding this comment.
There's no way target can be the appropriate global object. target isn't an object at all.
What you probable meant is "if the global does not implement target" or something like that?
|
This looks great; all the nits above are minor. Note that eventually we may want to propagate the this value to static operations on interfaces (e.g. |
|
@domenic OK, updated bits look good. Can you please file that followup issue to define the mixin/interface/member-list bits sanely and add a note pointing to it? My apologies for the lag here and for not being able to look at this again until Tuesday morning... :( |
|
Going to hold off on this until the Bikeshed rewrite lands, FYI. |
This consolidates the spec text for creating the functions for interface operations and namespace operations, using the more modern style seen in namespace operations (introduced in df8c9c4) as the base. Along the way, this fixes https://www.w3.org/Bugs/Public/show_bug.cgi?id=18547 (making all methods [ImplicitThis], and getting rid of [ImplicitThis]), and fixes whatwg/html#643 (a related issue on the HTML side). This also fixes #106, since we now explicitly use the CreateBuiltinFunction abstract operation.
|
This has been rebased and a pointer added to #164; should be ready to merge. |
webkit-commit-queue
pushed a commit
to WebKit/WebKit
that referenced
this pull request
May 20, 2021
…ttributes https://bugs.webkit.org/show_bug.cgi?id=223758 Reviewed by Sam Weinig. LayoutTests/imported/w3c: * web-platform-tests/WebIDL/ecmascript-binding/global-object-implicit-this-value.any-expected.txt: Added. * web-platform-tests/WebIDL/ecmascript-binding/global-object-implicit-this-value.any.html: Added. * web-platform-tests/WebIDL/ecmascript-binding/global-object-implicit-this-value.any.js: Added. * web-platform-tests/WebIDL/ecmascript-binding/global-object-implicit-this-value.any.worker-expected.txt: Added. * web-platform-tests/WebIDL/ecmascript-binding/global-object-implicit-this-value.any.worker.html: Added. * web-platform-tests/webaudio/the-audio-api/the-audioworklet-interface/audioworklet-registerprocessor-called-on-globalthis.https-expected.txt: Added. * web-platform-tests/webaudio/the-audio-api/the-audioworklet-interface/audioworklet-registerprocessor-called-on-globalthis.https.html: Added. * web-platform-tests/webaudio/the-audio-api/the-audioworklet-interface/processors/dummy-processor-globalthis.js: Added. Source/WebCore: This change introduces castThisValue<JSClass>, taking a step towards unification of |this| value casting between IDLAttribute and IDLOperation. The helper uses compile-time inheritance check to provide implicit |this| value for DOM global objects [1], replacing [ImplicitThis] extended attribute, which was removed from the spec [2] a while ago. IDLAttribute can't perform toThis() with ECMAMode::strict(), like IDLOperation now does, because CustomValue getters are called with |this| value of JSGlobalObject type, which gets tainted by JSScope::toThis(). #225397 will remove the need for toThis(), finally making |this| value casting consistent between attributes and operations. Also, this patch fixes `Object.create(window).location` to throw as per spec [1] by removing prototype chain traversal from toJSDOMWindow(), which aligns WebKit with Blink and Gecko. As DOM global objects are wrapped in proxies and require special casting, toJSDOMWindow() and friends are merged into toJSDOMGlobalObject<JSClass>, which is aware of inheritance / JSProxy. It replaces [CustomProxyToJSObject] extended attribute, which could be missed when adding new DOM global objects, fixing worklets' global functions not to throw when called on `globalThis`. This change reduces WebCore --release binary size by 0.2% (147 KB). [1] https://heycam.github.io/webidl/#dfn-attribute-getter (step 1.1.2.3) [2] whatwg/webidl#155 Tests: imported/w3c/web-platform-tests/WebIDL/ecmascript-binding/global-object-implicit-this-value.any.js imported/w3c/web-platform-tests/webaudio/the-audio-api/the-audioworklet-interface/audioworklet-registerprocessor-called-on-globalthis.https.html fast/css-custom-paint/registerPaintBindings.html http/tests/security/listener/*.html * Headers.cmake: * Modules/webaudio/AudioWorkletGlobalScope.idl: * WebCore.xcodeproj/project.pbxproj: * bindings/js/JSDOMAttribute.h: (WebCore::IDLAttribute::set): (WebCore::IDLAttribute::setPassingPropertyName): (WebCore::IDLAttribute::get): (WebCore::IDLAttribute::getPassingPropertyName): * bindings/js/JSDOMCastThisValue.h: Added. (WebCore::castThisValue): * bindings/js/JSDOMCastedThisErrorBehavior.h: Removed. * bindings/js/JSDOMGlobalObject.h: (WebCore::toJSDOMGlobalObject): * bindings/js/JSDOMOperation.h: (WebCore::IDLOperation::cast): * bindings/js/JSDOMWindowBase.cpp: * bindings/js/JSDOMWindowBase.h: (WebCore::toJSDOMWindow): * bindings/js/JSDOMWindowCustom.cpp: (WebCore::JSC_DEFINE_CUSTOM_GETTER): (WebCore::IDLOperation<JSDOMWindow>::cast): Deleted. * bindings/js/JSDocumentCustom.cpp: (WebCore::cachedDocumentWrapper): * bindings/js/JSEventTargetCustom.cpp: (WebCore::jsEventTargetCast): * bindings/js/JSEventTargetCustom.h: (WebCore::IDLOperation<JSEventTarget>::call): * bindings/js/JSRemoteDOMWindowBase.cpp: (WebCore::toJSRemoteDOMWindow): Deleted. * bindings/js/JSRemoteDOMWindowBase.h: * bindings/js/JSWorkerGlobalScopeBase.cpp: (WebCore::toJSDedicatedWorkerGlobalScope): Deleted. (WebCore::toJSWorkerGlobalScope): Deleted. (WebCore::toJSServiceWorkerGlobalScope): Deleted. * bindings/js/JSWorkerGlobalScopeBase.h: * bindings/js/JSWorkletGlobalScopeBase.cpp: (WebCore::toJSWorkletGlobalScope): Deleted. * bindings/js/JSWorkletGlobalScopeBase.h: * bindings/scripts/CodeGeneratorJS.pm: (ShouldGenerateToJSDeclaration): (IsAcceleratedDOMAttribute): (GenerateImplementation): * bindings/scripts/IDLAttributes.json: * bindings/scripts/test/JS/*: Updated. * inspector/InspectorController.cpp: (WebCore::InspectorController::canAccessInspectedScriptState const): * page/DOMWindow.idl: * page/RemoteDOMWindow.idl: * workers/DedicatedWorkerGlobalScope.idl: * workers/WorkerGlobalScope.idl: * workers/service/ServiceWorkerGlobalScope.idl: * worklets/PaintWorkletGlobalScope.idl: * worklets/WorkletGlobalScope.idl: Source/WebKit: Use inherits<T> instead of toJSDOMWindow() if the value is never a JSProxy. * WebProcess/InjectedBundle/InjectedBundle.cpp: (WebKit::InjectedBundle::reportException): Source/WebKitLegacy/mac: Use inherits<T> instead of toJSDOMWindow() if the value is never a JSProxy. * WebView/WebView.mm: (+[WebView _reportException:inContext:]): Source/WebKitLegacy/win: Use inherits<T> instead of toJSDOMWindow() if the value is never a JSProxy. * WebView.cpp: (WebView::reportException): LayoutTests: * fast/css-custom-paint/registerPaintBindings.html: * http/tests/security/listener/*: This is a progression: Blink and Gecko don't call event listeners belonging to destroyed frames. * js/property-of-window-as-prototype-expected.txt: Removed. * js/property-of-window-as-prototype.html: Removed. Canonical link: https://commits.webkit.org/237976@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@277830 268f45cc-cd09-0410-ab3c-d52691b4dbfc
bertogg
pushed a commit
to Igalia/webkit
that referenced
this pull request
May 23, 2021
…ttributes https://bugs.webkit.org/show_bug.cgi?id=223758 Reviewed by Sam Weinig. LayoutTests/imported/w3c: * web-platform-tests/WebIDL/ecmascript-binding/global-object-implicit-this-value.any-expected.txt: Added. * web-platform-tests/WebIDL/ecmascript-binding/global-object-implicit-this-value.any.html: Added. * web-platform-tests/WebIDL/ecmascript-binding/global-object-implicit-this-value.any.js: Added. * web-platform-tests/WebIDL/ecmascript-binding/global-object-implicit-this-value.any.worker-expected.txt: Added. * web-platform-tests/WebIDL/ecmascript-binding/global-object-implicit-this-value.any.worker.html: Added. * web-platform-tests/webaudio/the-audio-api/the-audioworklet-interface/audioworklet-registerprocessor-called-on-globalthis.https-expected.txt: Added. * web-platform-tests/webaudio/the-audio-api/the-audioworklet-interface/audioworklet-registerprocessor-called-on-globalthis.https.html: Added. * web-platform-tests/webaudio/the-audio-api/the-audioworklet-interface/processors/dummy-processor-globalthis.js: Added. Source/WebCore: This change introduces castThisValue<JSClass>, taking a step towards unification of |this| value casting between IDLAttribute and IDLOperation. The helper uses compile-time inheritance check to provide implicit |this| value for DOM global objects [1], replacing [ImplicitThis] extended attribute, which was removed from the spec [2] a while ago. IDLAttribute can't perform toThis() with ECMAMode::strict(), like IDLOperation now does, because CustomValue getters are called with |this| value of JSGlobalObject type, which gets tainted by JSScope::toThis(). #225397 will remove the need for toThis(), finally making |this| value casting consistent between attributes and operations. Also, this patch fixes `Object.create(window).location` to throw as per spec [1] by removing prototype chain traversal from toJSDOMWindow(), which aligns WebKit with Blink and Gecko. As DOM global objects are wrapped in proxies and require special casting, toJSDOMWindow() and friends are merged into toJSDOMGlobalObject<JSClass>, which is aware of inheritance / JSProxy. It replaces [CustomProxyToJSObject] extended attribute, which could be missed when adding new DOM global objects, fixing worklets' global functions not to throw when called on `globalThis`. This change reduces WebCore --release binary size by 0.2% (147 KB). [1] https://heycam.github.io/webidl/#dfn-attribute-getter (step 1.1.2.3) [2] whatwg/webidl#155 Tests: imported/w3c/web-platform-tests/WebIDL/ecmascript-binding/global-object-implicit-this-value.any.js imported/w3c/web-platform-tests/webaudio/the-audio-api/the-audioworklet-interface/audioworklet-registerprocessor-called-on-globalthis.https.html fast/css-custom-paint/registerPaintBindings.html http/tests/security/listener/*.html * Headers.cmake: * Modules/webaudio/AudioWorkletGlobalScope.idl: * WebCore.xcodeproj/project.pbxproj: * bindings/js/JSDOMAttribute.h: (WebCore::IDLAttribute::set): (WebCore::IDLAttribute::setPassingPropertyName): (WebCore::IDLAttribute::get): (WebCore::IDLAttribute::getPassingPropertyName): * bindings/js/JSDOMCastThisValue.h: Added. (WebCore::castThisValue): * bindings/js/JSDOMCastedThisErrorBehavior.h: Removed. * bindings/js/JSDOMGlobalObject.h: (WebCore::toJSDOMGlobalObject): * bindings/js/JSDOMOperation.h: (WebCore::IDLOperation::cast): * bindings/js/JSDOMWindowBase.cpp: * bindings/js/JSDOMWindowBase.h: (WebCore::toJSDOMWindow): * bindings/js/JSDOMWindowCustom.cpp: (WebCore::JSC_DEFINE_CUSTOM_GETTER): (WebCore::IDLOperation<JSDOMWindow>::cast): Deleted. * bindings/js/JSDocumentCustom.cpp: (WebCore::cachedDocumentWrapper): * bindings/js/JSEventTargetCustom.cpp: (WebCore::jsEventTargetCast): * bindings/js/JSEventTargetCustom.h: (WebCore::IDLOperation<JSEventTarget>::call): * bindings/js/JSRemoteDOMWindowBase.cpp: (WebCore::toJSRemoteDOMWindow): Deleted. * bindings/js/JSRemoteDOMWindowBase.h: * bindings/js/JSWorkerGlobalScopeBase.cpp: (WebCore::toJSDedicatedWorkerGlobalScope): Deleted. (WebCore::toJSWorkerGlobalScope): Deleted. (WebCore::toJSServiceWorkerGlobalScope): Deleted. * bindings/js/JSWorkerGlobalScopeBase.h: * bindings/js/JSWorkletGlobalScopeBase.cpp: (WebCore::toJSWorkletGlobalScope): Deleted. * bindings/js/JSWorkletGlobalScopeBase.h: * bindings/scripts/CodeGeneratorJS.pm: (ShouldGenerateToJSDeclaration): (IsAcceleratedDOMAttribute): (GenerateImplementation): * bindings/scripts/IDLAttributes.json: * bindings/scripts/test/JS/*: Updated. * inspector/InspectorController.cpp: (WebCore::InspectorController::canAccessInspectedScriptState const): * page/DOMWindow.idl: * page/RemoteDOMWindow.idl: * workers/DedicatedWorkerGlobalScope.idl: * workers/WorkerGlobalScope.idl: * workers/service/ServiceWorkerGlobalScope.idl: * worklets/PaintWorkletGlobalScope.idl: * worklets/WorkletGlobalScope.idl: Source/WebKit: Use inherits<T> instead of toJSDOMWindow() if the value is never a JSProxy. * WebProcess/InjectedBundle/InjectedBundle.cpp: (WebKit::InjectedBundle::reportException): Source/WebKitLegacy/mac: Use inherits<T> instead of toJSDOMWindow() if the value is never a JSProxy. * WebView/WebView.mm: (+[WebView _reportException:inContext:]): Source/WebKitLegacy/win: Use inherits<T> instead of toJSDOMWindow() if the value is never a JSProxy. * WebView.cpp: (WebView::reportException): LayoutTests: * fast/css-custom-paint/registerPaintBindings.html: * http/tests/security/listener/*: This is a progression: Blink and Gecko don't call event listeners belonging to destroyed frames. * js/property-of-window-as-prototype-expected.txt: Removed. * js/property-of-window-as-prototype.html: Removed. git-svn-id: http://svn.webkit.org/repository/webkit/trunk@277830 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This consolidates the spec text for creating the functions for interface operations and namespace operations, using the more modern style seen in namespace operations (introduced in df8c9c4) as the base.
Along the way, this fixes https://www.w3.org/Bugs/Public/show_bug.cgi?id=18547 (making all methods [ImplicitThis], and getting rid of [ImplicitThis]), and fixes whatwg/html#643 (a related issue on the HTML side).
This also fixes #106, since we now explicitly use the CreateBuiltinFunction abstract operation.
@bzbarsky please take a look! I'm rather pleased that this managed to fix the long-standing [ImplicitThis] issue.