feat: async change events in TextTrackList with EventTarget#queueTrigger (videojs#5332)

gkatsev · web-flow · commit 8c92cbfb3e98 · 2018-07-25T17:33:30.000-04:00
Trigger the change event on the next tick. This means that multiple changes to a track's mode will only result in a single change event on its associated TextTrackList rather than 3 events as it may be currently. Fixes videojs#5159
diff --git a/src/js/event-target.js b/src/js/event-target.js
@@ -2,6 +2,7 @@
  * @file src/js/event-target.js
  */
 import * as Events from './utils/events.js';
+import window from 'global/window';
 
 /**
  * `EventTarget` is a class that can have the same API as the DOM `EventTarget`. It
@@ -158,4 +159,38 @@ EventTarget.prototype.trigger = function(event) {
  */
 EventTarget.prototype.dispatchEvent = EventTarget.prototype.trigger;
 
+let EVENT_MAP;
+
+EventTarget.prototype.queueTrigger = function(event) {
+  // only set up EVENT_MAP if it'll be used
+  if (!EVENT_MAP) {
+    EVENT_MAP = new Map();
+  }
+
+  const type = event.type || event;
+  let map = EVENT_MAP.get(this);
+
+  if (!map) {
+    map = new Map();
+    EVENT_MAP.set(this, map);
+  }
+
+  const oldTimeout = map.get(type);
+
+  map.delete(type);
+  window.clearTimeout(oldTimeout);
+
+  const timeout = window.setTimeout(() => {
+    // if we cleared out all timeouts for the current target, delete its map
+    if (map.size === 0) {
+      map = null;
+      EVENT_MAP.delete(this);
+    }
+
+    this.trigger(event);
+  }, 0);
+
+  map.set(type, timeout);
+};
+
 export default EventTarget;
diff --git a/src/js/tracks/text-track-list.js b/src/js/tracks/text-track-list.js
@@ -28,7 +28,7 @@ class TextTrackList extends TrackList {
      * @fires TrackList#change
      */
     track.addEventListener('modechange', Fn.bind(this, function() {
-      this.trigger('change');
+      this.queueTrigger('change');
     }));
 
     const nonLanguageTextTrackKind = ['metadata', 'chapters'];
diff --git a/test/unit/event-target.test.js b/test/unit/event-target.test.js
@@ -0,0 +1,51 @@
+/* eslint-env qunit */
+import EventTarget from '../../src/js/event-target.js';
+import sinon from 'sinon';
+
+const { test } = QUnit;
+
+QUnit.module('EventTarget', {
+  beforeEach() {
+    this.clock = sinon.useFakeTimers();
+  },
+  afterEach() {
+    this.clock.restore();
+  }
+});
+
+test('EventTarget queueTrigger queues the event', function(t) {
+  const et = new EventTarget();
+  let changes = 0;
+  const changeHandler = function() {
+    changes++;
+  };
+
+  et.on('change', changeHandler);
+
+  et.queueTrigger('change');
+  t.equal(changes, 0, 'EventTarget did not trigger a change event yet');
+
+  this.clock.tick(1);
+  t.equal(changes, 1, 'EventTarget triggered a change event once the clock ticked');
+});
+
+test('EventTarget will only trigger event once with queueTrigger', function(t) {
+  const et = new EventTarget();
+  let changes = 0;
+  const changeHandler = function() {
+    changes++;
+  };
+
+  et.on('change', changeHandler);
+
+  et.queueTrigger('change');
+  t.equal(changes, 0, 'EventTarget did not trigger a change event yet');
+  et.queueTrigger('change');
+  t.equal(changes, 0, 'EventTarget did not trigger a change event yet');
+  et.queueTrigger('change');
+  t.equal(changes, 0, 'EventTarget did not trigger a change event yet');
+  et.queueTrigger('change');
+
+  this.clock.tick(100);
+  t.equal(changes, 1, 'EventTarget *only* triggered a change event once');
+});
diff --git a/test/unit/tracks/text-track-list.test.js b/test/unit/tracks/text-track-list.test.js
@@ -3,8 +3,17 @@ import TextTrackList from '../../../src/js/tracks/text-track-list.js';
 import TextTrack from '../../../src/js/tracks/text-track.js';
 import EventTarget from '../../../src/js/event-target.js';
 import TechFaker from '../tech/tech-faker';
+import sinon from 'sinon';
+
+QUnit.module('Text Track List', {
+  beforeEach() {
+    this.clock = sinon.useFakeTimers();
+  },
+  afterEach() {
+    this.clock.restore();
+  }
+});
 
-QUnit.module('Text Track List');
 QUnit.test('trigger "change" event when "modechange" is fired on a track', function(assert) {
   const tt = new EventTarget();
   const ttl = new TextTrackList([tt]);
@@ -15,11 +24,13 @@ QUnit.test('trigger "change" event when "modechange" is fired on a track', funct
 
   ttl.on('change', changeHandler);
   tt.trigger('modechange');
+  this.clock.tick(1);
 
   ttl.off('change', changeHandler);
   ttl.onchange = changeHandler;
 
   tt.trigger('modechange');
+  this.clock.tick(1);
   assert.equal(changes, 2, 'two change events should have fired');
 });
 
@@ -33,12 +44,14 @@ QUnit.test('trigger "change" event when mode changes on a TextTrack', function(a
 
   ttl.on('change', changeHandler);
   tt.mode = 'showing';
+  this.clock.tick(1);
 
   ttl.off('change', changeHandler);
   ttl.onchange = changeHandler;
 
   tt.mode = 'hidden';
   tt.mode = 'disabled';
+  this.clock.tick(1);
 
-  assert.equal(changes, 3, 'three change events should have fired');
+  assert.equal(changes, 2, 'two change events should have fired');
 });
diff --git a/test/unit/tracks/text-tracks.test.js b/test/unit/tracks/text-tracks.test.js
@@ -349,9 +349,11 @@ QUnit.test('removes cuechange event when text track is hidden for emulated track
   });
 
   tt.mode = 'showing';
+  this.clock.tick(1);
   assert.equal(numTextTrackChanges, 1,
     'texttrackchange should be called once for mode change');
   tt.mode = 'showing';
+  this.clock.tick(1);
   assert.equal(numTextTrackChanges, 2,
     'texttrackchange should be called once for mode change');
 
@@ -363,6 +365,7 @@ QUnit.test('removes cuechange event when text track is hidden for emulated track
     'texttrackchange should be triggered once for the cuechange');
 
   tt.mode = 'hidden';
+  this.clock.tick(1);
   assert.equal(numTextTrackChanges, 4,
     'texttrackchange should be called once for the mode change');
 
