807548 - error: addon-sdk: TEST FAILED: test-content-proxy.testCollections2 (failure)

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect, P1)

Description

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

error: addon-sdk: TEST FAILED: test-content-proxy.testCollections2 (failure)
error: addon-sdk: fail: body.childNodes is iterable
info: addon-sdk: Traceback (most recent call last):
  File "resource://39cb27c8-733d-4235-8881-144f986da8a6-at-jetpack/addon-sdk/lib/sdk/timers.js", line 31, in notify
    callback.apply(null, args);
  File "resource://39cb27c8-733d-4235-8881-144f986da8a6-at-jetpack/addon-sdk/lib/sdk/content/worker.js", line 109, in null
    self._emit.apply(self, JSON.parse(args));
  File "resource://39cb27c8-733d-4235-8881-144f986da8a6-at-jetpack/addon-sdk/lib/sdk/deprecated/events.js", line 123, in _emit
    return this._emitOnObject.apply(this, args);
  File "resource://39cb27c8-733d-4235-8881-144f986da8a6-at-jetpack/addon-sdk/lib/sdk/deprecated/events.js", line 153, in _emitOnObject
    listener.apply(targetObj, params);
  File "resource://39cb27c8-733d-4235-8881-144f986da8a6-at-jetpack/addon-sdk/lib/sdk/content/worker.js", line 228, in portEmit
    self._addonWorker._onContentScriptEvent.apply(self._addonWorker, arguments);
  File "resource://39cb27c8-733d-4235-8881-144f986da8a6-at-jetpack/addon-sdk/lib/sdk/content/worker.js", line 560, in _onContentScriptEvent
    this._port._emit.apply(this._port, arguments);
  File "resource://39cb27c8-733d-4235-8881-144f986da8a6-at-jetpack/addon-sdk/lib/sdk/deprecated/events.js", line 123, in _emit
    return this._emitOnObject.apply(this, args);
  File "resource://39cb27c8-733d-4235-8881-144f986da8a6-at-jetpack/addon-sdk/lib/sdk/deprecated/events.js", line 153, in _emitOnObject
    listener.apply(targetObj, params);
  File "resource://39cb27c8-733d-4235-8881-144f986da8a6-at-jetpack/addon-sdk/tests/test-content-proxy.js", line 84, in null
    test.assert(data.assertion, data.msg);
  File "resource://39cb27c8-733d-4235-8881-144f986da8a6-at-jetpack/addon-sdk/lib/sdk/deprecated/unit-test.js", line 137, in assert
    this.fail(message);
  File "resource://39cb27c8-733d-4235-8881-144f986da8a6-at-jetpack/addon-sdk/lib/sdk/deprecated/unit-test.js", line 75, in fail
    this.console.trace();


This test is failing on Mozilla Central for all of the runs that have completed so far in the Jetpack tree.

Comment 1

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

First m-c push that shows it is https://tbpl.mozilla.org/?noignore=1&jobname=jetpack&rev=71290adea0c1

https://tbpl.mozilla.org/?noignore=1&jobname=jetpack&rev=2cb6d26fcfe7 does not show it.

https://tbpl.mozilla.org/?noignore=1&jobname=jetpack&rev=626b7ea149bc was between those two, but did not run tests.

Comment 2

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Last good on inbound: https://tbpl.mozilla.org/?noignore=1&tree=Mozilla-Inbound&jobname=jetpack&rev=5a346b585255

First bad on inbound: https://tbpl.mozilla.org/?noignore=1&tree=Mozilla-Inbound&jobname=jetpack&rev=4ee6b1acd5ea

Comment 3

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Okay, I added some logging to testCollections2, and the "count" variable has different values on various branches.

On release, count == 3
On beta, aurora, and nightlies before a few days ago, count == 6
On recent nightlies, count == 7

The test only knows about 3 and 6.


On recent nightlies, the for() loop that goes through body.childNodes contains:
0, 1, 2, constructor, item, iterator, length

On beta, aurora, and older nightlies, the loop contains:
0, 1, 2, length, iterator, item


So constructor is now included in body.childNodes, while it wasn't previously.


Not sure what this means for us, but there it is...

Comment 4

[Peter Van der Beken [:peterv]]

Hmm, that doesn't seem right. Constructor shouldn't be enumerable :-/.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Is this just for xrays?  I certainly can't reproduce this with a basic testcase in a web page....

Can someone link me to the testcase source, please?  It doesn't seem to be in m-c.

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

(In reply to Boris Zbarsky (:bz) from comment #5)
> Is this just for xrays?  I certainly can't reproduce this with a basic
> testcase in a web page....
> 
> Can someone link me to the testcase source, please?  It doesn't seem to be
> in m-c.

Here's the test: https://github.com/mozilla/addon-sdk/blob/master/test/test-content-proxy.js#L567-597

createProxyTest() is defined here: https://github.com/mozilla/addon-sdk/blob/master/test/test-content-proxy.js#L13-57

Comment 7

[Boris Zbarsky [:bzbarsky]]

Yeah, that seems to be going through Xrays...  Let me see if I can reproduce.

Comment 8

[Boris Zbarsky [:bzbarsky]]

So XrayEnumerateNativeProperties does this:

  if (type == eInterfacePrototype &&
      nativePropertyHooks->mConstructorID != constructors::id::_ID_Count &&
      !AddStringToIDVector(cx, props, "constructor")) {
    return false;
  }

The only caller is DOMXrayTraits::enumerateNames.

The problem is that we don't pass in the enumeration flags, so can't tell what we should be doing here.  Fix coming up.

Comment 9

[Boris Zbarsky [:bzbarsky]]

Attachment: Enumerating an Xray should not see the 'constructor' property on DOM prototypes unless we're told to include non-enumerable properties.

Comment 10

[Boris Zbarsky [:bzbarsky]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/af47a345a5be

Comment 11

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/af47a345a5be