411327 - nsIXPCNativeCallContext should not inherit from nsISupports

Status: RESOLVED FIXED

Description

[Benjamin Smedberg]

Created attachment 296009 [details] [diff] [review]
nsAXPCNativeCallContext, rev. 1

nsIXPCNativeCallContext should not inherit from nsISupports:

* it doesn't need to be scriptable
* it's allocated on the stack and not refcounted
* nobody needs to QI it
* for moz2 that causes the static analysis tools to barf

This patch s/nsIXPCNativeCallContext/nsAXPCNativeCallContext/, otherwise keeping the signatures the same... although it looks like it would also be pretty simple to deCOM the signatures since most of them can't fail.

Blake, I wrote this against mozilla-central but I don't think it's that risky, and would be happier to land it in 1.9 since mozilla-central doesn't have build or test machines yet :-(

Comment 1

[Benjamin Smedberg]

Created attachment 296130 [details] [diff] [review]
nsAXPCNativeCallContext rev. 1 fixed

Comment 2

[Benjamin Smedberg]

Comment on attachment 296130 [details] [diff] [review]
nsAXPCNativeCallContext rev. 1 fixed

The first patch didn't have the new file.

Comment 3

[Blake Kaplan (:mrbkap)]

Comment on attachment 296130 [details] [diff] [review]
nsAXPCNativeCallContext rev. 1 fixed

Looks good to me. Please file a followup bug on deCOMtaminating nsAXPCCallContext.

Comment 4

[Benjamin Smedberg]

Comment on attachment 296130 [details] [diff] [review]
nsAXPCNativeCallContext rev. 1 fixed

This is fairly low-risk, reduces codesize and complexity, and helps moz2.

Comment 5

[Benjamin Smedberg]

I landed this yesterday.

Comment 6

[aaronr]

This is breaking my build when I --enable-webservices.  How do I get around this for now?  We (XForms) are working to remove our dependence on webservices, but we aren't there yet.

Comment 7

[Benjamin Smedberg]

aaronr, You probably just need to s/nsIXPCNativeCallContext/nsAXPCNativeCallContext/ in places just like the patch did.

Comment 8

[Boris Zbarsky [:bz] (no decent commit message means r-)]

This actually left a number of nsIXPCNativeCallContext in the tree.  Was there a reason to not just change them all?  There were people in #developers today being confused because their mxr searches were showing people using it, but they couldn't find the definition....

Comment 9

[Raúl Porcel]

This breaks other stuff as well:

