Stop using `rawObj instanceof`

NEW
Unassigned

Status

()

Firefox
Developer Tools: Console
a month ago
5 days ago

People

(Reporter: Oriol, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

a month ago
/devtools/server/actors/object.js uses lots of `rawObj instanceof Ci.something` to detect whether `rawObj` has that `something` interface.

However, this is not great. For example, before I added a previewer for Symbol objects in bug 1391274, you could run this in a privileged console:

    Object.setPrototypeOf(Object(Symbol('a')), document.body)

Then `rawObj instanceof Ci.nsIDOMElement` was true, so the console attempted to call various Element getters, producing an error:

    TypeError: 'get nodeType' called on an object that does not implement interface Node.

I think right now this is not a problem because all non-DOM classes have a custom stringifier, but I'm not sure. And this will become problematic again if a future version of ECMAScript adds new kinds of objects.

So instead of instanceof, I think something like this could be used:

  function hasInterface(rawObj, interface) {
    try {
      QueryInterface.call(rawObj, interface);
      return true;
    } catch (err) {
      return false;
    }
  }
  hasInterface(document.body, Ci.nsIDOMElement); // true
  hasInterface(Object.create(document.body), Ci.nsIDOMElement); // false
(In reply to Oriol Brufau [:Oriol] from comment #0)
> I think right now this is not a problem because all non-DOM classes have a
> custom stringifier, but I'm not sure. And this will become problematic again
> if a future version of ECMAScript adds new kinds of objects.

Reading the description of this bug, I'm not sure I follow what the concern here is. Is the worry that there might be a crash because of objects with weird prototypes? Or is this just a QOI issue where devtools will throw when exposed to these objects? Also, what's the link between this and custom stringifiers?
(Reporter)

Comment 2

5 days ago
(In reply to Blake Kaplan (:mrbkap) from comment #1)
> Reading the description of this bug, I'm not sure I follow what the concern
> here is. Is the worry that there might be a crash because of objects with
> weird prototypes? Or is this just a QOI issue where devtools will throw when
> exposed to these objects?

No crash, just that the object is initially identified as a DOM Element, but it is not. Not a big problem because the thrown error is catched and another previewer is used. But should not happen, the previewer should have some way to correctly identify that it's not an Element, and return false instead of throwing.

> Also, what's the link between this and custom stringifiers?

Sorry, I meant previewer, not stringifier. The link is that if there is no previewer defined for the class of the object, the console uses the previewers for the "Object" class as the default.
https://searchfox.org/mozilla-central/rev/9bab9dc5a9472e3c163ab279847d2249322c206e/devtools/server/actors/object.js#138-139

Note the DOMNode previewer that is in that default list, because DOM Nodes can have lots of different classes.

So before I added a custom previewer for the "Symbol" class, then DOMNode ran for the object, and wrongly identified it as a DOM Element because there was one in the [[Prototype]].

This problem does not happen for objects with an "Object" class (e.g. `Object.create(document.body)`) because DOMNode previewer blacklists it, it never consider objects with that class to be a DOM Node. An unreliable ad-hoc solution.
https://searchfox.org/mozilla-central/rev/9bab9dc5a9472e3c163ab279847d2249322c206e/devtools/server/actors/object.js#1786
You need to log in before you can comment on or make changes to this bug.