Bug 170749

Summary: Disable outdated WritableStream API
Product: WebKit Reporter: Dean Jackson <dino>
Component: New BugsAssignee: Dean Jackson <dino>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, benjamin, buildbot, cdumez, cmarcelo, commit-queue, dbates, esprehn+autocc, keith_miller, kondapallykalyan, mark.lam, msaboff, saam, sam, thorton, youennf
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Win tentative build fix
none
Archive of layout-test-results from ews122 for ios-simulator-wk2 none

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.