Merge pull request #32 from wilsonpage/dev

Large re-write to prevent frame conflicts and simplify code

4 files changed, 258 insertions, 116 deletions

diff --git a/index.js b/index.js
index 0f657b5..7efb9d5 100644
--- a/index.js
+++ b/index.js
@@ -1,6 +1,6 @@
 
 /**
- * DOM-Batch
+ * FastDom
  *
  * Eliminates layout thrashing
  * by batching DOM read/write
@@ -38,11 +38,11 @@
    * @constructor
    */
   function FastDom() {
+    this.frames = [];
     this.lastId = 0;
-    this.jobs = {};
     this.mode = null;
-    this.pending = false;
     this.queue = {
+      hash: {},
       read: [],
       write: []
     };
@@ -57,8 +57,16 @@
    */
   FastDom.prototype.read = function(fn, ctx) {
     var job = this.add('read', fn, ctx);
+
     this.queue.read.push(job.id);
-    this.request('read');
+
+    // If we're writing and a 'read' job
+    // comes in, we do have to schedule a new frame
+    var needsFrame = !this.batchPending || this.mode === 'writing';
+
+    // Schedule a new frame if need be
+    if (needsFrame) this.scheduleBatch();
+
     return job.id;
   };
 
@@ -71,74 +79,106 @@
    */
   FastDom.prototype.write = function(fn, ctx) {
     var job = this.add('write', fn, ctx);
+
     this.queue.write.push(job.id);
-    this.request('write');
+
+    // If we're emptying the read
+    // queue and a write comes in,
+    // we don't need to schedule a
+    // new frame. If we're writing
+    // and write comes in we don't
+    // need to schedule a new frame
+    var needsFrame = !this.batchPending;
+
+    // Schedule a new frame if need be
+    if (needsFrame) this.scheduleBatch();
+
     return job.id;
   };
 
   /**
-   * Removes a job from
-   * the 'reads' queue.
+   * Defers the given job
+   * by the number of frames
+   * specified.
+   *
+   * @param  {Number}   frame
+   * @param  {Function} fn
+   * @api public
+   */
+  FastDom.prototype.defer = function(frame, fn, ctx) {
+
+    // Accepts two arguments
+    if (typeof frame === 'function') {
+      ctx = fn;
+      fn = frame;
+      frame = 1;
+    }
+
+    var self = this;
+    var index = frame - 1;
+
+    return this.schedule(index, function() {
+      self.run({
+        fn: fn,
+        ctx: ctx
+      });
+    });
+  };
+
+  /**
+   * Clears a scheduled 'read',
+   * 'write' or 'defer' job.
    *
    * @param  {Number} id
    * @api public
    */
   FastDom.prototype.clear = function(id) {
-    var job = this.jobs[id];
-    if (!job) return;
-
-    // Clear reference
-    delete this.jobs[id];
 
     // Defer jobs are cleared differently
-    if (job.type === 'defer') {
-      caf(job.timer);
-      return;
+    if (typeof id === 'function') {
+      return this.clearFrame(id);
     }
 
+    var job = this.queue.hash[id];
+    if (!job) return;
+
     var list = this.queue[job.type];
     var index = list.indexOf(id);
+
+    // Clear references
+    delete this.queue.hash[id];
     if (~index) list.splice(index, 1);
   };
 
   /**
-   * Makes the decision as to
-   * whether a the frame needs
-   * to be scheduled.
+   * Clears a scheduled frame.
    *
-   * @param  {String} type
+   * @param  {Function} frame
    * @api private
    */
-  FastDom.prototype.request = function(type) {
-    var mode = this.mode;
-    var self = this;
-
-    // If we are currently writing, we don't
-    // need to schedule a new frame as this
-    // job will be emptied from the write queue
-    if (mode === 'writing' && type === 'write') return;
-
-    // If we are reading we don't need to schedule
-    // a new frame as this read will be emptied
-    // in the currently active read queue
-    if (mode === 'reading' && type === 'read') return;
-
-    // If we are reading we don't need to schedule
-    // a new frame and this write job will be run
-    // after the read queue has been emptied in the
-    // currently active frame.
-    if (mode === 'reading' && type === 'write') return;
+  FastDom.prototype.clearFrame = function(frame) {
+    var index = this.frames.indexOf(frame);
+    if (~index) this.frames.splice(index, 1);
+  };
 
-    // If there is already a frame
-    // scheduled, don't schedule another one
-    if (this.pending) return;
+  /**
+   * Schedules a new read/write
+   * batch if one isn't pending.
+   *
+   * @api private
+   */
+  FastDom.prototype.scheduleBatch = function() {
+    var self = this;
 
-    // Schedule frame (preserving context)
-    raf(function() { self.frame(); });
+    // Schedule batch for next frame
+    this.schedule(0, function() {
+      self.runBatch();
+      self.batchPending = false;
+    });
 
     // Set flag to indicate
     // a frame has been scheduled
-    this.pending = true;
+    this.batchPending = true;
   };
 
   /**
@@ -167,7 +207,7 @@
   FastDom.prototype.flush = function(list) {
     var id;
     while (id = list.shift()) {
-      this.run(this.jobs[id]);
+      this.run(this.queue.hash[id]);
     }
   };
 
@@ -177,12 +217,7 @@
    *
    * @api private
    */
-  FastDom.prototype.frame = function() {
-
-    // Set the pending flag to
-    // false so that any new requests
-    // that come in will schedule a new frame
-    this.pending = false;
+  FastDom.prototype.runBatch = function() {
 
     // Set the mode to 'reading',
     // then empty all read jobs
@@ -198,32 +233,6 @@
   };
 
   /**
-   * Defers the given job
-   * by the number of frames
-   * specified.
-   *
-   * @param  {Number}   frames
-   * @param  {Function} fn
-   * @api public
-   */
-  FastDom.prototype.defer = function(frames, fn, ctx) {
-    if (frames < 0) return;
-    var job = this.add('defer', fn, ctx);
-    var self = this;
-
-    (function wrapped() {
-      if (!(frames--)) {
-         self.run(job);
-         return;
-      }
-
-      job.timer = raf(wrapped);
-    })();
-
-    return job.id;
-  };
-
-  /**
    * Adds a new job to
    * the given queue.
    *
@@ -235,7 +244,7 @@
    */
   FastDom.prototype.add = function(type, fn, ctx) {
     var id = this.uniqueId();
-    return this.jobs[id] = {
+    return this.queue.hash[id] = {
       id: id,
       fn: fn,
       ctx: ctx,
@@ -244,17 +253,8 @@
   };
 
   /**
-   * Called when a callback errors.
-   * Overwrite this if you don't
-   * want errors inside your jobs
-   * to fail silently.
-   *
-   * @param {Error}
-   */
-  FastDom.prototype.onError = function(){};
-
-  /**
    * Runs a given job.
+   *
    * @param  {Object} job
    * @api private
    */
@@ -262,14 +262,75 @@
     var ctx = job.ctx || this;
 
     // Clear reference to the job
-    delete this.jobs[job.id];
+    delete this.queue.hash[job.id];
 
-    // Call the job in
-    try { job.fn.call(ctx); } catch(e) {
-      this.onError(e);
+    if (this.quiet) {
+      try { job.fn.call(ctx); } catch (e) {}
+    } else {
+      job.fn.call(ctx);
     }
   };
 
+  /**
+   * Starts of a rAF loop
+   * to empty the frame queue.
+   *
+   * @api private
+   */
+  FastDom.prototype.loop = function() {
+    var self = this;
+
+    // Don't start more than one loop
+    if (this.looping) return;
+
+    raf(function frame() {
+      var fn = self.frames.shift();
+
+      // Run the frame
+      if (fn) fn();
+
+      // If no more frames,
+      // stop looping
+      if (!self.frames.length) {
+        self.looping = false;
+        return;
+      }
+
+      raf(frame);
+    });
+
+    this.looping = true;
+  };
+
+  /**
+   * Adds a function to
+   * a specified index
+   * of the frame queue.
+   *
+   * @param  {Number}   index
+   * @param  {Function} fn
+   * @return {Function}
+   */
+  FastDom.prototype.schedule = function(index, fn) {
+
+    // Make sure this slot
+    // hasn't already been
+    // taken. If it has, try
+    // re-scheduling for the next slot
+    if (this.frames[index]) {
+      return this.schedule(index + 1, fn);
+    }
+
+    // Start the rAF
+    // loop to empty
+    // the frame queue
+    this.loop();
+
+    // Insert this function into
+    // the frames queue and return
+    return this.frames[index] = fn;
+  };
+
   // We only ever want there to be
   // one instance of FastDom in an app
   fastdom = fastdom || new FastDom();
diff --git a/test/test.clear.js b/test/test.clear.js
index 5194830..bea8031 100644
--- a/test/test.clear.js
+++ b/test/test.clear.js
@@ -59,8 +59,8 @@ suite('clear', function(){
 
   test('Should not run "defer" job if cleared', function(done) {
     var fastdom = new FastDom();
-    var write = sinon.spy();
-    var id = fastdom.defer(3, write);
+    var callback = sinon.spy();
+    var id = fastdom.defer(3, callback);
 
     fastdom.clear(id);
 
@@ -68,7 +68,7 @@ suite('clear', function(){
       raf(function() {
         raf(function() {
           raf(function() {
-            assert(!write.called);
+            assert(!callback.called);
             done();
           });
         });
@@ -79,7 +79,7 @@ suite('clear', function(){
   test('Should remove reference to the job if cleared', function(done) {
     var fastdom = new FastDom();
     var write = sinon.spy();
-    var id = fastdom.defer(2, write);
+    var id = fastdom.write(2, write);
 
     fastdom.clear(id);
 
@@ -87,11 +87,10 @@ suite('clear', function(){
       raf(function() {
         raf(function() {
           assert(!write.called);
-          assert(!fastdom.jobs[id]);
+          assert(!fastdom.queue.hash[id]);
           done();
         });
       });
     });
   });
-
-});
+});
\ No newline at end of file
diff --git a/test/test.defer.js b/test/test.defer.js
index 3e56a94..114287f 100644
--- a/test/test.defer.js
+++ b/test/test.defer.js
@@ -5,18 +5,15 @@ suite('defer', function(){
     var fastdom = new FastDom();
     var job = sinon.spy();
 
-    fastdom.defer(4, job);
+    fastdom.defer(3, job);
 
     raf(function() {
       assert(!job.called);
       raf(function() {
         assert(!job.called);
         raf(function() {
-          assert(!job.called);
-          raf(function() {
-            assert(job.called);
-            done();
-          });
+          assert(job.called);
+          done();
         });
       });
     });
@@ -33,18 +30,81 @@ suite('defer', function(){
     }, ctx);
   });
 
-  test('Should remove the reference to the job once run', function(done) {
+  test('Should run work at next frame if frames argument not supplied.', function(done) {
     var fastdom = new FastDom();
-    var callback = sinon.spy();
-    var id = fastdom.defer(2, callback);
+    var callback1 = sinon.spy();
+    var callback2 = sinon.spy();
+
+    fastdom.defer(callback1);
 
     raf(function() {
+      assert(callback1.called);
+      done();
+    });
+  });
+
+  test('Should run each job on a different frame.', function(done) {
+    var fastdom = new FastDom();
+    var callback1 = sinon.spy();
+    var callback2 = sinon.spy();
+    var callback3 = sinon.spy();
+
+    fastdom.defer(callback1);
+    fastdom.defer(callback2);
+    fastdom.defer(callback3);
+
+    raf(function() {
+      assert(callback1.called);
+      assert(!callback2.called);
+      assert(!callback3.called);
       raf(function() {
+        assert(callback2.called);
+        assert(!callback3.called);
         raf(function() {
-          assert(!fastdom.jobs[id]);
+          assert(callback3.called);
           done();
         });
       });
     });
   });
+
+  test('Should run fill empty frames before later work is run.', function(done) {
+    var fastdom = new FastDom();
+    var callback1 = sinon.spy();
+    var callback2 = sinon.spy();
+    var callback3 = sinon.spy();
+    var callback4 = sinon.spy();
+
+    // Frame 3
+    fastdom.defer(3, callback3);
+
+    // Frame 1
+    fastdom.defer(callback1);
+
+    // Frame 2
+    fastdom.defer(callback2);
+
+    // Frame 4
+    fastdom.defer(callback4);
+
+    raf(function() {
+      assert(callback1.called);
+      assert(!callback2.called);
+      assert(!callback3.called);
+      assert(!callback4.called);
+      raf(function() {
+        assert(callback2.called);
+        assert(!callback3.called);
+        assert(!callback4.called);
+        raf(function() {
+          assert(callback3.called);
+          assert(!callback4.called);
+          raf(function() {
+            assert(callback4.called);
+            done();
+          });
+        });
+      });
+    });
+  });
 });
diff --git a/test/test.set.js b/test/test.set.js
index 440848d..f5b2ae3 100644
--- a/test/test.set.js
+++ b/test/test.set.js
@@ -59,11 +59,17 @@ suite('set', function() {
         assert(!cb.called);
         done();
       });
+
+      // Should not have scheduled a new frame
+      assert(fastdom.frames.length === 0);
     });
   });
 
   test('Should call a write in the same frame if scheduled inside a read callback', function(done) {
     var fastdom = new FastDom();
+
+    fastdom.catchErrors = false;
+
     var cb = sinon.spy();
 
     fastdom.read(function() {
@@ -80,6 +86,9 @@ suite('set', function() {
         assert(!cb.called);
         done();
       });
+
+      // Should not have scheduled a new frame
+      assert(fastdom.frames.length === 0);
     });
   });
 
@@ -100,6 +109,9 @@ suite('set', function() {
         assert(cb.called);
         done();
       });
+
+      // Should not have scheduled a new frame
+      assert(fastdom.frames.length === 1);
     });
   });
 
@@ -134,10 +146,10 @@ suite('set', function() {
     fastdom.write(function(){});
 
     // Check there are four jobs stored
-    assert.equal(objectLength(fastdom.jobs), 4);
+    assert.equal(objectLength(fastdom.queue.hash), 4);
 
     raf(function() {
-      assert.equal(objectLength(fastdom.jobs), 0);
+      assert.equal(objectLength(fastdom.queue.hash), 0);
       done();
     });
   });
@@ -162,12 +174,12 @@ suite('set', function() {
     });
   });
 
-  test('Should call a registered onError handler when an error is thrown inside a job', function(done) {
+  test('Should no error if the `quiet` flag is set', function(done) {
     var fastdom = new FastDom();
     var err1 = { some: 'error1' };
     var err2 = { some: 'error2' };
 
-    fastdom.onError = sinon.spy();
+    fastdom.quiet = true;
 
     fastdom.read(function() {
       throw err1;
@@ -178,9 +190,19 @@ suite('set', function() {
     });
 
     raf(function() {
-      assert(fastdom.onError.calledTwice);
-      assert(fastdom.onError.getCall(0).calledWith(err1));
-      assert(fastdom.onError.getCall(1).calledWith(err2));
+      done();
+    });
+  });
+
+  test('Should stop rAF loop once frame queue is empty', function(done) {
+    var fastdom = new FastDom();
+    var callback = sinon.spy();
+
+    fastdom.read(callback);
+
+    raf(function() {
+      assert(callback.called);
+      assert(fastdom.looping === false);
       done();
     });
   });