Closed
Bug 410150
Opened 17 years ago
Closed 17 years ago
nsCacheService: only add observers that we really need
Categories
(Core :: Networking: Cache, defect)
Core
Networking: Cache
Tracking
()
VERIFIED
FIXED
mozilla1.9beta4
People
(Reporter: alfredkayser, Assigned: alfredkayser)
Details
Attachments
(1 file, 1 obsolete file)
|
9.29 KB,
patch
|
damons
:
approval1.9+
|
Details | Diff | Splinter Review |
1. Make better use of arrays and for loops to do the AddObserver/RemoveObserver calls.
2. Place the ifdef NECKO_OFFLINE_CACHE at the right locations.
3. Only Add/Remove observers for those types of cache that are build enabled.
This saves some code in general (±1.5K objectsize), but even more when compiling without DISK/OFFLINE caches, in which case also less observers are registered.
Attachment #294817 -
Flags: review?(dcamp)
| Assignee | ||
Updated•17 years ago
|
Assignee: nobody → alfredkayser
Comment 1•17 years ago
|
||
static char * prefList[]
might be better as :
static const char * prefList[]
Updated•17 years ago
|
Attachment #294817 -
Flags: review?(dcamp) → review+
| Assignee | ||
Updated•17 years ago
|
Attachment #294817 -
Flags: superreview?(cbiesinger)
Attachment #294817 -
Flags: approval1.9?
Comment 2•17 years ago
|
||
Do we need the sr on this? I'd be more comfortable with it, but don't know how available biesi is these days.
Comment 3•17 years ago
|
||
Comment on attachment 294817 [details] [diff] [review]
V1: Use array/for loops for the Add/Remove observers
Can't approve without appropriate sr ...
Attachment #294817 -
Flags: approval1.9?
Comment 4•17 years ago
|
||
Comment on attachment 294817 [details] [diff] [review]
V1: Use array/for loops for the Add/Remove observers
+ if (NS_FAILED(rv)) rv2 = rv;
while you're here, please put the body of the if on its own line
+ for (int i=0; i<NS_ARRAY_LENGTH(prefList); i++) {
+ // remove Disk cache pref observers
it's not only disk cache observers you're removing...
Attachment #294817 -
Flags: superreview?(cbiesinger) → superreview+
| Assignee | ||
Comment 5•17 years ago
|
||
Attachment #294817 -
Attachment is obsolete: true
Attachment #301852 -
Flags: approval1.9?
Comment 6•17 years ago
|
||
Comment on attachment 301852 [details] [diff] [review]
V2: With the comments cbiesinger incorporated: ready for checkin
a1.9+=damons
Attachment #301852 -
Flags: approval1.9? → approval1.9+
Updated•17 years ago
|
Keywords: checkin-needed
Updated•17 years ago
|
Version: unspecified → Trunk
Comment 7•17 years ago
|
||
Checking in netwerk/cache/src/nsCacheService.cpp;
/cvsroot/mozilla/netwerk/cache/src/nsCacheService.cpp,v <-- nsCacheService.cpp
new revision: 1.118; previous revision: 1.117
done
Status: NEW → RESOLVED
Closed: 17 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9beta4
| Assignee | ||
Comment 8•17 years ago
|
||
Verified:
libnecko.so
Total: -192 (+64/-256)
Code: -212 (+0/+0)
Data: +20 (+64/-256)
+32 (+64/-32) data.rel.ro (DATA)
+32 (+64/-32) UNDEF:libnecko.so:data.rel.ro
+32 prefList
+20 .nosyms.data.rel.ro
+12 observerList
-32 nsCacheProfilePrefObserver::Install()::C.260
-224 (+0/-224) text (DATA)
-224 (+0/-224) UNDEF:libnecko.so:text
-12 .nosyms.text
-62 nsCacheProfilePrefObserver::Install()
-150 nsCacheProfilePrefObserver::Remove()
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•