213910 - Components.interfaces should be accessible by IID

Status: RESOLVED FIXED

(Core :: XPConnect, defect)

Description

[Benjamin Smedberg]

In certain circumstances, XPConnects users may wish to use interfaces by IID
instead of by name... especially extensions that wish to be compatible with
multiple versions of the GRE/browser. Components.interfaces["someIID"] should
work like Components.classes["someCID"].

I'm willing to do this, but it would take me a while to get to it.

Comment 1

[David Bradley]

To be consistent with classes, it probably should be
Components.interfacesByID["{00000000-0000-0000-0000-000000000000}"]

Comment 2

[Benjamin Smedberg]

Mine, patch forthcoming.

Comment 3

[Benjamin Smedberg]

Attachment: implement Components.interfacesByID

Comment 4

[Benjamin Smedberg]

rginda: caillon said you might be interested in this for cview.

Comment 5

[timeless]

Comment on attachment 136288 [details] [diff] [review]
implement Components.interfacesByID

>+/**
>+* interface of Components.classesByID
			    ^^^^^^^ interfaces?
>+* (interesting stuff only reflected into JavaScript)
JavaScript? so this stuff can't be exposed to python?

>+*/
>+[scriptable, uuid(c99cffac-5aed-4267-ad2f-f4a4c9d4a081)]
>+interface nsIScriptableInterfacesByID : nsISupports
>+{
>+};

> [scriptable, uuid(42624f80-d26c-11d2-9842-006008962422)]
you changed an interface w/o bumping the uuid.
> interface nsIXPCComponents : nsISupports
> {
>     readonly attribute nsIScriptableInterfaces      interfaces;
>+    readonly attribute nsIScriptableInterfacesByID  interfacesByID;

also for abi reasons, should your item be at the end of the interface?

the rest of the patch looks familiar since i copied it for dumpdeps (work in
tree on raistlin, i'll post it late january).

Comment 6

[Benjamin Smedberg]

>>+* interface of Components.classesByID

I'll fix this typo.

> JavaScript? so this stuff can't be exposed to python?

This implementation is javascript-specific (I was copying the matching comments
from xpccomponents.idl)

> > [scriptable, uuid(42624f80-d26c-11d2-9842-006008962422)]
> you changed an interface w/o bumping the uuid.

Good catch, will change iid. I don't think that binary compatibility is an issue
here, so I'd prefer not to add the method at the end where it doesn't make
sense. As far as I know this interface exists solely so that xpconnect can
reflect it into javascript, and is not usable by py/plconnect.

--BDS

Comment 7

[David Bradley]

Comment on attachment 136288 [details] [diff] [review]
implement Components.interfacesByID

r=dbradley

With the changes mentioned previously. And I agree, I'd be very surprised to
find a use of this component outside of JS

Comment 8

[Johnny Stenback (:jst)]

Comment on attachment 136288 [details] [diff] [review]
implement Components.interfacesByID

- In nsXPCComponents_InterfacesByID::NewResolve():

+    if(mManager &&
+	JSVAL_IS_STRING(id) &&
+	38 == JS_GetStringLength(JSVAL_TO_STRING(id)) &&
+	nsnull != (name = JS_GetStringBytes(JSVAL_TO_STRING(id))))
+    {
+	 nsID iid;
+	 if (!iid.Parse(name))
+	     return NS_OK;

The use of JS_GetStringBytes() is bad here (just as in ClassesByID) since it
does a lossy conversion which can make a unicode string match an ID it's not
supposed to match. Use JS_GetStringChars() and convert to UTF8, then you're
guaranteed to never get false positives.

+	 nsCOMPtr<nsIJSIID> nsid = 
+	     dont_AddRef(NS_STATIC_CAST(nsIJSIID*, nsJSIID::NewID(info)));
+
+	 if(nsid)
+	 {
...
+	 }
+    }
+    return NS_OK;

Don't you want to throw an exception here if nsJSIID::NewID() returns null?

With that, sr=jst

Comment 9

[Stephen Donner [:stephend] Not actively reading bugmail]

Hi Benjamin -

I'm on mingw and I get the following build bustage, is it something I need to 
#include (a header file, perhaps?)

[01:04] <stephend> c:/moz_src/mozilla/js/src/xpconnect/src/xpccomponents.cpp: 
In member function
[01:04] <stephend> `virtual nsresult
[01:04] <stephend> nsXPCComponents_InterfacesByID::NewResolve
(nsIXPConnectWrappedNative*,
[01:04] <stephend> JSContext*, JSObject*, long int, unsigned int, JSObject**, 
PRBool*)':
[01:04] <stephend> 
c:/moz_src/mozilla/js/src/xpconnect/src/xpccomponents.cpp:470: no matching
[01:04] <stephend> function for call to 
`NS_ConvertUTF16toUTF8::NS_ConvertUTF16toUTF8(const
[01:04] <stephend> jschar*&)'
[01:04] <stephend> ../../../../dist/include/string/nsString.h:432: candidates 
are:
[01:04] <stephend> NS_ConvertUTF16toUTF8::NS_ConvertUTF16toUTF8(const 
NS_ConvertUTF16toUTF8&)
[01:04] <stephend> ../../../../dist/include/string/nsString.h:444:
[01:04] <stephend> NS_ConvertUTF16toUTF8::NS_ConvertUTF16toUTF8(char) <near 
match>
[01:04] <stephend> ../../../../dist/include/string/nsString.h:437:
[01:04] <stephend> NS_ConvertUTF16toUTF8::NS_ConvertUTF16toUTF8(const 
nsASingleFragmentString&)
[01:04] <stephend> ../../../../dist/include/string/nsString.h:436:
[01:04] <stephend> NS_ConvertUTF16toUTF8::NS_ConvertUTF16toUTF8(const 
nsAString&)
[01:04] <stephend> ../../../../dist/include/string/nsString.h:435:
[01:04] <stephend> NS_ConvertUTF16toUTF8::NS_ConvertUTF16toUTF8(const 
PRUnichar*, unsigned int)
[01:04] <stephend> ../../../../dist/include/string/nsString.h:434:
[01:04] <stephend> NS_ConvertUTF16toUTF8::NS_ConvertUTF16toUTF8(const 
PRUnichar*) <near match>
[01:04] <stephend> make[4]: *** [xpccomponents.o] Error 1

Comment 10

[Benjamin Smedberg]

stephend: not an #include... try this patch: and if it works, please get brendan
or someone to give sr; I am going to be mostly-gone until Jan. 1.

Index: xpccomponents.cpp
===================================================================
RCS file: /cvsroot/mozilla/js/src/xpconnect/src/xpccomponents.cpp,v
retrieving revision 1.64
diff -p -u -5 -r1.64 xpccomponents.cpp
--- xpccomponents.cpp   28 Dec 2003 04:37:57 -0000      1.64
+++ xpccomponents.cpp   28 Dec 2003 06:47:16 -0000
@@ -465,11 +465,11 @@ nsXPCComponents_InterfacesByID::NewResol
        JSVAL_IS_STRING(id) &&
        38 == JS_GetStringLength(JSVAL_TO_STRING(id)) &&
        nsnull != (name = JS_GetStringChars(JSVAL_TO_STRING(id))))
     {
         nsID iid;
-        if (!iid.Parse(NS_ConvertUCS2toUTF8(name).get()))
+        if (!iid.Parse(NS_ConvertUCS2toUTF8(NS_REINTERPRET_CAST(PRUnichar*,
name)).get()))
             return NS_OK;
  
         nsCOMPtr<nsIInterfaceInfo> info;
         mManager->GetInfoForIID(&iid, getter_AddRefs(info));
         if(!info)

Comment 11

[Stephen Donner [:stephend] Not actively reading bugmail]

C:\moz_src\mozilla\js\src\xpconnect\src>patch <patch.txt
(Stripping trailing CRs from patch.)
patching file xpccomponents.cpp
patch: **** malformed patch at line 16: name)).get()))

