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)
Core
Graphics: ImageLib
Tracking
()
RESOLVED
FIXED
People
(Reporter: pavlov, Assigned: nivedita)
Details
Attachments
(3 files)
(deleted),
patch
|
pavlov
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
brendan
:
review+
brendan
:
superreview+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
pavlov
:
review+
pavlov
:
superreview+
|
Details | Diff | Splinter Review |
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.
Reporter | ||
Comment 1•23 years ago
|
||
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
Assignee | ||
Comment 2•23 years ago
|
||
Added imagecache as an observer to the memory-pressure event.
Reporter | ||
Comment 3•23 years ago
|
||
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 4•23 years ago
|
||
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
Assignee | ||
Comment 5•23 years ago
|
||
Incorporated the comments given by Brenden
Assignee | ||
Comment 6•23 years ago
|
||
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 7•23 years ago
|
||
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+
Assignee | ||
Comment 8•23 years ago
|
||
defering the creation of nsresult untill needed.
Brenden,
Thanks for the info.
Reporter | ||
Comment 9•23 years ago
|
||
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+
Reporter | ||
Comment 10•23 years ago
|
||
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.
Description
•