From 9346a0afcf7779b70015e0f6afe9398fd558598d Mon Sep 17 00:00:00 2001
From: Joey Parrish <joeyparrish@users.noreply.github.com>
Date: Tue, 17 May 2022 11:19:20 -0700
Subject: [PATCH] refactor: Add utility to keep unused parameters (#4233)

Many times in our default configuration, we need to create an empty
default implementation of a callback.  The compiler wants to strip
out unused parameters from these functions, but this breaks our
runtime-type-checking for functions in configure().  We need a way to
keep the full function signature in default config callbacks in
compiled mode.

This adds a new utility: ConfigUtils.referenceParametersAndReturn().

It references the input parameters so the compiler doesn't remove them
from the calling function, and returns whatever value is specified.
The utility function is marked with `@noinline`, so the compiler won't
tamper with it.

Default config callbacks that use this utility will still bear the
complete function signature even in compiled mode.

The caller should look something like this:

```js
  const callback = (a, b, c, d) => {
    return referenceParametersAndReturn(
        [a, b, c, d],
        a);  // Can be anything, doesn't need to be one of the parameters
  };
```

See also https://github.com/shaka-project/shaka-player/pull/4231#discussion_r874710385
---
 lib/util/config_utils.js         | 25 +++++++++++++++++++++++++
 lib/util/player_configuration.js | 23 +++++++++--------------
 2 files changed, 34 insertions(+), 14 deletions(-)

diff --git a/lib/util/config_utils.js b/lib/util/config_utils.js
index c1d79be42c..b11ba6fbe8 100644
--- a/lib/util/config_utils.js
+++ b/lib/util/config_utils.js
@@ -127,4 +127,29 @@ shaka.util.ConfigUtils = class {
     last[fieldName.substring(nameStart).replace(/\\\./g, '.')] = value;
     return configObject;
   }
+
+  /**
+   * Reference the input parameters so the compiler doesn't remove them from
+   * the calling function.  Return whatever value is specified.
+   *
+   * This allows an empty or default implementation of a config callback that
+   * still bears the complete function signature even in compiled mode.
+   *
+   * The caller should look something like this:
+   *
+   *   const callback = (a, b, c, d) => {
+   *     return referenceParametersAndReturn(
+             [a, b, c, d],
+             a);  // Can be anything, doesn't need to be one of the parameters
+   *   };
+   *
+   * @param {!Array.<?>} parameters
+   * @param {T} returnValue
+   * @return {T}
+   * @template T
+   * @noinline
+   */
+  static referenceParametersAndReturn(parameters, returnValue) {
+    return parameters && returnValue;
+  }
 };
diff --git a/lib/util/player_configuration.js b/lib/util/player_configuration.js
index e9367ec2df..f795e42701 100644
--- a/lib/util/player_configuration.js
+++ b/lib/util/player_configuration.js
@@ -16,14 +16,6 @@ goog.require('shaka.util.ManifestParserUtils');
 goog.require('shaka.util.Platform');
 
 
-// TODO(vaage): Many times in our configs, we need to create an empty
-//  implementation of a method, but to avoid closure from removing unused
-//  parameters (and breaking our merge config code) we need to use each
-//  parameter. Is there a better solution to this problem than what we are
-//  doing now?
-//
-//  NOTE: Chrome App Content Security Policy prohibits usage of new Function()
-
 /**
  * @final
  * @export
@@ -113,11 +105,10 @@ shaka.util.PlayerConfiguration = class {
           'urn:uuid:f239e769-efa3-4850-9c16-a903c6932efb':
             'com.adobe.primetime',
         },
-        // Need some operation in the callback or else closure may remove calls
-        // to the function as it would be a no-op.  The operation can't just be
-        // a log message, because those are stripped in the compiled build.
         manifestPreprocessor: (element) => {
-          return element;
+          return shaka.util.ConfigUtils.referenceParametersAndReturn(
+              [element],
+              element);
         },
       },
       hls: {
@@ -138,7 +129,9 @@ shaka.util.PlayerConfiguration = class {
       // log message, because those are stripped in the compiled build.
       failureCallback: (error) => {
         shaka.log.error('Unhandled streaming error', error);
-        return [error];
+        return shaka.util.ConfigUtils.referenceParametersAndReturn(
+            [error],
+            undefined);
       },
       // When low latency streaming is enabled, rebufferingGoal will default to
       // 0.01 if not specified.
@@ -214,7 +207,9 @@ shaka.util.PlayerConfiguration = class {
       // to the function as it would be a no-op.  The operation can't just be a
       // log message, because those are stripped in the compiled build.
       progressCallback: (content, progress) => {
-        return [content, progress];
+        return shaka.util.ConfigUtils.referenceParametersAndReturn(
+            [content, progress],
+            undefined);
       },
 
       // By default we use persistent licenses as forces errors to surface if