fix: Fix compatibility for plugin factories

In v3.0, we changed our plugin interfaces so that all are factories
that return objects.  We no longer call those with "new" as we did in
v2.5.

We provided backward compatibility and a deprecation warning to alert
applications to update their usage of those interfaces.  However, this
compatibility shim was broken for ES6 classes, which behaved
differently than ES5 constructor functions.

This fixes the shim and adds regression tests.  The tests were tricky
because we use Babel to transpile our tests, but we needed raw,
untranspiled code to define a class to test these features.  So we use
eval and some associated trickery to get exactly the different kinds
of plugin registrations we intend to support.  These tests are skipped
on non-ES6 browsers.

Fixes shaka-project#2958

Change-Id: Ife8b63c0d89f4ea0aff085d3a4c156c4eb657604

build/conformance.textproto

@@ -300,13 +300,15 @@ requirement: {
   error_message: 'Using "eval" is not allowed'
   whitelist_regexp: 'demo/demo_utils.js'
   whitelist_regexp: 'node_modules/google-closure-library/closure/goog/base.js'
+  whitelist_regexp: 'test/util/functional_unit.js'
 }
 requirement: {
   type: BANNED_NAME
   value: 'eval'
   error_message: 'Using "eval" is not allowed'
   whitelist_regexp: 'demo/demo_utils.js'
   whitelist_regexp: 'node_modules/google-closure-library/closure/goog/base.js'
+  whitelist_regexp: 'test/util/functional_unit.js'
 }
 
 

lib/util/functional.js

@@ -86,14 +86,26 @@ shaka.util.Functional = class {
     // If this is a constructor, call it with our newly created object to
     // initialize it; if this isn't a constructor, the "this" shouldn't be used
     // since it should be "undefined".
-    let ret = factory.call(obj);  // eslint-disable-line no-restricted-syntax
-    // If it didn't return anything, assume it is a constructor and return our
-    // "this" value instead.
-    if (!ret) {
+    let ret;
+    try {
+      ret = factory.call(obj);  // eslint-disable-line no-restricted-syntax
+
+      // If it didn't return anything, assume it is a constructor and return our
+      // "this" value instead.
+      if (!ret) {
+        shaka.Deprecate.deprecateFeature(4,
+            'Factories requiring new',
+            'Factories should be plain functions');
+        ret = obj;
+      }
+    } catch (e) {
+      // This was an ES6 class, so it threw a TypeError because we didn't use
+      // "new".  Fall back to actually using "new".
       shaka.Deprecate.deprecateFeature(4,
           'Factories requiring new',
           'Factories should be plain functions');
-      ret = obj;
+      const FactoryAsClass = /** @type {function(new: T)} */(factory);
+      ret = new FactoryAsClass();
     }
     return ret;
   }

test/util/functional_unit.js

@@ -0,0 +1,68 @@
+/*! @license
+ * Shaka Player
+ * Copyright 2016 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+goog.require('shaka.util.Functional');
+
+describe('Functional', () => {
+  const Functional = shaka.util.Functional;
+
+  function supportsEs6Classes() {
+    // The callFactory tests should only be run on platforms that support ES6
+    // classes.  We need to use classes directly to ensure that callFactory is
+    // working correctly.
+    try {
+      eval('class Foo {}');
+      return true;
+    } catch (e) {  // eslint-disable-line no-restricted-syntax
+      return false;
+    }
+  }
+
+  filterDescribe('callFactory', supportsEs6Classes, () => {
+    // All of the following factories/functions/classes create objects with a
+    // field called "val" with a value of 1.  This is a type def to satisfy the
+    // compiler.
+    /** @typedef {{val: number}} */
+    let DummyObjType;
+
+    // Normally, our tests are transpiled by Babel to allow them to run on all
+    // browsers.  However, that would convert all of these into plain functions,
+    // which would defeat the purpose.  Therefore, we're using eval to make sure
+    // these get defined in exactly this way.  Furthermore, to make sure these
+    // are returned to names that are in scope of this test suite in strict mode
+    // (used by Babel), each eval must use an assignment syntax to a dummy
+    // variable, then return it.
+    const FactoryFunction = /** @type {function():DummyObjType} */(eval(
+        'const f = function() { return { val: 1 }; }; f;'));
+    const FactoryArrowFunction = /** @type {function():DummyObjType} */(eval(
+        'const f = () => { return { val: 1 }; }; f;'));
+    const Es5ConstructorFunction = /** @type {function():DummyObjType} */(eval(
+        'const f = function() { this.val = 1; }; f;'));
+    const Es6Class = /** @type {function():DummyObjType} */(eval(
+        'const f = class { constructor() { this.val = 1; } }; f;'));
+
+    it('supports true factory functions', () => {
+      const obj = Functional.callFactory(FactoryFunction);
+      expect(obj.val).toBe(1);
+    });
+
+    it('supports true factory arrow functions', () => {
+      const obj = Functional.callFactory(FactoryArrowFunction);
+      expect(obj.val).toBe(1);
+    });
+
+    it('supports ES5 constructor functions', () => {
+      const obj = Functional.callFactory(Es5ConstructorFunction);
+      expect(obj.val).toBe(1);
+    });
+
+    // Regression test for https://github.com/google/shaka-player/issues/2958
+    it('supports ES6 classes', () => {
+      const obj = Functional.callFactory(Es6Class);
+      expect(obj.val).toBe(1);
+    });
+  });
+});