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

ShouldCallPull: Assert desiredSize is not null #926

Merged
merged 2 commits into from May 23, 2018

Conversation

ricea
Copy link
Collaborator

@ricea ricea commented May 18, 2018

ReadableStreamDefaultControllerShouldCallPull implicitly assumes that
desiredSize is not null when it compares it to 0. This is guaranteed by
the check of
ReadableStreamDefaultControllerCanCloseOrEnqueue(controller), but it's
not obvious. Add an assert to make it explicit.

Also do the same for ReadableByteStreamControllerShouldCallPull.

No functional changes.

Closes #919.


Preview | Diff

ReadableStreamDefaultControllerShouldCallPull implicitly assumes that
desiredSize is not null when it compares it to 0. This is guaranteed by
the check of
ReadableStreamDefaultControllerCanCloseOrEnqueue(controller), but it's
not obvious. Add an assert to make it explicit.

Also do the same for ReadableByteStreamControllerShouldCallPull.

No functional changes.
@ricea
Copy link
Collaborator Author

ricea commented May 22, 2018

@domenic What do you think?

Copy link
Member

@domenic domenic left a comment

Choose a reason for hiding this comment

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

LGTM, sorry for delay!

index.bs Outdated
@@ -1826,6 +1826,7 @@ nothrow>ReadableStreamDefaultControllerShouldCallPull ( <var>controller</var> )<
1. If ! IsReadableStreamLocked(_stream_) is *true* and ! ReadableStreamGetNumReadRequests(_stream_) > *0*, return
*true*.
1. Let _desiredSize_ be ReadableStreamDefaultControllerGetDesiredSize(_controller_).
Copy link
Member

Choose a reason for hiding this comment

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

While here can you add a !?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done, thanks.

ReadableStreamDefaultControllerShouldCallPull was missing an exclamation mark
before ReadableStreamDefaultControllerGetDesiredSize. Fix it.
@ricea ricea merged commit a6de38e into whatwg:master May 23, 2018
@ricea ricea deleted the should-call-pull-assert branch May 23, 2018 07:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants