Bug 215607 - Add support for TransformStream
Summary: Add support for TransformStream
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Local Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: youenn fablet
URL:
Keywords: InRadar
: 151599 (view as bug list)
Depends on: 215448
Blocks:
  Show dependency treegraph
 
Reported: 2020-08-18 08:27 PDT by youenn fablet
Modified: 2022-08-02 14:08 PDT (History)
16 users (show)

See Also:


Attachments
Patch (162.99 KB, patch)
2020-08-18 08:39 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (173.83 KB, patch)
2020-08-18 10:32 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (163.21 KB, patch)
2020-08-26 01:38 PDT, youenn fablet
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description youenn fablet 2020-08-18 08:27:36 PDT
Add support for TransformStream
Comment 1 youenn fablet 2020-08-18 08:39:40 PDT
Created attachment 406788 [details]
Patch
Comment 2 EWS Watchlist 2020-08-18 08:40:26 PDT
This patch modifies the imported WPT tests. Please ensure that any changes on the tests (not coming from a WPT import) are exported to WPT. Please see https://trac.webkit.org/wiki/WPTExportProcess
Comment 3 youenn fablet 2020-08-18 10:32:50 PDT
Created attachment 406791 [details]
Patch
Comment 4 Sam Weinig 2020-08-18 11:01:07 PDT
Comment on attachment 406791 [details]
Patch

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

> Source/WebCore/page/RuntimeEnabledFeatures.h:296
> +    void setTransformStreamAPIEnabled(bool isEnabled) { m_isTransformStreamAPIEnabled = isEnabled; }
> +    bool transformStreamAPIEnabled() const { return m_isTransformStreamAPIEnabled; }

Unless this truly needs to be on the singleton, please use a Setting rather than a global RuntimeEnabledFeature.
Comment 5 youenn fablet 2020-08-18 11:17:48 PDT
(In reply to Sam Weinig from comment #4)
> Comment on attachment 406791 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=406791&action=review
> 
> > Source/WebCore/page/RuntimeEnabledFeatures.h:296
> > +    void setTransformStreamAPIEnabled(bool isEnabled) { m_isTransformStreamAPIEnabled = isEnabled; }
> > +    bool transformStreamAPIEnabled() const { return m_isTransformStreamAPIEnabled; }
> 
> Unless this truly needs to be on the singleton, please use a Setting rather
> than a global RuntimeEnabledFeature.

TransformStream are exposed to Workers, using a setting would require some additional work.
Comment 6 Sam Weinig 2020-08-18 11:21:05 PDT
(In reply to youenn fablet from comment #5)
> (In reply to Sam Weinig from comment #4)
> > Comment on attachment 406791 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=406791&action=review
> > 
> > > Source/WebCore/page/RuntimeEnabledFeatures.h:296
> > > +    void setTransformStreamAPIEnabled(bool isEnabled) { m_isTransformStreamAPIEnabled = isEnabled; }
> > > +    bool transformStreamAPIEnabled() const { return m_isTransformStreamAPIEnabled; }
> > 
> > Unless this truly needs to be on the singleton, please use a Setting rather
> > than a global RuntimeEnabledFeature.
> 
> TransformStream are exposed to Workers, using a setting would require some
> additional work.

ok.
Comment 7 Alex Christensen 2020-08-19 09:02:10 PDT
Comment on attachment 406791 [details]
Patch

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

> LayoutTests/imported/w3c/web-platform-tests/streams/transform-streams/reentrant-strategies.any.js:319
> +// }, 'writer.abort() inside size() should work');

Why are we commenting all this out?
Comment 8 youenn fablet 2020-08-20 09:14:00 PDT
(In reply to Alex Christensen from comment #7)
> Comment on attachment 406791 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=406791&action=review
> 
> > LayoutTests/imported/w3c/web-platform-tests/streams/transform-streams/reentrant-strategies.any.js:319
> > +// }, 'writer.abort() inside size() should work');
> 
> Why are we commenting all this out?

Ah, right, I forgot to undo these changes.
Tests passes but I will reupload a patch with tests enabled.
Comment 9 youenn fablet 2020-08-24 23:59:26 PDT
ping review
Comment 10 Radar WebKit Bug Importer 2020-08-25 08:28:13 PDT
<rdar://problem/67739054>
Comment 11 Alex Christensen 2020-08-25 12:18:45 PDT
Comment on attachment 406791 [details]
Patch

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

> Source/WebCore/Modules/streams/TransformStream.idl:2
> + * Copyright (C) 2015 Canon Inc.

Is this correct?  Was this implemented in 2015?

> LayoutTests/imported/w3c/web-platform-tests/streams/transform-streams/reentrant-strategies.any.js:41
> +// promise_test(() => {

This needs to not be commented out.
Comment 12 youenn fablet 2020-08-26 01:38:41 PDT
Created attachment 407282 [details]
Patch
Comment 13 youenn fablet 2020-08-27 02:41:04 PDT
Test failures are crashes that do not seem related to this patch
Comment 14 EWS 2020-08-27 03:00:25 PDT
Committed r266228: <https://trac.webkit.org/changeset/266228>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 407282 [details].
Comment 15 Brent Fulgham 2022-08-02 14:08:05 PDT
*** Bug 151599 has been marked as a duplicate of this bug. ***