237590 - nsIBinaryInputStream inherits from nsIInputStream but can't be QI'ed to it

Status: VERIFIED FIXED

(Core :: Networking, defect)

Description

[James Ross]

As http://lxr.mozilla.org/mozilla/source/xpcom/io/nsIBinaryInputStream.idl#37
shows, nsIBinaryInputStream inherits from nsIInputStream but attempting to QI it
gives this:

Component returned failure code: 0x80004002 (NS_NOINTERFACE)
[nsISupports.QueryInterface]" nsresult: "0x80004002 (NS_NOINTERFACE)" location:
"JS frame :: chrome://chatzilla/content/commands.js :: anonymous :: line 1402"
data: no

Comment 1

[James Ross]

Attachment: Adds nsIInputStream to the ISUPPORT list.

This allows the desired QI when tested here.

Comment 2

[Christian :Biesinger (don't email me, ping me on IRC)]

hm... from nsBinaryStreams.h:
// Derive from nsIObjectInputStream so this class can be used as a superclass
// by nsObjectInputStream.
class nsBinaryInputStream : public nsIObjectInputStream

so should this also support nsIObjectInputStream in QueryInterface?

Comment 3

[Darin Fisher]

Attachment: revised patch

Comment 4

[Brendan Eich [:brendan]]

Comment on attachment 144017 [details] [diff] [review]
revised patch

I'm not sure we want nsBinaryInputStream QI'ing to nsIObjectInputStream, nor
should nsBinaryOutputStream QI to nsIObjectOutputStream, because the
object-stream stubs call NS_NOTREACHED and return NS_ERROR_NOT_IMPLEMENTED.

It's true that the concrete class implements the interface.  If we do this,
then all is well in nsFastLoadFile.cpp.  If we don't include the
nsIObject*Stream interface in the corresponding NS_IMPL_ISUPPORTS calls in
nsBinaryStream.cpp, then I guess nsFastLoadFile.cpp needs to list 'em.

Thoughts?

/be

Comment 5

[Darin Fisher]

hmm... nsFastLoadFile.cpp does list nsIObject*Stream in its ISUPPORTS_INHERITED
macros.  so that's all fine-n-dandy.  however, i still think nsBinary*Stream
should list nsIObject*Stream in its QI list.  my reasoning is this: suppose in
some other component I want to create a class that is a wrapper for a
nsBinaryInputStream.  suppose I want to access
nsIObjectInputStream::GetBuffer/PutBuffer.  i'd need QI to work.

perhaps it should just be documented in nsBinary*Stream's contract that it does
not implement all of nsIObject*Stream.

Comment 6

[Darin Fisher]

or we could say that nsBinary*Stream inheriting from nsIObject*Stream is an
implementation detail of nsFastLoadFile.

Comment 7

[Brendan Eich [:brendan]]

Comment on attachment 144017 [details] [diff] [review]
revised patch

Sold.  Safe for 1.7 too, I'm approving as well as sr='ing.

/be

Comment 8

[Darin Fisher]

fixed-on-trunk for 1.7 final

Comment 9

[benc]

V/via cvs:
1.22darin%meer.net2004-04-03 09:09 fixes bug 237590 "nsIBinaryInputStream
inherits from nsIInputStream but can't be QI'ed to it" r=biesi sr=brendan a=brendan