Rearrange scheduling library (#3388)

To be more consistent with other parts of the platform:

* put the binding in a binding.dart file.

* rearrange some members of the Scheduler class to be more close to
  execution order.

* factor out Priority class into its own file.

* add more dart docs.

packages/flutter/lib/scheduler.dart

@@ -11,5 +11,6 @@
 /// For example, an idle-task is only executed when no animation is running.
 library scheduler;
 
-export 'src/scheduler/scheduler.dart';
+export 'src/scheduler/binding.dart';
+export 'src/scheduler/priority.dart';
 export 'src/scheduler/ticker.dart';

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

@@ -11,6 +11,8 @@ import 'dart:ui' show VoidCallback;
 import 'package:collection/collection.dart';
 import 'package:flutter/foundation.dart';
 
+import 'priority.dart';
+
 export 'dart:ui' show VoidCallback;
 
 /// Slows down animations by this factor to help in development.
@@ -24,11 +26,16 @@ double timeDilation = 1.0;
 /// common time base.
 typedef void FrameCallback(Duration timeStamp);
 
+/// Signature for the [Scheduler.schedulingStrategy] callback. Invoked
+/// whenever the system needs to decide whether a task at a given
+/// priority needs to be run.
+///
+/// Return true if a task with the given priority should be executed
+/// at this time, false otherwise.
+///
+/// See also [defaultSchedulingStrategy].
 typedef bool SchedulingStrategy({ int priority, Scheduler scheduler });
 
-/// An entry in the scheduler's priority queue.
-///
-/// Combines the task and its priority.
 class _TaskEntry {
   const _TaskEntry(this.task, this.priority);
   final VoidCallback task;
@@ -52,52 +59,17 @@ class _FrameCallbackEntry {
   StackTrace stack;
 }
 
-class Priority {
-  static const Priority idle = const Priority._(0);
-  static const Priority animation = const Priority._(100000);
-  static const Priority touch = const Priority._(200000);
-
-  /// Relative priorities are clamped by this offset.
-  ///
-  /// It is still possible to have priorities that are offset by more than this
-  /// amount by repeatedly taking relative offsets, but that's generally
-  /// discouraged.
-  static const int kMaxOffset = 10000;
-
-  const Priority._(this._value);
-
-  int get value => _value;
-  final int _value;
-
-  /// Returns a priority relative to this priority.
-  ///
-  /// A positive [offset] indicates a higher priority.
-  ///
-  /// The parameter [offset] is clamped to +/-[kMaxOffset].
-  Priority operator +(int offset) {
-    if (offset.abs() > kMaxOffset) {
-      // Clamp the input offset.
-      offset = kMaxOffset * offset.sign;
-    }
-    return new Priority._(_value + offset);
-  }
-
-  /// Returns a priority relative to this priority.
-  ///
-  /// A positive offset indicates a lower priority.
-  ///
-  /// The parameter [offset] is clamped to +/-[kMaxOffset].
-  Priority operator -(int offset) => this + (-offset);
-}
-
-/// Scheduler running tasks with specific priorities.
+/// Scheduler for running the following:
 ///
-/// Combines the task's priority with remaining time in a frame to decide when
-/// the task should be run.
+/// * _Frame callbacks_, triggered by the system's
+///   [ui.window.onBeginFrame] callback, for synchronising the
+///   application's behavior to the system's display. For example, the
+///   rendering layer uses this to drive its rendering pipeline.
 ///
-/// Tasks always run in the idle time after a frame has been committed.
+/// * Non-rendering tasks, to be run between frames. These are given a
+///   priority and are executed in priority order according to a
+///   [schedulingStrategy].
 abstract class Scheduler extends BindingBase {
-  /// Requires clients to use the [scheduler] singleton
 
   @override
   void initInstances() {
@@ -106,42 +78,61 @@ abstract class Scheduler extends BindingBase {
     ui.window.onBeginFrame = handleBeginFrame;
   }
 
-  static Scheduler _instance;
+  /// The current [Scheduler], if one has been created.
   static Scheduler get instance => _instance;
+  static Scheduler _instance;
 
+
+  /// The strategy to use when deciding whether to run a task or not.
+  /// 
+  /// Defaults to [defaultSchedulingStrategy].
   SchedulingStrategy schedulingStrategy = defaultSchedulingStrategy;
 
   static int _taskSorter (_TaskEntry e1, _TaskEntry e2) {
-    // Note that we inverse the priority.
     return -e1.priority.compareTo(e2.priority);
   }
   final PriorityQueue<_TaskEntry> _taskQueue = new HeapPriorityQueue<_TaskEntry>(_taskSorter);
 
-  /// Whether this scheduler already requested to be called from the event loop.
-  bool _hasRequestedAnEventLoopCallback = false;
-
-  /// Whether this scheduler already requested to be called at the beginning of
-  /// the next frame.
-  bool _hasRequestedABeginFrameCallback = false;
-
-  /// Schedules the given [task] with the given [priority].
+  /// Schedules the given `task` with the given `priority`.
+  ///
+  /// Tasks will be executed between frames, in priority order,
+  /// excluding tasks that are skipped by the current
+  /// [schedulingStrategy]. Tasks should be short (as in, up to a
+  /// millisecond), so as to not cause the regular frame callbacks to
+  /// get delayed.
   void scheduleTask(VoidCallback task, Priority priority) {
     bool isFirstTask = _taskQueue.isEmpty;
-    _taskQueue.add(new _TaskEntry(task, priority._value));
+    _taskQueue.add(new _TaskEntry(task, priority.value));
     if (isFirstTask)
       _ensureEventLoopCallback();
   }
 
+
+  // Whether this scheduler already requested to be called from the event loop.
+  bool _hasRequestedAnEventLoopCallback = false;
+
+  // Ensures that the scheduler is awakened by the event loop.
+  void _ensureEventLoopCallback() {
+    if (_hasRequestedAnEventLoopCallback)
+      return;
+    Timer.run(handleEventLoopCallback);
+    _hasRequestedAnEventLoopCallback = true;
+  }
+
   /// Invoked by the system when there is time to run tasks.
   void handleEventLoopCallback() {
     _hasRequestedAnEventLoopCallback = false;
     _runTasks();
   }
 
+  // Called when the system wakes up and at the end of each frame.
   void _runTasks() {
     if (_taskQueue.isEmpty)
       return;
     _TaskEntry entry = _taskQueue.first;
+    // TODO(floitsch): for now we only expose the priority. It might
+    // be interesting to provide more info (like, how long the task
+    // ran the last time, or how long is left in this frame).
     if (schedulingStrategy(priority: entry.priority, scheduler: this)) {
       try {
         (_taskQueue.removeFirst().task)();
@@ -152,27 +143,39 @@ abstract class Scheduler extends BindingBase {
     } else {
       // TODO(floitsch): we shouldn't need to request a frame. Just schedule
       // an event-loop callback.
-      _ensureBeginFrameCallback();
+      ensureVisualUpdate();
     }
   }
 
+
   int _nextFrameCallbackId = 0; // positive
   Map<int, _FrameCallbackEntry> _transientCallbacks = <int, _FrameCallbackEntry>{};
   final Set<int> _removedIds = new HashSet<int>();
 
+  /// The current number of transient frame callbacks scheduled.
+  ///
+  /// This is reset to zero just before all the currently scheduled
+  /// transient callbacks are invoked, at the start of a frame.
+  ///
+  /// This number is primarily exposed so that tests can verify that
+  /// there are no unexpected transient callbacks still registered
+  /// after a test's resources have been gracefully disposed.
   int get transientCallbackCount => _transientCallbacks.length;
 
   /// Schedules the given frame callback.
   ///
-  /// Adds the given callback to the list of frame-callbacks and ensures that a
+  /// Adds the given callback to the list of frame callbacks and ensures that a
   /// frame is scheduled.
   ///
   /// If `rescheduling` is true, the call must be in the context of a
   /// frame callback, and for debugging purposes the stack trace
   /// stored for this callback will be the same stack trace as for the
   /// current callback.
+  ///
+  /// Callbacks registered with this method can be canceled using
+  /// [cancelFrameCallbackWithId].
   int scheduleFrameCallback(FrameCallback callback, { bool rescheduling: false }) {
-    _ensureBeginFrameCallback();
+    ensureVisualUpdate();
     return addFrameCallback(callback, rescheduling: rescheduling);
   }
 
@@ -181,8 +184,8 @@ abstract class Scheduler extends BindingBase {
   /// Frame callbacks are executed at the beginning of a frame (see
   /// [handleBeginFrame]).
   ///
-  /// The registered callbacks are executed in the order in which they have been
-  /// registered.
+  /// These callbacks are executed in the order in which they have
+  /// been added.
   ///
   /// Callbacks registered with this method will not be invoked until
   /// a frame is requested. To register a callback and ensure that a
@@ -192,6 +195,9 @@ abstract class Scheduler extends BindingBase {
   /// frame callback, and for debugging purposes the stack trace
   /// stored for this callback will be the same stack trace as for the
   /// current callback.
+  ///
+  /// Callbacks registered with this method can be canceled using
+  /// [cancelFrameCallbackWithId].
   int addFrameCallback(FrameCallback callback, { bool rescheduling: false }) {
     _nextFrameCallbackId += 1;
     _transientCallbacks[_nextFrameCallbackId] = new _FrameCallbackEntry(callback, rescheduling: rescheduling);
@@ -201,69 +207,127 @@ abstract class Scheduler extends BindingBase {
   /// Cancels the callback of the given [id].
   ///
   /// Removes the given callback from the list of frame callbacks. If a frame
-  /// has been requested does *not* cancel that request.
+  /// has been requested, this does not also cancel that request.
+  ///
+  /// Frame callbacks are registered using [scheduleFrameCallback] or
+  /// [addFrameCallback].
   void cancelFrameCallbackWithId(int id) {
     assert(id > 0);
     _transientCallbacks.remove(id);
     _removedIds.add(id);
   }
 
+  /// Asserts that there are no registered transient callbacks; if
+  /// there are, prints their locations and throws an exception.
+  ///
+  /// This is expected to be called at the end of tests (the
+  /// flutter_test framework does it automatically in normal cases).
+  ///
+  /// Invoke this method when you expect there to be no transient
+  /// callbacks registered, in an assert statement with a message that
+  /// you want printed when a transient callback is registered:
+  ///
+  /// ```dart
+  /// assert(Scheduler.instance.debugAssertNoTransientCallbacks(
+  ///   'A leak of transient callbacks was detected while doing foo.'
+  /// ));
+  /// ```
+  ///
+  /// Does nothing if asserts are disabled. Always returns true.
+  bool debugAssertNoTransientCallbacks(String reason) {
+    assert(() {
+      if (transientCallbackCount > 0) {
+        FlutterError.reportError(new FlutterErrorDetails(
+          exception: reason,
+          library: 'scheduler library',
+          informationCollector: (StringBuffer information) {
+            information.writeln(
+              'There ${ transientCallbackCount == 1 ? "was one transient callback" : "were $transientCallbackCount transient callbacks" } '
+              'left. The stack traces for when they were registered are as follows:'
+            );
+            for (int id in _transientCallbacks.keys) {
+              _FrameCallbackEntry entry = _transientCallbacks[id];
+              information.writeln('-- callback $id --');
+              information.writeln(entry.stack);
+            }
+          }
+        ));
+      }
+      return true;
+    });
+    return true;
+  }
+
+
   final List<FrameCallback> _persistentCallbacks = new List<FrameCallback>();
 
   /// Adds a persistent frame callback.
   ///
-  /// Persistent callbacks are invoked after transient (non-persistent) frame
-  /// callbacks.
+  /// Persistent callbacks are invoked after transient
+  /// (non-persistent) frame callbacks.
   ///
-  /// Does *not* request a new frame. Conceptually, persistent
-  /// frame-callbacks are thus observers of begin-frame events. Since they are
-  /// executed after the transient frame-callbacks they can drive the rendering
-  /// pipeline.
+  /// Does *not* request a new frame. Conceptually, persistent frame
+  /// callbacks are observers of "begin frame" events. Since they are
+  /// executed after the transient frame callbacks they can drive the
+  /// rendering pipeline.
   void addPersistentFrameCallback(FrameCallback callback) {
     _persistentCallbacks.add(callback);
   }
 
+
   final List<FrameCallback> _postFrameCallbacks = new List<FrameCallback>();
 
   /// Schedule a callback for the end of this frame.
   ///
   /// Does *not* request a new frame.
   ///
-  /// The callback is run just after the persistent frame-callbacks (which is
-  /// when the main rendering pipeline has been flushed). If a frame is
-  /// in progress, but post frame-callbacks haven't been executed yet, then the
-  /// registered callback is still executed during the frame. Otherwise,
-  /// the registered callback is executed during the next frame.
+  /// This callback is run during a frame, just after the persistent
+  /// frame callbacks (which is when the main rendering pipeline has
+  /// been flushed). If a frame is in progress and post-frame
+  /// callbacks haven't been executed yet, then the registered
+  /// callback is still executed during the frame. Otherwise, the
+  /// registered callback is executed during the next frame.
   ///
-  /// The registered callbacks are executed in the order in which they have been
-  /// registered.
+  /// The callbacks are executed in the order in which they have been
+  /// added.
   void addPostFrameCallback(FrameCallback callback) {
     _postFrameCallbacks.add(callback);
   }
 
-  static bool get debugInFrame => _debugInFrame;
-  static bool _debugInFrame = false;
 
-  void _invokeTransientFrameCallbacks(Duration timeStamp) {
-    Timeline.startSync('Animate');
-    assert(_debugInFrame);
-    Map<int, _FrameCallbackEntry> callbacks = _transientCallbacks;
-    _transientCallbacks = new Map<int, _FrameCallbackEntry>();
-    callbacks.forEach((int id, _FrameCallbackEntry callbackEntry) {
-      if (!_removedIds.contains(id))
-        invokeFrameCallback(callbackEntry.callback, timeStamp, callbackEntry.stack);
-    });
-    _removedIds.clear();
-    Timeline.finishSync();
+  // Whether this scheduler already requested to be called at the beginning of
+  // the next frame.
+  bool _hasRequestedABeginFrameCallback = false;
+
+  /// If necessary, schedules a new frame by calling
+  /// [ui.window.scheduleFrame].
+  ///
+  /// After this is called, the engine will (eventually) invoke
+  /// [handleBeginFrame]. (This call might be delayed, e.g. if the
+  /// 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)
+      return;
+    ui.window.scheduleFrame();
+    _hasRequestedABeginFrameCallback = true;
   }
 
+  /// 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;
+
   /// Called by the engine to produce a new frame.
   ///
   /// This function first calls all the callbacks registered by
-  /// [scheduleFrameCallback]/[addFrameCallback], then calls all the callbacks
-  /// registered by [addPersistentFrameCallback], which typically drive the
-  /// rendering pipeline, and finally calls the callbacks registered by
-  /// [addPostFrameCallback].
+  /// [scheduleFrameCallback]/[addFrameCallback], then calls all the
+  /// callbacks registered by [addPersistentFrameCallback], which
+  /// typically drive the rendering pipeline, and finally calls the
+  /// callbacks registered by [addPostFrameCallback].
   void handleBeginFrame(Duration rawTimeStamp) {
     Timeline.startSync('Begin frame');
     assert(!_debugInFrame);
@@ -274,13 +338,13 @@ abstract class Scheduler extends BindingBase {
     _invokeTransientFrameCallbacks(timeStamp);
 
     for (FrameCallback callback in _persistentCallbacks)
-      invokeFrameCallback(callback, timeStamp);
+      _invokeFrameCallback(callback, timeStamp);
 
     List<FrameCallback> localPostFrameCallbacks =
         new List<FrameCallback>.from(_postFrameCallbacks);
     _postFrameCallbacks.clear();
     for (FrameCallback callback in localPostFrameCallbacks)
-      invokeFrameCallback(callback, timeStamp);
+      _invokeFrameCallback(callback, timeStamp);
 
     assert(() { _debugInFrame = false; return true; });
     Timeline.finishSync();
@@ -289,14 +353,25 @@ abstract class Scheduler extends BindingBase {
     _runTasks();
   }
 
-  /// Invokes the given [callback] with [timestamp] as argument.
-  ///
-  /// Wraps the callback in a try/catch and forwards any error to
-  /// [debugSchedulerExceptionHandler], if set. If not set, then simply prints
-  /// the error.
-  ///
-  /// Must not be called reentrantly from within a frame callback.
-  void invokeFrameCallback(FrameCallback callback, Duration timeStamp, [ StackTrace callbackStack ]) {
+  void _invokeTransientFrameCallbacks(Duration timeStamp) {
+    Timeline.startSync('Animate');
+    assert(_debugInFrame);
+    Map<int, _FrameCallbackEntry> callbacks = _transientCallbacks;
+    _transientCallbacks = new Map<int, _FrameCallbackEntry>();
+    callbacks.forEach((int id, _FrameCallbackEntry callbackEntry) {
+      if (!_removedIds.contains(id))
+        _invokeFrameCallback(callbackEntry.callback, timeStamp, callbackEntry.stack);
+    });
+    _removedIds.clear();
+    Timeline.finishSync();
+  }
+
+  // Invokes the given [callback] with [timestamp] as argument.
+  //
+  // Wraps the callback in a try/catch and forwards any error to
+  // [debugSchedulerExceptionHandler], if set. If not set, then simply prints
+  // the error.
+  void _invokeFrameCallback(FrameCallback callback, Duration timeStamp, [ StackTrace callbackStack ]) {
     assert(callback != null);
     assert(_FrameCallbackEntry.currentCallbackStack == null);
     assert(() { _FrameCallbackEntry.currentCallbackStack = callbackStack; return true; });
@@ -315,75 +390,15 @@ abstract class Scheduler extends BindingBase {
     }
     assert(() { _FrameCallbackEntry.currentCallbackStack = null; return true; });
   }
-
-  /// Asserts that there are no registered transient callbacks; if
-  /// there are, prints their locations and throws an exception.
-  ///
-  /// This is expected to be called at the end of tests (the
-  /// flutter_test framework does it automatically in normal cases).
-  ///
-  /// To invoke this method, call it, when you expect there to be no
-  /// transient callbacks registered, in an assert statement with a
-  /// message that you want printed when a transient callback is
-  /// registered, as follows:
-  ///
-  /// ```dart
-  /// assert(Scheduler.instance.debugAssertNoTransientCallbacks(
-  ///   'A leak of transient callbacks was detected while doing foo.'
-  /// ));
-  /// ```
-  ///
-  /// Does nothing if asserts are disabled. Always returns true.
-  bool debugAssertNoTransientCallbacks(String reason) {
-    assert(() {
-      if (transientCallbackCount > 0) {
-        FlutterError.reportError(new FlutterErrorDetails(
-          exception: reason,
-          library: 'scheduler library',
-          informationCollector: (StringBuffer information) {
-            information.writeln(
-              'There ${ transientCallbackCount == 1 ? "was one transient callback" : "were $transientCallbackCount transient callbacks" } '
-              'left. The stack traces for when they were registered are as follows:'
-            );
-            for (int id in _transientCallbacks.keys) {
-              _FrameCallbackEntry entry = _transientCallbacks[id];
-              information.writeln('-- callback $id --');
-              information.writeln(entry.stack);
-            }
-          }
-        ));
-      }
-      return true;
-    });
-    return true;
-  }
-
-  /// Ensures that the scheduler is woken by the event loop.
-  void _ensureEventLoopCallback() {
-    if (_hasRequestedAnEventLoopCallback)
-      return;
-    Timer.run(handleEventLoopCallback);
-    _hasRequestedAnEventLoopCallback = true;
-  }
-
-  // TODO(floitsch): "ensureVisualUpdate" doesn't really fit into the scheduler.
-  void ensureVisualUpdate() {
-    _ensureBeginFrameCallback();
-  }
-
-  /// Schedules a new frame.
-  void _ensureBeginFrameCallback() {
-    if (_hasRequestedABeginFrameCallback)
-      return;
-    ui.window.scheduleFrame();
-    _hasRequestedABeginFrameCallback = true;
-  }
 }
 
-// TODO(floitsch): for now we only expose the priority. It might be interesting
-// to provide more info (like, how long the task ran the last time).
+/// The default [SchedulingStrategy] for [Scheduler.schedulingStrategy].
+///
+/// If there are any frame callbacks registered, only runs tasks with
+/// a [Priority] of [Priority.animation] or higher. Otherwise, runs
+/// all tasks.
 bool defaultSchedulingStrategy({ int priority, Scheduler scheduler }) {
   if (scheduler.transientCallbackCount > 0)
-    return priority >= Priority.animation._value;
+    return priority >= Priority.animation.value;
   return true;
 }

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

+// Copyright 2016 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.
+
+/// A task priority, as passed to [Scheduler.scheduleTask].
+class Priority {
+  const Priority._(this._value);
+
+  /// The integer that describes this Priority value.
+  int get value => _value;
+  final int _value;
+
+  /// A task to run after all other tasks, when no animations are running.
+  static const Priority idle = const Priority._(0);
+
+  /// A task to run even when animations are running.
+  static const Priority animation = const Priority._(100000);
+
+  /// A task to run even when the user is interacting with the device.
+  static const Priority touch = const Priority._(200000);
+
+  /// Maximum offset by which to clamp relative priorities.
+  ///
+  /// It is still possible to have priorities that are offset by more
+  /// than this amount by repeatedly taking relative offsets, but that
+  /// is generally discouraged.
+  static const int kMaxOffset = 10000;
+
+  /// Returns a priority relative to this priority.
+  ///
+  /// A positive [offset] indicates a higher priority.
+  ///
+  /// The parameter [offset] is clamped to ±[kMaxOffset].
+  Priority operator +(int offset) {
+    if (offset.abs() > kMaxOffset) {
+      // Clamp the input offset.
+      offset = kMaxOffset * offset.sign;
+    }
+    return new Priority._(_value + offset);
+  }
+
+  /// Returns a priority relative to this priority.
+  ///
+  /// A positive offset indicates a lower priority.
+  ///
+  /// The parameter [offset] is clamped to ±[kMaxOffset].
+  Priority operator -(int offset) => this + (-offset);
+}

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

@@ -4,14 +4,22 @@
 
 import 'dart:async';
 
-import 'scheduler.dart';
+import 'binding.dart';
 
 /// Signature for the [onTick] constructor argument of the [Ticker] class.
+///
+/// The argument is the time that the object had spent enabled so far
+/// at the time of the callback being invoked.
 typedef void TickerCallback(Duration elapsed);
 
 /// Calls its callback once per animation frame.
+///
+/// When created, a ticker is initially disabled. Call [start] to
+/// enable the ticker.
+///
+/// See also [Scheduler.scheduleFrameCallback].
 class Ticker {
-  /// Constructs a ticker that will call [onTick] once per frame while running.
+  /// Creates a ticker that will call [onTick] once per frame while running.
   Ticker(TickerCallback onTick) : _onTick = onTick;
 
   final TickerCallback _onTick;
@@ -20,7 +28,11 @@ class Ticker {
   int _animationId;
   Duration _startTime;
 
-  /// Starts calling onTick once per animation frame.
+  /// Whether this ticker has scheduled a call to invoke its callback
+  /// on the next frame.
+  bool get isTicking => _completer != null;
+
+  /// Starts calling the ticker's callback once per animation frame.
   ///
   /// The returned future resolves once the ticker stops ticking.
   Future<Null> start() {
@@ -31,7 +43,7 @@ class Ticker {
     return _completer.future;
   }
 
-  /// Stops calling onTick.
+  /// Stops calling the ticker's callback.
   ///
   /// Causes the future returned by [start] to resolve.
   void stop() {
@@ -54,9 +66,6 @@ class Ticker {
     localCompleter.complete();
   }
 
-  /// Whether this ticker has scheduled a call to onTick
-  bool get isTicking => _completer != null;
-
   void _tick(Duration timeStamp) {
     assert(isTicking);
     assert(_animationId != null);