From 5ec4702d7ec4c1061ba98670dba4367e9bdaabd0 Mon Sep 17 00:00:00 2001 From: "pinkerton@netscape.com" Date: Fri, 30 Apr 1999 09:04:36 +0000 Subject: [PATCH] pinkerton/scc. appr chofmann. Bug #4127, et al. Fixing ownership models (more work to come). --- content/events/src/nsEventStateManager.h | 7 +- content/xul/document/src/nsXULDocument.cpp | 88 +++++++--------- docshell/base/nsWebShell.cpp | 2 + layout/base/nsPresContext.cpp | 70 +++++------- layout/base/nsPresShell.cpp | 44 +++----- layout/base/src/nsPresContext.cpp | 70 +++++------- layout/base/src/nsPresContext.h | 26 +++-- layout/events/src/nsEventStateManager.h | 7 +- layout/html/base/src/nsPresShell.cpp | 44 +++----- rdf/content/src/nsXULDocument.cpp | 88 +++++++--------- uriloader/base/nsDocLoader.cpp | 62 +++++------ webshell/src/nsDocLoader.cpp | 62 +++++------ webshell/src/nsDocumentViewer.cpp | 117 +++++++++------------ webshell/src/nsWebShell.cpp | 2 + xpfe/appshell/src/nsWebShellWindow.cpp | 12 +-- 15 files changed, 300 insertions(+), 401 deletions(-) diff --git a/content/events/src/nsEventStateManager.h b/content/events/src/nsEventStateManager.h index 325dc0fb57a8..04545b0d59fb 100644 --- a/content/events/src/nsEventStateManager.h +++ b/content/events/src/nsEventStateManager.h @@ -76,10 +76,9 @@ protected: nsIContent* mHoverContent; nsIContent* mCurrentFocus; PRInt32 mCurrentTabIndex; - - //Not refcnted - nsIPresContext* mPresContext; - nsIDocument* mDocument; + + nsIPresContext* mPresContext; // Not refcnted + nsIDocument* mDocument; // [OWNER], but doesn't need to be. }; extern nsresult NS_NewEventStateManager(nsIEventStateManager** aInstancePtrResult); diff --git a/content/xul/document/src/nsXULDocument.cpp b/content/xul/document/src/nsXULDocument.cpp index bd55bc762021..c95597ce1143 100644 --- a/content/xul/document/src/nsXULDocument.cpp +++ b/content/xul/document/src/nsXULDocument.cpp @@ -677,35 +677,40 @@ protected: nsresult AddNamedDataSource(const char* uri); + // IMPORTANT: The ownership implicit in the following member variables has been + // explicitly checked and set using nsCOMPtr for owning pointers and raw COM interface + // pointers for weak (ie, non owning) references. If you add any members to this + // class, please make the ownership explicit (pinkerton, scc). + // NOTE, THIS IS STILL IN PROGRESS, TALK TO PINK OR SCC BEFORE CHANGING - nsIArena* mArena; + nsCOMPtr mArena; nsVoidArray mObservers; nsAutoString mDocumentTitle; - nsIURL* mDocumentURL; - nsIURLGroup* mDocumentURLGroup; - nsIContent* mRootContent; + nsIURL* mDocumentURL; // [OWNER] ??? compare with loader + nsIURLGroup* mDocumentURLGroup; // [OWNER] leads to loader + nsIContent* mRootContent; // [OWNER] nsIDocument* mParentDocument; - nsIScriptContextOwner* mScriptContextOwner; - void* mScriptObject; + nsIScriptContextOwner* mScriptContextOwner; // [WEAK] it owns me! (indirectly) + void* mScriptObject; // ???? nsString mCharSetID; nsVoidArray mStyleSheets; - nsIDOMSelection* mSelection; + nsIDOMSelection* mSelection; // [OWNER] PRBool mDisplaySelection; nsVoidArray mPresShells; - nsINameSpaceManager* mNameSpaceManager; - nsIHTMLStyleSheet* mAttrStyleSheet; - nsIHTMLCSSStyleSheet* mInlineStyleSheet; + nsINameSpaceManager* mNameSpaceManager; // [OWNER] + nsIHTMLStyleSheet* mAttrStyleSheet; // [OWNER] + nsCOMPtr mInlineStyleSheet; nsElementMap mResources; - nsISupportsArray* mBuilders; - nsIRDFContentModelBuilder* mXULBuilder; - nsIRDFDataSource* mLocalDataSource; - nsIRDFDataSource* mDocumentDataSource; - nsILineBreaker* mLineBreaker; - nsIWordBreaker* mWordBreaker; - nsIContentViewerContainer* mContentViewerContainer; + nsISupportsArray* mBuilders; // [OWNER] of array, elements shouldn't own this, but they do + nsIRDFContentModelBuilder* mXULBuilder; // [OWNER] + nsIRDFDataSource* mLocalDataSource; // [OWNER] + nsIRDFDataSource* mDocumentDataSource; // [OWNER] + nsILineBreaker* mLineBreaker; // [OWNER] + nsIWordBreaker* mWordBreaker; // [OWNER] + nsIContentViewerContainer* mContentViewerContainer; // [WEAK] it owns me! (indirectly) nsString mCommand; - nsIRDFResource* mFragmentRoot; - nsVoidArray mSubDocuments; + nsIRDFResource* mFragmentRoot; // [OWNER] + nsVoidArray mSubDocuments; // [OWNER] of subelements }; PRInt32 XULDocumentImpl::gRefCnt = 0; @@ -721,8 +726,7 @@ nsIRDFResource* XULDocumentImpl::kXUL_element; // ctors & dtors XULDocumentImpl::XULDocumentImpl(void) - : mArena(nsnull), - mDocumentURL(nsnull), + : mDocumentURL(nsnull), mDocumentURLGroup(nsnull), mRootContent(nsnull), mParentDocument(nsnull), @@ -793,7 +797,8 @@ XULDocumentImpl::~XULDocumentImpl() NS_RELEASE(subdoc); } - // set all builder references to document to nsnull + // set all builder references to document to nsnull -- out of band notification + // to break ownership cycle if (mBuilders) { @@ -822,16 +827,13 @@ XULDocumentImpl::~XULDocumentImpl() NS_IF_RELEASE(mBuilders); NS_IF_RELEASE(mXULBuilder); NS_IF_RELEASE(mSelection); - NS_IF_RELEASE(mScriptContextOwner); NS_IF_RELEASE(mAttrStyleSheet); NS_IF_RELEASE(mRootContent); NS_IF_RELEASE(mDocumentURLGroup); NS_IF_RELEASE(mDocumentURL); - NS_IF_RELEASE(mArena); NS_IF_RELEASE(mNameSpaceManager); NS_IF_RELEASE(mLineBreaker); NS_IF_RELEASE(mWordBreaker); - NS_IF_RELEASE(mContentViewerContainer); NS_IF_RELEASE(mFragmentRoot); if (--gRefCnt == 0) { @@ -938,8 +940,9 @@ NS_IMPL_RELEASE(XULDocumentImpl); nsIArena* XULDocumentImpl::GetArena() { - NS_IF_ADDREF(mArena); - return mArena; + nsIArena* result = mArena; + NS_IF_ADDREF(result); + return result; } static @@ -988,13 +991,7 @@ XULDocumentImpl::PrepareToLoad( nsCOMPtr* created_parser, #endif if (aContainer && aContainer != mContentViewerContainer) - { - NS_IF_RELEASE(mContentViewerContainer); - - // AddRef and hold the container - NS_ADDREF(aContainer); mContentViewerContainer = aContainer; - } nsresult rv; @@ -1043,11 +1040,9 @@ XULDocumentImpl::PrepareToLoad( nsCOMPtr* created_parser, if (NS_SUCCEEDED(rv = nsComponentManager::CreateInstance(kHTMLCSSStyleSheetCID, nsnull, kIHTMLCSSStyleSheetIID, - (void**) &inlineSheet))) { + (void**)&inlineSheet))) { if (NS_SUCCEEDED(rv = inlineSheet->Init(syntheticURL, this))) { - mInlineStyleSheet = inlineSheet; - NS_ADDREF(mInlineStyleSheet); - + mInlineStyleSheet = dont_QueryInterface(inlineSheet); AddStyleSheet(mInlineStyleSheet); } NS_RELEASE(inlineSheet); @@ -1397,7 +1392,7 @@ XULDocumentImpl::CreateShell(nsIPresContext* aContext, } mPresShells.AppendElement(shell); - *aInstancePtrResult = shell; // addref implicit + *aInstancePtrResult = shell; // addref implicit in CreateInstance() return NS_OK; } @@ -1624,9 +1619,7 @@ XULDocumentImpl::SetScriptContextOwner(nsIScriptContextOwner *aScriptContextOwne if (!aScriptContextOwner && mRootContent) mRootContent->SetDocument(nsnull, PR_TRUE); - NS_IF_RELEASE(mScriptContextOwner); mScriptContextOwner = aScriptContextOwner; - NS_IF_ADDREF(mScriptContextOwner); } NS_IMETHODIMP @@ -2731,11 +2724,10 @@ XULDocumentImpl::SearchForNodeByID(const nsString& anID, NS_IMETHODIMP XULDocumentImpl::GetContentViewerContainer(nsIContentViewerContainer** aContainer) { - if (mContentViewerContainer != nsnull) - { - NS_ADDREF(mContentViewerContainer); - *aContainer = mContentViewerContainer; - } + NS_PRECONDITION ( aContainer, "Null Parameter into GetContentViewerContainer" ); + + *aContainer = mContentViewerContainer; + NS_IF_ADDREF(*aContainer); return NS_OK; } @@ -3134,11 +3126,11 @@ XULDocumentImpl::GetInlineStyleSheet(nsIHTMLCSSStyleSheet** aResult) return NS_ERROR_NULL_POINTER; } *aResult = mInlineStyleSheet; - if (nsnull == mInlineStyleSheet) { + if (!mInlineStyleSheet) { return NS_ERROR_NOT_AVAILABLE; // probably not the right error... } else { - NS_ADDREF(mInlineStyleSheet); + NS_ADDREF(*aResult); } return NS_OK; } @@ -3437,7 +3429,7 @@ XULDocumentImpl::Init(void) { nsresult rv; - if (NS_FAILED(rv = NS_NewHeapArena(&mArena, nsnull))) + if (NS_FAILED(rv = NS_NewHeapArena(getter_AddRefs(mArena), nsnull))) return rv; // Create a namespace manager so we can manage tags diff --git a/docshell/base/nsWebShell.cpp b/docshell/base/nsWebShell.cpp index afde1285b1af..85c1f6de8fd4 100644 --- a/docshell/base/nsWebShell.cpp +++ b/docshell/base/nsWebShell.cpp @@ -529,6 +529,8 @@ nsWebShell::~nsWebShell() NS_IF_RELEASE(mWindow); + ++mRefCnt; // hack will come back to this (pinkerton, scc) + NS_IF_RELEASE(mContentViewer); NS_IF_RELEASE(mDeviceContext); NS_IF_RELEASE(mPrefs); diff --git a/layout/base/nsPresContext.cpp b/layout/base/nsPresContext.cpp index cb7a88994610..a302edbe8b61 100644 --- a/layout/base/nsPresContext.cpp +++ b/layout/base/nsPresContext.cpp @@ -81,29 +81,20 @@ nsPresContext::~nsPresContext() Stop(); - if (nsnull != mImageGroup) { + if (mImageGroup) { // Interrupt any loading images. This also stops all looping // image animations. mImageGroup->Interrupt(); - NS_RELEASE(mImageGroup); } - NS_IF_RELEASE(mLinkHandler); - NS_IF_RELEASE(mContainer); + if (mEventManager) + mEventManager->SetPresContext(nsnull); // unclear if this is needed, but can't hurt - if (nsnull != mEventManager) { - mEventManager->SetPresContext(nsnull); - NS_RELEASE(mEventManager); - } - - NS_IF_RELEASE(mDeviceContext); // Unregister preference callbacks - if (nsnull != mPrefs) { + if (mPrefs) { mPrefs->UnregisterCallback("browser.", PrefChangedCallback, (void*)this); mPrefs->UnregisterCallback("intl.font2.", PrefChangedCallback, (void*)this); } - NS_IF_RELEASE(mPrefs); - NS_IF_RELEASE(mBaseURL); nsLayoutAtoms::ReleaseAtoms(); } @@ -253,13 +244,10 @@ nsPresContext::Init(nsIDeviceContext* aDeviceContext, nsIPref* aPrefs) { NS_ASSERTION(!(mInitialized == PR_TRUE), "attempt to reinit pres context"); - mDeviceContext = aDeviceContext; - NS_IF_ADDREF(mDeviceContext); + mDeviceContext = dont_QueryInterface(aDeviceContext); - mPrefs = aPrefs; - NS_IF_ADDREF(mPrefs); - - if (nsnull != mPrefs) { + mPrefs = dont_QueryInterface(aPrefs); + if (mPrefs) { // Register callbacks so we're notified when the preferences change mPrefs->RegisterCallback("browser.", PrefChangedCallback, (void*)this); mPrefs->RegisterCallback("intl.font2.", PrefChangedCallback, (void*)this); @@ -280,14 +268,13 @@ nsPresContext::Init(nsIDeviceContext* aDeviceContext, nsIPref* aPrefs) NS_IMETHODIMP nsPresContext::SetShell(nsIPresShell* aShell) { - NS_IF_RELEASE(mBaseURL); mShell = aShell; if (nsnull != mShell) { nsCOMPtr doc; if (NS_SUCCEEDED(mShell->GetDocument(getter_AddRefs(doc)))) { NS_ASSERTION(doc, "expect document here"); if (doc) { - doc->GetBaseURL(mBaseURL); + doc->GetBaseURL(*getter_AddRefs(mBaseURL)); } } } @@ -314,7 +301,7 @@ nsPresContext::GetPrefs(nsIPref** aResult) return NS_ERROR_NULL_POINTER; } *aResult = mPrefs; - NS_IF_ADDREF(mPrefs); + NS_IF_ADDREF(*aResult); return NS_OK; } @@ -366,7 +353,7 @@ nsPresContext::GetBaseURL(nsIURL** aResult) return NS_ERROR_NULL_POINTER; } *aResult = mBaseURL; - NS_IF_ADDREF(mBaseURL); + NS_IF_ADDREF(*aResult); return NS_OK; } @@ -459,7 +446,7 @@ nsPresContext::GetMetricsFor(const nsFont& aFont, nsIFontMetrics** aResult) } nsIFontMetrics* metrics = nsnull; - if (nsnull != mDeviceContext) { + if (mDeviceContext) { mDeviceContext->GetMetricsFor(aFont, metrics); } *aResult = metrics; @@ -572,7 +559,7 @@ nsPresContext::GetPixelsToTwips(float* aResult) const } float p2t = 1.0f; - if (nsnull != mDeviceContext) { + if (mDeviceContext) { mDeviceContext->GetDevUnitsToAppUnits(p2t); } *aResult = p2t; @@ -588,7 +575,7 @@ nsPresContext::GetTwipsToPixels(float* aResult) const } float app2dev = 1.0f; - if (nsnull != mDeviceContext) { + if (mDeviceContext) { mDeviceContext->GetAppUnitsToDevUnits(app2dev); } *aResult = app2dev; @@ -604,7 +591,7 @@ nsPresContext::GetScaledPixelsToTwips(float* aResult) const } float scale = 1.0f; - if (nsnull != mDeviceContext) + if (mDeviceContext) { float p2t; mDeviceContext->GetDevUnitsToAppUnits(p2t); @@ -623,7 +610,7 @@ nsPresContext::GetDeviceContext(nsIDeviceContext** aResult) const return NS_ERROR_NULL_POINTER; } *aResult = mDeviceContext; - NS_IF_ADDREF(mDeviceContext); + NS_IF_ADDREF(*aResult); return NS_OK; } @@ -635,26 +622,25 @@ nsPresContext::GetImageGroup(nsIImageGroup** aResult) return NS_ERROR_NULL_POINTER; } - if (nsnull == mImageGroup) { + if (!mImageGroup) { // Create image group - nsresult rv = NS_NewImageGroup(&mImageGroup); + nsresult rv = NS_NewImageGroup(getter_AddRefs(mImageGroup)); if (NS_OK != rv) { return rv; } // Initialize the image group - nsIURLGroup* urlGroup; - rv = mBaseURL->GetURLGroup(&urlGroup); + nsCOMPtr urlGroup; + rv = mBaseURL->GetURLGroup(getter_AddRefs(urlGroup)); if (rv == NS_OK) rv = mImageGroup->Init(mDeviceContext, urlGroup); - NS_IF_RELEASE(urlGroup); if (NS_OK != rv) { return rv; } } *aResult = mImageGroup; - NS_IF_ADDREF(mImageGroup); + NS_IF_ADDREF(*aResult); return NS_OK; } @@ -708,9 +694,9 @@ nsPresContext::StartLoadImage(const nsString& aURL, // Create image group if needed nsresult rv; - if (nsnull == mImageGroup) { + if (!mImageGroup) { nsCOMPtr group; - rv = GetImageGroup(getter_AddRefs(group)); + rv = GetImageGroup(getter_AddRefs(group)); // sets mImageGroup as side effect if (NS_OK != rv) { return rv; } @@ -819,10 +805,8 @@ nsPresContext::StopAllLoadImagesFor(nsIFrame* aTargetFrame) NS_IMETHODIMP nsPresContext::SetLinkHandler(nsILinkHandler* aHandler) -{ // XXX should probably be a WEAK reference - NS_IF_RELEASE(mLinkHandler); +{ mLinkHandler = aHandler; - NS_IF_ADDREF(aHandler); return NS_OK; } @@ -840,10 +824,8 @@ nsPresContext::GetLinkHandler(nsILinkHandler** aResult) NS_IMETHODIMP nsPresContext::SetContainer(nsISupports* aHandler) -{ // XXX should most likely be a WEAK reference - NS_IF_RELEASE(mContainer); +{ mContainer = aHandler; - NS_IF_ADDREF(aHandler); return NS_OK; } @@ -868,7 +850,7 @@ nsPresContext::GetEventStateManager(nsIEventStateManager** aManager) } if (nsnull == mEventManager) { - nsresult rv = NS_NewEventStateManager(&mEventManager); + nsresult rv = NS_NewEventStateManager(getter_AddRefs(mEventManager)); if (NS_OK != rv) { return rv; } @@ -878,7 +860,7 @@ nsPresContext::GetEventStateManager(nsIEventStateManager** aManager) mEventManager->SetPresContext(this); *aManager = mEventManager; - NS_IF_ADDREF(mEventManager); + NS_IF_ADDREF(*aManager); return NS_OK; } diff --git a/layout/base/nsPresShell.cpp b/layout/base/nsPresShell.cpp index c9513d8c4909..ef3bb1a4df89 100644 --- a/layout/base/nsPresShell.cpp +++ b/layout/base/nsPresShell.cpp @@ -372,11 +372,16 @@ protected: PRBool mInVerifyReflow; #endif - nsIDocument* mDocument; - nsIPresContext* mPresContext; - nsIStyleSet* mStyleSet; + // IMPORTANT: The ownership implicit in the following member variables has been + // explicitly checked and set using nsCOMPtr for owning pointers and raw COM interface + // pointers for weak (ie, non owning) references. If you add any members to this + // class, please make the ownership explicit (pinkerton, scc). + + nsIDocument* mDocument; // [WEAK] docViewer owns it so I don't have to + nsIPresContext* mPresContext; // [WEAK] docViewer owns it so I don't have to + nsCOMPtr mStyleSet; nsIFrame* mRootFrame; - nsIViewManager* mViewManager; + nsIViewManager* mViewManager; // [WEAK] docViewer owns it so I don't have to PRUint32 mUpdateCount; nsVoidArray mReflowCommands; PRUint32 mReflowLockCount; @@ -542,20 +547,10 @@ PresShell::~PresShell() // Disable paints during tear down of the frame tree mViewManager->DisableRefresh(); } - if (nsnull != mRootFrame) { + if (mRootFrame) mRootFrame->DeleteFrame(*mPresContext); - } - NS_IF_RELEASE(mViewManager); - - //Note: Release mPresContext after mViewManager - // XXX why? - NS_IF_RELEASE(mPresContext); - NS_IF_RELEASE(mStyleSet); - if (nsnull != mDocument) { + if (mDocument) mDocument->DeleteShell(this); - NS_RELEASE(mDocument); - } - mRefCnt = 0; delete mPlaceholderMap; } @@ -582,20 +577,16 @@ PresShell::Init(nsIDocument* aDocument, } mDocument = aDocument; - NS_ADDREF(aDocument); mViewManager = aViewManager; - NS_ADDREF(mViewManager); //doesn't add a ref since we own it... MMP mViewManager->SetViewObserver((nsIViewObserver *)this); // Bind the context to the presentation shell. mPresContext = aPresContext; - NS_ADDREF(aPresContext); aPresContext->SetShell(this); - mStyleSet = aStyleSet; - NS_ADDREF(aStyleSet); + mStyleSet = dont_QueryInterface(aStyleSet); nsCOMPtrdomselection; nsresult result = nsComponentManager::CreateInstance(kRangeListCID, nsnull, @@ -712,14 +703,14 @@ PresShell::GetStyleSet(nsIStyleSet** aResult) return NS_ERROR_NULL_POINTER; } *aResult = mStyleSet; - NS_IF_ADDREF(mStyleSet); + NS_IF_ADDREF(*aResult); return NS_OK; } NS_IMETHODIMP PresShell::GetActiveAlternateStyleSheet(nsString& aSheetTitle) { // first non-html sheet in style set that has title - if (nsnull != mStyleSet) { + if (mStyleSet) { PRInt32 count = mStyleSet->GetNumberOfDocStyleSheets(); PRInt32 index; nsAutoString textHtml("text/html"); @@ -746,7 +737,7 @@ PresShell::GetActiveAlternateStyleSheet(nsString& aSheetTitle) NS_IMETHODIMP PresShell::SelectAlternateStyleSheet(const nsString& aSheetTitle) { - if ((nsnull != mDocument) && (nsnull != mStyleSet)) { + if ((nsnull != mDocument) && mStyleSet) { PRInt32 count = mDocument->GetNumberOfStyleSheets(); PRInt32 index; nsAutoString textHtml("text/html"); @@ -2431,11 +2422,10 @@ PresShell::VerifyIncrementalReflow() // Create a new presentation shell to view the document. Use the // exact same style information that this document has. - nsIStyleSet* newSet; - rv = CloneStyleSet(mStyleSet, &newSet); + nsCOMPtr newSet; + rv = CloneStyleSet(mStyleSet, getter_AddRefs(newSet)); NS_ASSERTION(NS_SUCCEEDED(rv), "failed to clone style set"); rv = mDocument->CreateShell(cx, vm, newSet, &sh); - NS_RELEASE(newSet); NS_ASSERTION(NS_OK == rv, "failed to create presentation shell"); vm->SetViewObserver((nsIViewObserver *)((PresShell*)sh)); sh->InitialReflow(r.width, r.height); diff --git a/layout/base/src/nsPresContext.cpp b/layout/base/src/nsPresContext.cpp index cb7a88994610..a302edbe8b61 100644 --- a/layout/base/src/nsPresContext.cpp +++ b/layout/base/src/nsPresContext.cpp @@ -81,29 +81,20 @@ nsPresContext::~nsPresContext() Stop(); - if (nsnull != mImageGroup) { + if (mImageGroup) { // Interrupt any loading images. This also stops all looping // image animations. mImageGroup->Interrupt(); - NS_RELEASE(mImageGroup); } - NS_IF_RELEASE(mLinkHandler); - NS_IF_RELEASE(mContainer); + if (mEventManager) + mEventManager->SetPresContext(nsnull); // unclear if this is needed, but can't hurt - if (nsnull != mEventManager) { - mEventManager->SetPresContext(nsnull); - NS_RELEASE(mEventManager); - } - - NS_IF_RELEASE(mDeviceContext); // Unregister preference callbacks - if (nsnull != mPrefs) { + if (mPrefs) { mPrefs->UnregisterCallback("browser.", PrefChangedCallback, (void*)this); mPrefs->UnregisterCallback("intl.font2.", PrefChangedCallback, (void*)this); } - NS_IF_RELEASE(mPrefs); - NS_IF_RELEASE(mBaseURL); nsLayoutAtoms::ReleaseAtoms(); } @@ -253,13 +244,10 @@ nsPresContext::Init(nsIDeviceContext* aDeviceContext, nsIPref* aPrefs) { NS_ASSERTION(!(mInitialized == PR_TRUE), "attempt to reinit pres context"); - mDeviceContext = aDeviceContext; - NS_IF_ADDREF(mDeviceContext); + mDeviceContext = dont_QueryInterface(aDeviceContext); - mPrefs = aPrefs; - NS_IF_ADDREF(mPrefs); - - if (nsnull != mPrefs) { + mPrefs = dont_QueryInterface(aPrefs); + if (mPrefs) { // Register callbacks so we're notified when the preferences change mPrefs->RegisterCallback("browser.", PrefChangedCallback, (void*)this); mPrefs->RegisterCallback("intl.font2.", PrefChangedCallback, (void*)this); @@ -280,14 +268,13 @@ nsPresContext::Init(nsIDeviceContext* aDeviceContext, nsIPref* aPrefs) NS_IMETHODIMP nsPresContext::SetShell(nsIPresShell* aShell) { - NS_IF_RELEASE(mBaseURL); mShell = aShell; if (nsnull != mShell) { nsCOMPtr doc; if (NS_SUCCEEDED(mShell->GetDocument(getter_AddRefs(doc)))) { NS_ASSERTION(doc, "expect document here"); if (doc) { - doc->GetBaseURL(mBaseURL); + doc->GetBaseURL(*getter_AddRefs(mBaseURL)); } } } @@ -314,7 +301,7 @@ nsPresContext::GetPrefs(nsIPref** aResult) return NS_ERROR_NULL_POINTER; } *aResult = mPrefs; - NS_IF_ADDREF(mPrefs); + NS_IF_ADDREF(*aResult); return NS_OK; } @@ -366,7 +353,7 @@ nsPresContext::GetBaseURL(nsIURL** aResult) return NS_ERROR_NULL_POINTER; } *aResult = mBaseURL; - NS_IF_ADDREF(mBaseURL); + NS_IF_ADDREF(*aResult); return NS_OK; } @@ -459,7 +446,7 @@ nsPresContext::GetMetricsFor(const nsFont& aFont, nsIFontMetrics** aResult) } nsIFontMetrics* metrics = nsnull; - if (nsnull != mDeviceContext) { + if (mDeviceContext) { mDeviceContext->GetMetricsFor(aFont, metrics); } *aResult = metrics; @@ -572,7 +559,7 @@ nsPresContext::GetPixelsToTwips(float* aResult) const } float p2t = 1.0f; - if (nsnull != mDeviceContext) { + if (mDeviceContext) { mDeviceContext->GetDevUnitsToAppUnits(p2t); } *aResult = p2t; @@ -588,7 +575,7 @@ nsPresContext::GetTwipsToPixels(float* aResult) const } float app2dev = 1.0f; - if (nsnull != mDeviceContext) { + if (mDeviceContext) { mDeviceContext->GetAppUnitsToDevUnits(app2dev); } *aResult = app2dev; @@ -604,7 +591,7 @@ nsPresContext::GetScaledPixelsToTwips(float* aResult) const } float scale = 1.0f; - if (nsnull != mDeviceContext) + if (mDeviceContext) { float p2t; mDeviceContext->GetDevUnitsToAppUnits(p2t); @@ -623,7 +610,7 @@ nsPresContext::GetDeviceContext(nsIDeviceContext** aResult) const return NS_ERROR_NULL_POINTER; } *aResult = mDeviceContext; - NS_IF_ADDREF(mDeviceContext); + NS_IF_ADDREF(*aResult); return NS_OK; } @@ -635,26 +622,25 @@ nsPresContext::GetImageGroup(nsIImageGroup** aResult) return NS_ERROR_NULL_POINTER; } - if (nsnull == mImageGroup) { + if (!mImageGroup) { // Create image group - nsresult rv = NS_NewImageGroup(&mImageGroup); + nsresult rv = NS_NewImageGroup(getter_AddRefs(mImageGroup)); if (NS_OK != rv) { return rv; } // Initialize the image group - nsIURLGroup* urlGroup; - rv = mBaseURL->GetURLGroup(&urlGroup); + nsCOMPtr urlGroup; + rv = mBaseURL->GetURLGroup(getter_AddRefs(urlGroup)); if (rv == NS_OK) rv = mImageGroup->Init(mDeviceContext, urlGroup); - NS_IF_RELEASE(urlGroup); if (NS_OK != rv) { return rv; } } *aResult = mImageGroup; - NS_IF_ADDREF(mImageGroup); + NS_IF_ADDREF(*aResult); return NS_OK; } @@ -708,9 +694,9 @@ nsPresContext::StartLoadImage(const nsString& aURL, // Create image group if needed nsresult rv; - if (nsnull == mImageGroup) { + if (!mImageGroup) { nsCOMPtr group; - rv = GetImageGroup(getter_AddRefs(group)); + rv = GetImageGroup(getter_AddRefs(group)); // sets mImageGroup as side effect if (NS_OK != rv) { return rv; } @@ -819,10 +805,8 @@ nsPresContext::StopAllLoadImagesFor(nsIFrame* aTargetFrame) NS_IMETHODIMP nsPresContext::SetLinkHandler(nsILinkHandler* aHandler) -{ // XXX should probably be a WEAK reference - NS_IF_RELEASE(mLinkHandler); +{ mLinkHandler = aHandler; - NS_IF_ADDREF(aHandler); return NS_OK; } @@ -840,10 +824,8 @@ nsPresContext::GetLinkHandler(nsILinkHandler** aResult) NS_IMETHODIMP nsPresContext::SetContainer(nsISupports* aHandler) -{ // XXX should most likely be a WEAK reference - NS_IF_RELEASE(mContainer); +{ mContainer = aHandler; - NS_IF_ADDREF(aHandler); return NS_OK; } @@ -868,7 +850,7 @@ nsPresContext::GetEventStateManager(nsIEventStateManager** aManager) } if (nsnull == mEventManager) { - nsresult rv = NS_NewEventStateManager(&mEventManager); + nsresult rv = NS_NewEventStateManager(getter_AddRefs(mEventManager)); if (NS_OK != rv) { return rv; } @@ -878,7 +860,7 @@ nsPresContext::GetEventStateManager(nsIEventStateManager** aManager) mEventManager->SetPresContext(this); *aManager = mEventManager; - NS_IF_ADDREF(mEventManager); + NS_IF_ADDREF(*aManager); return NS_OK; } diff --git a/layout/base/src/nsPresContext.h b/layout/base/src/nsPresContext.h index 6f6ad8deb541..534b4330664e 100644 --- a/layout/base/src/nsPresContext.h +++ b/layout/base/src/nsPresContext.h @@ -23,7 +23,9 @@ #include "nsVoidArray.h" #include "nsFont.h" #include "nsCRT.h" -class nsIImageGroup; +#include "nsCOMPtr.h" +#include "nsIImageGroup.h" + // Base class for concrete presentation context classes class nsPresContext : public nsIPresContext { @@ -99,23 +101,29 @@ protected: nsPresContext(); virtual ~nsPresContext(); - nsIPresShell* mShell; - nsIPref* mPrefs; + // IMPORTANT: The ownership implicit in the following member variables has been + // explicitly checked and set using nsCOMPtr for owning pointers and raw COM interface + // pointers for weak (ie, non owning) references. If you add any members to this + // class, please make the ownership explicit (pinkerton, scc). + + nsIPresShell* mShell; // [WEAK] + nsCOMPtr mPrefs; nsRect mVisibleArea; - nsIDeviceContext* mDeviceContext; - nsIImageGroup* mImageGroup; - nsILinkHandler* mLinkHandler; - nsISupports* mContainer; + nsCOMPtr mDeviceContext; // could be weak, but better safe than sorry. Cannot reintroduce cycles + // since there is no dependency from gfx back to layout. + nsCOMPtr mImageGroup; + nsILinkHandler* mLinkHandler; // [WEAK] + nsISupports* mContainer; // [WEAK] nsFont mDefaultFont; nsFont mDefaultFixedFont; PRInt32 mFontScaler; nscolor mDefaultColor; nscolor mDefaultBackgroundColor; nsVoidArray mImageLoaders; - nsIEventStateManager* mEventManager; + nsCOMPtr mEventManager; nsCompatibility mCompatibilityMode; nsWidgetRendering mWidgetRenderingMode; - nsIURL* mBaseURL; + nsCOMPtr mBaseURL; PRBool mStopped; #ifdef DEBUG diff --git a/layout/events/src/nsEventStateManager.h b/layout/events/src/nsEventStateManager.h index 325dc0fb57a8..04545b0d59fb 100644 --- a/layout/events/src/nsEventStateManager.h +++ b/layout/events/src/nsEventStateManager.h @@ -76,10 +76,9 @@ protected: nsIContent* mHoverContent; nsIContent* mCurrentFocus; PRInt32 mCurrentTabIndex; - - //Not refcnted - nsIPresContext* mPresContext; - nsIDocument* mDocument; + + nsIPresContext* mPresContext; // Not refcnted + nsIDocument* mDocument; // [OWNER], but doesn't need to be. }; extern nsresult NS_NewEventStateManager(nsIEventStateManager** aInstancePtrResult); diff --git a/layout/html/base/src/nsPresShell.cpp b/layout/html/base/src/nsPresShell.cpp index c9513d8c4909..ef3bb1a4df89 100644 --- a/layout/html/base/src/nsPresShell.cpp +++ b/layout/html/base/src/nsPresShell.cpp @@ -372,11 +372,16 @@ protected: PRBool mInVerifyReflow; #endif - nsIDocument* mDocument; - nsIPresContext* mPresContext; - nsIStyleSet* mStyleSet; + // IMPORTANT: The ownership implicit in the following member variables has been + // explicitly checked and set using nsCOMPtr for owning pointers and raw COM interface + // pointers for weak (ie, non owning) references. If you add any members to this + // class, please make the ownership explicit (pinkerton, scc). + + nsIDocument* mDocument; // [WEAK] docViewer owns it so I don't have to + nsIPresContext* mPresContext; // [WEAK] docViewer owns it so I don't have to + nsCOMPtr mStyleSet; nsIFrame* mRootFrame; - nsIViewManager* mViewManager; + nsIViewManager* mViewManager; // [WEAK] docViewer owns it so I don't have to PRUint32 mUpdateCount; nsVoidArray mReflowCommands; PRUint32 mReflowLockCount; @@ -542,20 +547,10 @@ PresShell::~PresShell() // Disable paints during tear down of the frame tree mViewManager->DisableRefresh(); } - if (nsnull != mRootFrame) { + if (mRootFrame) mRootFrame->DeleteFrame(*mPresContext); - } - NS_IF_RELEASE(mViewManager); - - //Note: Release mPresContext after mViewManager - // XXX why? - NS_IF_RELEASE(mPresContext); - NS_IF_RELEASE(mStyleSet); - if (nsnull != mDocument) { + if (mDocument) mDocument->DeleteShell(this); - NS_RELEASE(mDocument); - } - mRefCnt = 0; delete mPlaceholderMap; } @@ -582,20 +577,16 @@ PresShell::Init(nsIDocument* aDocument, } mDocument = aDocument; - NS_ADDREF(aDocument); mViewManager = aViewManager; - NS_ADDREF(mViewManager); //doesn't add a ref since we own it... MMP mViewManager->SetViewObserver((nsIViewObserver *)this); // Bind the context to the presentation shell. mPresContext = aPresContext; - NS_ADDREF(aPresContext); aPresContext->SetShell(this); - mStyleSet = aStyleSet; - NS_ADDREF(aStyleSet); + mStyleSet = dont_QueryInterface(aStyleSet); nsCOMPtrdomselection; nsresult result = nsComponentManager::CreateInstance(kRangeListCID, nsnull, @@ -712,14 +703,14 @@ PresShell::GetStyleSet(nsIStyleSet** aResult) return NS_ERROR_NULL_POINTER; } *aResult = mStyleSet; - NS_IF_ADDREF(mStyleSet); + NS_IF_ADDREF(*aResult); return NS_OK; } NS_IMETHODIMP PresShell::GetActiveAlternateStyleSheet(nsString& aSheetTitle) { // first non-html sheet in style set that has title - if (nsnull != mStyleSet) { + if (mStyleSet) { PRInt32 count = mStyleSet->GetNumberOfDocStyleSheets(); PRInt32 index; nsAutoString textHtml("text/html"); @@ -746,7 +737,7 @@ PresShell::GetActiveAlternateStyleSheet(nsString& aSheetTitle) NS_IMETHODIMP PresShell::SelectAlternateStyleSheet(const nsString& aSheetTitle) { - if ((nsnull != mDocument) && (nsnull != mStyleSet)) { + if ((nsnull != mDocument) && mStyleSet) { PRInt32 count = mDocument->GetNumberOfStyleSheets(); PRInt32 index; nsAutoString textHtml("text/html"); @@ -2431,11 +2422,10 @@ PresShell::VerifyIncrementalReflow() // Create a new presentation shell to view the document. Use the // exact same style information that this document has. - nsIStyleSet* newSet; - rv = CloneStyleSet(mStyleSet, &newSet); + nsCOMPtr newSet; + rv = CloneStyleSet(mStyleSet, getter_AddRefs(newSet)); NS_ASSERTION(NS_SUCCEEDED(rv), "failed to clone style set"); rv = mDocument->CreateShell(cx, vm, newSet, &sh); - NS_RELEASE(newSet); NS_ASSERTION(NS_OK == rv, "failed to create presentation shell"); vm->SetViewObserver((nsIViewObserver *)((PresShell*)sh)); sh->InitialReflow(r.width, r.height); diff --git a/rdf/content/src/nsXULDocument.cpp b/rdf/content/src/nsXULDocument.cpp index bd55bc762021..c95597ce1143 100644 --- a/rdf/content/src/nsXULDocument.cpp +++ b/rdf/content/src/nsXULDocument.cpp @@ -677,35 +677,40 @@ protected: nsresult AddNamedDataSource(const char* uri); + // IMPORTANT: The ownership implicit in the following member variables has been + // explicitly checked and set using nsCOMPtr for owning pointers and raw COM interface + // pointers for weak (ie, non owning) references. If you add any members to this + // class, please make the ownership explicit (pinkerton, scc). + // NOTE, THIS IS STILL IN PROGRESS, TALK TO PINK OR SCC BEFORE CHANGING - nsIArena* mArena; + nsCOMPtr mArena; nsVoidArray mObservers; nsAutoString mDocumentTitle; - nsIURL* mDocumentURL; - nsIURLGroup* mDocumentURLGroup; - nsIContent* mRootContent; + nsIURL* mDocumentURL; // [OWNER] ??? compare with loader + nsIURLGroup* mDocumentURLGroup; // [OWNER] leads to loader + nsIContent* mRootContent; // [OWNER] nsIDocument* mParentDocument; - nsIScriptContextOwner* mScriptContextOwner; - void* mScriptObject; + nsIScriptContextOwner* mScriptContextOwner; // [WEAK] it owns me! (indirectly) + void* mScriptObject; // ???? nsString mCharSetID; nsVoidArray mStyleSheets; - nsIDOMSelection* mSelection; + nsIDOMSelection* mSelection; // [OWNER] PRBool mDisplaySelection; nsVoidArray mPresShells; - nsINameSpaceManager* mNameSpaceManager; - nsIHTMLStyleSheet* mAttrStyleSheet; - nsIHTMLCSSStyleSheet* mInlineStyleSheet; + nsINameSpaceManager* mNameSpaceManager; // [OWNER] + nsIHTMLStyleSheet* mAttrStyleSheet; // [OWNER] + nsCOMPtr mInlineStyleSheet; nsElementMap mResources; - nsISupportsArray* mBuilders; - nsIRDFContentModelBuilder* mXULBuilder; - nsIRDFDataSource* mLocalDataSource; - nsIRDFDataSource* mDocumentDataSource; - nsILineBreaker* mLineBreaker; - nsIWordBreaker* mWordBreaker; - nsIContentViewerContainer* mContentViewerContainer; + nsISupportsArray* mBuilders; // [OWNER] of array, elements shouldn't own this, but they do + nsIRDFContentModelBuilder* mXULBuilder; // [OWNER] + nsIRDFDataSource* mLocalDataSource; // [OWNER] + nsIRDFDataSource* mDocumentDataSource; // [OWNER] + nsILineBreaker* mLineBreaker; // [OWNER] + nsIWordBreaker* mWordBreaker; // [OWNER] + nsIContentViewerContainer* mContentViewerContainer; // [WEAK] it owns me! (indirectly) nsString mCommand; - nsIRDFResource* mFragmentRoot; - nsVoidArray mSubDocuments; + nsIRDFResource* mFragmentRoot; // [OWNER] + nsVoidArray mSubDocuments; // [OWNER] of subelements }; PRInt32 XULDocumentImpl::gRefCnt = 0; @@ -721,8 +726,7 @@ nsIRDFResource* XULDocumentImpl::kXUL_element; // ctors & dtors XULDocumentImpl::XULDocumentImpl(void) - : mArena(nsnull), - mDocumentURL(nsnull), + : mDocumentURL(nsnull), mDocumentURLGroup(nsnull), mRootContent(nsnull), mParentDocument(nsnull), @@ -793,7 +797,8 @@ XULDocumentImpl::~XULDocumentImpl() NS_RELEASE(subdoc); } - // set all builder references to document to nsnull + // set all builder references to document to nsnull -- out of band notification + // to break ownership cycle if (mBuilders) { @@ -822,16 +827,13 @@ XULDocumentImpl::~XULDocumentImpl() NS_IF_RELEASE(mBuilders); NS_IF_RELEASE(mXULBuilder); NS_IF_RELEASE(mSelection); - NS_IF_RELEASE(mScriptContextOwner); NS_IF_RELEASE(mAttrStyleSheet); NS_IF_RELEASE(mRootContent); NS_IF_RELEASE(mDocumentURLGroup); NS_IF_RELEASE(mDocumentURL); - NS_IF_RELEASE(mArena); NS_IF_RELEASE(mNameSpaceManager); NS_IF_RELEASE(mLineBreaker); NS_IF_RELEASE(mWordBreaker); - NS_IF_RELEASE(mContentViewerContainer); NS_IF_RELEASE(mFragmentRoot); if (--gRefCnt == 0) { @@ -938,8 +940,9 @@ NS_IMPL_RELEASE(XULDocumentImpl); nsIArena* XULDocumentImpl::GetArena() { - NS_IF_ADDREF(mArena); - return mArena; + nsIArena* result = mArena; + NS_IF_ADDREF(result); + return result; } static @@ -988,13 +991,7 @@ XULDocumentImpl::PrepareToLoad( nsCOMPtr* created_parser, #endif if (aContainer && aContainer != mContentViewerContainer) - { - NS_IF_RELEASE(mContentViewerContainer); - - // AddRef and hold the container - NS_ADDREF(aContainer); mContentViewerContainer = aContainer; - } nsresult rv; @@ -1043,11 +1040,9 @@ XULDocumentImpl::PrepareToLoad( nsCOMPtr* created_parser, if (NS_SUCCEEDED(rv = nsComponentManager::CreateInstance(kHTMLCSSStyleSheetCID, nsnull, kIHTMLCSSStyleSheetIID, - (void**) &inlineSheet))) { + (void**)&inlineSheet))) { if (NS_SUCCEEDED(rv = inlineSheet->Init(syntheticURL, this))) { - mInlineStyleSheet = inlineSheet; - NS_ADDREF(mInlineStyleSheet); - + mInlineStyleSheet = dont_QueryInterface(inlineSheet); AddStyleSheet(mInlineStyleSheet); } NS_RELEASE(inlineSheet); @@ -1397,7 +1392,7 @@ XULDocumentImpl::CreateShell(nsIPresContext* aContext, } mPresShells.AppendElement(shell); - *aInstancePtrResult = shell; // addref implicit + *aInstancePtrResult = shell; // addref implicit in CreateInstance() return NS_OK; } @@ -1624,9 +1619,7 @@ XULDocumentImpl::SetScriptContextOwner(nsIScriptContextOwner *aScriptContextOwne if (!aScriptContextOwner && mRootContent) mRootContent->SetDocument(nsnull, PR_TRUE); - NS_IF_RELEASE(mScriptContextOwner); mScriptContextOwner = aScriptContextOwner; - NS_IF_ADDREF(mScriptContextOwner); } NS_IMETHODIMP @@ -2731,11 +2724,10 @@ XULDocumentImpl::SearchForNodeByID(const nsString& anID, NS_IMETHODIMP XULDocumentImpl::GetContentViewerContainer(nsIContentViewerContainer** aContainer) { - if (mContentViewerContainer != nsnull) - { - NS_ADDREF(mContentViewerContainer); - *aContainer = mContentViewerContainer; - } + NS_PRECONDITION ( aContainer, "Null Parameter into GetContentViewerContainer" ); + + *aContainer = mContentViewerContainer; + NS_IF_ADDREF(*aContainer); return NS_OK; } @@ -3134,11 +3126,11 @@ XULDocumentImpl::GetInlineStyleSheet(nsIHTMLCSSStyleSheet** aResult) return NS_ERROR_NULL_POINTER; } *aResult = mInlineStyleSheet; - if (nsnull == mInlineStyleSheet) { + if (!mInlineStyleSheet) { return NS_ERROR_NOT_AVAILABLE; // probably not the right error... } else { - NS_ADDREF(mInlineStyleSheet); + NS_ADDREF(*aResult); } return NS_OK; } @@ -3437,7 +3429,7 @@ XULDocumentImpl::Init(void) { nsresult rv; - if (NS_FAILED(rv = NS_NewHeapArena(&mArena, nsnull))) + if (NS_FAILED(rv = NS_NewHeapArena(getter_AddRefs(mArena), nsnull))) return rv; // Create a namespace manager so we can manage tags diff --git a/uriloader/base/nsDocLoader.cpp b/uriloader/base/nsDocLoader.cpp index 8a34a7c8fe4f..3b58fc05d96b 100644 --- a/uriloader/base/nsDocLoader.cpp +++ b/uriloader/base/nsDocLoader.cpp @@ -874,19 +874,26 @@ private: static PRBool IsBusyEnumerator(void* aElement, void* aData); public: - nsIDocumentLoaderFactory* m_DocFactory; + nsCOMPtr m_DocFactory; protected: - nsIURL* mDocumentUrl; - nsISupportsArray* m_LoadingDocsList; + + // IMPORTANT: The ownership implicit in the following member variables has been + // explicitly checked and set using nsCOMPtr for owning pointers and raw COM interface + // pointers for weak (ie, non owning) references. If you add any members to this + // class, please make the ownership explicit (pinkerton, scc). + + nsIURL* mDocumentUrl; // [OWNER] ???compare with document + nsCOMPtr m_LoadingDocsList; nsVoidArray mChildGroupList; nsVoidArray mDocObservers; - nsILoadAttribs* m_LoadAttrib; - nsIStreamObserver* mStreamObserver; - nsIContentViewerContainer* mContainer; + nsCOMPtr m_LoadAttrib; + nsCOMPtr mStreamObserver; // ??? unclear what to do here + nsIContentViewerContainer* mContainer; // [WEAK] it owns me! - nsDocLoaderImpl* mParent; + nsDocLoaderImpl* mParent; // [OWNER] but upside down ownership model + // needs to be fixed*** /* * The following counts are for the current document loader only. They * do not take into account URLs being loaded by child document loaders. @@ -914,19 +921,16 @@ nsDocLoaderImpl::nsDocLoaderImpl() mDocumentUrl = nsnull; mParent = nsnull; - mStreamObserver = nsnull; mContainer = nsnull; mForegroundURLs = 0; mTotalURLs = 0; mIsLoadingDocument = PR_FALSE; - NS_NewISupportsArray(&m_LoadingDocsList); - NS_NewLoadAttribs(&m_LoadAttrib); - - m_DocFactory = new nsDocFactoryImpl(); - NS_ADDREF(m_DocFactory); + NS_NewISupportsArray(getter_AddRefs(m_LoadingDocsList)); + NS_NewLoadAttribs(getter_AddRefs(m_LoadAttrib)); + m_DocFactory = do_QueryInterface(NS_STATIC_CAST(nsIDocumentLoaderFactory*, new nsDocFactoryImpl())); PR_LOG(gDocLoaderLog, PR_LOG_DEBUG, ("DocLoader:%p: created.\n", this)); @@ -941,11 +945,6 @@ nsDocLoaderImpl::~nsDocLoaderImpl() NS_RELEASE(mParent); } - NS_IF_RELEASE(m_LoadingDocsList); - NS_IF_RELEASE(m_DocFactory); - NS_IF_RELEASE(m_LoadAttrib); - NS_IF_RELEASE(mStreamObserver); - NS_IF_RELEASE(mContainer); NS_IF_RELEASE(mDocumentUrl); PR_LOG(gDocLoaderLog, PR_LOG_DEBUG, @@ -1015,9 +1014,7 @@ done: NS_IMETHODIMP nsDocLoaderImpl::SetDocumentFactory(nsIDocumentLoaderFactory* aFactory) { - NS_IF_RELEASE(m_DocFactory); - m_DocFactory = aFactory; - NS_IF_ADDREF(m_DocFactory); + m_DocFactory = dont_QueryInterface(aFactory); return NS_OK; } @@ -1025,8 +1022,8 @@ nsDocLoaderImpl::SetDocumentFactory(nsIDocumentLoaderFactory* aFactory) NS_IMETHODIMP nsDocLoaderImpl::GetDocumentFactory(nsIDocumentLoaderFactory** aResult) { - NS_IF_ADDREF(m_DocFactory); *aResult = m_DocFactory; + NS_IF_ADDREF(*aResult); return NS_OK; } @@ -1101,9 +1098,7 @@ nsDocLoaderImpl::LoadDocument(const nsString& aURLSpec, m_LoadAttrib->SetLocalIP(aLocalIP); } - NS_IF_RELEASE(mStreamObserver); - mStreamObserver = anObserver; - NS_IF_ADDREF(mStreamObserver); + mStreamObserver = dont_QueryInterface(anObserver); rv = loader->Bind(aURLSpec, aPostData, nsnull); @@ -1182,7 +1177,7 @@ nsDocLoaderImpl::Stop(void) * Release the Stream Observer... * It will be set on the next LoadDocument(...) */ - NS_IF_RELEASE(mStreamObserver); + mStreamObserver = do_QueryInterface(0); // to be replaced with null_nsCOMPtr() return NS_OK; } @@ -1234,9 +1229,7 @@ nsDocLoaderImpl::RemoveObserver(nsIDocumentLoaderObserver* aObserver) NS_IMETHODIMP nsDocLoaderImpl::SetContainer(nsIContentViewerContainer* aContainer) { - NS_IF_RELEASE(mContainer); mContainer = aContainer; - NS_IF_ADDREF(mContainer); return NS_OK; } @@ -1250,7 +1243,7 @@ nsDocLoaderImpl::GetContainer(nsIContentViewerContainer** aResult) rv = NS_ERROR_NULL_POINTER; } else { *aResult = mContainer; - NS_IF_ADDREF(mContainer); + NS_IF_ADDREF(*aResult); } return rv; } @@ -1270,13 +1263,10 @@ nsDocLoaderImpl::CreateURL(nsIURL** aInstancePtrResult, } else { rv = NS_NewURL(&url, aURLSpec, aBaseURL, aContainer, this); if (NS_SUCCEEDED(rv)) { - nsILoadAttribs* loadAttributes; - - rv = url->GetLoadAttribs(&loadAttributes); - if (NS_SUCCEEDED(rv)) { + nsCOMPtr loadAttributes; + rv = url->GetLoadAttribs(getter_AddRefs(loadAttributes)); + if (loadAttributes) loadAttributes->Clone(m_LoadAttrib); - NS_RELEASE(loadAttributes); - } } *aInstancePtrResult = url; } @@ -1341,7 +1331,7 @@ NS_IMETHODIMP nsDocLoaderImpl::GetDefaultLoadAttributes(nsILoadAttribs*& aLoadAttribs) { aLoadAttribs = m_LoadAttrib; - NS_IF_ADDREF(m_LoadAttrib); + NS_IF_ADDREF(aLoadAttribs); return NS_OK;; } diff --git a/webshell/src/nsDocLoader.cpp b/webshell/src/nsDocLoader.cpp index 8a34a7c8fe4f..3b58fc05d96b 100644 --- a/webshell/src/nsDocLoader.cpp +++ b/webshell/src/nsDocLoader.cpp @@ -874,19 +874,26 @@ private: static PRBool IsBusyEnumerator(void* aElement, void* aData); public: - nsIDocumentLoaderFactory* m_DocFactory; + nsCOMPtr m_DocFactory; protected: - nsIURL* mDocumentUrl; - nsISupportsArray* m_LoadingDocsList; + + // IMPORTANT: The ownership implicit in the following member variables has been + // explicitly checked and set using nsCOMPtr for owning pointers and raw COM interface + // pointers for weak (ie, non owning) references. If you add any members to this + // class, please make the ownership explicit (pinkerton, scc). + + nsIURL* mDocumentUrl; // [OWNER] ???compare with document + nsCOMPtr m_LoadingDocsList; nsVoidArray mChildGroupList; nsVoidArray mDocObservers; - nsILoadAttribs* m_LoadAttrib; - nsIStreamObserver* mStreamObserver; - nsIContentViewerContainer* mContainer; + nsCOMPtr m_LoadAttrib; + nsCOMPtr mStreamObserver; // ??? unclear what to do here + nsIContentViewerContainer* mContainer; // [WEAK] it owns me! - nsDocLoaderImpl* mParent; + nsDocLoaderImpl* mParent; // [OWNER] but upside down ownership model + // needs to be fixed*** /* * The following counts are for the current document loader only. They * do not take into account URLs being loaded by child document loaders. @@ -914,19 +921,16 @@ nsDocLoaderImpl::nsDocLoaderImpl() mDocumentUrl = nsnull; mParent = nsnull; - mStreamObserver = nsnull; mContainer = nsnull; mForegroundURLs = 0; mTotalURLs = 0; mIsLoadingDocument = PR_FALSE; - NS_NewISupportsArray(&m_LoadingDocsList); - NS_NewLoadAttribs(&m_LoadAttrib); - - m_DocFactory = new nsDocFactoryImpl(); - NS_ADDREF(m_DocFactory); + NS_NewISupportsArray(getter_AddRefs(m_LoadingDocsList)); + NS_NewLoadAttribs(getter_AddRefs(m_LoadAttrib)); + m_DocFactory = do_QueryInterface(NS_STATIC_CAST(nsIDocumentLoaderFactory*, new nsDocFactoryImpl())); PR_LOG(gDocLoaderLog, PR_LOG_DEBUG, ("DocLoader:%p: created.\n", this)); @@ -941,11 +945,6 @@ nsDocLoaderImpl::~nsDocLoaderImpl() NS_RELEASE(mParent); } - NS_IF_RELEASE(m_LoadingDocsList); - NS_IF_RELEASE(m_DocFactory); - NS_IF_RELEASE(m_LoadAttrib); - NS_IF_RELEASE(mStreamObserver); - NS_IF_RELEASE(mContainer); NS_IF_RELEASE(mDocumentUrl); PR_LOG(gDocLoaderLog, PR_LOG_DEBUG, @@ -1015,9 +1014,7 @@ done: NS_IMETHODIMP nsDocLoaderImpl::SetDocumentFactory(nsIDocumentLoaderFactory* aFactory) { - NS_IF_RELEASE(m_DocFactory); - m_DocFactory = aFactory; - NS_IF_ADDREF(m_DocFactory); + m_DocFactory = dont_QueryInterface(aFactory); return NS_OK; } @@ -1025,8 +1022,8 @@ nsDocLoaderImpl::SetDocumentFactory(nsIDocumentLoaderFactory* aFactory) NS_IMETHODIMP nsDocLoaderImpl::GetDocumentFactory(nsIDocumentLoaderFactory** aResult) { - NS_IF_ADDREF(m_DocFactory); *aResult = m_DocFactory; + NS_IF_ADDREF(*aResult); return NS_OK; } @@ -1101,9 +1098,7 @@ nsDocLoaderImpl::LoadDocument(const nsString& aURLSpec, m_LoadAttrib->SetLocalIP(aLocalIP); } - NS_IF_RELEASE(mStreamObserver); - mStreamObserver = anObserver; - NS_IF_ADDREF(mStreamObserver); + mStreamObserver = dont_QueryInterface(anObserver); rv = loader->Bind(aURLSpec, aPostData, nsnull); @@ -1182,7 +1177,7 @@ nsDocLoaderImpl::Stop(void) * Release the Stream Observer... * It will be set on the next LoadDocument(...) */ - NS_IF_RELEASE(mStreamObserver); + mStreamObserver = do_QueryInterface(0); // to be replaced with null_nsCOMPtr() return NS_OK; } @@ -1234,9 +1229,7 @@ nsDocLoaderImpl::RemoveObserver(nsIDocumentLoaderObserver* aObserver) NS_IMETHODIMP nsDocLoaderImpl::SetContainer(nsIContentViewerContainer* aContainer) { - NS_IF_RELEASE(mContainer); mContainer = aContainer; - NS_IF_ADDREF(mContainer); return NS_OK; } @@ -1250,7 +1243,7 @@ nsDocLoaderImpl::GetContainer(nsIContentViewerContainer** aResult) rv = NS_ERROR_NULL_POINTER; } else { *aResult = mContainer; - NS_IF_ADDREF(mContainer); + NS_IF_ADDREF(*aResult); } return rv; } @@ -1270,13 +1263,10 @@ nsDocLoaderImpl::CreateURL(nsIURL** aInstancePtrResult, } else { rv = NS_NewURL(&url, aURLSpec, aBaseURL, aContainer, this); if (NS_SUCCEEDED(rv)) { - nsILoadAttribs* loadAttributes; - - rv = url->GetLoadAttribs(&loadAttributes); - if (NS_SUCCEEDED(rv)) { + nsCOMPtr loadAttributes; + rv = url->GetLoadAttribs(getter_AddRefs(loadAttributes)); + if (loadAttributes) loadAttributes->Clone(m_LoadAttrib); - NS_RELEASE(loadAttributes); - } } *aInstancePtrResult = url; } @@ -1341,7 +1331,7 @@ NS_IMETHODIMP nsDocLoaderImpl::GetDefaultLoadAttributes(nsILoadAttribs*& aLoadAttribs) { aLoadAttribs = m_LoadAttrib; - NS_IF_ADDREF(m_LoadAttrib); + NS_IF_ADDREF(aLoadAttribs); return NS_OK;; } diff --git a/webshell/src/nsDocumentViewer.cpp b/webshell/src/nsDocumentViewer.cpp index c7f5df5e84c1..3adc8c16cdae 100644 --- a/webshell/src/nsDocumentViewer.cpp +++ b/webshell/src/nsDocumentViewer.cpp @@ -102,17 +102,26 @@ private: const nsRect& aBounds, nsScrollPreference aScrolling); - protected: - nsIViewManager* mViewManager; - nsIView* mView; - nsIWidget* mWindow; - nsIContentViewerContainer* mContainer; - nsIDeviceContext *mDeviceContext; +protected: - nsIDocument* mDocument; - nsIPresContext* mPresContext; - nsIPresShell* mPresShell; - nsIStyleSheet* mUAStyleSheet; + // IMPORTANT: The ownership implicit in the following member variables has been + // explicitly checked and set using nsCOMPtr for owning pointers and raw COM interface + // pointers for weak (ie, non owning) references. If you add any members to this + // class, please make the ownership explicit (pinkerton, scc). + + nsIContentViewerContainer* mContainer; // [WEAK] it owns me! + nsCOMPtr mDeviceContext; // ??? can't hurt, but... + nsIView* mView; // [WEAK] cleaned up by view mgr + + // the following six items are explicitly in this order + // so they will be destroyed in the reverse order (pinkerton, scc) + nsCOMPtr mDocument; + nsCOMPtr mWindow; // ??? should we really own it? + nsCOMPtr mViewManager; + nsCOMPtr mPresContext; + nsCOMPtr mPresShell; + + nsCOMPtr mUAStyleSheet; }; //Class IDs @@ -142,10 +151,9 @@ DocumentViewerImpl::DocumentViewerImpl() } DocumentViewerImpl::DocumentViewerImpl(nsIPresContext* aPresContext) + : mPresContext(dont_QueryInterface(aPresContext)) { NS_INIT_REFCNT(); - mPresContext = aPresContext; - NS_IF_ADDREF(aPresContext); } // ISupports implementation... @@ -179,11 +187,7 @@ nsresult DocumentViewerImpl::QueryInterface(REFNSIID aIID, void** aInstancePtr) DocumentViewerImpl::~DocumentViewerImpl() { - // Release windows and views - NS_IF_RELEASE(mViewManager); - NS_IF_RELEASE(mWindow); - - if (nsnull != mDocument) { + if (mDocument) { //Break global object circular reference on the document //created in the DocViewer Init nsIScriptContextOwner *mOwner = mDocument->GetScriptContextOwner(); @@ -196,28 +200,17 @@ DocumentViewerImpl::~DocumentViewerImpl() } NS_RELEASE(mOwner); - mDocument->SetScriptContextOwner(nsnull); + mDocument->SetScriptContextOwner(nsnull); // out of band cleanup of webshell } - NS_RELEASE(mDocument); } - if (nsnull != mDeviceContext) { + if (mDeviceContext) mDeviceContext->FlushFontCache(); - NS_RELEASE(mDeviceContext); - } - // Note: release context then shell - NS_IF_RELEASE(mPresContext); - if (nsnull != mPresShell) { - // Break circular reference first + if (mPresShell) { + // Break circular reference (or something) mPresShell->EndObservingDocument(); - - // Then release the shell - NS_RELEASE(mPresShell); } - - NS_IF_RELEASE(mUAStyleSheet); - NS_IF_RELEASE(mContainer); } @@ -230,32 +223,26 @@ DocumentViewerImpl::~DocumentViewerImpl() NS_IMETHODIMP DocumentViewerImpl::BindToDocument(nsISupports *aDoc, const char *aCommand) { - nsresult rv; - - NS_PRECONDITION(nsnull == mDocument, "Viewer is already bound to a document!"); + NS_PRECONDITION(!mDocument, "Viewer is already bound to a document!"); #ifdef NOISY_VIEWER printf("DocumentViewerImpl::BindToDocument\n"); #endif - rv = aDoc->QueryInterface(kIDocumentIID, (void**)&mDocument); - if (nsnull != mDocument) { - } + nsresult rv; + mDocument = do_QueryInterface(aDoc,&rv); return rv; } NS_IMETHODIMP DocumentViewerImpl::SetContainer(nsIContentViewerContainer* aContainer) { - NS_IF_RELEASE(mContainer); mContainer = aContainer; - - if (nsnull != aContainer) { - if (nsnull != mPresContext) { - mPresContext->SetContainer(aContainer); - } - NS_ADDREF(mContainer); - } + + // this seems wrong if someone passes in nsnull for |aContainer|. it will leave + // |mPresContext| pointing to the old container??? (pinkerton, scc) + if (aContainer && mPresContext) + mPresContext->SetContainer(aContainer); return NS_OK; } @@ -264,7 +251,7 @@ NS_IMETHODIMP DocumentViewerImpl::GetContainer(nsIContentViewerContainer*& aResult) { aResult = mContainer; - NS_IF_ADDREF(mContainer); + NS_IF_ADDREF(aResult); return NS_OK; } @@ -278,17 +265,16 @@ DocumentViewerImpl::Init(nsNativeWidget aNativeParent, { nsresult rv; - if (nsnull == mDocument) { + if (!mDocument) { return NS_ERROR_NULL_POINTER; } - mDeviceContext = aDeviceContext; - NS_IF_ADDREF(mDeviceContext); + mDeviceContext = dont_QueryInterface(aDeviceContext); PRBool makeCX = PR_FALSE; - if (nsnull == mPresContext) { + if (!mPresContext) { // Create presentation context - rv = NS_NewGalleyContext(&mPresContext); + rv = NS_NewGalleyContext(getter_AddRefs(mPresContext)); if (NS_OK != rv) { return rv; } @@ -334,7 +320,7 @@ DocumentViewerImpl::Init(nsNativeWidget aNativeParent, if (NS_OK == rv) { // Now make the shell for the document rv = mDocument->CreateShell(mPresContext, mViewManager, styleSet, - &mPresShell); + getter_AddRefs(mPresShell)); NS_RELEASE(styleSet); if (NS_OK == rv) { // Initialize our view manager @@ -368,7 +354,7 @@ DocumentViewerImpl::Init(nsNativeWidget aNativeParent, NS_IMETHODIMP DocumentViewerImpl::Stop(void) { - if (nsnull != mPresContext) { + if (mPresContext) { mPresContext->Stop(); } return NS_OK; @@ -377,10 +363,7 @@ DocumentViewerImpl::Stop(void) NS_IMETHODIMP DocumentViewerImpl::SetUAStyleSheet(nsIStyleSheet* aUAStyleSheet) { - NS_IF_RELEASE(mUAStyleSheet); - mUAStyleSheet = aUAStyleSheet; - NS_IF_ADDREF(mUAStyleSheet); - + mUAStyleSheet = dont_QueryInterface(aUAStyleSheet); return NS_OK; } @@ -388,7 +371,7 @@ NS_IMETHODIMP DocumentViewerImpl::GetDocument(nsIDocument*& aResult) { aResult = mDocument; - NS_IF_ADDREF(mDocument); + NS_IF_ADDREF(aResult); return NS_OK; } @@ -396,7 +379,7 @@ NS_IMETHODIMP DocumentViewerImpl::GetPresShell(nsIPresShell*& aResult) { aResult = mPresShell; - NS_IF_ADDREF(mPresShell); + NS_IF_ADDREF(aResult); return NS_OK; } @@ -404,7 +387,7 @@ NS_IMETHODIMP DocumentViewerImpl::GetPresContext(nsIPresContext*& aResult) { aResult = mPresContext; - NS_IF_ADDREF(mPresContext); + NS_IF_ADDREF(aResult); return NS_OK; } @@ -583,7 +566,7 @@ DocumentViewerImpl::CreateStyleSet(nsIDocument* aDocument, // different sets for different media nsresult rv; - if (nsnull == mUAStyleSheet) { + if (!mUAStyleSheet) { NS_WARNING("unable to load UA style sheet"); } @@ -596,7 +579,7 @@ DocumentViewerImpl::CreateStyleSet(nsIDocument* aDocument, (*aStyleSet)->AddDocStyleSheet(sheet, aDocument); NS_RELEASE(sheet); } - if (nsnull != mUAStyleSheet) { + if (mUAStyleSheet) { (*aStyleSet)->AppendBackstopStyleSheet(mUAStyleSheet); } } @@ -613,7 +596,7 @@ nsresult DocumentViewerImpl::MakeWindow(nsNativeWidget aNativeParent, rv = nsComponentManager::CreateInstance(kViewManagerCID, nsnull, kIViewManagerIID, - (void **)&mViewManager); + getter_AddRefs(mViewManager)); nsCOMPtr dx; mPresContext->GetDeviceContext(getter_AddRefs(dx)); @@ -632,7 +615,7 @@ nsresult DocumentViewerImpl::MakeWindow(nsNativeWidget aNativeParent, rv = nsComponentManager::CreateInstance(kViewCID, nsnull, kIViewIID, - (void **)&mView); + (void**)&mView); static NS_DEFINE_IID(kWidgetCID, NS_CHILD_CID); if ((NS_OK != rv) || (NS_OK != mView->Init(mViewManager, tbounds, @@ -648,7 +631,7 @@ nsresult DocumentViewerImpl::MakeWindow(nsNativeWidget aNativeParent, // Setup hierarchical relationship in view manager mViewManager->SetRootView(mView); - mView->GetWidget(mWindow); + mView->GetWidget(*getter_AddRefs(mWindow)); //set frame rate to 25 fps mViewManager->SetFrameRate(25); @@ -665,7 +648,7 @@ NS_IMETHODIMP DocumentViewerImpl::CreateDocumentViewerUsing(nsIPresContext* aPresContext, nsIDocumentViewer*& aResult) { - if (nsnull == mDocument) { + if (!mDocument) { // XXX better error return NS_ERROR_NULL_POINTER; } diff --git a/webshell/src/nsWebShell.cpp b/webshell/src/nsWebShell.cpp index afde1285b1af..85c1f6de8fd4 100644 --- a/webshell/src/nsWebShell.cpp +++ b/webshell/src/nsWebShell.cpp @@ -529,6 +529,8 @@ nsWebShell::~nsWebShell() NS_IF_RELEASE(mWindow); + ++mRefCnt; // hack will come back to this (pinkerton, scc) + NS_IF_RELEASE(mContentViewer); NS_IF_RELEASE(mDeviceContext); NS_IF_RELEASE(mPrefs); diff --git a/xpfe/appshell/src/nsWebShellWindow.cpp b/xpfe/appshell/src/nsWebShellWindow.cpp index 048b0ef54dd3..fb656585152b 100644 --- a/xpfe/appshell/src/nsWebShellWindow.cpp +++ b/xpfe/appshell/src/nsWebShellWindow.cpp @@ -359,14 +359,12 @@ NS_METHOD nsWebShellWindow::Close() { ExitModalLoop(); + if ( mWebShell ) { + mWebShell->Destroy(); + NS_IF_RELEASE(mWebShell); + } + NS_IF_RELEASE(mWindow); - NS_IF_RELEASE(mWebShell); - /* note: this next Release() seems like the right thing to do, but it doesn't - appear exactly necessary, and we are afraid of possible repercussions - unexplored at this time. ("this time" being a stability release crunch.) - Revisit this later!? - */ -// Release(); nsIAppShellService* appShell; nsresult rv = nsServiceManager::GetService(kAppShellServiceCID, kIAppShellServiceIID,