ia64-unknown-linux-gnu-g++ -o nsXPCToolsModule.o -c -fvisibility=hidden -DOSTYPE=\"Linux2.6\" -DOSARCH=Linux -DGENTOO_LIB_DIR=\"/usr/lib\" -DJSFILE -DJS_THREADSAFE  -I../../../../../dist/include/xpcom -I../../../../../dist/include/xpconnect -I../../../../../dist/include/js -I../../../../../dist/include/string -I../../../../../dist/include   -I../../../../../dist/include/unstable -I/usr/include/nspr     -I../../../../../dist/sdk/include    -fPIC  -DGENTOO_NSPLUGINS_DIR=\"/usr/lib/nsplugins\" -DGENTOO_NSBROWSER_PLUGINS_DIR=\"/usr/lib/nsbrowser/plugins\"  -fno-rtti -fno-handle-exceptions  -Wconversion -Wpointer-arith -Woverloaded-virtual -Wsynth -Wno-ctor-dtor-privacy -Wno-non-virtual-dtor -Wno-long-long -pipe -fPIC -Wno-return-type -w -fshort-wchar -pthread -pipe  -DNDEBUG -DTRIMMED -Os  -DGENTOO_NSPLUGINS_DIR=\"/usr/lib/nsplugins\" -DGENTOO_NSBROWSER_PLUGINS_DIR=\"/usr/lib/nsbrowser/plugins\"  -DMOZILLA_CLIENT -include ../../../../../mozilla-config.h -Wp,-MD,.deps/nsXPCToolsModule.pp nsXPCToolsModule.cpp
nsXPCToolsCompiler.cpp: In member function 'virtual nsresult nsXPCToolsCompiler::CompileFile(nsILocalFile*, PRBool)':
nsXPCToolsCompiler.cpp:96: error: 'nsIXPCNativeCallContext' was not declared in this scope
nsXPCToolsCompiler.cpp:96: error: template argument 1 is invalid
nsXPCToolsCompiler.cpp:96: error: invalid type in declaration before ';' token
nsXPCToolsCompiler.cpp:97: error: no matching function for call to 'getter_AddRefs(int&)'
nsXPCToolsCompiler.cpp:104: error: base operand of '->' is not a pointer
nsXPCToolsCompiler.cpp:110: error: base operand of '->' is not a pointer
gmake[5]: *** [nsXPCToolsCompiler.o] Error 1
gmake[5]: *** Waiting for unfinished jobs....
gmake[5]: Leaving directory `/var/tmp/portage/net-libs/xulrunner-1.9_pre20080117/work/mozilla/js/src/xpconnect/tools/src'
gmake[4]: *** [libs] Error 2

Comment 10

[Raúl Porcel]

After doing s/nsIXPCNativeCallContext/nsAXPCNativeCallContext/ in all files with nsIXPCNativeCallContext(nsXPCToolsCompiler.cpp amongst others) i get this:

ia64-unknown-linux-gnu-g++ -o nsXPCToolsModule.o -c -fvisibility=hidden -DOSTYPE=\"Linux2.6\" -DOSARCH=Linux -DGENTOO_LIB_DIR=\"/usr/lib\" -DJSFILE -DJS_THREADSAFE  -I../../../../../dist/include/xpcom -I../../../../../dist/include/xpconnect -I../../../../../dist/include/js -I../../../../../dist/include/string -I../../../../../dist/include   -I../../../../../dist/include/unstable -I/usr/include/nspr     -I../../../../../dist/sdk/include    -fPIC  -DGENTOO_NSPLUGINS_DIR=\"/usr/lib/nsplugins\" -DGENTOO_NSBROWSER_PLUGINS_DIR=\"/usr/lib/nsbrowser/plugins\"  -fno-rtti -fno-handle-exceptions  -Wconversion -Wpointer-arith -Woverloaded-virtual -Wsynth -Wno-ctor-dtor-privacy -Wno-non-virtual-dtor -Wno-long-long -pipe -fPIC -Wno-return-type -w -fshort-wchar -pthread -pipe  -DNDEBUG -DTRIMMED -Os  -DGENTOO_NSPLUGINS_DIR=\"/usr/lib/nsplugins\" -DGENTOO_NSBROWSER_PLUGINS_DIR=\"/usr/lib/nsbrowser/plugins\"  -DMOZILLA_CLIENT -include ../../../../../mozilla-config.h -Wp,-MD,.deps/nsXPCToolsModule.pp nsXPCToolsModule.cpp
../../../../../dist/include/unstable/nsCOMPtr.h: In instantiation of 'nsDerivedSafe<nsAXPCNativeCallContext>':
nsXPCToolsCompiler.cpp:104:   instantiated from here
../../../../../dist/include/unstable/nsCOMPtr.h:200: error: no members matching 'nsAXPCNativeCallContext::AddRef' in 'class nsAXPCNativeCallContext'
../../../../../dist/include/unstable/nsCOMPtr.h:201: error: no members matching 'nsAXPCNativeCallContext::Release' in 'class nsAXPCNativeCallContext'
gmake[5]: *** [nsXPCToolsCompiler.o] Error 1
gmake[5]: *** Waiting for unfinished jobs....
gmake[5]: Leaving directory `/var/tmp/portage/net-libs/xulrunner-1.9_pre20080117/work/mozilla/js/src/xpconnect/tools/src'
gmake[4]: *** [libs] Error 2

Comment 11

[Benjamin Smedberg]

