From e166b906a7e1f9f72702dac9e3ecdf3ae7589805 Mon Sep 17 00:00:00 2001 From: "bryner%brianryner.com" Date: Mon, 30 Jan 2006 18:57:38 +0000 Subject: [PATCH] Fix crashing if the document loadgroup is cancelled while there are pending tree image loads (bug 324988) r+sr=bzbarsky --- .../xul/base/src/tree/src/nsTreeBodyFrame.cpp | 82 +++++++------------ .../xul/base/src/tree/src/nsTreeBodyFrame.h | 16 +++- 2 files changed, 43 insertions(+), 55 deletions(-) diff --git a/layout/xul/base/src/tree/src/nsTreeBodyFrame.cpp b/layout/xul/base/src/tree/src/nsTreeBodyFrame.cpp index 9d67a662bd1..f6937796a77 100644 --- a/layout/xul/base/src/tree/src/nsTreeBodyFrame.cpp +++ b/layout/xul/base/src/tree/src/nsTreeBodyFrame.cpp @@ -110,18 +110,12 @@ static NS_DEFINE_CID(kWidgetCID, NS_CHILD_CID); // Enumeration function that cancels all the image requests in our cache -PR_STATIC_CALLBACK(PRBool) -CancelImageRequest(nsHashKey* aKey, void* aData, void* aClosure) +PR_STATIC_CALLBACK(PLDHashOperator) +CancelImageRequest(const nsAString& aKey, + nsTreeImageCacheEntry aEntry, void* aData) { - nsISupports* supports = NS_STATIC_CAST(nsISupports*, aData); - nsCOMPtr request = do_QueryInterface(supports); - nsCOMPtr observer; - request->GetDecoderObserver(getter_AddRefs(observer)); - NS_ASSERTION(observer, "No observer? We're leaking!"); - request->Cancel(NS_ERROR_FAILURE); - imgIDecoderObserver* observer2 = observer; - NS_RELEASE(observer2); // Balance out the addref from GetImage() - return PR_TRUE; + aEntry.request->Cancel(NS_BINDING_ABORTED); + return PL_DHASH_NEXT; } // @@ -150,7 +144,7 @@ NS_INTERFACE_MAP_END_INHERITING(nsLeafBoxFrame) // Constructor nsTreeBodyFrame::nsTreeBodyFrame(nsIPresShell* aPresShell) -:nsLeafBoxFrame(aPresShell), mTreeBoxObject(nsnull), mImageCache(nsnull), +:nsLeafBoxFrame(aPresShell), mTreeBoxObject(nsnull), mScrollbar(nsnull), mHorzScrollbar(nsnull), mColScrollContent(nsnull), mColScrollView(nsnull), mHorzPosition(0), mHorzWidth(0), mRowHeight(0), mIndentation(0), mStringWidth(-1), mTopRowIndex(0), mFocused(PR_FALSE), mHasFixedRowCount(PR_FALSE), @@ -164,10 +158,7 @@ nsTreeBodyFrame::nsTreeBodyFrame(nsIPresShell* aPresShell) // Destructor nsTreeBodyFrame::~nsTreeBodyFrame() { - if (mImageCache) { - mImageCache->Enumerate(CancelImageRequest); - delete mImageCache; - } + mImageCache.EnumerateRead(CancelImageRequest, nsnull); delete mSlots; } @@ -251,6 +242,7 @@ nsTreeBodyFrame::Init(nsPresContext* aPresContext, nsIContent* aContent, mIndentation = GetIndentation(); mRowHeight = GetRowHeight(); + NS_ENSURE_TRUE(mImageCache.Init(16), NS_ERROR_OUT_OF_MEMORY); return rv; } @@ -1742,29 +1734,27 @@ nsTreeBodyFrame::GetImage(PRInt32 aRowIndex, nsTreeColumn* aCol, PRBool aUseCont uri->GetSpec(spec); CopyUTF8toUTF16(spec, imageSrc); } - nsStringKey key(imageSrc); - if (mImageCache) { - // Look the image up in our cache. - nsCOMPtr imgReq = getter_AddRefs(NS_STATIC_CAST(imgIRequest*, mImageCache->Get(&key))); - if (imgReq) { - // Find out if the image has loaded. - PRUint32 status; - imgReq->GetImageStatus(&status); - imgReq->GetImage(aResult); // We hand back the image here. The GetImage call addrefs *aResult. - PRUint32 numFrames = 1; - if (*aResult) - (*aResult)->GetNumFrames(&numFrames); + // Look the image up in our cache. + nsTreeImageCacheEntry entry; + if (mImageCache.Get(imageSrc, &entry)) { + // Find out if the image has loaded. + PRUint32 status; + imgIRequest *imgReq = entry.request; + imgReq->GetImageStatus(&status); + imgReq->GetImage(aResult); // We hand back the image here. The GetImage call addrefs *aResult. + PRUint32 numFrames = 1; + if (*aResult) + (*aResult)->GetNumFrames(&numFrames); - if ((!(status & imgIRequest::STATUS_LOAD_COMPLETE)) || numFrames > 1) { - // We either aren't done loading, or we're animating. Add our row as a listener for invalidations. - nsCOMPtr obs; - imgReq->GetDecoderObserver(getter_AddRefs(obs)); - nsCOMPtr listener(do_QueryInterface(obs)); - if (listener) - listener->AddCell(aRowIndex, aCol); - return NS_OK; - } + if ((!(status & imgIRequest::STATUS_LOAD_COMPLETE)) || numFrames > 1) { + // We either aren't done loading, or we're animating. Add our row as a listener for invalidations. + nsCOMPtr obs; + imgReq->GetDecoderObserver(getter_AddRefs(obs)); + nsCOMPtr listener(do_QueryInterface(obs)); + if (listener) + listener->AddCell(aRowIndex, aCol); + return NS_OK; } } @@ -1815,15 +1805,8 @@ nsTreeBodyFrame::GetImage(PRInt32 aRowIndex, nsTreeColumn* aCol, PRBool aUseCont // In a case it was already cached. imageRequest->GetImage(aResult); - - if (!mImageCache) { - mImageCache = new nsSupportsHashtable(16); - if (!mImageCache) - return NS_ERROR_OUT_OF_MEMORY; - } - mImageCache->Put(&key, imageRequest); - imgIDecoderObserver* decoderObserverPtr = imgDecoderObserver; - NS_ADDREF(decoderObserverPtr); // Will get released when we remove the cache entry. + nsTreeImageCacheEntry cacheEntry = { imageRequest, imgDecoderObserver }; + mImageCache.Put(imageSrc, cacheEntry); } return NS_OK; } @@ -3682,11 +3665,8 @@ NS_IMETHODIMP nsTreeBodyFrame::ClearStyleAndImageCaches() { mStyleCache.Clear(); - if (mImageCache) { - mImageCache->Enumerate(CancelImageRequest); - delete mImageCache; - } - mImageCache = nsnull; + mImageCache.EnumerateRead(CancelImageRequest, nsnull); + mImageCache.Clear(); mScrollbar = nsnull; return NS_OK; } diff --git a/layout/xul/base/src/tree/src/nsTreeBodyFrame.h b/layout/xul/base/src/tree/src/nsTreeBodyFrame.h index 7056b1deed7..c21399d34e0 100644 --- a/layout/xul/base/src/tree/src/nsTreeBodyFrame.h +++ b/layout/xul/base/src/tree/src/nsTreeBodyFrame.h @@ -54,6 +54,14 @@ #include "nsTreeColumns.h" #include "nsTreeImageListener.h" #include "nsAutoPtr.h" +#include "nsDataHashtable.h" + +// An entry in the tree's image cache +struct nsTreeImageCacheEntry +{ + nsCOMPtr request; + nsCOMPtr listener; +}; // The actual frame that paints the cells and rows. class nsTreeBodyFrame : public nsLeafBoxFrame, @@ -344,11 +352,11 @@ protected: // Data Members // (the power set of all row properties). nsTreeStyleCache mStyleCache; - // A hashtable that maps from URLs to image requests. The URL is provided - // by the view or by the style context. The style context represents - // a resolved :-moz-tree-cell-image (or twisty) pseudo-element. + // A hashtable that maps from URLs to image request/listener pairs. The URL + // is provided by the view or by the style context. The style context + // represents a resolved :-moz-tree-cell-image (or twisty) pseudo-element. // It maps directly to an imgIRequest. - nsSupportsHashtable* mImageCache; + nsDataHashtable mImageCache; // Our scrollbars. nsIFrame* mScrollbar;