You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
As part of #963, we will have to specify CountQueuingStrategy and ByteLengthQueuingStrategy in WebIDL. There are a couple of things about these classes that are "strange" from a WebIDL point-of-view:
size() is a function (as opposed to a method), but it is inherited
highWaterMark is added by the constructor as a property of the object, rather than coming from the prototype.
I think problem 2. can be fixed quite easily by making highWaterMark a normal interface attribute.
Problem 1. is harder. WebIDL doesn't seem to have functions as such at all. The closest thing is "Callback functions", but it doesn't appear that they can exist as attributes on an interface. There are also "static operations", which are a bit like functions, but appear as properties of the constructor, not the prototype.
Would this mean that CountQueuingStrategy.prototype.size() must always be called with a proper CountQueuingStrategy object as this context? Correct me if I'm wrong, but it seems like the WebIDL spec requires a brand check on all operations:
If target is an interface, and op is not a static operation:
2.1. Let esValue be the this value, if it is not null or undefined, or realm’s global object otherwise. (This will subsequently cause a TypeError in a few steps, if the global object does not implement target and [LenientThis] is not specified.)
2.2. If esValue is a platform object, then perform a security check, passing esValue, id, and "method".
2.3. If esValue does not implement the interface target, throw a TypeError.
This would be a problem in the current streams specification, since we always call the size algorithm as a plain function with no this context.
I don't want to give up on calling size as a plain function. The principle that it is a pure function is important to the mental model of streams. Making it a method gives the implication that it can be stateful.
The brand check is necessary for the correct functioning of WebIDL-generated code, at least in Blink. Before it can call a C++ method, it needs to make sure it's calling it on an object of the correct type.
The way I'm currently doing it in my in-progress implementation in Blink is this:
[
Exposed=(Window,Worker,Worklet),
Constructor(QueuingStrategyInit init)
] interface CountQueuingStrategy {
readonly attribute any highWaterMark;//size is an accessor that returns a function.
readonly attribute any size;
};
For what it's worth, I would prefer to introduce a new Web IDL extended attribute for this, as it's pretty weird to have a getter that returns a function, instead of just having a function directly.
For what it's worth, I would prefer to introduce a new Web IDL extended attribute for this, as it's pretty weird to have a getter that returns a function, instead of just having a function directly.
Yes, an extended attribute would make sense. In C++, the behaviour corresponds to a static method, but in the WebIDL world it's more like a normal method that doesn't take a this value. So perhaps if you annotate an ordinary method like [NoThis] then it would mostly appear like a normal method from JavaScript, but the bindings code would translate it to a call to a static method in C++.
Can we get use counters on CountQueuingStrategy and ByteQueuingStrategy? If they are not used almost at all, we could consider more drastic design changes or removals. I doubt that's the case, but it might be, and I think we should do due dilligence.
Can we get use counters on CountQueuingStrategy and ByteQueuingStrategy? If they are not used almost at all, we could consider more drastic design changes or removals. I doubt that's the case, but it might be, and I think we should do due dilligence.
i.e. they assume you can access CountQueuingStrategy.prototype.size. That is not possible, even with this strategy, because the getter will do a thisArg check. You can do
conststrategy=newCountQueuingStrategy({highWaterMark: 0});constsize=strategy.size;// passes the getter thisArg checksize(5);// no thisArg while callingsize.call(undefined,chunk);// no thisArg while calling
but I'm really unsure if this is worth it. I think maybe we should just specify size() as a normal Web IDL method.
My point was this is all not really worth it. The benefits are so small (smaller than I'd realized) and the spec/implementation complexity relatively large. I think we should just make them normal methods.
Hmm, but if we make them normal methods, then we have to do similar tricks to what we're doing for UnderlyingSource/UnderlyingSink dictionaries. I.e. convert to a QueuingStrategy dictionary in prose and save the original object. So the simplicity gains are not as nice as I thought. OK, maybe I'll just continue with the current path...
Alternately we could add infrastructure to Web IDL to allow invoking methods of a dictionary on the original object, somehow, to handle both cases. But I'm not sure that's a good idea.
Activity
domenic commentedon Jul 9, 2019
I think it's OK to make size() a normal method, although there is some risk of back-compat problems if people are using it in strange ways.
If we think that is not likely to work, we have a few workarounds:
anyand return a function object.As always, I apologize for this mess.
MattiasBuelens commentedon Jul 9, 2019
Would this mean that
CountQueuingStrategy.prototype.size()must always be called with a properCountQueuingStrategyobject asthiscontext? Correct me if I'm wrong, but it seems like the WebIDL spec requires a brand check on all operations:This would be a problem in the current streams specification, since we always call the size algorithm as a plain function with no
thiscontext.ricea commentedon Jul 10, 2019
I don't want to give up on calling
sizeas a plain function. The principle that it is a pure function is important to the mental model of streams. Making it a method gives the implication that it can be stateful.The brand check is necessary for the correct functioning of WebIDL-generated code, at least in Blink. Before it can call a C++ method, it needs to make sure it's calling it on an object of the correct type.
The way I'm currently doing it in my in-progress implementation in Blink is this:
[ Exposed=(Window,Worker,Worklet), Constructor(QueuingStrategyInit init) ] interface CountQueuingStrategy { readonly attribute any highWaterMark; // size is an accessor that returns a function. readonly attribute any size; };This still has slightly odd semantics:
works as expected, but
throws an exception.
Implement QueuingStrategies in C++
Implement QueuingStrategies in C++
Implement QueuingStrategies in C++
domenic commentedon Jul 23, 2019
For what it's worth, I would prefer to introduce a new Web IDL extended attribute for this, as it's pretty weird to have a getter that returns a function, instead of just having a function directly.
ricea commentedon Jul 24, 2019
Yes, an extended attribute would make sense. In C++, the behaviour corresponds to a static method, but in the WebIDL world it's more like a normal method that doesn't take a
thisvalue. So perhaps if you annotate an ordinary method like[NoThis]then it would mostly appear like a normal method from JavaScript, but the bindings code would translate it to a call to a static method in C++.Bug 1566341 [wpt PR 17804] - Implement QueuingStrategies in C++, a=te…
Bug 1566341 [wpt PR 17804] - Implement QueuingStrategies in C++, a=te…
Implement QueuingStrategies in C++
Bug 1566341 [wpt PR 17804] - Implement QueuingStrategies in C++, a=te…
Bug 1566341 [wpt PR 17804] - Implement QueuingStrategies in C++, a=te…
Bug 1566341 [wpt PR 17804] - Implement QueuingStrategies in C++, a=te…
domenic commentedon Nov 3, 2019
Can we get use counters on CountQueuingStrategy and ByteQueuingStrategy? If they are not used almost at all, we could consider more drastic design changes or removals. I doubt that's the case, but it might be, and I think we should do due dilligence.
1 remaining item
ricea commentedon Nov 5, 2019
I'm adding some in https://chromium-review.googlesource.com/c/chromium/src/+/1899236 but it will be weeks before we have an answer.
Use counters for Streams Strategies
ricea commentedon Mar 16, 2020
Usage is certainly low. CountQueuingStrategy is used on roughly 0.000006% of page loads: https://chromestatus.com/metrics/feature/timeline/popularity/3082.
ByteLengthQueuingStrategy is used on about 0.00004% of page loads: https://chromestatus.com/metrics/feature/timeline/popularity/3083.
Given how easily they can be polyfilled, removal might be an option.
Rewrite to use Web IDL, and generally modernize
Rewrite to use Web IDL, and generally modernize
Rewrite to use Web IDL, and generally modernize
domenic commentedon Apr 15, 2020
I'm updating the tests with the "Specify size as a
readonly attribute Function size;" strategy, and noticing that a lot of them do `things likei.e. they assume you can access
CountQueuingStrategy.prototype.size. That is not possible, even with this strategy, because the getter will do a thisArg check. You can dobut I'm really unsure if this is worth it. I think maybe we should just specify
size()as a normal Web IDL method.ricea commentedon Apr 16, 2020
In Blink we simply fail those tests:
https://cs.chromium.org/chromium/src/third_party/blink/web_tests/external/wpt/streams/count-queuing-strategy.any-expected.txt
Those tests will have to be changed whatever we do, so I don't think it should be a deciding factor.
ExE-Boss commentedon Apr 16, 2020
Alternatively, we could create
sizeas a data property in a custom binding, similar to howDOMExceptionhas a custom binding.Optionally, we could make it possible to specify data properties in WebIDL.
domenic commentedon Apr 16, 2020
My point was this is all not really worth it. The benefits are so small (smaller than I'd realized) and the spec/implementation complexity relatively large. I think we should just make them normal methods.
domenic commentedon Apr 16, 2020
Hmm, but if we make them normal methods, then we have to do similar tricks to what we're doing for UnderlyingSource/UnderlyingSink dictionaries. I.e. convert to a QueuingStrategy dictionary in prose and save the original object. So the simplicity gains are not as nice as I thought. OK, maybe I'll just continue with the current path...
Alternately we could add infrastructure to Web IDL to allow invoking methods of a dictionary on the original object, somehow, to handle both cases. But I'm not sure that's a good idea.
Rewrite to use Web IDL, and generally modernize
Rewrite to use Web IDL, and generally modernize
Bug 1566341 [wpt PR 17804] - Implement QueuingStrategies in C++, a=te…