Fix a bug in StreamQueue.cancel().

Fix a bug in StreamQueue.cancel().

If cancel was called on the queue before anything else, it wouldn't call
StreamSubscription.cancel.

R=lrn@google.com

Committed: https://github.com/dart-lang/async/commit/5f908468abc0ecd10224d47033c3cb95a5c33b44

[nweiz, 2015-07-13 21:54:34]

nweiz@google.com changed reviewers:
+ lrn@google.com

[Lasse Reichstein Nielsen, 2015-07-14 07:28:10]

https://codereview.chromium.org/1239543004/diff/1/lib/src/stream_queue.dart
File lib/src/stream_queue.dart (right):

https://codereview.chromium.org/1239543004/diff/1/lib/src/stream_queue.dart#n...
lib/src/stream_queue.dart:245: return request.future;
That doesn't look right either - you subscribe to the stream, but don't set up
the callbacks. For a non-immediate cancel, that would mean it won't get the
onDone event, so the _CancelRequest never completes.

On the other hand, if you haven't subscribed yet, then it means that the cancel
is the first request, so the _eventQueue is empty.
In that case, you can just cancel it immediately, no need to enqueue anything.
So maybe change the enqueue-event case condition to:
 if (_isDone) return new Future.value();
 if (!immediate && _eventQueue.isNotEmpty) {
    assert(_subscription != null);
    ...
 }
 if (_subscription == null) _subscription = _sourceStream.listen(null);
 ...

Alternatively, andprobably better, fix this at the root cause: That a
_CancelRequest doesn't listen before it cancels if there is no subscription. See
below.

https://codereview.chromium.org/1239543004/diff/1/lib/src/stream_queue.dart#n...
lib/src/stream_queue.dart:535: _completer.complete();
This skips creating the subscription if it's null.
That's fine in the _isDone case, but not what you want in the not-subscribed-yet
case.
So change the condition here to `_streamQueue._isDone`, and add
`_streamQueue._ensureListening();` at the start of the else branch - or maybe a
litte more conservative:
 if (_streamQueue._subscription == null) _streamQueue._ensureListening();

[nweiz, 2015-07-14 20:13:49]

Code review changes

[nweiz, 2015-07-14 20:14:08]

I think my version works as written, but you're right that yours is cleaner.

https://codereview.chromium.org/1239543004/diff/1/lib/src/stream_queue.dart
File lib/src/stream_queue.dart (right):

https://codereview.chromium.org/1239543004/diff/1/lib/src/stream_queue.dart#n...
lib/src/stream_queue.dart:535: _completer.complete();
On 2015/07/14 07:28:10, Lasse Reichstein Nielsen wrote:
> This skips creating the subscription if it's null.
> That's fine in the _isDone case, but not what you want in the
not-subscribed-yet
> case.
> So change the condition here to `_streamQueue._isDone`, and add
> `_streamQueue._ensureListening();` at the start of the else branch - or maybe
a
> litte more conservative:
>  if (_streamQueue._subscription == null) _streamQueue._ensureListening();

Done.

[Lasse Reichstein Nielsen, 2015-07-15 08:56:43]

lgtm

[nweiz, 2015-07-15 20:36:06]

Committed patchset #2 (id:20001) manually as
5f908468abc0ecd10224d47033c3cb95a5c33b44 (presubmit successful).