Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(1)

Issue 2823563002: Unified error handling for WritableStream (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 months ago by Adam Rice
Modified:
4 months ago
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.
4 months 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 ...
4 months 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 ...
4 months 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: ...
4 months ago (2017-04-18 02:55:45 UTC) #17
tyoshino (SeeGerritForStatus)
lgtm
4 months 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
4 months 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)
4 months 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
4 months ago (2017-04-19 05:24:58 UTC) #38
commit-bot: I haz the power
4 months 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 b40b6558b