Skip to content

Specifying QueuingStrategies in WebIDL #1005

Closed

Description

@ricea

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:

  1. size() is a function (as opposed to a method), but it is inherited
  2. 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.

Activity

  1. domenic commented on Jul 9, 2019

    @domenic
    Member

    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:

    • Use any and return a function object.
    • Introduce a new Web IDL extended attribute for methods that removes the brand check.

    As always, I apologize for this mess.

  2. MattiasBuelens commented on Jul 9, 2019

    @MattiasBuelens
    Collaborator

    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:

    1. 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.

  3. ricea commented on Jul 10, 2019

    @ricea
    CollaboratorAuthor

    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;
    };

    This still has slightly odd semantics:

    const size = new CountQueuingStrategy({}).size;
    console.assert(size() == 1);

    works as expected, but

    const size = CountQueuingStrategy.prototype.size;

    throws an exception.

  4. domenic commented on Jul 23, 2019

    @domenic
    Member

    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.

  5. ricea commented on Jul 24, 2019

    @ricea
    CollaboratorAuthor

    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++.

  6. added a commit that references this issue on Aug 23, 2019
  7. domenic commented on Nov 3, 2019

    @domenic
    Member

    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.

  8. 1 remaining item

  9. ricea commented on Nov 5, 2019

    @ricea
    CollaboratorAuthor

    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'm adding some in https://chromium-review.googlesource.com/c/chromium/src/+/1899236 but it will be weeks before we have an answer.

  10. added a commit that references this issue on Nov 5, 2019
  11. ricea commented on Mar 16, 2020

    @ricea
    CollaboratorAuthor

    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.

  12. added 3 commits that reference this issue on Apr 13, 2020
    0674415
    c5b5e05
    ca89482
  13. domenic commented on Apr 15, 2020

    @domenic
    Member

    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 like

      assert_equals(CountQueuingStrategy.prototype.size.call(thisValue, chunk), 1);

    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

    const strategy = new CountQueuingStrategy({ highWaterMark: 0 });
    const size = strategy.size; // passes the getter thisArg check
    
    size(5); // no thisArg while calling
    size.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.

  14. ricea commented on Apr 16, 2020

    @ricea
    CollaboratorAuthor

    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.

  15. ExE-Boss commented on Apr 16, 2020

    @ExE-Boss

    Alternatively, we could create size as a data property in a custom binding, similar to how DOMException has a custom binding.


    Optionally, we could make it possible to specify data properties in WebIDL.

  16. domenic commented on Apr 16, 2020

    @domenic
    Member

    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.

  17. domenic commented on Apr 16, 2020

    @domenic
    Member

    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.

  18. added a commit that references this issue on Jun 11, 2020
    caf3cea
  19. added a commit that references this issue on Jun 3, 2021
    7dbcfed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions