Closed Bug 607767 Opened 9 years ago Closed 9 years ago

test-observer-service fails on "observer-service.add() should pass subject (({}) != ({}))"

Categories

(Core :: XPConnect, defect)

x86
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla2.0b7
Tracking Status
blocking2.0 --- beta7+

People

(Reporter: myk, Assigned: mrbkap)

References

Details

Attachments

(1 file, 2 obsolete files)

On the latest Firefox trunk nightly, test-observer-service fails on "observer-service.add() should pass subject (({}) != ({}))".

--------------------------------------------------------------------------------
(addon-sdk)myk@myk:~/Projects/addon-sdk/packages/jetpack-core$ cfx test -F observer-service
Using binary at '/home/myk/bin/firefox'.
Using profile at '/tmp/tmpjxbBsm.mozrunner'.

(firefox-bin:23173): GLib-WARNING **: g_set_prgname() called multiple times
......error: TEST FAILED: test-observer-service.testObserverService (failure)
error: fail: observer-service.add() should pass subject (({}) != ({}))
info: Traceback (most recent call last):
  File "resource://jetpack-core-jetpack-core-lib/timer.js", line 64, in notifyOnTimeout
    this._callback.apply(null, this._params);
  File "resource://jetpack-core-jetpack-core-lib/unit-test.js", line 255, in anonymous
    timer.setTimeout(function() { onDone(self); }, 0);
  File "resource://jetpack-core-jetpack-core-lib/unit-test.js", line 280, in runNextTest
    self.start({test: test, onDone: runNextTest});
  File "resource://jetpack-core-jetpack-core-lib/unit-test.js", line 298, in start
    this.test.testFunction(this);
  File "resource://jetpack-core-jetpack-core-lib/unit-test.js", line 63, in runTest
    test(runner);
  File "resource://jetpack-core-jetpack-core-tests/test-observer-service.js", line 55, in anonymous
    "observer-service.add() should pass subject");
  File "resource://jetpack-core-jetpack-core-lib/unit-test.js", line 227, in assertEqual
    this.fail(message);
  File "resource://jetpack-core-jetpack-core-lib/unit-test.js", line 145, in fail
    console.trace();
.....
11 of 12 tests passed.
FAIL
Total time: 1.513529 seconds
Program terminated unsuccessfully.
--------------------------------------------------------------------------------

This looks like one of the failures in bug 607764 and another in bug 607113, suggesting a common cause.

Atul, Peter: any thoughts on what that common cause might be?
Myk, can you explain how this can be reproduced? Where should one pull from,
what needs to be built, etc etc?
Pull from http://github.com/mozilla/addon-sdk or its Hg mirror at
https://hg.mozilla.org/labs/jetpack-sdk/.  Then enter your local clone's
directory in a terminal and run "source bin/activate" ("bin\activate.bat" on
Windows).  Then enter packages/jetpack-core/ and run "cfx test -F symbiont" or
"cfx test -F observer-service".

That command will look for Firefox, but if it doesn't find it (or if you think
it might be finding a different binary than the one you want to test with),
specify "--binary=/path/to/binary" on that command line.
Erm, copy/paste fail; I meant to say that you should run "cfx test -F observer-service" as the last step.
Peter, this is more sandbox inconsistency.  In this case, an nsIURI is being passed as the subject to nsIObserverService.notifyObservers outside a sandbox.  Inside the sandbox, an nsIObserver grabs this subject, and outside the sandbox the original nsIURI is compared to the subject grabbed inside the sandbox using ==, and they're not equal.

e.g.:

  var Cc = Components.classes;
  var Ci = Components.interfaces;
  var sp = Cc["@mozilla.org/systemprincipal;1"].
           createInstance(Ci.nsIPrincipal);
  var s = Components.utils.Sandbox(sp);
  Components.utils.evalInSandbox(
    'var global = this;' +
    'var os = Components.classes["@mozilla.org/observer-service;1"].' +
    '         getService(Components.interfaces.nsIObserverService);' +
    'os.addObserver({' +
    '   observe: function (subject) global.subject = subject' +
    '}, "foo", false);',
    s
  );
  var subject = Cc["@mozilla.org/network/io-service;1"].
                getService(Ci.nsIIOService).
                newURI("http://www.foo.com", null, null);
  Cc["@mozilla.org/observer-service;1"].
    getService(Ci.nsIObserverService).
    notifyObservers(subject, "foo", "");
  s.subject == subject; /* false */

Incidentally, s.subject is simply an nsISupports.  It has to be QI'ed to nsIURI.

Here's an example without sandboxes that works as expected:

  var Cc = Components.classes;
  var Ci = Components.interfaces;
  var os = Cc["@mozilla.org/observer-service;1"].
           getService(Ci.nsIObserverService);
  var osSubject = null;
  os.addObserver({
    observe: function (subject) osSubject = subject
  }, "foo", false);
  var subject = Cc["@mozilla.org/network/io-service;1"].
                getService(Ci.nsIIOService).
                newURI("http://www.foo.com", null, null);
  os.notifyObservers(subject, "foo", "");
  osSubject == subject; /* true */

Here, osSubject is already an nsIURI and doesn't need to be QI'ed.
With the fixes for bugs 607799, 607863, 606585, and bug 608142 this still fails
with the same message as in comment 0.
Attached patch Proposed fix (obsolete) — Splinter Review
The problem is that we thought that the URI object was an outer window because NoHelper objects had an innerObject hook (!). This removes the ability for XPConnect objects to confuse us like that any more.
Assignee: nobody → mrbkap
Status: NEW → ASSIGNED
Attachment #486991 - Flags: review?(peterv)
Attachment #486991 - Flags: review?(peterv) → review+
With mrbkap's patch applied the observer-service tests pass.
http://hg.mozilla.org/mozilla-central/rev/1c265164a571
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Component: Jetpack SDK → XPConnect
Product: Mozilla Labs → Core
QA Contact: jetpack-sdk → xpconnect
Target Milestone: -- → mozilla2.0b7
This patch caused the Mochitest-5 boxes to go orange on Linux opt and Linux64 opt (other platforms pending right now).  I haven't backed the patch out, but I'm reopening this bug to alert the right people.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
blocking2.0: --- → beta7+
This also seems to have caused another failure on osx:

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1288552231.1288554189.10288.gz
Ugh, nsStandardURL claims to implement nsIClassInfo but doesn't actually return any interfaces from GetInterfaces (it only wants the MAIN_THREAD_ONLY flag); so the new XPCWrappedNative doesn't actually know that it implements nsIURI.
mrbkap's updated fix for this fixes this testcase (patch yet to be attached here :).
Attached patch Proposed fix v2 (obsolete) — Splinter Review
This patch attempts to detect this case and if the object's classinfo lies about the interfaces it implements, we just use the set from the old wrapped native. Note that the returned wrapped native *must* have nsISupports in its set.
Attachment #486991 - Attachment is obsolete: true
Attachment #487439 - Flags: review?(jst)
Attachment #487439 - Flags: review?(jst) → review+
Attached patch Proposed fix v3Splinter Review
This fixes confusion with the history object and also fixes a webconsole test.
Attachment #487439 - Attachment is obsolete: true
Attachment #487488 - Flags: review?(jst)
Attachment #487488 - Flags: review?(jst) → review+
http://hg.mozilla.org/mozilla-central/rev/9ec3b67b2fd7
Status: REOPENED → RESOLVED
Closed: 9 years ago9 years ago
Resolution: --- → FIXED
Depends on: 609362
You need to log in before you can comment on or make changes to this bug.