A blinking cursor should push only one frame (#3445) (#3506)

Prior to this patch, we were pushing two frames each time the cursor blinked.
In turning the cursor on or off, the markNeedsPaint call was triggering another
frame to be scheduled because we cleared a bit in the scheduler at the
beginning of the frame instead of at the end of the frame.

To implement scheduling correctly, we actually need two bits: one for
ensureVisualUpdate, which just promises to get to the end of the pipeline soon,
and scheduleFrame, which promises to get to the beginning of the pipeline soon.

(Reland)

packages/flutter/lib/src/scheduler/binding.dart

@@ -86,7 +86,7 @@ abstract class SchedulerBinding extends BindingBase {
   void initServiceExtensions() {
     super.initServiceExtensions();
     registerNumericServiceExtension(
-      name: 'timeDilation', 
+      name: 'timeDilation',
       getter: () => timeDilation,
       setter: (double value) {
         timeDilation = value;
@@ -94,9 +94,8 @@ abstract class SchedulerBinding extends BindingBase {
     );
   }
 
-
   /// The strategy to use when deciding whether to run a task or not.
-  /// 
+  ///
   /// Defaults to [defaultSchedulingStrategy].
   SchedulingStrategy schedulingStrategy = defaultSchedulingStrategy;
 
@@ -119,7 +118,6 @@ abstract class SchedulerBinding extends BindingBase {
       _ensureEventLoopCallback();
   }
 
-
   // Whether this scheduler already requested to be called from the event loop.
   bool _hasRequestedAnEventLoopCallback = false;
 
@@ -155,11 +153,10 @@ abstract class SchedulerBinding extends BindingBase {
     } else {
       // TODO(floitsch): we shouldn't need to request a frame. Just schedule
       // an event-loop callback.
-      ensureVisualUpdate();
+      scheduleFrame();
     }
   }
 
-
   int _nextFrameCallbackId = 0; // positive
   Map<int, _FrameCallbackEntry> _transientCallbacks = <int, _FrameCallbackEntry>{};
   final Set<int> _removedIds = new HashSet<int>();
@@ -187,7 +184,7 @@ abstract class SchedulerBinding extends BindingBase {
   /// Callbacks registered with this method can be canceled using
   /// [cancelFrameCallbackWithId].
   int scheduleFrameCallback(FrameCallback callback, { bool rescheduling: false }) {
-    ensureVisualUpdate();
+    scheduleFrame();
     return addFrameCallback(callback, rescheduling: rescheduling);
   }
 
@@ -270,7 +267,6 @@ abstract class SchedulerBinding extends BindingBase {
     return true;
   }
 
-
   final List<FrameCallback> _persistentCallbacks = new List<FrameCallback>();
 
   /// Adds a persistent frame callback.
@@ -286,7 +282,6 @@ abstract class SchedulerBinding extends BindingBase {
     _persistentCallbacks.add(callback);
   }
 
-
   final List<FrameCallback> _postFrameCallbacks = new List<FrameCallback>();
 
   /// Schedule a callback for the end of this frame.
@@ -306,10 +301,11 @@ abstract class SchedulerBinding extends BindingBase {
     _postFrameCallbacks.add(callback);
   }
 
+  // Whether this scheduler as requested that handleBeginFrame be called soon.
+  bool _hasScheduledFrame = false;
 
-  // Whether this scheduler already requested to be called at the beginning of
-  // the next frame.
-  bool _hasRequestedABeginFrameCallback = false;
+  // Whether this scheduler is currently producing a frame in handleBeginFrame.
+  bool _isProducingFrame = false;
 
   /// If necessary, schedules a new frame by calling
   /// [ui.window.scheduleFrame].
@@ -319,19 +315,17 @@ abstract class SchedulerBinding extends BindingBase {
   /// device's screen is turned off it will typically be delayed until
   /// the screen is on and the application is visible.)
   void ensureVisualUpdate() {
-    if (_hasRequestedABeginFrameCallback)
+    if (_isProducingFrame)
       return;
-    ui.window.scheduleFrame();
-    _hasRequestedABeginFrameCallback = true;
+    scheduleFrame();
   }
 
-  /// Whether the scheduler is currently handling a "begin frame"
-  /// callback.
-  ///
-  /// True while [handleBeginFrame] is running in checked mode. False
-  /// otherwise.
-  static bool get debugInFrame => _debugInFrame;
-  static bool _debugInFrame = false;
+  void scheduleFrame() {
+    if (_hasScheduledFrame)
+      return;
+    ui.window.scheduleFrame();
+    _hasScheduledFrame = true;
+  }
 
   /// Called by the engine to produce a new frame.
   ///
@@ -342,23 +336,24 @@ abstract class SchedulerBinding extends BindingBase {
   /// callbacks registered by [addPostFrameCallback].
   void handleBeginFrame(Duration rawTimeStamp) {
     Timeline.startSync('Frame');
-    assert(!_debugInFrame);
-    assert(() { _debugInFrame = true; return true; });
+    assert(!_isProducingFrame);
+    _isProducingFrame = true;
+    _hasScheduledFrame = false;
     Duration timeStamp = new Duration(
         microseconds: (rawTimeStamp.inMicroseconds / timeDilation).round());
-    _hasRequestedABeginFrameCallback = false;
     _invokeTransientFrameCallbacks(timeStamp);
 
     for (FrameCallback callback in _persistentCallbacks)
       _invokeFrameCallback(callback, timeStamp);
 
+    _isProducingFrame = false;
+
     List<FrameCallback> localPostFrameCallbacks =
         new List<FrameCallback>.from(_postFrameCallbacks);
     _postFrameCallbacks.clear();
     for (FrameCallback callback in localPostFrameCallbacks)
       _invokeFrameCallback(callback, timeStamp);
 
-    assert(() { _debugInFrame = false; return true; });
     Timeline.finishSync();
 
     // All frame-related callbacks have been executed. Run lower-priority tasks.
@@ -367,7 +362,7 @@ abstract class SchedulerBinding extends BindingBase {
 
   void _invokeTransientFrameCallbacks(Duration timeStamp) {
     Timeline.startSync('Animate');
-    assert(_debugInFrame);
+    assert(_isProducingFrame);
     Map<int, _FrameCallbackEntry> callbacks = _transientCallbacks;
     _transientCallbacks = new Map<int, _FrameCallbackEntry>();
     callbacks.forEach((int id, _FrameCallbackEntry callbackEntry) {

packages/flutter/lib/src/widgets/binding.dart

@@ -54,7 +54,7 @@ abstract class WidgetsBinding extends BindingBase implements GestureBinding, Ren
   void initInstances() {
     super.initInstances();
     _instance = this;
-    buildOwner.onBuildScheduled = ensureVisualUpdate;
+    buildOwner.onBuildScheduled = _handleBuildScheduled;
     ui.window.onLocaleChanged = handleLocaleChanged;
     ui.window.onPopRoute = handlePopRoute;
     ui.window.onAppLifecycleStateChanged = handleAppLifecycleStateChanged;
@@ -183,9 +183,23 @@ abstract class WidgetsBinding extends BindingBase implements GestureBinding, Ren
     _didFirstFrame = true;
   }
 
+  void _handleBuildScheduled() {
+    // If we're in the process of building dirty elements, we're know that any
+    // builds that are scheduled will be run this frame, which means we don't
+    // need to schedule another frame.
+    if (_buildingDirtyElements)
+      return;
+    scheduleFrame();
+  }
+
+  bool _buildingDirtyElements = false;
+
   @override
   void beginFrame() {
+    assert(!_buildingDirtyElements);
+    _buildingDirtyElements = true;
     buildOwner.buildDirtyElements();
+    _buildingDirtyElements = false;
     super.beginFrame();
     buildOwner.finalizeTree();
     // TODO(ianh): Following code should not be included in release mode, only profile and debug mode