Bug 170749 - Disable outdated WritableStream API
Summary: Disable outdated WritableStream API
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: Dean Jackson
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-04-11 14:17 PDT by Dean Jackson
Modified: 2017-04-17 14:24 PDT (History)
16 users (show)

See Also:


Attachments
Patch (8.88 KB, patch)
2017-04-11 14:19 PDT, Dean Jackson
no flags Details | Formatted Diff | Diff
Patch (102.86 KB, patch)
2017-04-14 22:17 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (42.80 KB, patch)
2017-04-14 22:21 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (105.27 KB, patch)
2017-04-14 22:26 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (105.81 KB, patch)
2017-04-14 22:54 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (106.43 KB, patch)
2017-04-15 12:22 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Win tentative build fix (117.40 KB, patch)
2017-04-15 16:10 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews122 for ios-simulator-wk2 (817.34 KB, application/zip)
2017-04-15 18:54 PDT, Build Bot
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Dean Jackson 2017-04-11 14:17:07 PDT
Disable outdated WritableStream API
Comment 1 Dean Jackson 2017-04-11 14:19:27 PDT
<rdar://problem/31446233>
Comment 2 Dean Jackson 2017-04-11 14:19:58 PDT
Created attachment 306849 [details]
Patch
Comment 3 Tim Horton 2017-04-11 14:26:05 PDT
Comment on attachment 306849 [details]
Patch

rs=me
Comment 4 Dean Jackson 2017-04-11 14:31:22 PDT
Committed r215250: <http://trac.webkit.org/changeset/215250>
Comment 5 youenn fablet 2017-04-12 09:18:19 PDT
I am surprised some tests in LayoutTests/streams do not fail.
Were they disabled? Are we sure WritableStream is disabled?
I'll update my build sometimes today and will have a look at it at that time.
Comment 6 youenn fablet 2017-04-13 09:28:01 PDT
WritableStream does not seem disabled.
Probably related to WTF/wtf/Platform.h which defines ENABLE_WRITABLE_STREAM_API to 1 if not already defined.
The same applies to ReadableStream and ReadableByteStream.
Comment 7 youenn fablet 2017-04-13 21:53:04 PDT
To fix things, we could probably:
- replace WRITABLE_STREAM_API, READABLE_STREAM_API and READABLE_BYTE_STREAM_API by STREAMS_API, enabled by default.
- Add a writable_stream and readable_byte_stream runtime flags, enabled only for rwt.
Comment 8 youenn fablet 2017-04-14 22:17:29 PDT
Reopening to attach new patch.
Comment 9 youenn fablet 2017-04-14 22:17:32 PDT
Created attachment 307184 [details]
Patch
Comment 10 youenn fablet 2017-04-14 22:21:30 PDT
Created attachment 307186 [details]
Patch
Comment 11 youenn fablet 2017-04-14 22:26:20 PDT
Created attachment 307188 [details]
Patch
Comment 12 youenn fablet 2017-04-14 22:54:12 PDT
Created attachment 307189 [details]
Patch
Comment 13 youenn fablet 2017-04-15 12:22:16 PDT
Created attachment 307201 [details]
Patch
Comment 14 youenn fablet 2017-04-15 13:11:21 PDT
I verified that WritableStream is undefined and ReadableStream{{type: "bytes"}) is throwing.
Comment 15 youenn fablet 2017-04-15 13:11:35 PDT
(In reply to youenn fablet from comment #14)
> I verified that WritableStream is undefined and ReadableStream{{type:
> "bytes"}) is throwing.

except in rwt/drt
Comment 16 youenn fablet 2017-04-15 16:10:15 PDT
Created attachment 307204 [details]
Win tentative build fix
Comment 17 Sam Weinig 2017-04-15 16:54:48 PDT
What's the value in having the ENABLE_STREAMS_API at all? Is anyone planning on not shipping with it?
Comment 18 Build Bot 2017-04-15 18:54:48 PDT
Comment on attachment 307204 [details]
Win tentative build fix

Attachment 307204 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/3542606

New failing tests:
compositing/absolute-inside-out-of-view-fixed.html
Comment 19 Build Bot 2017-04-15 18:54:50 PDT
Created attachment 307213 [details]
Archive of layout-test-results from ews122 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews122  Port: ios-simulator-wk2  Platform: Mac OS X 10.11.6
Comment 20 youenn fablet 2017-04-15 22:29:23 PDT
(In reply to Sam Weinig from comment #17)
> What's the value in having the ENABLE_STREAMS_API at all? Is anyone planning
> on not shipping with it?

Looking at Source/WTF/FeatureDefines.h, the default ENABLE_READABLE_STREAM_API is 0 for win, although this might be overridden somewhere else in win build system, not sure about that.

I also got feedback from time to time that I broke the build for some non-upstreamed ports that were disabling ENABLE_STREAMS_API.

I agree we should consider removing it.
Comment 21 WebKit Commit Bot 2017-04-17 14:24:41 PDT
Comment on attachment 307204 [details]
Win tentative build fix

Clearing flags on attachment: 307204

Committed r215429: <http://trac.webkit.org/changeset/215429>
Comment 22 WebKit Commit Bot 2017-04-17 14:24:43 PDT
All reviewed patches have been landed.  Closing bug.