FYI, before the editor code can use a weak reference for the ViewManager, but first
the ViewManager's
implementation class has to derive from nsSupportsWeakReference.
--== Kin ==--
Randell Jesup wrote:
> Simon Fraser <[EMAIL PROTECTED]> writes:
> >> 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.
>
> But where's the guarantee? It strikes me as an unwritten and
> unchecked (and almost uncommented) assumption on a a series of interface
> implementations, one of which might change someday and break it. I'd think
> that weak references of this type (direct Release) are inherently breaking
> the XPCOM model.
>
> Why not nsCOMPtr it and be safe? Or use the weak reference stuff? If
> something else we're holding guarantees it'll be around and won't go away,
> do we gain anything by subverting the refcount here?
>
> BTW, the function already does real "weak references" to other things
> (NS_GetWeakReference is deprecated, BTW):
>
> in nsEditor::Init:
> mDocWeak = getter_AddRefs( NS_GetWeakReference(aDoc) ); // weak reference to doc
> mPresShellWeak = getter_AddRefs( NS_GetWeakReference(aPresShell) ); // weak
>reference to pres shell
> mSelConWeak = getter_AddRefs( NS_GetWeakReference(aSelCon) ); // weak reference
>to selectioncontroller
> nsCOMPtr<nsIPresShell> ps = do_QueryReferent(mPresShellWeak);
>
> Or at least do this:
>
> in nsEditor::Init:
>
> ps->GetViewManager(&mViewManager);
> if (!mViewManager) {return NS_ERROR_NULL_POINTER;}
>
> and in nsEditor::~nsEditor:
>
> NS_IF_RELEASE(mViewManager);
>
> or
>
> (with nsCOMPtr<nsIViewManager> mViewManager)
> ps->GetViewManager(getter_AddRefs(mViewManager));
> if (!mViewManager) {return NS_ERROR_NULL_POINTER;}
>
> or some such.
>
> --
> Randell Jesup, Worldgate Communications, ex-Scala, ex-Amiga OS team ('88-94)
> [EMAIL PROTECTED]
