1141354 - "can't convert null to object" error in diff.js

Status: RESOLVED INCOMPLETE

(Add-on SDK Graveyard :: General, defect)

Description

[Kris Maglione [:kmag]]

This is happening when a combination of two extensions are installed, Extension Test and Yahoo Toolbar 4.0.0. I haven't figured out exactly what's causing it, but the proximal cause is that having Extension Test installs somehow causes the `owners` property of the left data structure to be `null`, which diff.js interprets as an object, and tries to read the properties of.

I've given up trying to understand the flow that leads to this situation, since every bit of related SDK code is arcane, highly abstract, and completely undocumented. In any case, I'm fairly certain that diff.js needs to treat `null` the same as it treats `undefined`, rather than the same as it treats objets.

  Message: TypeError: can't convert null to object
  Stack:
    calculate@resource://gre/modules/addons/XPIProvider.jsm -> file:///home/kris/review-profiles/firefox36/extensions/jid1-XdW6rSDieZeTkA@jetpack/bootstrap.js -> resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/diffpatcher/diff.js:24:3
calculate/<@resource://gre/modules/addons/XPIProvider.jsm -> file:///home/kris/review-profiles/firefox36/extensions/jid1-XdW6rSDieZeTkA@jetpack/bootstrap.js -> resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/diffpatcher/diff.js:36:17
calculate@resource://gre/modules/addons/XPIProvider.jsm -> file:///home/kris/review-profiles/firefox36/extensions/jid1-XdW6rSDieZeTkA@jetpack/bootstrap.js -> resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/diffpatcher/diff.js:29:3
calculate/<@resource://gre/modules/addons/XPIProvider.jsm -> file:///home/kris/review-profiles/firefox36/extensions/jid1-XdW6rSDieZeTkA@jetpack/bootstrap.js -> resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/diffpatcher/diff.js:36:17
calculate@resource://gre/modules/addons/XPIProvider.jsm -> file:///home/kris/review-profiles/firefox36/extensions/jid1-XdW6rSDieZeTkA@jetpack/bootstrap.js -> resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/diffpatcher/diff.js:29:3
calculate/<@resource://gre/modules/addons/XPIProvider.jsm -> file:///home/kris/review-profiles/firefox36/extensions/jid1-XdW6rSDieZeTkA@jetpack/bootstrap.js -> resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/diffpatcher/diff.js:36:17
calculate@resource://gre/modules/addons/XPIProvider.jsm -> file:///home/kris/review-profiles/firefox36/extensions/jid1-XdW6rSDieZeTkA@jetpack/bootstrap.js -> resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/diffpatcher/diff.js:29:3
@resource://gre/modules/addons/XPIProvider.jsm -> file:///home/kris/review-profiles/firefox36/extensions/jid1-XdW6rSDieZeTkA@jetpack/bootstrap.js -> resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/diffpatcher/diff.js:18:10
dispatch@resource://gre/modules/addons/XPIProvider.jsm -> file:///home/kris/review-profiles/firefox36/extensions/jid1-XdW6rSDieZeTkA@jetpack/bootstrap.js -> resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/method/core.js:119:12
reactor<.onStep@resource://gre/modules/addons/XPIProvider.jsm -> file:///home/kris/review-profiles/firefox36/extensions/jid1-XdW6rSDieZeTkA@jetpack/bootstrap.js -> resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/ui/frame/model.js:127:19
Reactor.prototype.onNext@resource://gre/modules/addons/XPIProvider.jsm -> file:///home/kris/review-profiles/firefox36/extensions/jid1-XdW6rSDieZeTkA@jetpack/bootstrap.js -> resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/event/utils.js:248:3
Reactor.prototype.run/<@resource://gre/modules/addons/XPIProvider.jsm -> file:///home/kris/review-profiles/firefox36/extensions/jid1-XdW6rSDieZeTkA@jetpack/bootstrap.js -> resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/event/utils.js:251:32
emit@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/event/core.js:97:9
receive@resource://gre/modules/addons/XPIProvider.jsm -> file:///home/kris/review-profiles/firefox36/extensions/jid1-XdW6rSDieZeTkA@jetpack/bootstrap.js -> resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/event/utils.js:115:5
transform/next@resource://gre/modules/addons/XPIProvider.jsm -> file:///home/kris/review-profiles/firefox36/extensions/jid1-XdW6rSDieZeTkA@jetpack/bootstrap.js -> resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/event/utils.js:36:24
map/<@resource://gre/modules/addons/XPIProvider.jsm -> file:///home/kris/review-profiles/firefox36/extensions/jid1-XdW6rSDieZeTkA@jetpack/bootstrap.js -> resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/event/utils.js:60:60
transform/<@resource://gre/modules/addons/XPIProvider.jsm -> file:///home/kris/review-profiles/firefox36/extensions/jid1-XdW6rSDieZeTkA@jetpack/bootstrap.js -> resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/event/utils.js:43:29
emit@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/event/core.js:97:9
receive@resource://gre/modules/addons/XPIProvider.jsm -> file:///home/kris/review-profiles/firefox36/extensions/jid1-XdW6rSDieZeTkA@jetpack/bootstrap.js -> resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/event/utils.js:115:5
merges/</<@resource://gre/modules/addons/XPIProvider.jsm -> file:///home/kris/review-profiles/firefox36/extensions/jid1-XdW6rSDieZeTkA@jetpack/bootstrap.js -> resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/event/utils.js:187:31
emit@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/event/core.js:97:9
receive@resource://gre/modules/addons/XPIProvider.jsm -> file:///home/kris/review-profiles/firefox36/extensions/jid1-XdW6rSDieZeTkA@jetpack/bootstrap.js -> resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/event/utils.js:115:5
lift/</<@resource://gre/modules/addons/XPIProvider.jsm -> file:///home/kris/review-profiles/firefox36/extensions/jid1-XdW6rSDieZeTkA@jetpack/bootstrap.js -> resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/event/utils.js:163:7
emit@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/event/core.js:97:9
receive@resource://gre/modules/addons/XPIProvider.jsm -> file:///home/kris/review-profiles/firefox36/extensions/jid1-XdW6rSDieZeTkA@jetpack/bootstrap.js -> resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/event/utils.js:115:5
transform/next@resource://gre/modules/addons/XPIProvider.jsm -> file:///home/kris/review-profiles/firefox36/extensions/jid1-XdW6rSDieZeTkA@jetpack/bootstrap.js -> resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/event/utils.js:36:24
filter/<@resource://gre/modules/addons/XPIProvider.jsm -> file:///home/kris/review-profiles/firefox36/extensions/jid1-XdW6rSDieZeTkA@jetpack/bootstrap.js -> resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/event/utils.js:53:7
transform/<@resource://gre/modules/addons/XPIProvider.jsm -> file:///home/kris/review-profiles/firefox36/extensions/jid1-XdW6rSDieZeTkA@jetpack/bootstrap.js -> resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/event/utils.js:43:29
emit@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/event/core.js:97:9
receive@resource://gre/modules/addons/XPIProvider.jsm -> file:///home/kris/review-profiles/firefox36/extensions/jid1-XdW6rSDieZeTkA@jetpack/bootstrap.js -> resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/event/utils.js:115:5
lift/</<@resource://gre/modules/addons/XPIProvider.jsm -> file:///home/kris/review-profiles/firefox36/extensions/jid1-XdW6rSDieZeTkA@jetpack/bootstrap.js -> resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/event/utils.js:163:7
emit@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/event/core.js:97:9
receive@resource://gre/modules/addons/XPIProvider.jsm -> file:///home/kris/review-profiles/firefox36/extensions/jid1-XdW6rSDieZeTkA@jetpack/bootstrap.js -> resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/event/utils.js:115:5
InputPort.prototype.observe@resource://gre/modules/addons/XPIProvider.jsm -> file:///home/kris/review-profiles/firefox36/extensions/jid1-XdW6rSDieZeTkA@jetpack/bootstrap.js -> resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/input/system.js:100:5

