Closed Bug 94454 Opened 23 years ago Closed 23 years ago

Have the image cache flush when memory is low.

Categories

(Core :: Graphics: ImageLib, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: pavlov, Assigned: nivedita)

Details

Attachments

(3 files)

The image cache needs add its self as an observer of the memory thread so that it can flush the cache when memory is low.
See http://lxr.mozilla.org/seamonkey/source/xpcom/base/nsIMemory.idl#40 for how to hookup to this. imgCache is the class in imagelib that should listen to the observer.
Assignee: pavlov → nivedita
Added imagecache as an observer to the memory-pressure event.
Comment on attachment 65564 [details] [diff] [review] patch file to flush image cache when the memory is low r=pavlov
Attachment #65564 - Flags: review+
Comment on attachment 65564 [details] [diff] [review] patch file to flush image cache when the memory is low >+PR_STATIC_CALLBACK(nsresult) >+Initialize(nsIModule* aSelf) >+{ >+ imgCache::Init(); >+ return PR_TRUE; >+} >+ > PR_STATIC_CALLBACK(void) >-ImageModuleDestructor(nsIModule *self) >+Shutdown(nsIModule* aSelf) > { > imgCache::Shutdown(); > } These two static callbacks may be non-static on some systems, in order to be callbacks, so they should have unique names -- prefix their given names above with imgCache_ or some such. >+nsresult imgCache::Init() >+{ >+ nsresult rv; >+ imgCache* cache = new imgCache(); Test for null result from operator new and fail with NS_ERROR_OUT_OF_MEMORY if so. >+ >+ nsCOMPtr<nsIObserverService> os = do_GetService("@mozilla.org/observer-service;1", &rv); >+ if (NS_SUCCEEDED(rv) && os) >+ os->AddObserver(cache, "memory-pressure", PR_TRUE); Capture os->AddObserver()'s result in rv, and return it instead of NS_OK below: >+ return NS_OK; >+} >+ > /* void clearCache (in boolean chrome); */ > NS_IMETHODIMP imgCache::ClearCache(PRBool chrome) > { >@@ -255,3 +268,12 @@ > return PR_TRUE; > } > >+ >+NS_IMETHODIMP >+imgCache::Observe(nsISupports* aSubject, const char* aTopic, const PRUnichar* aSomeData) >+{ >+ if (nsCRT::strcmp(aTopic, "memory-pressure") == 0) We've finally figured out that all Mozilla platforms have working strcmp (:-) so no need for nsCRT:: here -- if you get rid of it, you'll be ahead of the game when we soon purge most str* wrapper methods from nsCRT. >+ ClearCache(PR_TRUE); >+ >+ return NS_OK; >+} Fix these issues and I'll gladly sr=brendan@mozilla.org. /be
Incorporated the comments given by Brenden
Brenden, I am not clear on the static call backs being non-static on some systems. Could you please elaborate on it. I am wondering in these cases how would the static intializers and etc be taken care of. what should be taken care while writing static methods or static blocks.
Comment on attachment 66245 [details] [diff] [review] patch file incorporating the comments given by Brenden >+PR_STATIC_CALLBACK(nsresult) >+imgCache_Initialize(nsIModule* aSelf) >+{ >+ imgCache::Init(); >+ return PR_TRUE; >+} >+ > PR_STATIC_CALLBACK(void) >-ImageModuleDestructor(nsIModule *self) >+imgCache_Shutdown(nsIModule* aSelf) > { > imgCache::Shutdown(); > } See http://lxr.mozilla.org/mozilla/source/nsprpub/pr/include/prtypes.h#147 for the macro that (on windows) defines PR_STATIC_CALLBACK to expand without the static storage class/visibility specifier. The callbacks on Windows are therefore extern, not static -- still singleton functions, but with names that pollute the entire application's global namespace, not with names scoped to the file. >+nsresult imgCache::Init() >+{ >+ nsresult rv; >+ imgCache* cache = new imgCache(); >+ if(!cache) return NS_ERROR_OUT_OF_MEMORY; >+ >+ nsCOMPtr<nsIObserverService> os = do_GetService("@mozilla.org/observer-service;1", &rv); >+ if (NS_SUCCEEDED(rv) && os) >+ rv = os->AddObserver(cache, "memory-pressure", PR_TRUE); >+ return rv; >+} Bonus style points (change only if you agree) for moving the nsresult rv; declaration down to just before it's actually needed. Thanks for the nit-picks, sr=brendan@mozilla.org. /be
Attachment #66245 - Flags: superreview+
Attachment #66245 - Flags: review+
defering the creation of nsresult untill needed. Brenden, Thanks for the info.
Comment on attachment 66246 [details] [diff] [review] incorporating the comment given by Brenden r=/sr= from previous patch
Attachment #66246 - Flags: superreview+
Attachment #66246 - Flags: review+
fix checked in. great job nivedita!
Status: NEW → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: