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]
