From d7f27b711281f6eb3cc939440589ce476f84f54f Mon Sep 17 00:00:00 2001
From: Gary Katsevman <git@gkatsev.com>
Date: Thu, 5 Jul 2018 16:29:03 -0400
Subject: [PATCH] perf: setTimeout and requestAnimationFrame memory leak
 (#5294)

Our setTimeout and requestAnimationFrame methods added dispose handlers so that they get cancelled if the component is disposed before they get a chance to run. However, we were only clearing out these dispose handlers if we cleared the timeout or the rAF manually. Instead make sure that we remove the dispose handler when it is no longer needed.

Fixes #5199.
---
 src/js/component.js         | 24 +++++++++++++++---
 test/unit/component.test.js | 49 +++++++++++++++++++++++++++++++++++++
 2 files changed, 69 insertions(+), 4 deletions(-)

diff --git a/src/js/component.js b/src/js/component.js
index 6e7f2489f..8a1a1a5cd 100644
--- a/src/js/component.js
+++ b/src/js/component.js
@@ -1245,10 +1245,18 @@ class Component {
    * @see [Similar to]{@link https://developer.mozilla.org/en-US/docs/Web/API/WindowTimers/setTimeout}
    */
   setTimeout(fn, timeout) {
+    // declare as variables so they are properly available in timeout function
+    // eslint-disable-next-line
+    var timeoutId, disposeFn;
+
     fn = Fn.bind(this, fn);
 
-    const timeoutId = window.setTimeout(fn, timeout);
-    const disposeFn = () => this.clearTimeout(timeoutId);
+    timeoutId = window.setTimeout(() => {
+      this.off('dispose', disposeFn);
+      fn();
+    }, timeout);
+
+    disposeFn = () => this.clearTimeout(timeoutId);
 
     disposeFn.guid = `vjs-timeout-${timeoutId}`;
 
@@ -1371,11 +1379,19 @@ class Component {
    * @see [Similar to]{@link https://developer.mozilla.org/en-US/docs/Web/API/window/requestAnimationFrame}
    */
   requestAnimationFrame(fn) {
+    // declare as variables so they are properly available in rAF function
+    // eslint-disable-next-line
+    var id, disposeFn;
+
     if (this.supportsRaf_) {
       fn = Fn.bind(this, fn);
 
-      const id = window.requestAnimationFrame(fn);
-      const disposeFn = () => this.cancelAnimationFrame(id);
+      id = window.requestAnimationFrame(() => {
+        this.off('dispose', disposeFn);
+        fn();
+      });
+
+      disposeFn = () => this.cancelAnimationFrame(id);
 
       disposeFn.guid = `vjs-raf-${id}`;
       this.on('dispose', disposeFn);
diff --git a/test/unit/component.test.js b/test/unit/component.test.js
index db35e8d88..d3de87948 100644
--- a/test/unit/component.test.js
+++ b/test/unit/component.test.js
@@ -957,6 +957,55 @@ QUnit.test('*AnimationFrame methods fall back to timers if rAF not supported', f
   window.cancelAnimationFrame = oldCAF;
 });
 
+QUnit.test('setTimeout should remove dispose handler on trigger', function(assert) {
+  const comp = new Component(getFakePlayer());
+  const el = comp.el();
+  const data = DomData.getData(el);
+
+  comp.setTimeout(() => {}, 1);
+
+  assert.equal(data.handlers.dispose.length, 2, 'we got a new dispose handler');
+  assert.ok(/vjs-timeout-\d/.test(data.handlers.dispose[1].guid), 'we got a new dispose handler');
+
+  this.clock.tick(1);
+
+  assert.equal(data.handlers.dispose.length, 1, 'we removed our dispose handle');
+
+  comp.dispose();
+});
+
+QUnit.test('requestAnimationFrame should remove dispose handler on trigger', function(assert) {
+  const comp = new Component(getFakePlayer());
+  const el = comp.el();
+  const data = DomData.getData(el);
+  const oldRAF = window.requestAnimationFrame;
+  const oldCAF = window.cancelAnimationFrame;
+
+  // Stub the window.*AnimationFrame methods with window.setTimeout methods
+  // so we can control when the callbacks are called via sinon's timer stubs.
+  window.requestAnimationFrame = (fn) => window.setTimeout(fn, 1);
+  window.cancelAnimationFrame = (id) => window.clearTimeout(id);
+
+  // Make sure the component thinks it supports rAF.
+  comp.supportsRaf_ = true;
+
+  const spyRAF = sinon.spy();
+
+  comp.requestAnimationFrame(spyRAF);
+
+  assert.equal(data.handlers.dispose.length, 2, 'we got a new dispose handler');
+  assert.ok(/vjs-raf-\d/.test(data.handlers.dispose[1].guid), 'we got a new dispose handler');
+
+  this.clock.tick(1);
+
+  assert.equal(data.handlers.dispose.length, 1, 'we removed our dispose handle');
+
+  comp.dispose();
+
+  window.requestAnimationFrame = oldRAF;
+  window.cancelAnimationFrame = oldCAF;
+});
+
 QUnit.test('$ and $$ functions', function(assert) {
   const comp = new Component(getFakePlayer());
   const contentEl = document.createElement('div');