Bug 151356 - [Streams API] Implement IsReadableStreamDisturbed according to spec
Summary: [Streams API] Implement IsReadableStreamDisturbed according to spec
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Xabier Rodríguez Calvar
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-11-17 12:33 PST by Xabier Rodríguez Calvar
Modified: 2015-11-19 01:21 PST (History)
4 users (show)

See Also:


Attachments
Patch (10.59 KB, patch)
2015-11-17 12:38 PST, Xabier Rodríguez Calvar
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Xabier Rodríguez Calvar 2015-11-17 12:33:21 PST
[Streams API] Implement IsReadableStreamDisturbed according to spec
Comment 1 Xabier Rodríguez Calvar 2015-11-17 12:38:08 PST
Created attachment 265697 [details]
Patch
Comment 2 Darin Adler 2015-11-18 08:31:31 PST
Comment on attachment 265697 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=265697&action=review

> Source/WebCore/testing/Internals.idl:413
> +    [Conditional=STREAMS_API, CallWith=ScriptState] boolean isReadableStreamDisturbed(any stream);

Is there really no indirect way to test this “disturbed” state? That’s what we should be doing. I think exposing the internal disturbed flag is an inferior way to do the testing. The specification does not actually require a “disturbed” flag; just that everything works “as if” there is one. I suppose we are inheriting this testing strategy from whoever is creating the test suite, though.
Comment 3 Xabier Rodríguez Calvar 2015-11-18 09:48:35 PST
(In reply to comment #2)
> Is there really no indirect way to test this “disturbed” state? That’s what
> we should be doing. I think exposing the internal disturbed flag is an
> inferior way to do the testing. The specification does not actually require
> a “disturbed” flag; just that everything works “as if” there is one. I
> suppose we are inheriting this testing strategy from whoever is creating the
> test suite, though.

Yes, I am doing it like this precisely because there are such tests. Actually it is kinda surprinsing that they exist in such way.

Anyway, the flag is exposed thru the test internals so never to the user in the browser.
Comment 4 youenn fablet 2015-11-18 10:21:21 PST
AFAIK, disturbed flag is only needed by fetch API atm.

It will be testable as usual once fetch API becomes available to WebKit which I hope will happen in a near future.

The reference testsuite is built to ensure spec and ref impl work well.
Comment 5 WebKit Commit Bot 2015-11-19 01:21:53 PST
Comment on attachment 265697 [details]
Patch

Clearing flags on attachment: 265697

Committed r192621: <http://trac.webkit.org/changeset/192621>
Comment 6 WebKit Commit Bot 2015-11-19 01:21:56 PST
All reviewed patches have been landed.  Closing bug.