Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: Add utility to keep unused parameters #4233

Merged
merged 1 commit into from May 17, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't come up in any situation where we actually call this at the moment, but technically this approach won't work correctly if returnValue is falsey.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It definitely will. true && 0 === 0, true && undefined === undefined, etc. It works this way for any truthy value, not just true. And parameters (being a non-nullable Array) is always truthy, even if it's an empty array for some reason.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see, I made a mistake while testing this.

}
};
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 @@ -215,7 +208,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