Chromium Code Reviews
Help | Chromium Project | Sign in
(2)

Issue 2823563002: Unified error handling for WritableStream (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 weeks, 1 day ago by Adam Rice
Modified:
1 week, 3 days ago
Reviewers:
tyoshino, domenic
CC:
blink-reviews, chromium-reviews
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Unified error handling for WritableStream Port the standard changes in https://github.com/whatwg/streams/pull/721. There are a number of behavioural changes related to error handling, which are listed at the above URL. This implementation has no known deviations from the standard. The brings this implementation up to parity with https://github.com/whatwg/streams/commit/e7bf9293d3e0b26f9221dada3723e31707db8c32. This CL also removes failing test expectations. Issue 626703 and 711529 cover lines that were removed from TestExpectations. BUG=711254, 626703, 711529, 684543 Review-Url: https://codereview.chromium.org/2823563002 Cr-Commit-Position: refs/heads/master@{#465498} Committed: https://chromium.googlesource.com/chromium/src/+/6703f4df6ad92a629ea4ac736f6ff26402ef139e

Patch Set 1 #

Patch Set 2 : Rebase #

Patch Set 3 : Remove failing expectations #

Patch Set 4 : properties.js tests now fully pass. Remove expectations. #

Patch Set 5 : SharedWorker tests failing due to issue 712124. Redisable them. #

Total comments: 4

Patch Set 6 : Wrap case statements in blocks #

Patch Set 7 : Rebase #

Patch Set 8 : Re-enable newly working sharedworker tests #

Patch Set 9 : remove local copy of piping/multiple-propagation test as it is fail #

Unified diffs Side-by-side diffs Delta from patch set Stats (+233 lines, -958 lines) Patch
M third_party/WebKit/LayoutTests/TestExpectations View 1 2 3 4 5 6 7 8 4 chunks +0 lines, -13 lines 0 comments Download
D third_party/WebKit/LayoutTests/external/wpt/streams/writable-streams/aborting-expected.txt View 1 2 1 chunk +0 lines, -52 lines 0 comments Download
D third_party/WebKit/LayoutTests/external/wpt/streams/writable-streams/aborting.dedicatedworker-expected.txt View 1 2 1 chunk +0 lines, -52 lines 0 comments Download
D third_party/WebKit/LayoutTests/external/wpt/streams/writable-streams/aborting.serviceworker.https-expected.txt View 1 2 1 chunk +0 lines, -53 lines 0 comments Download
D third_party/WebKit/LayoutTests/external/wpt/streams/writable-streams/aborting.sharedworker-expected.txt View 1 2 3 4 5 6 7 1 chunk +0 lines, -52 lines 0 comments Download
D third_party/WebKit/LayoutTests/external/wpt/streams/writable-streams/close-expected.txt View 1 2 1 chunk +0 lines, -21 lines 0 comments Download
D third_party/WebKit/LayoutTests/external/wpt/streams/writable-streams/close.dedicatedworker-expected.txt View 1 2 1 chunk +0 lines, -21 lines 0 comments Download
D third_party/WebKit/LayoutTests/external/wpt/streams/writable-streams/close.serviceworker.https-expected.txt View 1 2 1 chunk +0 lines, -22 lines 0 comments Download
D third_party/WebKit/LayoutTests/external/wpt/streams/writable-streams/close.sharedworker-expected.txt View 1 2 3 4 5 6 7 1 chunk +0 lines, -21 lines 0 comments Download
D third_party/WebKit/LayoutTests/external/wpt/streams/writable-streams/constructor-expected.txt View 1 2 1 chunk +0 lines, -15 lines 0 comments Download
D third_party/WebKit/LayoutTests/external/wpt/streams/writable-streams/constructor.dedicatedworker-expected.txt View 1 2 1 chunk +0 lines, -15 lines 0 comments Download
D third_party/WebKit/LayoutTests/external/wpt/streams/writable-streams/constructor.serviceworker.https-expected.txt View 1 2 1 chunk +0 lines, -16 lines 0 comments Download
D third_party/WebKit/LayoutTests/external/wpt/streams/writable-streams/constructor.sharedworker-expected.txt View 1 2 3 4 5 6 7 1 chunk +0 lines, -15 lines 0 comments Download
D third_party/WebKit/LayoutTests/external/wpt/streams/writable-streams/error-expected.txt View 1 2 1 chunk +0 lines, -8 lines 0 comments Download
D third_party/WebKit/LayoutTests/external/wpt/streams/writable-streams/error.dedicatedworker-expected.txt View 1 2 1 chunk +0 lines, -8 lines 0 comments Download
D third_party/WebKit/LayoutTests/external/wpt/streams/writable-streams/error.serviceworker.https-expected.txt View 1 2 1 chunk +0 lines, -10 lines 0 comments Download
D third_party/WebKit/LayoutTests/external/wpt/streams/writable-streams/error.sharedworker-expected.txt View 1 2 3 4 5 6 7 1 chunk +0 lines, -8 lines 0 comments Download
D third_party/WebKit/LayoutTests/external/wpt/streams/writable-streams/properties-expected.txt View 1 2 3 1 chunk +0 lines, -47 lines 0 comments Download
D third_party/WebKit/LayoutTests/external/wpt/streams/writable-streams/properties.dedicatedworker-expected.txt View 1 2 3 1 chunk +0 lines, -47 lines 0 comments Download
D third_party/WebKit/LayoutTests/external/wpt/streams/writable-streams/properties.serviceworker.https-expected.txt View 1 2 3 1 chunk +0 lines, -48 lines 0 comments Download
D third_party/WebKit/LayoutTests/external/wpt/streams/writable-streams/properties.sharedworker-expected.txt View 1 2 3 4 5 6 7 1 chunk +0 lines, -47 lines 0 comments Download
D third_party/WebKit/LayoutTests/http/tests/streams/piping/multiple-propagation.js View 1 2 3 4 5 6 7 8 1 chunk +0 lines, -139 lines 0 comments Download
D third_party/WebKit/LayoutTests/http/tests/streams/piping/multiple-propagation.https.html View 1 2 3 4 5 6 7 8 1 chunk +0 lines, -14 lines 0 comments Download
D third_party/WebKit/LayoutTests/http/tests/streams/piping/multiple-propagation.https-expected.txt View 1 2 3 4 5 6 7 8 1 chunk +0 lines, -25 lines 0 comments Download
M third_party/WebKit/Source/core/streams/WritableStream.js View 1 2 3 4 5 24 chunks +233 lines, -189 lines 0 comments Download
Commit queue not available (can’t edit this change).

Messages

Total messages: 41 (32 generated)
Adam Rice
I think this is good to go now. Please review.
1 week, 5 days ago (2017-04-17 09:28:03 UTC) #9
domenic
lgtm!! A little unclear how this will interact with the "remove asserts" CL, but seems ...
1 week, 5 days ago (2017-04-17 22:20:28 UTC) #13
Adam Rice
On 2017/04/17 22:20:28, domenic wrote: > A little unclear how this will interact with the ...
1 week, 4 days ago (2017-04-18 02:55:31 UTC) #16
Adam Rice
https://codereview.chromium.org/2823563002/diff/80001/third_party/WebKit/Source/core/streams/WritableStream.js File third_party/WebKit/Source/core/streams/WritableStream.js (right): https://codereview.chromium.org/2823563002/diff/80001/third_party/WebKit/Source/core/streams/WritableStream.js#newcode48 third_party/WebKit/Source/core/streams/WritableStream.js:48: const ERRORED = 3; On 2017/04/17 22:20:28, domenic wrote: ...
1 week, 4 days ago (2017-04-18 02:55:45 UTC) #17
tyoshino
lgtm
1 week, 4 days ago (2017-04-18 04:32:46 UTC) #18
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2823563002/100001
1 week, 4 days ago (2017-04-18 06:41:08 UTC) #23
commit-bot: I haz the power
Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_ng/builds/424423)
1 week, 4 days ago (2017-04-18 10:08:24 UTC) #25
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2823563002/150001
1 week, 3 days ago (2017-04-19 05:24:58 UTC) #38
commit-bot: I haz the power
1 week, 3 days ago (2017-04-19 05:30:20 UTC) #41
Message was sent while issue was closed.
Committed patchset #9 (id:150001) as
https://chromium.googlesource.com/chromium/src/+/6703f4df6ad92a629ea4ac736f6f...
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld cc6ac46