Closed Bug 411327 Opened 12 years ago Closed 12 years ago

nsIXPCNativeCallContext should not inherit from nsISupports

Categories

(Core :: XPConnect, defect)

defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: benjamin, Assigned: benjamin)

References

Details

(Keywords: memory-footprint)

Attachments

(1 file, 1 obsolete file)

Attached patch nsAXPCNativeCallContext, rev. 1 (obsolete) — Splinter Review
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 :-(
Attachment #296009 - Flags: review?(mrbkap)
Comment on attachment 296130 [details] [diff] [review]
nsAXPCNativeCallContext rev. 1 fixed

The first patch didn't have the new file.
Attachment #296130 - Attachment description: ns → nsAXPCNativeCallContext rev. 1 fixed
Attachment #296130 - Attachment is patch: true
Attachment #296130 - Attachment mime type: application/octet-stream → text/plain
Attachment #296130 - Flags: review?(mrbkap)
Attachment #296009 - Attachment is obsolete: true
Attachment #296009 - Flags: review?(mrbkap)
Comment on attachment 296130 [details] [diff] [review]
nsAXPCNativeCallContext rev. 1 fixed

Looks good to me. Please file a followup bug on deCOMtaminating nsAXPCCallContext.
Attachment #296130 - Flags: review?(mrbkap) → review+
Comment on attachment 296130 [details] [diff] [review]
nsAXPCNativeCallContext rev. 1 fixed

This is fairly low-risk, reduces codesize and complexity, and helps moz2.
Attachment #296130 - Flags: approval1.9?
Attachment #296130 - Flags: approval1.9? → approval1.9+
Assignee: benjamin → nobody
Status: ASSIGNED → NEW
Assignee: nobody → benjamin
I landed this yesterday.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
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.
aaronr, You probably just need to s/nsIXPCNativeCallContext/nsAXPCNativeCallContext/ in places just like the patch did.
Keywords: footprint
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....
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

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
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.
Depends on: 412665
(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.
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...
Depends on: 412811
Depends on: 412527
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.
You need to log in before you can comment on or make changes to this bug.