Add zone task support for request-anim.

tools/dom/templates/html/impl/impl_Window.darttemplate

Index: tools/dom/templates/html/impl/impl_Window.darttemplate
diff --git a/tools/dom/templates/html/impl/impl_Window.darttemplate b/tools/dom/templates/html/impl/impl_Window.darttemplate
index 03bd4da9f4cd1786063d1542753b852017515229..057a2b8a7305f9cf424b75af377a31113d303f00 100644
--- a/tools/dom/templates/html/impl/impl_Window.darttemplate
+++ b/tools/dom/templates/html/impl/impl_Window.darttemplate
@@ -4,6 +4,77 @@
 
 part of $LIBRARYNAME;
 
+typedef void RemoveFrameRequestMapping(int id);
+
+/**
+ * The task object representing animation-frame requests.
+ *
+ * For historical reasons, [Window.requestAnimationFrame] returns an integer
+ * to users. However, zone-tasks must be unique objects, and an integer thus
+ * doesn't work. The [Window] class thus keeps a mapping from the id to the

[Lasse Reichstein Nielsen, 2016/06/07 12:57:34]

second "thus" in two lines. Consider rewording (maybe just remove "thus" here).

[Lasse Reichstein Nielsen, 2016/06/07 12:57:34]

id -> integer ID

[floitsch, 2016/06/10 15:45:29]

reworded.

[floitsch, 2016/06/10 15:45:29]

Done.

+ * corresponding task object. All zone related operations work on this task
+ * object, whereas users of [Window.requestAnimationFrame] only see the integer
+ * id.

[Lasse Reichstein Nielsen, 2016/06/07 12:57:34]

id -> ID

[floitsch, 2016/06/10 15:45:28]

Done.

+ *
+ * Since this mapping takes space, it must be removed as soon as the animation-

[Lasse Reichstein Nielsen, 2016/06/07 12:57:34]

takes -> takes up

[floitsch, 2016/06/10 15:45:29]

Done.

+ * frame task triggered. The default-implementations do this automatically, but

[Lasse Reichstein Nielsen, 2016/06/07 12:57:34]

default implementations (no '-')

Should probably not be plural - the user won't care that you have more than one
implementation internally, it's just the default.

[Lasse Reichstein Nielsen, 2016/06/07 12:57:34]

triggered -> has triggered (or: is triggered, or: triggers)

[floitsch, 2016/06/10 15:45:29]

Done.

[floitsch, 2016/06/10 15:45:29]

Done.

+ * intercepted implementations of `requestAnimationFrame` must ensure to
+ * call the [AnimationFrameRequestSpecification.removeMapping] function that is
+ * provided in the task specification.
+ */
+abstract class AnimationFrameTask {
+  /** The id that is returned to users. */

[Lasse Reichstein Nielsen, 2016/06/07 12:57:34]

id -> ID

[floitsch, 2016/06/10 15:45:28]

Done.

+  int get id;
+
+  /** The function that is invoked when the user cancels the request. */
+  void cancel(Window window);
+
+  /** The zone in which the task should run (and cancel). */

[Lasse Reichstein Nielsen, 2016/06/07 12:57:34]

(and cancel) -> (or be cancelled)

[floitsch, 2016/06/10 15:45:28]

No cancel anymore. -> removed.

+  Zone get zone;
+}
+
+class _AnimationFrameTask implements AnimationFrameTask {
+  final int id;
+  final Zone zone;
+  final FrameRequestCallback _callback;
+
+  _AnimationFrameTask(this.id, this.zone, this._callback);
+
+  void cancel(Window window) {
+    window._cancelAnimationFrame(this.id);
+  }
+}
+
+/**
+ * The task specification for an animation-frame request.
+ *
+ * The task specification of animation-frame requests include a `removeMapping`
+ * function. This function must be called by the created task before it

[Lasse Reichstein Nielsen, 2016/06/07 12:57:34]

include->includes (or specification -> specifications).

[floitsch, 2016/06/10 15:45:29]

Done.

+ * runs the frame-request callback.

[Lasse Reichstein Nielsen, 2016/06/07 12:57:34]

Sentence is hard to parse (what does "it" refer to, the function or the task?)
Also not clear who must call the function (the task?) and whether it happens
automatically.

[floitsch, 2016/06/10 15:45:29]

Removed it.

+ */
+class AnimationFrameRequestSpecification implements TaskSpecification {
+  final Window window;

[Lasse Reichstein Nielsen, 2016/06/07 12:57:34]

Document. 

[floitsch, 2016/06/10 15:45:29]

Done.

+  /**
+   * The callback that should be executed when the animation-frame is ready.
+   *
+   * Note that the callback hasn't been registered in any zone yet.
+   */
+  final FrameRequestCallback callback;
+
+  /**
+   * The function that should be invoked by the task before running [callback].
+   *
+   * See [AnimationFrame].
+   */
+  final RemoveFrameRequestMapping removeMapping;

[Lasse Reichstein Nielsen, 2016/06/07 12:57:34]

Does it make sense to override the removeMapping function?

If not, could it just be a fixed function that you must remember to call?

[floitsch, 2016/06/10 15:45:29]

Moved it as a static function on AnimationFrameTask.

+
+  AnimationFrameRequestSpecification(
+      this.window, this.callback, this.removeMapping);
+
+  String get name => "dart.html.request-animation-frame";
+  bool get isOneShot => true;
+}
+
 @DocsEditable()
 $if DART2JS
 $(ANNOTATIONS)@Native("Window,DOMWindow")
