Skip to content

[worklets] Should paintWorklet be a static attribute? #410

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

Open
nhiroki opened this issue May 22, 2017 · 5 comments
Open

[worklets] Should paintWorklet be a static attribute? #410

nhiroki opened this issue May 22, 2017 · 5 comments

Comments

@nhiroki
Copy link
Contributor

nhiroki commented May 22, 2017

IIUC, CSS is not an instance but an interface (and will be changed to namespace?), so paintWorklet could be a static attribute[2] like this:

partial interface CSS {
    [SameObject] static readonly attribute Worklet paintWorklet;
};

[1] https://heycam.github.io/webidl/#idl-namespaces
[2] https://heycam.github.io/webidl/#idl-static-attributes-and-operations

@yuki3
Copy link

yuki3 commented May 25, 2017

@domenic, could you give us advice?

My questions are "what realm should be used when creating a new platform object in static IDL attributes/operations?" and "is it allowed (or good) to create a new platform object in static IDL attributes/operations?"

In CSSOM, it says that

Specifications that define static functions on the CSS interface and want to store some state should store the state on the current global object’s associated Document.

So, CSS.paintWorklet in the first comment seems going to create a platform object (a Worklet) in the Current realm. However, is this well-spec'ed? I have several questions here.

  1. Is it okay to create a new platform object in static IDL attributes/operations? Like document.createElement and navigator.permissions, it shouldn't be static at all?
  2. Is this CSSOM-specific rule? Should we have a general rule about static IDL attributes/operations? CSSOM should follow the general rule?

In case of paintWorklet, it was/is originally window.paintWorklet and there is no problem, but it will be moved into CSS interface (or namespace) and will be CSS.paintWorklet. I'm not sure how they should be moved. Maybe it should be css.paintWorklet where css is a singleton like navigator?

@domenic
Copy link
Contributor

domenic commented May 30, 2017

Should paintWorklet be a static attribute?

So, my understanding was that CSS was going to be a namespace, or maybe was already; @zcorpan worked with me on getting that working. In that case it's just an attribute, but it's automatically "static", since namespaces are themselves "static". I guess that hasn't happened yet though? In which case adding static makes sense. But switching to namespaces would be better...

My questions are "what realm should be used when creating a new platform object in static IDL attributes/operations?"

It should be "the current realm", since in static attributes/operations, there is no this object to look at the relevant realm of.

and "is it allowed (or good) to create a new platform object in static IDL attributes/operations?"

I think it is fine, but see below.

So, CSS.paintWorklet in the first comment seems going to create a platform object (a Worklet) in the Current realm.

My interpretation was that each Document has an associated paint worklet, which is a Worklet. This is created at the same time the document is created, maybe. Then the paintWorklet attribute just returns the current global object's associated Document's paint worklet.

Of course this could be done lazily too, as an implementation detail.

Is this CSSOM-specific rule? Should we have a general rule about static IDL attributes/operations? CSSOM should follow the general rule?

I agree things are a bit confusing here. I probably was the one who suggested "should store the state on the current global object’s associated Document." But I am not sure why that is better than just "the current global object". All the "associated Document" bit does is make things confusing when document.open() gets involved. So maybe if we simplified it to just "the current global object" as the rule for all static things (and all namespace things), it would be better.

Maybe it should be css.paintWorklet where css is a singleton like navigator?

I don't think this should be necessary. We want to be able to create a world where you can use namespaces. It's very silly right now to have things like Navigator + navigator, i.e. singleton classes which can never be constructed by authors. That is very un-JavaScriptey. Namespaces are the right solution for the singleton pattern in JavaScript. So we should make sure they are clear and easy to use.

@zcorpan
Copy link
Member

zcorpan commented May 30, 2017

Changing to CSS namespace is w3c/csswg-drafts#437

@yuki3
Copy link

yuki3 commented May 31, 2017

Thanks for the detailed explanation. In summary, it's fine that static attributes/operations create/return a platform object, and in that case, the current realm should be used instead of the relevant realm of this object.

I hope that it'll be clarified in Web IDL (or already?) and CSSOM simply follows the general rules. Anyway, my questions are resolved. Thanks a lot.

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Aug 24, 2017
Before this CL, |paintWorklet| attribute is defined on Window interface, but
that should be defined on CSS interface[1].

According to the spec discussion[2], "CSS" will be changed from "interface" to
"namespace", but "namespace" is not supported yet in Blink. As a stopgap, this
CL changes "paintWorklet" to a static attribute to emulate an
attribute-on-namespace and moves it to "CSS" interface.

[1] https://drafts.css-houdini.org/css-paint-api-1/#dom-css-paintworklet
[2] w3c/css-houdini-drafts#410

Bug: 719303
Change-Id: I01f371f0d4f9a193352d1ede1fe28632d52cf312
Reviewed-on: https://chromium-review.googlesource.com/630899
WPT-Export-Revision: c3dcf02b902dbdbf35d3f700d897b71e1dbc4042
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Aug 25, 2017
Before this CL, "paintWorklet" attribute is defined on "Window" interface, but
that should be defined on "CSS" interface[1].

According to the spec discussion[2], "CSS" will be changed from "interface" to
"namespace", but "namespace" is not supported yet in Blink. As a stopgap, this
CL changes "paintWorklet" to a static attribute and moves it to "CSS" interface
to emulate an attribute-on-namespace.

