[web] Fix race condition in widget benchmarks (#53952)

dev/benchmarks/macrobenchmarks/lib/src/web/recorder.dart

@@ -305,8 +305,7 @@ abstract class SceneBuilderRecorder extends Recorder {
 ///   }
 /// }
 /// ```
-abstract class WidgetRecorder extends Recorder
-    implements RecordingWidgetsBindingListener {
+abstract class WidgetRecorder extends Recorder implements FrameRecorder {
   WidgetRecorder({@required String name}) : super._(name);
 
   /// Creates a widget to be benchmarked.
@@ -318,9 +317,10 @@ abstract class WidgetRecorder extends Recorder
   Widget createWidget();
 
   @override
-  Profile profile;
+  VoidCallback didStop;
 
-  final Completer<Profile> _profileCompleter = Completer<Profile>();
+  Profile profile;
+  Completer<void> _runCompleter;
 
   Stopwatch _drawFrameStopwatch;
 
@@ -340,27 +340,32 @@ abstract class WidgetRecorder extends Recorder
     if (profile.shouldContinue()) {
       window.scheduleFrame();
     } else {
-      _profileCompleter.complete(profile);
+      didStop();
+      _runCompleter.complete();
     }
   }
 
   @override
   void _onError(dynamic error, StackTrace stackTrace) {
-    _profileCompleter.completeError(error, stackTrace);
+    _runCompleter.completeError(error, stackTrace);
   }
 
   @override
-  Future<Profile> run() {
-    profile = Profile(name: name);
+  Future<Profile> run() async {
+    _runCompleter = Completer<void>();
+    final Profile localProfile = profile = Profile(name: name);
     final _RecordingWidgetsBinding binding =
         _RecordingWidgetsBinding.ensureInitialized();
     final Widget widget = createWidget();
     binding._beginRecording(this, widget);
 
-    _profileCompleter.future.whenComplete(() {
+    try {
+      await _runCompleter.future;
+      return localProfile;
+    } finally {
+      _runCompleter = null;
       profile = null;
-    });
-    return _profileCompleter.future;
+    }
   }
 }
 
@@ -371,8 +376,7 @@ abstract class WidgetRecorder extends Recorder
 /// another frame that clears the screen. It repeats this process, measuring the
 /// performance of frames that render the widget and ignoring the frames that
 /// clear the screen.
-abstract class WidgetBuildRecorder extends Recorder
-    implements RecordingWidgetsBindingListener {
+abstract class WidgetBuildRecorder extends Recorder implements FrameRecorder {
   WidgetBuildRecorder({@required String name}) : super._(name);
 
   /// Creates a widget to be benchmarked.
@@ -383,9 +387,10 @@ abstract class WidgetBuildRecorder extends Recorder
   Widget createWidget();
 
   @override
-  Profile profile;
+  VoidCallback didStop;
 
-  final Completer<Profile> _profileCompleter = Completer<Profile>();
+  Profile profile;
+  Completer<void> _runCompleter;
 
   Stopwatch _drawFrameStopwatch;
 
@@ -427,26 +432,31 @@ abstract class WidgetBuildRecorder extends Recorder
       showWidget = !showWidget;
       _hostState._setStateTrampoline();
     } else {
-      _profileCompleter.complete(profile);
+      didStop();
+      _runCompleter.complete();
     }
   }
 
   @override
   void _onError(dynamic error, StackTrace stackTrace) {
-    _profileCompleter.completeError(error, stackTrace);
+    _runCompleter.completeError(error, stackTrace);
   }
 
   @override
-  Future<Profile> run() {
-    profile = Profile(name: name);
+  Future<Profile> run() async {
+    _runCompleter = Completer<void>();
+    final Profile localProfile = profile = Profile(name: name);
     final _RecordingWidgetsBinding binding =
         _RecordingWidgetsBinding.ensureInitialized();
     binding._beginRecording(this, _WidgetBuildRecorderHost(this));
 
-    _profileCompleter.future.whenComplete(() {
+    try {
+      await _runCompleter.future;
+      return localProfile;
+    } finally {
+      _runCompleter = null;
       profile = null;
-    });
-    return _profileCompleter.future;
+    }
   }
 }
 
@@ -698,9 +708,10 @@ String _ratioToPercent(double value) {
 
 /// Implemented by recorders that use [_RecordingWidgetsBinding] to receive
 /// frame life-cycle calls.
-abstract class RecordingWidgetsBindingListener {
-  /// The profile where the benchmark is collecting metrics.
-  Profile profile;
+abstract class FrameRecorder {
+  /// Called by the recorder when it stops recording and doesn't need to collect
+  /// any more data.
+  set didStop(VoidCallback cb);
 
   /// Called just before calling [SchedulerBinding.handleDrawFrame].
   void frameWillDraw();
@@ -739,19 +750,31 @@ class _RecordingWidgetsBinding extends BindingBase
     return WidgetsBinding.instance as _RecordingWidgetsBinding;
   }
 
-  RecordingWidgetsBindingListener _listener;
+  FrameRecorder _recorder;
   bool _hasErrored = false;
 
-  void _beginRecording(
-      RecordingWidgetsBindingListener recorder, Widget widget) {
+  /// To short-circuit all frame lifecycle methods when the benchmark has
+  /// stopped collecting data.
+  bool _benchmarkStopped = false;
+
+  void _beginRecording(FrameRecorder recorder, Widget widget) {
+    if (_recorder != null) {
+      throw Exception(
+        'Cannot call _RecordingWidgetsBinding._beginRecording more than once',
+      );
+    }
     final FlutterExceptionHandler originalOnError = FlutterError.onError;
 
+    recorder.didStop = () {
+      _benchmarkStopped = true;
+    };
+
     // Fail hard and fast on errors. Benchmarks should not have any errors.
     FlutterError.onError = (FlutterErrorDetails details) {
       _haltBenchmarkWithError(details.exception, details.stack);
       originalOnError(details);
     };
-    _listener = recorder;
+    _recorder = recorder;
     runApp(widget);
   }
 
@@ -759,22 +782,17 @@ class _RecordingWidgetsBinding extends BindingBase
     if (_hasErrored) {
       return;
     }
-    _listener._onError(error, stackTrace);
+    _recorder._onError(error, stackTrace);
     _hasErrored = true;
   }
 
-  /// To avoid calling [Profile.shouldContinue] every time [scheduleFrame] is
-  /// called, we cache this value at the beginning of the frame.
-  bool _benchmarkStopped = false;
-
   @override
   void handleBeginFrame(Duration rawTimeStamp) {
-    // Don't keep on truckin' if there's an error.
-    if (_hasErrored) {
+    // Don't keep on truckin' if there's an error or the benchmark has stopped.
+    if (_hasErrored || _benchmarkStopped) {
       return;
     }
     try {
-      _benchmarkStopped = !_listener.profile.shouldContinue();
       super.handleBeginFrame(rawTimeStamp);
     } catch (error, stackTrace) {
       _haltBenchmarkWithError(error, stackTrace);
@@ -784,22 +802,23 @@ class _RecordingWidgetsBinding extends BindingBase
 
   @override
   void scheduleFrame() {
-    // Don't keep on truckin' if there's an error.
-    if (!_benchmarkStopped && !_hasErrored) {
-      super.scheduleFrame();
+    // Don't keep on truckin' if there's an error or the benchmark has stopped.
+    if (_hasErrored || _benchmarkStopped) {
+      return;
     }
+    super.scheduleFrame();
   }
 
   @override
   void handleDrawFrame() {
-    // Don't keep on truckin' if there's an error.
-    if (_hasErrored) {
+    // Don't keep on truckin' if there's an error or the benchmark has stopped.
+    if (_hasErrored || _benchmarkStopped) {
       return;
     }
     try {
-      _listener.frameWillDraw();
+      _recorder.frameWillDraw();
       super.handleDrawFrame();
-      _listener.frameDidDraw();
+      _recorder.frameDidDraw();
     } catch (error, stackTrace) {
       _haltBenchmarkWithError(error, stackTrace);
       rethrow;