@@ -29,9 +100,7 @@ $endif
    */
   Future<num> get animationFrame {
     var completer = new Completer<num>.sync();
-    requestAnimationFrame((time) {
-      completer.complete(time);
-    });
+    requestAnimationFrame(completer.complete);
     return completer.future;
   }
 
@@ -97,6 +166,12 @@ $if DART2JS
   }
 
   /**
+   * Maps animation-frame request ids to their task objects.

[Lasse Reichstein Nielsen, 2016/06/07 12:57:34]

ids -> IDs

[floitsch, 2016/06/10 15:45:28]

Done.

+   */
+  static final Map<int, _AnimationFrameTask> _animationFrameTasks =
+      <int, _AnimationFrameTask>{};
+
+  /**
    * Called to draw an animation frame and then request the window to repaint
    * after [callback] has finished (creating the animation).
    *
@@ -115,7 +190,35 @@ $if DART2JS
   @DomName('Window.requestAnimationFrame')
   int requestAnimationFrame(FrameRequestCallback callback) {
     _ensureRequestAnimationFrame();
-    return _requestAnimationFrame(_wrapZone/*<dynamic, num>*/(callback));
+    if (identical(Zone.current, Zone.ROOT)) {
+      return _requestAnimationFrame(callback);
+    }
+    var spec = new AnimationFrameRequestSpecification(
+        this, callback, _removeTaskMapping);
+    var task = Zone.current.createTask/*<AnimationFrameTask>*/(
+        _createAnimationFrameTask, spec);
+    _animationFrameTasks[task.id] = task;
+    return task.id;
+  }
+
+  static _AnimationFrameTask _createAnimationFrameTask(
+      AnimationFrameRequestSpecification spec, Zone zone) {
+    var task;
+    var id = spec.window._requestAnimationFrame((num time) {
+      spec.removeMapping(task.id);
+      zone.runTask(_runAnimationFrame, task, time);
+    });
+    var callback = zone.registerUnaryCallback(spec.callback);
+    task = new _AnimationFrameTask(id, zone, callback);
+    return task;
+  }
+
+  static void _removeTaskMapping(int id) {
+    _animationFrameTasks.remove(id);
+  }
+
+  static void _runAnimationFrame(_AnimationFrameTask task, num time) {
+    task._callback(time);
   }
 
   /**
@@ -128,7 +231,12 @@ $if DART2JS
    */
   void cancelAnimationFrame(int id) {
     _ensureRequestAnimationFrame();
-    _cancelAnimationFrame(id);
+    var task = _animationFrameTasks[id];

[Lasse Reichstein Nielsen, 2016/06/07 12:57:35]

Just do _animationFrameTasks.remove(id). That also returns the current value.

[floitsch, 2016/06/10 15:45:29]

Done.

+    if (task == null) {
+      _cancelAnimationFrame(id);

[Lasse Reichstein Nielsen, 2016/06/07 12:57:34]

and return?
(otherwise you should reach `task.cancel(this)` below with a null `task`. Add
test to catch this?)

[floitsch, 2016/06/10 15:45:29]

Done.

+    }
+    _animationFrameTasks.remove(id);
+    task.cancel(this);
   }
 
   @JSName('requestAnimationFrame')