Bug 1337743 - Document misuse of instanceof in Marionette; r?whimboo
MozReview-Commit-ID: IbUyQd0xCAI
--- a/testing/marionette/assert.js
+++ b/testing/marionette/assert.js
@@ -282,16 +282,18 @@ assert.string = function (obj, msg = "")
* |obj| is returned unaltered.
*
* @throws {InvalidArgumentError}
* If |obj| is not an object.
*/
assert.object = function (obj, msg = "") {
msg = msg || error.pprint`Expected ${obj} to be an object`;
return assert.that(o => {
+ // unable to use instanceof because LHS and RHS may come from
+ // different globals
let s = Object.prototype.toString.call(o);
return s == "[object Object]" || s == "[object nsJSIID]";
})(obj);
};
/**
* Asserts that |prop| is in |obj|.
*
--- a/testing/marionette/error.js
+++ b/testing/marionette/error.js
@@ -43,20 +43,31 @@ const BUILTIN_ERRORS = new Set([
"URIError",
]);
this.EXPORTED_SYMBOLS = ["error"].concat(Array.from(ERRORS));
this.error = {};
/**
- * Checks if obj is an instance of the Error prototype in a safe manner.
- * Prefer using this over using instanceof since the Error prototype
- * isn't unique across browsers, and XPCOM nsIException's are special
- * snowflakes.
+ * Check if |val| is an instance of the |Error| prototype.
+ *
+ * Because error objects may originate from different globals, comparing
+ * the prototype of the left hand side with the prototype property from
+ * the right hand side, which is what |instanceof| does, will not work.
+ * If the LHS and RHS come from different globals, this check will always
+ * fail because the two objects will not have the same identity.
+ *
+ * Therefore it is not safe to use |instanceof| in any multi-global
+ * situation, e.g. in content across multiple Window objects or anywhere
+ * in chrome scope.
+ *
+ * This function also contains a special check if |val| is an XPCOM
+ * |nsIException| because they are special snowflakes and may indeed
+ * cause Firefox to crash if used with |instanceof|.
*
* @param {*} val
* Any value that should be undergo the test for errorness.
* @return {boolean}
* True if error, false otherwise.
*/
error.isError = function (val) {
if (val === null || typeof val != "object") {