Fix coverage collection crash (#21015)

* Fix coverage collection crash

Based on Jason's patch in https://github.com/flutter/flutter/pull/19546/

This is more or less the same but I tried to avoid using `dynamic`.

* Improve argument and variable names in flutter_platform

* Don't bother with reduce, since the order is guaranteed.

packages/flutter_tools/lib/src/test/coverage_collector.dart

@@ -45,35 +45,29 @@ class CoverageCollector extends TestWatcher {
     assert(observatoryUri != null);
 
     final int pid = process.pid;
-    int exitCode;
-    // Synchronization is enforced by the API contract. Error handling
-    // synchronization is done in the code below where `exitCode` is checked.
-    // Callback cannot throw.
-    process.exitCode.then<Null>((int code) { // ignore: unawaited_futures
-      exitCode = code;
-    });
-    if (exitCode != null)
-      throw new Exception('Failed to collect coverage, process terminated before coverage could be collected.');
-
     printTrace('pid $pid: collecting coverage data from $observatoryUri...');
-    final Map<String, dynamic> data = await coverage
-        .collect(observatoryUri, false, false)
-        .timeout(
-          const Duration(minutes: 2),
-          onTimeout: () {
-            throw new Exception('Timed out while collecting coverage.');
-          },
-        );
-    printTrace(() {
-      final StringBuffer buf = new StringBuffer()
-          ..write('pid $pid ($observatoryUri): ')
-          ..write(exitCode == null
-              ? 'collected coverage data; merging...'
-              : 'process terminated prematurely with exit code $exitCode; aborting');
-      return buf.toString();
-    }());
-    if (exitCode != null)
-      throw new Exception('Failed to collect coverage, process terminated while coverage was being collected.');
+
+    Map<String, dynamic> data;
+    final Future<void> processComplete = process.exitCode
+      .then<void>((int code) {
+        throw new Exception('Failed to collect coverage, process terminated prematurely with exit code $code.');
+      });
+    final Future<void> collectionComplete = coverage.collect(observatoryUri, false, false)
+      .then<void>((Map<String, dynamic> result) {
+        if (result == null)
+          throw new Exception('Failed to collect coverage.');
+        data = result;
+      })
+      .timeout(
+        const Duration(minutes: 10),
+        onTimeout: () {
+          throw new Exception('Timed out while collecting coverage.');
+        },
+      );
+    await Future.any<void>(<Future<void>>[ processComplete, collectionComplete ]);
+    assert(data != null);
+
+    printTrace('pid $pid ($observatoryUri): collected coverage data; merging...');
     _addHitmap(coverage.createHitmap(data['coverage']));
     printTrace('pid $pid ($observatoryUri): done merging coverage data into global coverage map.');
   }

packages/flutter_tools/lib/src/test/flutter_platform.dart

@@ -367,7 +367,7 @@ class _FlutterPlatform extends PlatformPlugin {
     _testCount += 1;
     final StreamController<dynamic> localController = new StreamController<dynamic>();
     final StreamController<dynamic> remoteController = new StreamController<dynamic>();
-    final Completer<Null> testCompleteCompleter = new Completer<Null>();
+    final Completer<_AsyncError> testCompleteCompleter = new Completer<_AsyncError>();
     final _FlutterPlatformStreamSinkWrapper<dynamic> remoteSink = new _FlutterPlatformStreamSinkWrapper<dynamic>(
       remoteController.sink,
       testCompleteCompleter.future,
@@ -384,13 +384,14 @@ class _FlutterPlatform extends PlatformPlugin {
     return remoteChannel;
   }
 
-  Future<Null> _startTest(
+  Future<_AsyncError> _startTest(
     String testPath,
     StreamChannel<dynamic> controller,
-    int ourTestCount) async {
+    int ourTestCount,
+  ) async {
     printTrace('test $ourTestCount: starting test $testPath');
 
-    dynamic outOfBandError; // error that we couldn't send to the harness that we need to send via our future
+    _AsyncError outOfBandError; // error that we couldn't send to the harness that we need to send via our future
 
     final List<_Finalizer> finalizers = <_Finalizer>[]; // Will be run in reverse order.
     bool subprocessActive = false;
@@ -440,8 +441,7 @@ class _FlutterPlatform extends PlatformPlugin {
         mainDart = await compiler.compile(mainDart);
 
         if (mainDart == null) {
-          controller.sink.addError(
-              _getErrorMessage('Compilation failed', testPath, shellPath));
+          controller.sink.addError(_getErrorMessage('Compilation failed', testPath, shellPath));
           return null;
         }
       }
@@ -630,7 +630,7 @@ class _FlutterPlatform extends PlatformPlugin {
         controller.sink.addError(error, stack);
       } else {
         printError('unhandled error during test:\n$testPath\n$error\n$stack');
-        outOfBandError ??= error;
+        outOfBandError ??= new _AsyncError(error, stack);
       }
     } finally {
       printTrace('test $ourTestCount: cleaning up...');
@@ -644,7 +644,7 @@ class _FlutterPlatform extends PlatformPlugin {
             controller.sink.addError(error, stack);
           } else {
             printError('unhandled error during finalization of test:\n$testPath\n$error\n$stack');
-            outOfBandError ??= error;
+            outOfBandError ??= new _AsyncError(error, stack);
           }
         }
       }
@@ -659,10 +659,10 @@ class _FlutterPlatform extends PlatformPlugin {
     assert(controllerSinkClosed);
     if (outOfBandError != null) {
       printTrace('test $ourTestCount: finished with out-of-band failure');
-      throw outOfBandError;
+    } else {
+      printTrace('test $ourTestCount: finished');
     }
-    printTrace('test $ourTestCount: finished');
-    return null;
+    return outOfBandError;
   }
 
   String _createListenerDart(List<_Finalizer> finalizers, int ourTestCount,
@@ -902,10 +902,24 @@ class _FlutterPlatform extends PlatformPlugin {
   }
 }
 
+// The [_shellProcessClosed] future can't have errors thrown on it because it
+// crosses zones (it's fed in a zone created by the test package, but listened
+// to by a parent zone, the same zone that calls [close] below).
+//
+// This is because Dart won't let errors that were fed into a Future in one zone
+// propagate to listeners in another zone. (Specifically, the zone in which the
+// future was completed with the error, and the zone in which the listener was
+// registered, are what matters.)
+//
+// Because of this, the [_shellProcessClosed] future takes an [_AsyncError]
+// object as a result. If it's null, it's as if it had completed correctly; if
+// it's non-null, it contains the error and stack trace of the actual error, as
+// if it had completed with that error.
 class _FlutterPlatformStreamSinkWrapper<S> implements StreamSink<S> {
   _FlutterPlatformStreamSinkWrapper(this._parent, this._shellProcessClosed);
+
   final StreamSink<S> _parent;
-  final Future<void> _shellProcessClosed;
+  final Future<_AsyncError> _shellProcessClosed;
 
   @override
   Future<void> get done => _done.future;
@@ -913,12 +927,19 @@ class _FlutterPlatformStreamSinkWrapper<S> implements StreamSink<S> {
 
   @override
   Future<dynamic> close() {
-   Future.wait<dynamic>(<Future<dynamic>>[
+    Future.wait<dynamic>(<Future<dynamic>>[
       _parent.close(),
       _shellProcessClosed,
     ]).then<void>(
-      (List<dynamic> value) {
-        _done.complete();
+      (List<dynamic> futureResults) {
+        assert(futureResults.length == 2);
+        assert(futureResults.first == null);
+        if (futureResults.last is _AsyncError) {
+          _done.completeError(futureResults.last.error, futureResults.last.stack);
+        } else {
+          assert(futureResults.last == null);
+          _done.complete();
+        }
       },
       onError: _done.completeError,
     );
@@ -932,3 +953,10 @@ class _FlutterPlatformStreamSinkWrapper<S> implements StreamSink<S> {
   @override
   Future<dynamic> addStream(Stream<S> stream) => _parent.addStream(stream);
 }
+
+@immutable
+class _AsyncError {
+  const _AsyncError(this.error, this.stack);
+  final dynamic error;
+  final StackTrace stack;
+}
\ No newline at end of file