Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(97)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 1234163002: Fix a WebSocket crash. (Closed)

Created:
5 years, 5 months ago by nweiz
Modified:
5 years, 5 months ago
Reviewers:
Anders Johnsen
CC:
reviews_dartlang.org
Base URL:
git@github.com:dart-lang/sdk.git@master
Target Ref:
refs/heads/master
Visibility:
Public.

Description

Fix a WebSocket crash. Previously, a WebSocket would crash if it was closed after its StreamSubscription was canceled. Now, it tracks whether the subscription was canceled by canceling and nulling out its own internal subscription. Fixes #23845 R=ajohnsen@google.com Committed: https://github.com/dart-lang/sdk/commit/bdd5803006ef0ff58bc19b100d3e6300402fe07d

Patch Set 1 #

Patch Set 2 : changelog entry #

Total comments: 3

Patch Set 3 : Code review changes #

Unified diffs Side-by-side diffs Delta from patch set Stats (+45 lines, -2 lines) Patch
M CHANGELOG.md View 1 1 chunk +7 lines, -0 lines 0 comments Download
M sdk/lib/io/websocket_impl.dart View 3 chunks +6 lines, -2 lines 0 comments Download
M tests/standalone/io/web_socket_test.dart View 1 2 2 chunks +32 lines, -0 lines 0 comments Download

Messages

Total messages: 9 (2 generated)
nweiz
5 years, 5 months ago (2015-07-15 01:24:29 UTC) #2
nweiz
Re-sending to Anders because Søren is OOO.
5 years, 5 months ago (2015-07-15 01:48:28 UTC) #4
Anders Johnsen
LGTM, thank you! https://codereview.chromium.org/1234163002/diff/20001/tests/standalone/io/web_socket_test.dart File tests/standalone/io/web_socket_test.dart (right): https://codereview.chromium.org/1234163002/diff/20001/tests/standalone/io/web_socket_test.dart#newcode193 tests/standalone/io/web_socket_test.dart:193: webSocket.listen(null).cancel(); Can you add a test ...
5 years, 5 months ago (2015-07-15 06:32:48 UTC) #5
nweiz
Code review changes
5 years, 5 months ago (2015-07-15 20:29:29 UTC) #6
nweiz
https://codereview.chromium.org/1234163002/diff/20001/tests/standalone/io/web_socket_test.dart File tests/standalone/io/web_socket_test.dart (right): https://codereview.chromium.org/1234163002/diff/20001/tests/standalone/io/web_socket_test.dart#newcode193 tests/standalone/io/web_socket_test.dart:193: webSocket.listen(null).cancel(); On 2015/07/15 06:32:48, Anders Johnsen wrote: > Can ...
5 years, 5 months ago (2015-07-15 20:29:34 UTC) #7
nweiz
Committed patchset #3 (id:40001) manually as bdd5803006ef0ff58bc19b100d3e6300402fe07d (presubmit successful).
5 years, 5 months ago (2015-07-15 20:30:10 UTC) #8
Anders Johnsen
5 years, 5 months ago (2015-07-20 11:59:21 UTC) #9
Message was sent while issue was closed. 
https://codereview.chromium.org/1234163002/diff/20001/tests/standalone/io/web...
File tests/standalone/io/web_socket_test.dart (right):

https://codereview.chromium.org/1234163002/diff/20001/tests/standalone/io/web...
tests/standalone/io/web_socket_test.dart:193: webSocket.listen(null).cancel();
On 2015/07/15 20:29:34, nweiz wrote:
> On 2015/07/15 06:32:48, Anders Johnsen wrote:
> > Can you add a test where you close before cancel?
> 
> Done.

Cool, thank you!

Powered by Google App Engine
This is Rietveld 408576698