604362 - xpcom module is broken (making a bunch of other modules broken)

Status: RESOLVED FIXED

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

Description

[Dietrich Ayala (:dietrich)]

cfx testpkgs -F anymodulethatusesxpcom

results in:

error: An exception occurred.
Traceback (most recent call last):
  File "resource://testpkgs-jetpack-core-lib/xpcom.js", line 41, in 
TypeError: Cu.import is not a function

and no tests are run.

Comment 1

[Dietrich Ayala (:dietrich)]

Irakli says it's working ok on Mac. (panel for example had a test failure, but passed all the rest)

Comment 2

[Dietrich Ayala (:dietrich)]

Also, might be a nightly build problem, not related to Jetpack at all.

Comment 3

[Dietrich Ayala (:dietrich)]

Everything still works on the last beta, so maybe a problem with the nightly. I didn't think I'd updated, so didn't suspect it at first.

Comment 4

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

I think there's a nightly problem. Firebug is also broken.

More in bug 604233 & http://groups.google.com/group/firebug/browse_thread/thread/e29d9d71f8154ded

Comment 5

[Rob Campbell [:rc] (:robcee)]

seeing bustage in several of my jetpacks. They don't appear to run and didn't see anything on the Error Console.

Coming from today's nightly.

today's build: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b8pre)
Gecko/20101014 Firefox/4.0b8pre
built from:
http://hg.mozilla.org/mozilla-central/rev/ad0a0be8be74

Comment 6

[Atul Varma [:atul]]

ok, just talked to mrbkap, apparently his compartments stuff did just land and in particular massively changed the behavior of components.utils.evalinsandbox. he will be talking to me soon about it and we'll figure out how to proceed.

Comment 7

[Atul Varma [:atul]]

Just talked to mrbkap again and we are crossing our fingers that we're broken for the same reason that mozmill is broken in bug 604363. mrbkap is working on a fix for that ASAP so we'll see if it fixes us too.

i'll also try to do some exploratory poking-around on my own to find out what i can.

Comment 8

[Atul Varma [:atul]]

Yup, this is actually a duplicate of bug 604363. Or at least, mrbkap's fix for 604363 was pushed here:

  http://hg.mozilla.org/mozilla-central/rev/ed24c21e6497

and I pulled mozilla-central and built Firefox and re-ran cfx testall and the "Cu.import is not a function" is no more. I have no idea if the fix actually fixes bug 604363, though, since that's Mozmill bustage, but since it fixes this, I'll mark it as resolved.

Note, though, that this bugfix won't actually appear on nightlies until the next nightlies are generated.

Comment 10

[Rob Campbell [:rc] (:robcee)]

reopening as that fix has not fixed jetpacks in latest (20101015) mac nightly.

Comment 11

[Mike Beltzner [:beltzner, not reading bugmail]]

Might this actually be more related to bug 604368?

Comment 12

[Peter Van der Beken [:peterv]]

Is the problem in comment 0 not fixed, or is there some other brokenness?

Comment 13

[Dietrich Ayala (:dietrich)]

(In reply to comment #12)
> Is the problem in comment 0 not fixed, or is there some other brokenness?

The problem in comment #0 is not fixed in the 10/15 nightly.

Comment 14

[Peter Van der Beken [:peterv]]

Atul, can you retest and see whether it's still broken for you too?

Comment 15

[Atul Varma [:atul]]

ugh, it is indeed broken for me and i do not know why. I have a debug build on my own system that's built as of changeset ed24c21e6497 (mrbkap's fix) on m-c and the Cu.import error is not raised, yet the latest nightly does indeed still raise the Cu.import error.

Comment 16

[Atul Varma [:atul]]

Hmm, so I pulled from m-c again and the very latest one, bb01920c2708, does work (Cu.import is defined). Yet building Firefox off the nightly revision on my machine, which is rev 19cb42fa4554, does not work.

That means that sometime between changeset ed24c21e6497 (mrbkap's initial fix) and 19cb42fa4554 (the nightly), a regression for this bug occurred, and from that point to bb01920c2708, the bug was fixed once more. Yet I'm not seeing any changesets that could have made any obvious fixes/regressions to the bug.

I am really confused.

Comment 17

[Drew Willcoxon :adw]

It looks like injecting Cu into the sandbox strips it of its `import` property.  If you run this in the JS console of the most recent nightly you can see Cu.import is undefined, but C.utils.import is OK:

  var Cc = Components.classes;
  var Ci = Components.interfaces;
  var sp = Cc["@mozilla.org/systemprincipal;1"].
           createInstance(Ci.nsIPrincipal);
  var s = Components.utils.Sandbox(sp);
  s.a = [];
  s.Cu = Components.utils;
  s.C = Components;
  Components.utils.evalInSandbox(
    "a.push(Cu.import || '(none)');" +
    "a.push(C.utils.import || '(none)');",
    s);
  s.a.join("\n");

Injecting Cu into chrome sandboxes like this is how Cuddlefish works.

I saw this recent test fix by Blake that I thought was related:

http://hg.mozilla.org/mozilla-central/rev/83a06cce6b14

He's checking that expandos are visible in a chrome sandbox, kind of like we are.  The difference is that his test doesn't inject into the sandbox the object on which the expando is defined, like we do.  this.document is available inside the sandbox (I think) because the sandbox is created with a window.

Comment 18

[Peter Van der Beken [:peterv]]

This happens because for Cu we're for now creating a new XPCWrappedNative in the sandbox, and that hasn't been QI'ed to nsIXPCComponents_Utils. Components has classinfo, so that doesn't need to be QI'ed, and then getting .utils will QI in the sandbox for us.

Comment 19

[Peter Van der Beken [:peterv]]

Attachment: v1

Comment 20

[Peter Van der Beken [:peterv]]

BTW, even with that patch running the tests fails with:

Traceback (most recent call last):
  File "resource://testpkgs-jetpack-core-lib/list.js", line 166, in 
    _iterator: function _iterator(onKeys) {
  File "resource://testpkgs-jetpack-core-lib/traits.js", line 151, in compose
    return Composition(_compose.apply(null, traits.map(TraitDescriptor)));
  File "resource://testpkgs-jetpack-core-lib/traits.js", line 75, in TraitDescriptor
    ) ? object._trait(TraitDescriptor) : _trait(object)
  File "resource://testpkgs-jetpack-core-lib/traits/core.js", line 136, in trait
    let descriptor = getOwnPropertyDescriptor(properties, key);
  File "resource://testpkgs-jetpack-core-lib/list.js", line 112, in 
    get length() this._keyValueMap.length,
TypeError: this._keyValueMap is null
No tests were run
FAIL

It doesn't look like this is a regression from compartments though. Unless this is a known problem we should spin it off to a separate bug anyway.

Comment 21

[Peter Van der Beken [:peterv]]

http://hg.mozilla.org/tracemonkey/rev/b4b5641fa789
http://hg.mozilla.org/mozilla-central/rev/f9f10c04dceb

Comment 22

[Mike Beltzner [:beltzner, not reading bugmail]]

We need this on the beta7 relbranch, as well, which is:

mozilla-central GECKO20b7pre_20101006_RELBRANCH

Comment 23

[Myk Melez [:myk] [@mykmelez]]

The Add-on SDK is no longer a Mozilla Labs experiment and has become a big enough project to warrant its own Bugzilla product, so the "Add-on SDK" product has been created for it, and I am moving its bugs to that product.

To filter bugmail related to this change, filter on the word "looptid".