Comment 1

[Kris Maglione [:kmag]]

Apparently this is caused by registering a web progress listener for content windows and accessing the `webProgress.DOMWindow` property from the `onStateChange` callback.

Comment 2

[Erik Vold [:erikvold] (please needinfo? me)]

(In reply to Kris Maglione [:kmag] from comment #1)
> Apparently this is caused by registering a web progress listener for content
> windows and accessing the `webProgress.DOMWindow` property from the
> `onStateChange` callback.

Hey Kris, it sounds like you've identified two issues here, does that seem right?

1. "I'm fairly certain that diff.js needs to treat `null` the same as it treats `undefined`" sounds correct
2. The statement above, which I'm not sure I follow exactly, could you explain this in some more detail?

Comment 3

[Kris Maglione [:kmag]]

I think it's only one issue, the treatment of nulls in diff.js. comment 1 is just about what piece of my code seems to trigger it, which is essentially doing this for all content windows:

let listener = {
    onStateChange: wrapCallback(function onStateChange(webProgress, request, stateFlags, status) {
        var window = webProgress.DOMWindow;
    }
};
window.QueryInterface(Ci.nsIInterfaceRequestor)
      .getInterface(Ci.nsIWebNavigation)
      .QueryInterface(Ci.nsIWebProgress)
      .addProgressListener(listener, Ci.nsIWebProgress.NOTIFY_STATE_DOCUMENT);

Changing `DOMWindow` to `DOMWindowID` fixes the problem in 36, but not in 39. In 39, it happens even without my add-on installed, as long as I start with the `-jsconsole` flag.

Comment 4

[Firefox Bug Husbandry Bot]

https://bugzilla.mozilla.org/show_bug.cgi?id=1399562