Refactored vsync for animations and non-animations uses

extensions/amp-font/0.1/fontloader.js

@@ -151,7 +151,7 @@ export class FontLoader {
       // Create DOM elements
       this.createElements_();
       // Measure until timeout (or font load).
-      const vsyncTask = {
+      const vsyncTask = vsync.createTask({
         measure: () => {
           if (this.fontLoadResolved_) {
             resolve();
@@ -160,13 +160,11 @@ export class FontLoader {
           } else if (this.compareMeasurements_()) {
             resolve();
           } else {
-            vsync.run(vsyncTask);
+            vsyncTask();
           }
-        },
-        mutate: () => {}
-      };
-      // TODO(dvoytenko): Fix https://github.com/ampproject/amphtml/issues/839.
-      vsync.run(vsyncTask);
+        }
+      });
+      vsyncTask();
     });
   }
 

src/animation.js

@@ -132,6 +132,9 @@ class AnimationPlayer {
    */
   constructor(vsync, segments, defaultCurve, duration) {
 
+    /** @private @const {!Vsync} */
+    this.vsync_ = vsync;
+
     /** @private @const {!Array<!SegmentRuntime_>} */
     this.segments_ = [];
     for (let i = 0; i < segments.length; i++) {
@@ -177,12 +180,9 @@ class AnimationPlayer {
     });
 
     /** @const */
-    this.task_ = vsync.createTask({
+    this.task_ = this.vsync_.createAnimTask({
       mutate: this.stepMutate_.bind(this)
     });
-
-    // TODO(dvoytenko): slow requestAnimationFrame buster, e.g. when Tab becomes
-    // inactive.
   }
 
   /**
@@ -229,7 +229,12 @@ class AnimationPlayer {
   start_() {
     this.startTime_ = timer.now();
     this.running_ = true;
-    this.task_(this.state_);
+    if (this.vsync_.canAnimate()) {
+      this.task_(this.state_);
+    } else {
+      log.warn(TAG_, 'cannot animate');
+      this.complete_(/* success */ false, /* dir */ 0);
+    }
   }
 
   /**
@@ -306,7 +311,12 @@ class AnimationPlayer {
     if (normLinearTime == 1) {
       this.complete_(/* success */ true, /* dir */ 0);
     } else {
-      this.task_(this.state_);
+      if (this.vsync_.canAnimate()) {
+        this.task_(this.state_);
+      } else {
+        log.warn(TAG_, 'cancel animation');
+        this.complete_(/* success */ false, /* dir */ 0);
+      }
     }
   }
 

src/motion.js

@@ -204,7 +204,7 @@ class Motion {
     this.velocityY_ = this.maxVelocityY_;
     const boundStep = this.stepContinue_.bind(this);
     const boundComplete = this.completeContinue_.bind(this, true);
-    return this.vsync_.runMutateSeries(boundStep, 5000)
+    return this.vsync_.runAnimMutateSeries(boundStep, 5000)
         .then(boundComplete, boundComplete);
   }
 

src/resources.js

@@ -428,11 +428,7 @@ export class Resources {
       return;
     }
     this.vsyncScheduled_ = true;
-    if (!this.docState_.isHidden()) {
-      this.vsync_.mutate(() => this.doPass_());
-    } else {
-      this.schedulePass(16);
-    }
+    this.vsync_.mutate(() => this.doPass_());
   }
 
   /**

src/vsync.js

@@ -14,19 +14,28 @@
  * limitations under the License.
  */
 
+import {Pass} from './pass';
 import {getService} from './service';
 import {log} from './log';
 import {timer} from './timer';
 import {viewerFor} from './viewer';
 
 
+/** @const {time} */
+const FRAME_TIME = 16;
+
+/**
+ * @typedef {Object<string, *>}
+ */
+let VsyncState;
+
 /**
  * @typedef {{
- *   measure: (function(Object<string,*>)|undefined),
- *   mutate: (function(Object<string,*>)|undefined)
+ *   measure: (function(!VsyncState)|undefined),
+ *   mutate: (function(!VsyncState)|undefined)
  * }}
  */
-class VsyncTaskSpec {}
+let VsyncTaskSpec;
 
 
 /**
@@ -41,11 +50,15 @@ export class Vsync {
 
   /**
    * @param {!Window} win
+   * @param {!Viewer} viewer
    */
