Add ReadableStreamDefaultControllerHasBackpressure operation and use it in TransformStream

[ricea]

The TransformStream implementation currently looks at desiredSize to get
an approximation of whether there is backpressure on the readable side
or not. This is fragile and not really clean enough for standardisation.

Add an abstract operation ReadableStreamDefaultControllerHasBackpressure
which reflect the ReadableStream's own notion of whether there is
backpressure. Use it in TransformStream to accurately detect when an
enqueue() operation has filled the queue.

Since ReadableStream always considers itself to have backpressure until
the underlying source start() method completes, use the same convention
in TransformStream. This simplifies some logic.

[ricea]

Bikeshedding welcome.

[ricea]

A couple of questions to spur discussion:

Should this be an exported abstract operation rather than a polymorphic method? Does it matter?

Should this be a public operation? Alternatively, should ReadableStream*Controller have a "ready" promise which provides the functionality of TransformStreamReadableReadyPromise?

[domenic]

+1 on the general idea. There are a lot of changes here in the semantics, as the *ShouldCallPull abstract ops are much more in-depth, but in general it seems like the new semantics are better and more likely to cover edge cases.

Speaking of those edge cases, it might be worth writing some tests to cover them. E.g. do steps 1-5 of https://streams.spec.whatwg.org/#readable-stream-default-controller-should-call-pull ever cause us to return early? Can we trigger that from a test?

Should this be an exported abstract operation rather than a polymorphic method? Does it matter?

Since it's polymorphic, that seems harder. I'm not sure what you mean by exported, but it seems unlikely, as this is somewhat of an implementation detail.

Should this be a public operation? Alternatively, should ReadableStream*Controller have a "ready" promise which provides the functionality of TransformStreamReadableReadyPromise?

Heh, then the controllers really would look like writable streams...

I don't think it's necessary, because I can't see how an underlying source would use it. Maybe if we didn't have pull() {}, but only start() {}, then you could use that promise to implement pull() {}. But we have pull(), which takes care of this use case, I think.

[ricea]

The user-visible behaviour change is one extra microtask before the first transform() is called when starting without backpressure. I think. Counting microtask iterations is tricky. Testing the number of microtask iterations is even trickier, so I'm not going to try. We make up for the extra call to pull() at startup by avoiding unnecessary calls to pull() when a read() is in process, so on balance it works out well.

In #777 I argue for always starting with backpressure by default, which would make this difference even less consequential.

This means there isn't anything I can test here. However, thanks to your hint I noticed that calling HasBackpressure before start() has completed will always return true, so I just hardcoded that value, which then let me simplify TSDefaultSource's start() method.

When a close or error is triggered by the TransformStream itself, it always sets _readableClosed, which prevents reaching the call to HasBackpressure. If it gets triggered by the consumer calling cancel() than the TransformStream is notified by the source cancel() method before we get a chance to call HasBackpressure, setting _readableClosed.

As part of my work in in restructuring the TransformStream implementation, I realised that the controller.enqueue() method which calls HasBackpressure will be stream-type specific. So there's no need for it to be polymorphic, and I've replaced it with a ReadableStreamDefaultControllerHasBackpressure abstract operation.

The benefit of having a controller.ready on ReadableStream would be that custom TransformStream-alike objects could share the same logic the built-in one does. But it's not very compelling. Implementing transforms without using TransformStream is a very niche case.

[domenic]

Now this is a nice simplification.

If you're confident this doesn't need any test changes, then please go ahead and merge :). Maybe the commit message can explain some of the things discovered here.