bz, I fixed up the ones which I could test and are part of our regular products. Webservices is not, but it is fixed by 412665. Raúl, please file a separate bug for your issues... I suspect they are caused by a special compile flag (--enable-xpconnect-tools probably)... the patch here can give guidance... you no longer use nsCOMPtr on nsAXPCNativeCallContext, just use a raw pointer.

Comment 12

[aaronr]

(In reply to comment #10)
> After doing s/nsIXPCNativeCallContext/nsAXPCNativeCallContext/ in all files
> with nsIXPCNativeCallContext(nsXPCToolsCompiler.cpp amongst others) i get this:
> 
> ia64-unknown-linux-gnu-g++ -o nsXPCToolsModule.o -c -fvisibility=hidden
> -DOSTYPE=\"Linux2.6\" -DOSARCH=Linux -DGENTOO_LIB_DIR=\"/usr/lib\" -DJSFILE
> -DJS_THREADSAFE  -I../../../../../dist/include/xpcom
> -I../../../../../dist/include/xpconnect -I../../../../../dist/include/js
> -I../../../../../dist/include/string -I../../../../../dist/include  
> -I../../../../../dist/include/unstable -I/usr/include/nspr    
> -I../../../../../dist/sdk/include    -fPIC 
> -DGENTOO_NSPLUGINS_DIR=\"/usr/lib/nsplugins\"
> -DGENTOO_NSBROWSER_PLUGINS_DIR=\"/usr/lib/nsbrowser/plugins\"  -fno-rtti
> -fno-handle-exceptions  -Wconversion -Wpointer-arith -Woverloaded-virtual
> -Wsynth -Wno-ctor-dtor-privacy -Wno-non-virtual-dtor -Wno-long-long -pipe -fPIC
> -Wno-return-type -w -fshort-wchar -pthread -pipe  -DNDEBUG -DTRIMMED -Os 
> -DGENTOO_NSPLUGINS_DIR=\"/usr/lib/nsplugins\"
> -DGENTOO_NSBROWSER_PLUGINS_DIR=\"/usr/lib/nsbrowser/plugins\"  -DMOZILLA_CLIENT
> -include ../../../../../mozilla-config.h -Wp,-MD,.deps/nsXPCToolsModule.pp
> nsXPCToolsModule.cpp
> ../../../../../dist/include/unstable/nsCOMPtr.h: In instantiation of
> 'nsDerivedSafe<nsAXPCNativeCallContext>':
> nsXPCToolsCompiler.cpp:104:   instantiated from here
> ../../../../../dist/include/unstable/nsCOMPtr.h:200: error: no members matching
> 'nsAXPCNativeCallContext::AddRef' in 'class nsAXPCNativeCallContext'
> ../../../../../dist/include/unstable/nsCOMPtr.h:201: error: no members matching
> 'nsAXPCNativeCallContext::Release' in 'class nsAXPCNativeCallContext'
> gmake[5]: *** [nsXPCToolsCompiler.o] Error 1
> gmake[5]: *** Waiting for unfinished jobs....
> gmake[5]: Leaving directory
> `/var/tmp/portage/net-libs/xulrunner-1.9_pre20080117/work/mozilla/js/src/xpconnect/tools/src'
> gmake[4]: *** [libs] Error 2
> 

Raul, I think you are seeing that error because you just did a simple string replacement.  Your nsIXPCNativeCallContext was probably a nsCOMPtr.  Your nsAXPCNativeCallContext variable should be a regular pointer, not a comptr.

Comment 13

[Boris Zbarsky [:bz] (no decent commit message means r-)]

I guess I feel that changing code that will obviously break with your change to not be broken, even if you can't test it, is a good idea.  But I seem to recall that we have fundamental philosophical differences there...

Comment 14

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I agree with bz, there's no reason to not at least try to fix the code.

At worst, your fix won't make anything worse, and when someone finds the breakage, they can look at the CVS log and will be immediately pointed to the right bug so they can figure out how to fix it. If you don't touch the code, then it's still broken but it's a lot more work to figure out what changed to break it.