xpconnect does not correctly reflect nsIClassInfo interface

VERIFIED FIXED

Status

()

Core
XPConnect
VERIFIED FIXED
17 years ago
17 years ago

People

(Reporter: John Bandhauer, Assigned: David Bradley)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

17 years ago
I screwed up in building the mechanism for reflecting the nsIClassInfo interface
of a wrapped native. Are rule is that classes that *have* classinfo will have an
wrappednativeproto. That wrappednativeproto may or may not be shared (in the
plugin instance case we don't want to share the proto because that will screwup
our the delegation after our JS proto hacking). If the class has no classinfo
then we build a wrapper without a wrappednativeproto. 

So, there are 3 types of wrappers in respect to their protos: 
 1) shared proto 
 2) non-shared proto
 3) no proto

In the case where we are trying to build a wrapper for the nsIClassInfo
interface (i.e. we QI'd an object to nsIClassInfo), the current code will do
type '2'. But it should do type '3'.

The object we are trying to wrap exposes nsIClassInfo. But that classinfo does
not *describe* the current object. It describes other objects.

Recall that we have decreed that it is OK to break the xpcom identity rules with
respect to nsIClassInfo. For a given class we say that all instances of that
class can be QI'd to a single instance object that implements nsIClassInfo. That
classinfo object will describe the attributes of the class. But it does not
describe the attributes of itself.

In the 'normal' case we use the info from the classinfo object to know how to
reflect native instance objects into JS. We need special handling to deal with
reflecting the nsIClassInfo interface itself. 

When we are building a wrapper for the nsIClassInfo interface, the current code
does the right thing in trying to not *share* the wrappednativeproto of the
instance objects. But, it is wrong to even build a wrappednativeproto at all.
The whole point of the wrappednativeproto is to allow us to share metainfo about
the class. But the metainfo here describes the class of the instances, not the
class of the classinfo objects.

Here is a simple test case...

///////////////////////////////////////////////////////////////////////
const nsIClassInfo = Components.interfaces.nsIClassInfo;

var foo = new Components.Exception("foo exception");
var bar = new Components.Exception("bar exception");
print(foo);
print(bar);
print(foo == bar);

var ciFoo = foo.QueryInterface(nsIClassInfo);
var ciBar = bar.QueryInterface(nsIClassInfo);

print(ciFoo);
print(ciBar);
print(ciFoo == ciBar);
///////////////////////////////////////////////////////////////////////

I chose Components.Exception because I know that the XPCException class
implements nsIClassInfo.

This should print something like:

[Exception... "foo exception"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  locati
on: "JS frame :: ci.js :: <TOP_LEVEL> :: line 19"  data: no]
[Exception... "bar exception"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  locati
on: "JS frame :: ci.js :: <TOP_LEVEL> :: line 20"  data: no]
false
[xpconnect wrapped nsIClassInfo @ 0x1137be0]
[xpconnect wrapped nsIClassInfo @ 0x1137be0]
true

In the current code it will assert instead because it thinks that ciFoo
implements nsIXPCException which has a toString method that the code tries to
call. But this turns out not to be the case.

The fix for all this is simple. We just use the knowledge that we are building a
wrapper around nsIClassInfo to select type '3' instead of type '2'. 
I'll attach a patch.
(Reporter)

Comment 1

17 years ago
Created attachment 44747 [details] [diff] [review]
proposed fix
sr=jst
(Assignee)

Comment 3

17 years ago
r=dbradley
Status: NEW → ASSIGNED
(Reporter)

Comment 4

17 years ago
dbradley: can you integrate this into your other reviewed xpconnect changes and 
check this in for me? Thanks.
(Assignee)

Comment 5

17 years ago
Will do.
(Assignee)

Comment 6

17 years ago
Patch commited
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED

Comment 7

17 years ago
Marking Verified - 
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.