Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Unified Diff: polyfill.js

Issue 29697596: Issue 6388 - Wrap inherited function properties as well (Closed)
Patch Set: Simplify our approach Created Feb. 16, 2018, 1:30 p.m.
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: polyfill.js
diff --git a/polyfill.js b/polyfill.js
index f80c98f6ee6efed6b1da4d2fca0107723985f736..cf104289d1a3a627341470ac9fc7780dc50a6786 100644
--- a/polyfill.js
+++ b/polyfill.js
@@ -78,60 +78,59 @@
if (!func)
return;
- let descriptor = Object.getOwnPropertyDescriptor(object, name);
-
- delete descriptor["get"];
- delete descriptor["set"];
-
- descriptor.value = function(...args)
- {
- let callStack = new Error().stack;
+ // If the property is not writable assigning will fail, so we use
+ // Object.defineProperty here instead. Assuming the property isn't
+ // inherited its other attributes (e.g. enumerable) are presereved,
Manish Jethani 2018/02/16 14:03:53 Nit: "preserved" (spelling)
kzar 2018/02/16 16:41:08 Done.
+ // except accessor properties since we're specifying a value.
Manish Jethani 2018/02/16 14:03:53 Nit: "except accessor attributes" (?)
kzar 2018/02/16 16:41:08 How's this?
Manish Jethani 2018/02/16 16:47:38 Since "enumerable" is referred to as an attribute
+ Object.defineProperty(object, name, {
+ value: function(...args)
Manish Jethani 2018/02/16 14:03:53 Since we're using an object literal here now we ca
kzar 2018/02/16 16:41:08 Good point, eslint picked that up I just forgot to
+ {
kzar 2018/02/16 13:36:46 Sorry for changing the indentation here, it makes
+ let callStack = new Error().stack;
- if (typeof args[args.length - 1] == "function")
- return func.apply(object, args);
+ if (typeof args[args.length - 1] == "function")
+ return func.apply(object, args);
- // If the last argument is undefined, we drop it from the list assuming
- // it stands for the optional callback. We must do this, because we have
- // to replace it with our own callback. If we simply append our own
- // callback to the list, it won't match the signature of the function and
- // will cause an exception.
- if (typeof args[args.length - 1] == "undefined")
- args.pop();
+ // If the last argument is undefined, we drop it from the list assuming
+ // it stands for the optional callback. We must do this, because we have
+ // to replace it with our own callback. If we simply append our own
+ // callback to the list, it won't match the signature of the function
+ // and will cause an exception.
+ if (typeof args[args.length - 1] == "undefined")
+ args.pop();
- let resolvePromise = null;
- let rejectPromise = null;
+ let resolvePromise = null;
+ let rejectPromise = null;
- func.call(object, ...args, result =>
- {
- let error = browser.runtime.lastError;
- if (error && !portClosedBeforeResponseError.test(error.message))
+ func.call(object, ...args, result =>
{
- // runtime.lastError is already an Error instance on Edge, while on
- // Chrome it is a plain object with only a message property.
- if (!(error instanceof Error))
+ let error = browser.runtime.lastError;
+ if (error && !portClosedBeforeResponseError.test(error.message))
{
- error = new Error(error.message);
+ // runtime.lastError is already an Error instance on Edge, while on
+ // Chrome it is a plain object with only a message property.
+ if (!(error instanceof Error))
+ {
+ error = new Error(error.message);
+
+ // Add a more helpful stack trace.
+ error.stack = callStack;
+ }
- // Add a more helpful stack trace.
- error.stack = callStack;
+ rejectPromise(error);
}
+ else
+ {
+ resolvePromise(result);
+ }
+ });
- rejectPromise(error);
- }
- else
+ return new Promise((resolve, reject) =>
{
- resolvePromise(result);
- }
- });
-
- return new Promise((resolve, reject) =>
- {
- resolvePromise = resolve;
- rejectPromise = reject;
- });
- };
-
- Object.defineProperty(object, name, descriptor);
+ resolvePromise = resolve;
+ rejectPromise = reject;
+ });
+ }
+ });
}
function wrapRuntimeOnMessage()
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld