Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add [Exposed=*] to expose on all globals #526

Merged
merged 3 commits into from
Nov 2, 2021

Conversation

littledan
Copy link
Collaborator

@littledan littledan commented Feb 22, 2018

This patch adds support in WebIDL for declaring an interface available
in all contexts.

Closes #468

This PR follows the syntax and outline suggested by @tobie . Thanks for your clear instructions here.


Preview | Diff

index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
@annevk
Copy link
Member

annevk commented Feb 23, 2018

I think before landing this we should try to address the question at #468 (comment).

index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
@littledan littledan force-pushed the exposed-star branch 2 times, most recently from 35b291d to 2e23cfb Compare February 23, 2018 14:42
@littledan
Copy link
Collaborator Author

@annevk Thanks for making this link. I think we should work out an answer in conjunction with TC39 to this question, given that ECMAScript includes things that are pretty equivalent in power/appropriateness to put everywhere to TextEncoder and URL. I'm fine with letting this PR be blocked on this question.

@tobie tobie added the do not merge yet Pull request must not be merged per rationale in comment label Feb 26, 2018
@littledan
Copy link
Collaborator Author

We should include the jsidl label on this issue, since all ES APIs will need to be exposed to all globals

@tobie tobie added the jsidl label Oct 20, 2018
@leobalter
Copy link

@littledan I believe we should follow up on this PR for the proper ShadowRealms integration, right?

@littledan
Copy link
Collaborator Author

We could add this for ShadowRealms, or we could just add [Exposed=ShadowRealm]. The choice depends on whether we want to go around adding lots of things to Worklets which are not currently exposed, and which we might decide to expose on ShadowRealm. In offline conversations with different HTML editors, I heard mixed opinions on this question, cc @domenic @annevk . Given that, I thought that it might be better to start with the more conservative approach of just adding things specifically to ShadowRealm.

@leobalter

This comment has been minimized.

@domenic
Copy link
Member

domenic commented Sep 22, 2021

I think it'd be better to add [Exposed=*]? Saying that in worklets you have to access these APIs indirectly via shadow realms seems strange.

@annevk
Copy link
Member

annevk commented Sep 23, 2021

In an offline discussion tc39/ecma262#1120 came up, but since we did not change JavaScript to support minimal realms (i.e., a subset of what is defined in JavaScript), we should not do so here by excluding worklets. (As that would make worklets the new minimal realms due to organizational boundaries.)

As I understand it the primary motivation for having minimal realms in worklets was to avoid steering web developers down the wrong path, but in practice this hasn't been much of a problem.

This is a long way of saying that I agree with @domenic, but I thought it might be useful to add some context.

cc @padenot @codehag @Ms2ger

@littledan
Copy link
Collaborator Author

OK, thanks for the clarifications everyone, sounds like I was the confused/misunderstanding one. Does that mean we should move ahead with [Exposed=*]?

This patch adds support in WebIDL for declaring an interface available
in all contexts.

Closes whatwg#468
@Ms2ger
Copy link
Member

Ms2ger commented Oct 1, 2021

Rebased and rephrased a bit - what's next here?

index.bs Outdated Show resolved Hide resolved
Co-authored-by: ExE Boss <3889017+ExE-Boss@users.noreply.github.com>
@Ms2ger Ms2ger requested a review from annevk October 7, 2021 15:04
@annevk annevk requested review from EdgarChen and removed request for annevk October 8, 2021 10:24
Copy link
Member

@EdgarChen EdgarChen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, thanks!

@Ms2ger Ms2ger requested a review from TimothyGu October 12, 2021 08:40
@EdgarChen
Copy link
Member

@Ms2ger it would be helpful if you could make a list of interfaces which [Exposed=*] is proposed somewhere publicly. Thanks!

Copy link
Member

@TimothyGu TimothyGu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@Ms2ger
Copy link
Member

Ms2ger commented Nov 2, 2021

@Ms2ger it would be helpful if you could make a list of interfaces which [Exposed=*] is proposed somewhere publicly. Thanks!

Besides the ones already listed in tc39/proposal-shadowrealm#331, we might propose some parts of Crypto and Performance - please do let me know if you'd like me to present the list in some other way

