Skip to content

Commit

Permalink
refactor: Add utility to keep unused parameters (#4233)
Browse files Browse the repository at this point in the history
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 #4231 (comment)
  • Loading branch information
joeyparrish committed May 17, 2022
1 parent de0a5b6 commit 9346a0a
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 14 deletions.
25 changes: 25 additions & 0 deletions lib/util/config_utils.js
Expand Up @@ -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;
}
};
23 changes: 9 additions & 14 deletions lib/util/player_configuration.js
Expand Up @@ -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
Expand Down Expand Up @@ -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: {
Expand All @@ -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.
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 9346a0a

Please sign in to comment.