Fix behavior of handleDrawFrame() in benchmark mode. (#25049)

dev/benchmarks/microbenchmarks/lib/stocks/build_bench.dart

@@ -3,8 +3,6 @@
 // found in the LICENSE file.
 
 import 'dart:async';
-import 'dart:ui' as ui;
-
 import 'package:flutter_test/flutter_test.dart';
 import 'package:flutter/material.dart';
 import 'package:flutter/rendering.dart';
@@ -46,9 +44,7 @@ Future<void> main() async {
       // frames are missed, etc.
       // We use Timer.run to ensure there's a microtask flush in between
       // the two calls below.
-      Timer.run(() { ui.window.onBeginFrame(Duration(milliseconds: iterations * 16)); });
+      await tester.pumpBenchmark(Duration(milliseconds: iterations * 16));
-      Timer.run(() { ui.window.onDrawFrame(); });
-      await tester.idle(); // wait until the frame has run (also uses Timer.run)
       iterations += 1;
     }
     watch.stop();

dev/benchmarks/microbenchmarks/lib/stocks/layout_bench.dart

@@ -3,7 +3,6 @@
 // found in the LICENSE file.
 
 import 'dart:async';
-import 'dart:ui' as ui;
 
 import 'package:flutter_test/flutter_test.dart';
 import 'package:flutter/material.dart';
@@ -33,27 +32,15 @@ Future<void> main() async {
     await tester.pump(); // Start drawer animation
     await tester.pump(const Duration(seconds: 1)); // Complete drawer animation
 
-    // Disable calls from the engine which would interfere with the benchmark.
-    ui.window.onBeginFrame = null;
-    ui.window.onDrawFrame = null;
-
     final TestViewConfiguration big = TestViewConfiguration(size: const Size(360.0, 640.0));
     final TestViewConfiguration small = TestViewConfiguration(size: const Size(355.0, 635.0));
     final RenderView renderView = WidgetsBinding.instance.renderView;
-    binding.framePolicy = LiveTestWidgetsFlutterBindingFramePolicy.fullyLive;
+    binding.framePolicy = LiveTestWidgetsFlutterBindingFramePolicy.benchmark;
 
     watch.start();
     while (watch.elapsed < kBenchmarkTime) {
       renderView.configuration = (iterations % 2 == 0) ? big : small;
-      // We don't use tester.pump() because we're trying to drive it in an
+      await tester.pumpBenchmark(Duration(milliseconds: iterations * 16));
-      // artificially high load to find out how much CPU each frame takes.
-      // This differs from normal benchmarks which might look at how many
-      // frames are missed, etc.
-      // We use Timer.run to ensure there's a microtask flush in between
-      // the two calls below.
-      Timer.run(() { binding.handleBeginFrame(Duration(milliseconds: iterations * 16)); });
-      Timer.run(() { binding.handleDrawFrame(); });
-      await tester.idle(); // wait until the frame has run (also uses Timer.run)
       iterations += 1;
     }
     watch.stop();

packages/flutter/test/scheduler/benchmarks_test.dart

@@ -0,0 +1,68 @@
+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+import 'dart:async';
+
+import 'package:flutter/foundation.dart';
+import 'package:flutter/widgets.dart';
+import 'package:flutter_test/flutter_test.dart';
+
+class TestBinding extends LiveTestWidgetsFlutterBinding {
+  TestBinding();
+
+  int framesBegun = 0;
+  int framesDrawn = 0;
+
+  bool handleBeginFrameMicrotaskRun;
+
+  @override
+  void handleBeginFrame(Duration rawTimeStamp) {
+    handleBeginFrameMicrotaskRun = false;
+    framesBegun += 1;
+    Future<void>.microtask(() { handleBeginFrameMicrotaskRun = true; });
+    super.handleBeginFrame(rawTimeStamp);
+  }
+
+  @override
+  void handleDrawFrame() {
+    if (!handleBeginFrameMicrotaskRun) {
+      throw "Microtasks scheduled by 'handledBeginFrame' must be run before 'handleDrawFrame'.";
+    }
+    framesDrawn += 1;
+    super.handleDrawFrame();
+  }
+}
+
+Future<void> main() async {
+  final TestBinding binding = TestBinding();
+
+  test('test pumpBenchmark() only runs one frame', () async {
+    await benchmarkWidgets((WidgetTester tester) async {
+      const Key root = Key('root');
+      binding.attachRootWidget(Container(key: root));
+      await tester.pump();
+
+      expect(binding.framesBegun, greaterThan(0));
+      expect(binding.framesDrawn, greaterThan(0));
+
+      final Element appState = tester.element(find.byKey(root));
+      binding.framePolicy = LiveTestWidgetsFlutterBindingFramePolicy.benchmark;
+
+      final int startFramesBegun = binding.framesBegun;
+      final int startFramesDrawn = binding.framesDrawn;
+      expect(startFramesBegun, equals(startFramesDrawn));
+
+      appState.markNeedsBuild();
+
+      await tester.pumpBenchmark(const Duration(milliseconds: 16));
+
+      final int endFramesBegun = binding.framesBegun;
+      final int endFramesDrawn = binding.framesDrawn;
+      expect(endFramesBegun, equals(endFramesDrawn));
+
+      expect(endFramesBegun, equals(startFramesBegun + 1));
+      expect(endFramesDrawn, equals(startFramesDrawn + 1));
+    });
+  });
+}

packages/flutter_test/lib/src/binding.dart

@@ -985,7 +985,8 @@ enum LiveTestWidgetsFlutterBindingFramePolicy {
   /// This is intended to be used by benchmarks (hence the name) that drive the
   /// pipeline directly. It tells the binding to entirely ignore requests for a
   /// frame to be scheduled, while still allowing frames that are pumped
-  /// directly (invoking [Window.onBeginFrame] and [Window.onDrawFrame]) to run.
+  /// directly to run (either by using [WidgetTester.pumpBenchmark] or invoking
+  /// [Window.onBeginFrame] and [Window.onDrawFrame]).
   ///
   /// The [SchedulerBinding.hasScheduledFrame] property will never be true in
   /// this mode. This can cause unexpected effects. For instance,
@@ -1143,8 +1144,7 @@ class LiveTestWidgetsFlutterBinding extends TestWidgetsFlutterBinding {
       _pendingFrame.complete(); // unlocks the test API
       _pendingFrame = null;
       _expectingFrame = false;
-    } else {
+    } else if (framePolicy != LiveTestWidgetsFlutterBindingFramePolicy.benchmark) {
-      assert(framePolicy != LiveTestWidgetsFlutterBindingFramePolicy.benchmark);
       ui.window.scheduleFrame();
     }
   }

packages/flutter_test/lib/src/widget_tester.dart

@@ -257,6 +257,34 @@ class WidgetTester extends WidgetController implements HitTestDispatcher, Ticker
     return TestAsyncUtils.guard<void>(() => binding.pump(duration, phase));
   }
 
+  /// Triggers a frame after `duration` amount of time, return as soon as the frame is drawn.
+  ///
+  /// This enables driving an artificially high CPU load by rendering frames in
+  /// a tight loop. It must be used with the frame policy set to
+  /// [LiveTestWidgetsFlutterBindingFramePolicy.benchmark].
+  ///
+  /// Similarly to [pump], this doesn't actually wait for `duration`, just
+  /// advances the clock.
+  Future<void> pumpBenchmark(Duration duration) async {
+    assert(() {
+        final TestWidgetsFlutterBinding widgetsBinding = binding;
+        return widgetsBinding is LiveTestWidgetsFlutterBinding &&
+               widgetsBinding.framePolicy == LiveTestWidgetsFlutterBindingFramePolicy.benchmark;
+    }());
+
+    dynamic caughtException;
+    void handleError(dynamic error, StackTrace stackTrace) => caughtException ??= error;
+
+    Future<void>.microtask(() { binding.handleBeginFrame(duration); }).catchError(handleError);
+    await idle();
+    Future<void>.microtask(() { binding.handleDrawFrame(); }).catchError(handleError);
+    await idle();
+
+    if (caughtException != null) {
+      throw caughtException;
+    }
+  }
+
   /// Repeatedly calls [pump] with the given `duration` until there are no
   /// longer any frames scheduled. This will call [pump] at least once, even if
   /// no frames are scheduled when the function is called, to flush any pending