Did you mean this, below, instead?

Index: xpccomponents.cpp
===================================================================
RCS file: /cvsroot/mozilla/js/src/xpconnect/src/xpccomponents.cpp,v
retrieving revision 1.64
diff -u -r1.64 xpccomponents.cpp
--- xpccomponents.cpp   28 Dec 2003 04:37:57 -0000      1.64
+++ xpccomponents.cpp   28 Dec 2003 07:15:07 -0000
@@ -467,7 +467,9 @@
        nsnull != (name = JS_GetStringChars(JSVAL_TO_STRING(id))))
     {
         nsID iid;
-        if (!iid.Parse(NS_ConvertUCS2toUTF8(name).get()))
+        if
+(!iid.Parse(NS_ConvertUCS2toUTF8(NS_REINTERPRET_CAST(PRUnichar*,
+name)).get())))
             return NS_OK;

         nsCOMPtr<nsIInterfaceInfo> info;

Comment 12

[Stephen Donner [:stephend] Not actively reading bugmail]

Attachment: Fixes MinGW build bustage.

Comment 13

[Stephen Donner [:stephend] Not actively reading bugmail]

Attachment: (Fixed spacing issue of previous patch)

So sorry.

Comment 14

[[e-mail]]

Comment on attachment 138054 [details] [diff] [review]
(Fixed spacing issue of previous patch)

Nit: shouldn't that be a const PRUnichar *?

Comment 15

[David G King]

To build with MinGW on Win32, it must be a const PRUnichar*. If not, the build
fails with an error.

Also, I'm rather sure the end bracket count is one to many.

Otherwise, the amended patch fixes my problems.

Comment 16

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Any idea why this turned the Camino tinderbox orange?

Comment 17

[Stephen Donner [:stephend] Not actively reading bugmail]

I seem to build fine without it being declared as const, but yes, my revised
patch (necessary whitespace changes due to line endings) had one too many end
braces.

Can we get this in soon?

Comment 18

[Stephen Donner [:stephend] Not actively reading bugmail]

Attachment: Patch to fix MinGW bustage

Comment 19

[Stephen Donner [:stephend] Not actively reading bugmail]

Comment on attachment 138142 [details] [diff] [review]
Patch to fix MinGW bustage

Brendan, sr for this MinGW build bustage?

Comment 20

[Brendan Eich [:brendan]]

Comment on attachment 138142 [details] [diff] [review]
Patch to fix MinGW bustage

Sure -- build bustage fixes generally don't need review, if the patcher knows
how to fix and there is no doubt about the fix.

I'm surprised this didn't bust some other build already.  Asking dbaron via r?
for his thoughts.

/be

Comment 21

[Stephen Donner [:stephend] Not actively reading bugmail]

Attachment 138142 [details] [diff] landed to fix MinGW build bustage.