@EdgarChen EdgarChen removed the do not merge yet Pull request must not be merged per rationale in comment label Nov 2, 2021
@EdgarChen EdgarChen merged commit 59dae6f into whatwg:main Nov 2, 2021
webkit-commit-queue pushed a commit to WebKit/WebKit that referenced this pull request Nov 3, 2021
https://bugs.webkit.org/show_bug.cgi?id=231082

Reviewed by Chris Dumez.

Adds a shorthand to expose interfaces/attributes on Window, Workers*,
and the forthcoming ShadowRealm global object.

See whatwg/webidl#468 and
whatwg/webidl#526 for details.

* bindings/scripts/CodeGenerator.pm:
(shouldPropertyBeExposed):
* bindings/scripts/CodeGeneratorJS.pm:
(GenerateRuntimeEnableConditionalStringForExposed):
* bindings/scripts/IDLParser.pm:
(parseExtendedAttributeRest2):
* bindings/scripts/preprocess-idls.pl:
* bindings/scripts/test/AudioWorkletGlobalScopeConstructors.idl:
* bindings/scripts/test/DOMWindowConstructors.idl:
* bindings/scripts/test/DedicatedWorkerGlobalScopeConstructors.idl:
* bindings/scripts/test/ExposedStar.idl: Added.
* bindings/scripts/test/JS/JSDOMWindow.cpp:
(WebCore::jsDOMWindow_ExposedStarConstructorGetter):
(WebCore::JSC_DEFINE_CUSTOM_GETTER):
* bindings/scripts/test/JS/JSDedicatedWorkerGlobalScope.cpp:
(WebCore::jsDedicatedWorkerGlobalScope_ExposedStarConstructorGetter):
(WebCore::JSC_DEFINE_CUSTOM_GETTER):
* bindings/scripts/test/JS/JSExposedStar.cpp: Added.
(WebCore::JSExposedStarDOMConstructor::prototypeForStructure):
(WebCore::JSExposedStarDOMConstructor::initializeProperties):
(WebCore::JSExposedStarPrototype::finishCreation):
(WebCore::JSExposedStar::JSExposedStar):
(WebCore::JSExposedStar::finishCreation):
(WebCore::JSExposedStar::createPrototype):
(WebCore::JSExposedStar::prototype):
(WebCore::JSExposedStar::getConstructor):
(WebCore::JSC_DEFINE_CUSTOM_GETTER):
(WebCore::jsExposedStarPrototypeFunction_operationForAllContextsBody):
(WebCore::JSC_DEFINE_HOST_FUNCTION):
(WebCore::jsExposedStarPrototypeFunction_operationJustForWindowContextsBody):
(WebCore::jsExposedStarPrototypeFunction_operationJustForWorkerContextsBody):
(WebCore::JSExposedStar::subspaceForImpl):
(WebCore::JSExposedStar::analyzeHeap):
(WebCore::toJSNewlyCreated):
(WebCore::toJS):
(WebCore::JSExposedStar::toWrapped):
* bindings/scripts/test/JS/JSExposedStar.h: Added.
(WebCore::JSExposedStar::create):
(WebCore::JSExposedStar::createStructure):
(WebCore::JSExposedStar::subspaceFor):
(WebCore::JSExposedStar::wrapped const):
(WebCore::toJS):
(WebCore::toJSNewlyCreated):
* bindings/scripts/test/JS/JSPaintWorkletGlobalScope.cpp:
(WebCore::jsPaintWorkletGlobalScope_ExposedStarConstructorGetter):
(WebCore::JSC_DEFINE_CUSTOM_GETTER):
* bindings/scripts/test/JS/JSServiceWorkerGlobalScope.cpp:
(WebCore::jsServiceWorkerGlobalScope_ExposedStarConstructorGetter):
(WebCore::JSC_DEFINE_CUSTOM_GETTER):
* bindings/scripts/test/PaintWorkletGlobalScopeConstructors.idl:
* bindings/scripts/test/ServiceWorkerGlobalScopeConstructors.idl:
* bindings/scripts/test/SupplementalDependencies.dep:



Canonical link: https://commits.webkit.org/243822@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@285196 268f45cc-cd09-0410-ab3c-d52691b4dbfc
philn pushed a commit to philn/old-webkit that referenced this pull request Nov 3, 2021
https://bugs.webkit.org/show_bug.cgi?id=231082

