Comparing XPCOM Objects broken

[Philipp Kewisch]

Hello Folks,

I haven't found a regression range for this yet, but I just noticed
that the method we've been using to compare wrapped xpcom objects
seems to be broken. Maybe someone can help out and tell me how to
better do this.

The background is that we have a few places where we need to compare
two objects for equality, in Javascript.The naive approach would be to
use a == compare, but this doesn't work since an object can be wrapped
in multiple ways (either not wrapped, or using different interfaces,
or even using the same interfaces but wrapped differently), which
causes the compare to fail.

What has been working before, but has always been giving us a strange
feeling is using supports-interface-pointer:
function compareObject(a, b, iid) {
var sip1 = Components.classes["@mozilla.org/supports-interface-
pointer;1"]
.createInstance(Components.interfaces.nsISupportsInterfacePointer);

var sip2 = Components.classes["@mozilla.org/supports-interface-
pointer;1"]
.createInstance(Components.interfaces.nsISupportsInterfacePointer);

sip1.data = a;
sip2.data = b;
sip1.dataIID = iid;
sip2.dataIID = iid;

return sip1.data == sip2.data;
}

Using comm-central, this no longer works. Now the big question is, how
do we solve this using todays code? Is there some API we have missed?
Working around this by using a member of the object for comparison
would mean a substantial amount of work so I would like to avoid that.

Thanks in advance,
Philipp

[Boris Zbarsky]

On 5/19/11 4:07 AM, Philipp Kewisch wrote:
> Using comm-central, this no longer works.

In what cases? What are the objects in question when this no longer works?

-Boris

[Ehsan Akhgari]

I was under the impression that comparing XPCOM objects should consist
of QIing them to nsISupports and compare the pointer values.

Ehsan

[Boris Zbarsky]

On 5/19/11 1:46 PM, Ehsan Akhgari wrote:
> I was under the impression that comparing XPCOM objects should consist
> of QIing them to nsISupports and compare the pointer values.

That's effectively what the posted code using
nsISupportsInterfacePointer is doing.

-Boris

[Philipp Kewisch]

On May 19, 2:54 pm, Boris Zbarsky <bzbar...@mit.edu> wrote:
> In what cases?  What are the objects in question when this no longer works?

I haven't tested it for other objects, but specifically for these it
doesn't work. The control flow is that I have an observer
(calAlarmMonitor) that registers with an observer service
(calAlarmService). Adding the observer works perfectly, but removing
the observer again doesn't remove it from the interface bag. The
remove function compares all array objects with the passed object to
remove the right one.

The visible effect is that a shutdown/startup of the alarm service
duplicates the alarms, since then two observers are registered. This
happens when the computer goes into sleep state and wakes up again.

This is the observer used:
http://mxr.mozilla.org/comm-central/source/calendar/base/src/calAlarmMonitor.js#57

It is added to the alarm service here:
http://mxr.mozilla.org/comm-central/source/calendar/base/src/calAlarmMonitor.js#100

Which delegates to the listener/interface bag:
http://mxr.mozilla.org/comm-central/source/calendar/base/src/calUtils.js#1177
Using the Interface calIAlarmServiceObserver:
http://mxr.mozilla.org/comm-central/source/calendar/base/public/calIAlarmService.idl#49

The compareObjects function is similar to the one posted above.

Sorry for the high number of involved files, I can try to create a
reduced testcase if needed. Let me know if I can provide additional
info.

[Neil]

Philipp Kewisch wrote:

>http://mxr.mozilla.org/comm-central/source/calendar/base/src/calAlarmMonitor.js#100
>
>
Weird. The alarm service is an XPCOM object, so it should be getting
wrappers for any passed-in objects anyway. And you definitely pass in
the same object twice, so you should get the same wrapper twice, or
failing that, they should both wrap the same wrapped JS object, and you
should therefore be able compare them directly with ==.

--
Warning: May contain traces of nuts.

[Philipp Kewisch]

On May 20, 12:26 pm, Neil <n...@parkwaycc.co.uk> wrote:
> failing that, they should both wrap the same wrapped JS object, and you
> should therefore be able compare them directly with ==.

Direct compare fails too. I also tried just QIing to nsISupports and
then comparing, but to no avail.

[Justin Lebar]

It would be really helpful if you could bisect this. I touched this code in 9ad5c138c2d5, and that might somehow be causing your problem, although I suspect that if it were, lots of other things would be broken.

[Philipp Kewisch]

On May 20, 2:35 pm, Justin Lebar <justin.le...@gmail.com> wrote:
> It would be really helpful if you could bisect this.  I touched this code in 9ad5c138c2d5, and that might somehow be causing your problem, although I suspect that if it were, lots of other things would be broken.

I'm having a bit trouble compiling older versions of comm-central with
mozilla-central, but when I started I had a mozilla-central revision
before 9ad5c138c2d5. It neither works on that version I had before,
nor on the latest mozilla-central so 9ad5c138c2d5 is neither the
culprit nor the savior.

I'll let you know when I found out when or if it used to work.

[Philipp Kewisch]

Back again. I'm having trouble finding the exact revision since my
special case requires a combination of working comm-central/mozilla-
central, but at least I'm a bit closer. This seems to have happened
somewhere during the component manifest merges, which makes it
especially troubling. Next I fear I need to make a reduced testcase
that depends only on mozilla-central.

Known Good: 2010-06-05

Known Bad: 2010-08-20

I'll let you know when I have found a closer regression range, that
includes actual revision numbers. Maybe someone already has an idea
what could be the cause with the above info.

Philipp

[Philipp Kewisch]

I couldn't get too close via bisect, due to build errors in mozilla-
central. Also those results don't really match with what I found out
later via testing nightlies. Anyway, here goes:

Last working revision:
changeset: 46996:fe832b373724
user: Benjamin Smedberg <benj...@smedbergs.us>
date: Thu Jun 10 12:51:43 2010 -0400
summary: Bug 570488 part B - fix and test loading XPT files from
JARs, needed for omnijar, r=mwu

First known bad revision:
changeset: 47029:014f02585f6d
user: Benjamin Smedberg <benj...@smedbergs.us>
date: Tue Jun 22 20:21:34 2010 -0400
summary: Rename NSGetModule to NSModule in various build scripts
for bug 568691.

Next, I tried nightly builds. As mentioned, the dates don't really fit
together with the above revisions, but maybe I tried the wrong
nightlies? I used the nightlies from
ftp://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2010/07/2010-xxxx-mozilla-central/

Last working Nightly:
Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:2.0b2pre) Gecko/20100701
Minefield/4.0b2pre

First bad Nightly:
Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:2.0b2pre) Gecko/20100702
Minefield/4.0b2pre

Anything else to note before I file a bug?

[Justin Lebar]

> Anything else to note before I file a bug?

I think you should go for it.

[Boris Zbarsky]

On 6/8/11 12:07 AM, Philipp Kewisch wrote:
> Last working Nightly:
> Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:2.0b2pre) Gecko/20100701
> Minefield/4.0b2pre
>
> First bad Nightly:
> Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:2.0b2pre) Gecko/20100702
> Minefield/4.0b2pre
>
> Anything else to note before I file a bug?

Nope. Just make sure to include the about:buildconfig strings from
those nightlies in the bug report.

-Boris

[Philipp Kewisch]

Bug is filed, see https://bugzilla.mozilla.org/show_bug.cgi?id=662915