Clean up CancelableOperation.

[lrhn]

Some clean-ups and simplifications of the code.
Should not change any behavior, but differences in future chaining
can potentially change timing.

[natebosch]

👍

Looks like a big improvement

[natebosch]

Filed an issue for one of the diagnostics that is showing up: dart-lang/sdk#58677

I'm a little unsure whether the choice to change the cancel return from Future<dynamic> to Future<void> is too breaking to land with the rest. We might want to consider holding that part back until we can dig on whether it will break much existing code.

[lrhn]

I'll roll back the visible type changes, just to be sure.

[natebosch]

This change may have broken some tests. I'm going to dig a bit and see if I can figure out why.

[natebosch]

Here is a small repro case that shows the difference in behavior. This test passes before this PR and fails after.

  test('canceled operation does not call .then callback', () async {
    var completer = Completer<void>();
    var hasInvokedThen = false;
    var operation = CancelableOperation.fromFuture(completer.future).then((_) {
      hasInvokedThen = true;
    });
    operation.cancel();
    completer.complete();
    await pumpEventQueue();
    expect(hasInvokedThen, false);
  });

[natebosch]

This is what is causing a few tests to fail. Before the onValue would not be called if the returned operation is canceled, but now it only matters whether this operation is canceled.

[lrhn]

Good point. I'll fix that.