Use Task.FromCanceled<TResult>() on NETSTANDARD1_3

[justinvp]

Task.FromCanceled<int>(cancellationToken) can be used on NETSTANDARD1_3 in TaskUtilities.GetCancelledZeroTask().

[dnfclas]

Hi @justinvp, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!

The agreement was validated by .NET Foundation and real humans are currently evaluating your PR.

TTYL, DNFBOT;

[cesarblum]

Not sure what the benefit of this is. I'm inclined not to take it since it only adds code with no clear improvement.

[justinvp]

Benefits:

• Avoids an unnecessary TaskCompletionSource<int> allocation on Net Standard.
• The implementation of GetCancelledZeroTask() now matches GetCancelledTask() (the method above it).
• This comment: // Task<int>.FromCanceled doesn't return Task<int> is misguided and confusing. There is a method that returns a canceled Task<int>: Task.FromCanceled<int>() (whoever wrote the comment wasn't looking at the right method).

[justinvp]

Another benefit:

• FrameRequestStream.ValidateState now matches FrameResponseStream.ValidateState, passing along the cancellation token to the canceled task when the state is Open.

[justinvp]

Unrelated to this change, but I noticed that FrameResponseStream.ValidateState only returns a canceled task for the Aborted case when cancellationToken.IsCancellationRequested is true, whereas here it is always returning a canceled task for Aborted regardless of cancellationToken.IsCancellationRequested.

FrameResponseStream.ValidateState (src):

                case FrameStreamState.Aborted:
                    if (cancellationToken.IsCancellationRequested)
                    {
                        // Aborted state only throws on write if cancellationToken requests it
                        return TaskUtilities.GetCancelledTask(cancellationToken);
                    }
                    break;

Is this by design, or should FrameRequestStream be modified to behave the same as FrameResponseStream?

[benaadams]

Is by design #567 (comment) and the reading and writing are different.

[mikeharder]

@CesarBS, @halter73: I do think we should merge this for the reasons listed above by @justinvp.

[mikeharder]

I would add an overload GetCancelledZeroTask() which calls GetCancelledZeroTask(CancellationToken.None). This is cleaner than requiring the caller to pass default(CancellationToken) or CancellationToken.None.

[justinvp]

How about making it an optional parameter with a default value of default(CancellationToken)?

[mikeharder]

Sure, either would be fine.

[cesarblum]

@justinvp @mikeharder Yeah, the point about avoiding an allocation made me change my mind 😄 It's sad though that we can't have this on all frameworks.

[justinvp]

Sorry for not making the benefits more clear from the start. I'll update the PR, addressing @mikeharder's feedback, later today.

[justinvp]

Feedback addressed. Thanks for the review.

[cesarblum]