As reported in bug 80327 (http://bugzilla.mozilla.org/show_bug.cgi?id=80327),
I hit a crash due to (apparently) view->GetViewManager() giving an nsnull
view. In checking this out, I found that about 1/3 of uses of
nsIViewManager use it straight and do NS_RELEASE(), and the other 2/3rds
use nsCOMPtr's. Also, quite a lot of those that use nsCOMPtr's still don't
check for failure to get it. Even the code in nsView.cpp (which holds
the nsIViewManager) is inconsistent on whether it's guaranteed to exist.
I'm working on a patch for this (I have a patch for the case I hit in the
bug), but it's scattered over about 20-30 files.
What's the _proper_ behavior? Should I nuke all the non-nsCOMPtr uses?
(maybe leave the one in HandleEvent in nsView.cpp for speed?)
Does this count as a case of (mentioned in the discussion on style in
porkjockeys):
use nsCOMPtr<>
If you don't know how to use it, start looking in the code for
examples. The general rule is that the very act of typing NS_RELEASE
should be a signal to you to question your code: "Should I be using
nsCOMPtr here?". Generally the only valid use of NS_RELEASE are when you
are storing refcounted pointers in a long-lived datastructure.
Since this will affect a lot of files in layout and content, plus several
(if not all) of docshell, dom, editor, view, extensions, webshell, and
widget, how should I get this patch applied?
Most cases are somewhat like this:
nsIViewManager *vm;
view->GetViewManager(vm);
vm->GetWidget(&window);
NS_IF_RELEASE(vm);
becomes:
nsCOMPtr<nsIViewManager> vm;
view->GetViewManager(*getter_AddRefs(vm));
if (vm) {
vm->GetWidget(&window);
}
A few are rather more complicated due to having to unwind operations if
we can't get the viewmanager. None are really bad, IMHO.
--
Randell Jesup, Worldgate Communications, ex-Scala, ex-Amiga OS team ('88-94)
[EMAIL PROTECTED]
