revert: perf: Cache currentTime and buffered from Flash (#3705) (#3778)

This reverts commit 45ffa810fb.

src/js/tech/flash-cache.js

@@ -1,31 +0,0 @@
-/**
- * @file flash-cache.js
- *
- * Auto-caching method wrapper to avoid calling through to Flash too
- * often.
- */
-
-/**
- * Returns a new getter function that returns a cached value if
- * invoked multiple times within the specified duration.
- *
- * @param {Function} getter the function to be cached
- * @param {Number} cacheDuration the number of milliseconds to cache
- * results for
- * @return {Function} a new function that returns cached results if
- * invoked multiple times within the cache duration
- */
-export default function timeExpiringCache(getter, cacheDuration) {
-  const result = function cachedGetter() {
-    const now = new Date().getTime();
-
-    if (now - result.lastCheckTime_ >= cacheDuration) {
-      result.lastCheckTime_ = now;
-      result.cache_ = getter();
-    }
-    return result.cache_;
-  };
-
-  result.lastCheckTime_ = -Infinity;
-  return result;
-}

src/js/tech/flash.js

@@ -10,7 +10,6 @@ import * as Dom from '../utils/dom.js';
 import * as Url from '../utils/url.js';
 import { createTimeRange } from '../utils/time-ranges.js';
 import FlashRtmpDecorator from './flash-rtmp';
-import timeExpiringCache from './flash-cache.js';
 import Component from '../component';
 import window from 'global/window';
 import assign from 'object.assign';
@@ -60,34 +59,6 @@ class Flash extends Tech {
     this.on('seeked', function() {
       this.lastSeekTarget_ = undefined;
     });
-
-    // calling into the SWF can be expensive, especially if Flash is
-    // busy rendering video frames
-    // automatically cache commonly used properties for a short period
-    // of time so that multiple calls within a short time period don't
-    // all pay a big performance penalty for properties that change
-    // relatively slowly over time
-    const getCurrentTimeCached = timeExpiringCache(() => {
-      return this.el_.vjs_getProperty('currentTime');
-    }, 100);
-
-    this.currentTime = (time) => {
-      // when seeking make the reported time keep up with the requested time
-      // by reading the time we're seeking to
-      if (this.seeking()) {
-        return this.lastSeekTarget_ || 0;
-      }
-
-      return getCurrentTimeCached();
-    };
-    this.buffered = timeExpiringCache(() => {
-      const ranges = this.el_.vjs_getProperty('buffered');
-
-      if (ranges.length === 0) {
-        return createTimeRange();
-      }
-      return createTimeRange(ranges[0][0], ranges[0][1]);
-    }, 100);
   }
 
   /**
@@ -242,6 +213,14 @@ class Flash extends Tech {
    * @return {Number} Current time
    * @method currentTime
    */
+  currentTime(time) {
+    // when seeking make the reported time keep up with the requested time
+    // by reading the time we're seeking to
+    if (this.seeking()) {
+      return this.lastSeekTarget_ || 0;
+    }
+    return this.el_.vjs_getProperty('currentTime');
+  }
 
   /**
    * Get current source
@@ -315,6 +294,14 @@ class Flash extends Tech {
    * @return {TimeRangeObject}
    * @method buffered
    */
+  buffered() {
+    const ranges = this.el_.vjs_getProperty('buffered');
+
+    if (ranges.length === 0) {
+      return createTimeRange();
+    }
+    return createTimeRange(ranges[0][0], ranges[0][1]);
+  }
 
   /**
    * Get fullscreen support -

test/unit/tech/flash-cache.test.js

@@ -1,50 +0,0 @@
-/* eslint-env qunit */
-import timeExpiringCache from '../../../src/js/tech/flash-cache.js';
-import sinon from 'sinon';
-
-QUnit.module('Flash Cache', {
-  beforeEach() {
-    this.clock = sinon.useFakeTimers();
-  },
-  afterEach() {
-    this.clock.restore();
-  }
-});
-
-QUnit.test('calls the getter on first invocation', function(assert) {
-  let calls = 0;
-  const cached = timeExpiringCache(() => calls++, 100);
-
-  QUnit.equal(cached(), 0, 'returned a value');
-  QUnit.equal(calls, 1, 'called the getter');
-});
-
-QUnit.test('caches results', function(assert) {
-  let calls = 0;
-  const cached = timeExpiringCache(() => calls++, 100);
-
-  for (let i = 0; i < 31; i++) {
-    QUnit.equal(cached(), 0, 'returned a cached value');
-  }
-  QUnit.equal(calls, 1, 'only called getter once');
-});
-
-QUnit.test('refreshes the cache after the result expires', function(assert) {
-  let calls = 0;
-  const cached = timeExpiringCache(() => calls++, 1);
-
-  QUnit.equal(cached(), 0, 'returned a value');
-  QUnit.equal(cached(), 0, 'returned a cached value');
-  QUnit.equal(calls, 1, 'only called getter once');
-  this.clock.tick(1);
-
-  QUnit.equal(cached(), 1, 'returned a value');
-  QUnit.equal(cached(), 1, 'returned a cached value');
-  QUnit.equal(calls, 2, 'called getter again');
-
-  this.clock.tick(10);
-  QUnit.equal(calls, 2, 'only refreshes on-demand');
-  QUnit.equal(cached(), 2, 'returned a value');
-  QUnit.equal(cached(), 2, 'returned a cached value');
-  QUnit.equal(calls, 3, 'called getter again');
-});

test/unit/tech/flash.test.js

@@ -31,7 +31,8 @@ QUnit.test('Flash.canPlaySource', function(assert) {
 });
 
 QUnit.test('currentTime', function(assert) {
-  const mockFlash = new MockFlash();
+  const getCurrentTime = Flash.prototype.currentTime;
+  const setCurrentTime = Flash.prototype.setCurrentTime;
   let seekingCount = 0;
   let seeking = false;
   let setPropVal;
@@ -39,42 +40,44 @@ QUnit.test('currentTime', function(assert) {
   let result;
 
   // Mock out a Flash instance to avoid creating the swf object
-  mockFlash.el_ = {
-    /* eslint-disable camelcase */
-    vjs_setProperty(prop, val) {
-      setPropVal = val;
+  const mockFlash = {
+    el_: {
+      /* eslint-disable camelcase */
+      vjs_setProperty(prop, val) {
+        setPropVal = val;
+      },
+      vjs_getProperty() {
+        return getPropVal;
+      }
+      /* eslint-enable camelcase */
     },
-    vjs_getProperty() {
-      return getPropVal;
+    seekable() {
+      return createTimeRange(5, 1000);
+    },
+    trigger(event) {
+      if (event === 'seeking') {
+        seekingCount++;
+      }
+    },
+    seeking() {
+      return seeking;
     }
-    /* eslint-enable camelcase */
-  };
-  mockFlash.seekable = function() {
-    return createTimeRange(5, 1000);
-  };
-  mockFlash.trigger = function(event) {
-    if (event === 'seeking') {
-      seekingCount++;
-    }
-  };
-  mockFlash.seeking = function() {
-    return seeking;
   };
 
   // Test the currentTime getter
   getPropVal = 3;
-  result = mockFlash.currentTime();
+  result = getCurrentTime.call(mockFlash);
   assert.equal(result, 3, 'currentTime is retreived from the swf element');
 
   // Test the currentTime setter
-  mockFlash.setCurrentTime(10);
+  setCurrentTime.call(mockFlash, 10);
   assert.equal(setPropVal, 10, 'currentTime is set on the swf element');
   assert.equal(seekingCount, 1, 'triggered seeking');
 
   // Test current time while seeking
-  mockFlash.setCurrentTime(20);
+  setCurrentTime.call(mockFlash, 20);
   seeking = true;
-  result = mockFlash.currentTime();
+  result = getCurrentTime.call(mockFlash);
   assert.equal(result,
               20,
               'currentTime is retrieved from the lastSeekTarget while seeking');
@@ -84,13 +87,13 @@ QUnit.test('currentTime', function(assert) {
   assert.equal(seekingCount, 2, 'triggered seeking');
 
   // clamp seeks to seekable
-  mockFlash.setCurrentTime(1001);
-  result = mockFlash.currentTime();
+  setCurrentTime.call(mockFlash, 1001);
+  result = getCurrentTime.call(mockFlash);
   assert.equal(result, mockFlash.seekable().end(0), 'clamped to the seekable end');
   assert.equal(seekingCount, 3, 'triggered seeking');
 
-  mockFlash.setCurrentTime(1);
-  result = mockFlash.currentTime();
+  setCurrentTime.call(mockFlash, 1);
+  result = getCurrentTime.call(mockFlash);
   assert.equal(result, mockFlash.seekable().start(0), 'clamped to the seekable start');
   assert.equal(seekingCount, 4, 'triggered seeking');
 });