-  constructor(win) {
+  constructor(win, viewer) {
     /** @const {!Window} */
     this.win = win;
 
+    /** @private @const {!Viewer} */
+    this.viewer_ = viewer;
+
     /** @private @const {function(function())}  */
     this.raf_ = this.getRaf_();
 
@@ -57,7 +70,7 @@ export class Vsync {
 
     /**
      * States for tasks in the next frame in the same order.
-     * @private {!Array<!Object>}
+     * @private {!Array<!VsyncState>}
      */
     this.states_ = [];
 
@@ -66,57 +79,46 @@ export class Vsync {
      * @private {boolean}
      */
     this.scheduled_ = false;
+
+    /** @const {!Function} */
+    this.boundRunScheduledTasks_ = this.runScheduledTasks_.bind(this);
+
+    /** @const {!Pass} */
+    this.pass_ = new Pass(this.boundRunScheduledTasks_, FRAME_TIME);
+
+    // When the document changes visibility, vsync has to reschedule the queue
+    // processing.
+    this.viewer_.onVisibilityChanged(() => {
+      if (this.scheduled_) {
+        this.forceSchedule_();
+      }
+    });
   }
 
   /**
+   * Runs vsync task: measure followed by mutate.
+   *
+   * If state is not provided, the value passed to the measure and mutate
+   * will be undefined.
+   *
    * @param {!VsyncTaskSpec} task
-   * @param {!Object<string, *>|undefined} opt_state
+   * @param {!VsyncState=} opt_state
    */
   run(task, opt_state) {
-    // Do not request animation frames when the document is not visible.
-    if (!viewerFor(this.win).isVisible()) {
-      log.fine('VSYNC', 'Did not schedule a vsync request, ' +
-          'because document was invisible.');
-      return;
-    }
-    const state = opt_state || {};
     this.tasks_.push(task);
-    this.states_.push(state);
-
-    if (this.scheduled_) {
-      return;
-    }
-    this.scheduled_ = true;
-
-    // Schedule actual animation frame and then run tasks.
-    this.raf_(() => {
-      this.runScheduledTasks();
-    });
+    this.states_.push(opt_state || {});
+    this.schedule_();
   }
 
   /**
-   * Runs all scheduled tasks. This is typically called in an RAF
-   * callback. Tests may call this method to force execution of
-   * tasks without waiting.
-   * @visibleForTesting
+   * Creates a function that will call {@link run} method.
+   * @param {!VsyncTaskSpec} task
+   * @return {function(!VsyncState=)}
    */
-  runScheduledTasks() {
-    this.scheduled_ = false;
-    // TODO(malteubl) Avoid array allocation with a double buffer.
-    const tasks = this.tasks_;
-    const states = this.states_;
-    this.tasks_ = [];
-    this.states_ = [];
-    for (let i = 0; i < tasks.length; i++) {
-      if (tasks[i].measure) {
-        tasks[i].measure(states[i]);
-      }
-    }
-    for (let i = 0; i < tasks.length; i++) {
-      if (tasks[i].mutate) {
-        tasks[i].mutate(states[i]);
-      }
-    }
+  createTask(task) {
+    return opt_state => {
+      this.run(task, opt_state);
+    };
   }
 
   /**
@@ -136,33 +138,64 @@ export class Vsync {
   }
 
   /**
+   * Whether the runtime is allowed to animate at this time.
+   * @return {boolean}
+   */
+  canAnimate() {
+    return this.viewer_.isVisible();
+  }
+
+  /**
+   * Runs the animation vsync task. This operation can only run when animations
+   * are allowed. Otherwise, this method returns `false` and exits.
    * @param {!VsyncTaskSpec} task
-   * @return {function((!Object<string, *>|undefined))}
+   * @param {!VsyncState=} opt_state
+   * @return {boolean}
    */
-  createTask(task) {
+  runAnim(task, opt_state) {
+    // Do not request animation frames when the document is not visible.
+    if (!this.canAnimate()) {
+      log.warn('Vsync',
+          'Did not schedule a vsync request, because document was invisible');
+      return false;
+    }
+    this.run(task, opt_state);
+    return true;
+  }
+
+  /**
+   * Creates an animation vsync task. This operation can only run when
+   * animations are allowed. Otherwise, this closure returns `false` and exits.
+   * @param {!VsyncTaskSpec} task
+   * @return {function(!VsyncState=):boolean}
+   */
+  createAnimTask(task) {
     return opt_state => {
-      this.run(task, opt_state);
+      return this.runAnim(task, opt_state);
     };
   }
 
   /**
    * Runs the series of mutates until the mutator returns a false value.
-   * @param {function(time, time, !Object<string,*>):boolean} mutator The
+   * @param {function(time, time, !VsyncState):boolean} mutator The
    *   mutator callback. Only expected to do DOM writes, not reads. If the
    *   returned value is true, the vsync task will be repeated, otherwise it
    *   will be completed. The arguments are: timeSinceStart:time,
-   *   timeSincePrev:time and state:Object<string, *>.
+   *   timeSincePrev:time and state:VsyncState.
    * @param {number=} opt_timeout Optional timeout that will force the series
    *   to complete and reject the promise.
    * @return {!Promise} Returns the promise that will either resolve on when
    *   the vsync series are completed or reject in case of failure, such as
    *   timeout.
    */
-  runMutateSeries(mutator, opt_timeout) {
+  runAnimMutateSeries(mutator, opt_timeout) {
+    if (!this.canAnimate()) {
+      return Promise.reject();
+    }
     return new Promise((resolve, reject) => {
       const startTime = timer.now();
       let prevTime = 0;
-      const task = this.createTask({
+      const task = this.createAnimTask({
         mutate: state => {
           const timeSinceStart = timer.now() - startTime;
           const res = mutator(timeSinceStart, timeSinceStart - prevTime, state);
@@ -180,6 +213,50 @@ export class Vsync {
     });
   }
 
+  /** @private */
+  schedule_() {
+    if (this.scheduled_) {
+      return;
+    }
+    // Schedule actual animation frame and then run tasks.
+    this.scheduled_ = true;
+    this.forceSchedule_();
+  }
+
+  /** @private */
+  forceSchedule_() {
+    if (this.canAnimate()) {
+      this.raf_(this.boundRunScheduledTasks_);
+    } else {
+      this.pass_.schedule();
+    }
+  }
+
+  /**
+   * Runs all scheduled tasks. This is typically called in an RAF
+   * callback. Tests may call this method to force execution of
+   * tasks without waiting.
+   * @private
+   */
+  runScheduledTasks_() {
+    this.scheduled_ = false;
+    // TODO(malteubl) Avoid array allocation with a double buffer.
+    const tasks = this.tasks_;
+    const states = this.states_;
+    this.tasks_ = [];
+    this.states_ = [];
+    for (let i = 0; i < tasks.length; i++) {
+      if (tasks[i].measure) {
+        tasks[i].measure(states[i]);
+      }
+    }
+    for (let i = 0; i < tasks.length; i++) {
+      if (tasks[i].mutate) {
+        tasks[i].mutate(states[i]);
+      }
+    }
+  }
+
   /**
    * @return {function(function())} requestAnimationFrame or polyfill.
    */
@@ -194,7 +271,7 @@ export class Vsync {
       const now = new Date().getTime();
       // By default we take 16ms between frames, but if the last frame is say
       // 10ms ago, we only want to wait 6ms.
-      const timeToCall = Math.max(0, 16 - (now - lastTime));
+      const timeToCall = Math.max(0, FRAME_TIME - (now - lastTime));
       lastTime = now + timeToCall;
       this.win.setTimeout(fn, timeToCall);
     };
@@ -208,6 +285,6 @@ export class Vsync {
  */
 export function vsyncFor(window) {
   return getService(window, 'vsync', () => {
-    return new Vsync(window);
+    return new Vsync(window, viewerFor(window));
   });
 };

test/functional/test-amp-ad.js

@@ -186,7 +186,7 @@ describe('amp-ad', () => {
         expect(ampAd.shouldSendIntersectionChanges_).to.be.true;
         expect(ampAd.iframeLayoutBox_).to.be.null;
         expect(posts).to.have.length(0);
-        ampAd.getVsync().runScheduledTasks();
+        ampAd.getVsync().runScheduledTasks_();
         expect(posts).to.have.length(1);
       });
     });