[1] https://drafts.css-houdini.org/css-paint-api-1/#dom-css-paintworklet
[2] w3c/css-houdini-drafts#410

Bug: 719303
Change-Id: I01f371f0d4f9a193352d1ede1fe28632d52cf312
Reviewed-on: https://chromium-review.googlesource.com/630899
Commit-Queue: Hiroki Nakagawa 
Reviewed-by: Xida Chen 
Reviewed-by: Ian Kilpatrick 
Reviewed-by: Kentaro Hara 
WPT-Export-Revision: ff3957a21bc1bc8bf5df4e44f29832e424dfa49e
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Aug 25, 2017
Before this CL, "paintWorklet" attribute is defined on "Window" interface, but
that should be defined on "CSS" interface[1].

According to the spec discussion[2], "CSS" will be changed from "interface" to
"namespace", but "namespace" is not supported yet in Blink. As a stopgap, this
CL changes "paintWorklet" to a static attribute and moves it to "CSS" interface
to emulate an attribute-on-namespace.

[1] https://drafts.css-houdini.org/css-paint-api-1/#dom-css-paintworklet
[2] w3c/css-houdini-drafts#410

Bug: 719303
Change-Id: I01f371f0d4f9a193352d1ede1fe28632d52cf312
Reviewed-on: https://chromium-review.googlesource.com/630899
Commit-Queue: Hiroki Nakagawa 
Reviewed-by: Yuki Shiino 
Reviewed-by: Xida Chen 
Reviewed-by: Ian Kilpatrick 
Reviewed-by: Kentaro Hara 
Cr-Commit-Position: refs/heads/master@{#497319}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Aug 25, 2017
Before this CL, "paintWorklet" attribute is defined on "Window" interface, but
that should be defined on "CSS" interface[1].

According to the spec discussion[2], "CSS" will be changed from "interface" to
"namespace", but "namespace" is not supported yet in Blink. As a stopgap, this
CL changes "paintWorklet" to a static attribute and moves it to "CSS" interface
to emulate an attribute-on-namespace.

[1] https://drafts.css-houdini.org/css-paint-api-1/#dom-css-paintworklet
[2] w3c/css-houdini-drafts#410

Bug: 719303
Change-Id: I01f371f0d4f9a193352d1ede1fe28632d52cf312
Reviewed-on: https://chromium-review.googlesource.com/630899
Commit-Queue: Hiroki Nakagawa 
Reviewed-by: Yuki Shiino 
Reviewed-by: Xida Chen 
Reviewed-by: Ian Kilpatrick 
Reviewed-by: Kentaro Hara 
Cr-Commit-Position: refs/heads/master@{#497319}
scheib pushed a commit to scheib/chromium that referenced this issue Aug 25, 2017
Before this CL, "paintWorklet" attribute is defined on "Window" interface, but
that should be defined on "CSS" interface[1].

According to the spec discussion[2], "CSS" will be changed from "interface" to
"namespace", but "namespace" is not supported yet in Blink. As a stopgap, this
CL changes "paintWorklet" to a static attribute and moves it to "CSS" interface
to emulate an attribute-on-namespace.

[1] https://drafts.css-houdini.org/css-paint-api-1/#dom-css-paintworklet
[2] w3c/css-houdini-drafts#410

Bug: 719303
Change-Id: I01f371f0d4f9a193352d1ede1fe28632d52cf312
Reviewed-on: https://chromium-review.googlesource.com/630899
Commit-Queue: Hiroki Nakagawa 
Reviewed-by: Yuki Shiino 
Reviewed-by: Xida Chen 
Reviewed-by: Ian Kilpatrick 
Reviewed-by: Kentaro Hara 
Cr-Commit-Position: refs/heads/master@{#497319}
rachelandrew pushed a commit to rachelandrew/web-platform-tests that referenced this issue Nov 8, 2017
Before this CL, "paintWorklet" attribute is defined on "Window" interface, but
that should be defined on "CSS" interface[1].

According to the spec discussion[2], "CSS" will be changed from "interface" to
"namespace", but "namespace" is not supported yet in Blink. As a stopgap, this
CL changes "paintWorklet" to a static attribute and moves it to "CSS" interface
to emulate an attribute-on-namespace.

[1] https://drafts.css-houdini.org/css-paint-api-1/#dom-css-paintworklet
[2] w3c/css-houdini-drafts#410

Bug: 719303
Change-Id: I01f371f0d4f9a193352d1ede1fe28632d52cf312
Reviewed-on: https://chromium-review.googlesource.com/630899
Commit-Queue: Hiroki Nakagawa 
Reviewed-by: Yuki Shiino 
Reviewed-by: Xida Chen 
Reviewed-by: Ian Kilpatrick 
Reviewed-by: Kentaro Hara 
Cr-Commit-Position: refs/heads/master@{#497319}
yurket added a commit to yurket/servo that referenced this issue Feb 20, 2018
yurket added a commit to yurket/servo that referenced this issue Feb 22, 2018
nupurbaghel pushed a commit to paavininanda/servo that referenced this issue Mar 14, 2018
@domenic
Copy link
Contributor

domenic commented Oct 16, 2020

This issue's labels should be changed to one of the css-paint-api-* labels, instead of worklets-1.

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

No branches or pull requests

5 participants