In article <[EMAIL PROTECTED]>,
Randell Jesup <[EMAIL PROTECTED]> wrote:
> Randell Jesup <[EMAIL PROTECTED]> writes:
> >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.
>
> Also, is this a bug? It certainly doesn't look safe:
>
> from editor/base/nsEditor.cpp:
>
> ps->GetViewManager(&mViewManager);
> if (!mViewManager) {return NS_ERROR_NULL_POINTER;}
> mViewManager->Release(); //we want a weak link
>
> Direct calls to Release()? Isn't that very bad form? What stops
> mViewManager from going away on us?? (The hold on ps might, but then why
> do we need to Release() the mViewManager here? Can't we just release it at
> the appropriate time, when we no longer need it?)
>
> If this really is to be a weak reference, shouldn't it use nsIWeakReference?
Not necessarily. If you can guarantee that the view manager will
outlive you (as we should be able to in this case), there is no need
for nsIWeakReference stuff.
Simon
--
Simon Fraser Entomologist
[EMAIL PROTECTED] http://people.netscape.com/sfraser/
