311240 - XPCOMJavaProxy "equals()" method doesn't check actual XPCOM object

Status: RESOLVED FIXED

(Core Graveyard :: Java to XPCOM Bridge, defect)

Description

[jhp (no longer active)]

The |XPCOMJavaProxy.proxyEquals()| handles the |Object.equals()| calls.  But it
only checks if the Java objects are the same;  the method doesn't check if the
underlying XPCOM objects are the same (in the case where the Java objects are
different but representing the same XPCOM object).

Comment 1

[jhp (no longer active)]

Attachment: patch

Comment 2

[jhp (no longer active)]

Still not completely sure about this patch, but whatever the final fix is, it should be checked in to 1.8 branch.  Adding "javaconnect-to-branch" to whiteboard.

Comment 3

[jhp (no longer active)]

OK, so now I understand better why we need this patch.  The issue presents itself when we have an XPCOM object that implements multiple interfaces.  If passed to Java as InterfaceA, JavaXPCOM creates a Java proxy for that interface.  Later, if that XPCOM object is passed to Java as InterfaceB, JavaXPCOM creates another Java proxy for that interface.

So we have two Java proxies that implement different interfaces but represent the same object.  If we try to compare the two Java proxies for equality, it fails.  This patch overrides the |equals()| method to compare the underlying XPCOM objects.

It would be great if I could add an interface to an existing Java proxy, but that doesn't seem to be possible.

What would be best would be to create a Java proxy with all the interfaces that an XPCOM object implements.  But it seems that the only way to get all the implemented interfaces is if the XPCOM object implements "nsIClassInfo", correct?  Is there another way to get the implemented interfaces of an object?

Comment 4

[Benjamin Smedberg]

If an object doesn't implement classinfo, then you're out of luck. And there are cases where classinfo doesn't list all the interfaces implemented by an object, since we overload the meaning of classinfo to support DOM scripting :-(

xpconnect solves this by dynamically adding tearoffs to an object, but if Java can't do that then this patch is the right way to go.

Comment 5

[jhp (no longer active)]

If I understand tearoffs correctly, the it won't work with java.lang.Object's |equals| method.  So I'll just have to stick with this patch to overload |equals| to check the XPCOM objects themselves.

Comment 6

[jhp (no longer active)]

I think I've asked this before, but just to be sure: when comparing two XPCOM objects together, I need to QI them to |nsISupports| first, right?

Comment 7

[Benjamin Smedberg]

Yes.

Comment 8

[jhp (no longer active)]

Attachment: patch v2

Turns out I was doing quite a few things wrong.  First, I should be saving the root pointer of the XPCOM object (i.e. QI'd to nsISupports).  Second, in the convenience methods (i.e. |GetComponentManager|), I was creating a new Java proxy each time; in this patch, I make all requests for Java objects go though |GetNewOrUsedJavaObject|, which takes care of searching the hash tables and creating new Java proxies if necessary.

Comment 9

[Benjamin Smedberg]

Comment on attachment 207551 [details] [diff] [review]
patch v2

I don't actually know all the java proxy stuff very well, but this looks good from an XPCOM identity point of view.

a=me for branches as this is javaxpcom-only

Comment 10

[jhp (no longer active)]

Checked in to trunk.

Comment 11

[Benjamin Smedberg]

Fixed on MOZILLA_1_8_BRANCH and MOZILLA_1_8_0_BRANCH.