Reviewed by Chris Dumez.

Adds a shorthand to expose interfaces/attributes on Window, Workers*,
and the forthcoming ShadowRealm global object.

See whatwg/webidl#468 and
whatwg/webidl#526 for details.

* bindings/scripts/CodeGenerator.pm:
(shouldPropertyBeExposed):
* bindings/scripts/CodeGeneratorJS.pm:
(GenerateRuntimeEnableConditionalStringForExposed):
* bindings/scripts/IDLParser.pm:
(parseExtendedAttributeRest2):
* bindings/scripts/preprocess-idls.pl:
* bindings/scripts/test/AudioWorkletGlobalScopeConstructors.idl:
* bindings/scripts/test/DOMWindowConstructors.idl:
* bindings/scripts/test/DedicatedWorkerGlobalScopeConstructors.idl:
* bindings/scripts/test/ExposedStar.idl: Added.
* bindings/scripts/test/JS/JSDOMWindow.cpp:
(WebCore::jsDOMWindow_ExposedStarConstructorGetter):
(WebCore::JSC_DEFINE_CUSTOM_GETTER):
* bindings/scripts/test/JS/JSDedicatedWorkerGlobalScope.cpp:
(WebCore::jsDedicatedWorkerGlobalScope_ExposedStarConstructorGetter):
(WebCore::JSC_DEFINE_CUSTOM_GETTER):
* bindings/scripts/test/JS/JSExposedStar.cpp: Added.
(WebCore::JSExposedStarDOMConstructor::prototypeForStructure):
(WebCore::JSExposedStarDOMConstructor::initializeProperties):
(WebCore::JSExposedStarPrototype::finishCreation):
(WebCore::JSExposedStar::JSExposedStar):
(WebCore::JSExposedStar::finishCreation):
(WebCore::JSExposedStar::createPrototype):
(WebCore::JSExposedStar::prototype):
(WebCore::JSExposedStar::getConstructor):
(WebCore::JSC_DEFINE_CUSTOM_GETTER):
(WebCore::jsExposedStarPrototypeFunction_operationForAllContextsBody):
(WebCore::JSC_DEFINE_HOST_FUNCTION):
(WebCore::jsExposedStarPrototypeFunction_operationJustForWindowContextsBody):
(WebCore::jsExposedStarPrototypeFunction_operationJustForWorkerContextsBody):
(WebCore::JSExposedStar::subspaceForImpl):
(WebCore::JSExposedStar::analyzeHeap):
(WebCore::toJSNewlyCreated):
(WebCore::toJS):
(WebCore::JSExposedStar::toWrapped):
* bindings/scripts/test/JS/JSExposedStar.h: Added.
(WebCore::JSExposedStar::create):
(WebCore::JSExposedStar::createStructure):
(WebCore::JSExposedStar::subspaceFor):
(WebCore::JSExposedStar::wrapped const):
(WebCore::toJS):
(WebCore::toJSNewlyCreated):
* bindings/scripts/test/JS/JSPaintWorkletGlobalScope.cpp:
(WebCore::jsPaintWorkletGlobalScope_ExposedStarConstructorGetter):
(WebCore::JSC_DEFINE_CUSTOM_GETTER):
* bindings/scripts/test/JS/JSServiceWorkerGlobalScope.cpp:
(WebCore::jsServiceWorkerGlobalScope_ExposedStarConstructorGetter):
(WebCore::JSC_DEFINE_CUSTOM_GETTER):
* bindings/scripts/test/PaintWorkletGlobalScopeConstructors.idl:
* bindings/scripts/test/ServiceWorkerGlobalScopeConstructors.idl:
* bindings/scripts/test/SupplementalDependencies.dep:


git-svn-id: http://svn.webkit.org/repository/webkit/trunk@285196 268f45cc-cd09-0410-ab3c-d52691b4dbfc
@littledan
Copy link
Collaborator Author

This change looks great, but I don't think it handles the ShadowRealm case yet, since that is not an interface.

@annevk
Copy link
Member

annevk commented Jun 8, 2022

@littledan that's a good point. I suppose we should put that requirement in HTML along with the overall ShadowRealm integration?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

Exposing an interface "on all globals"
9 participants