Closed
Bug 779461
Opened 12 years ago
Closed 12 years ago
Do not transfer the ownership of the key string from nsCacheRequest to nsCacheEntry
Categories
(Core :: Networking: Cache, defect)
Tracking
()
RESOLVED
FIXED
mozilla17
People
(Reporter: michal, Assigned: michal)
Details
Attachments
(1 file, 1 obsolete file)
12.89 KB,
patch
|
Details | Diff | Splinter Review |
It doesn't seem to be a big performance win to transfer the string from nsCacheRequest to nsCacheEntry, because strings share the buffer. Having the key available in the request for its whole lifecycle is good e.g. for debugging and logging (e.g. bug #729182 needs it).
Attachment #647912 -
Flags: review?(honzab.moz)
Comment 1•12 years ago
|
||
Comment on attachment 647912 [details] [diff] [review] fix Review of attachment 647912 [details] [diff] [review]: ----------------------------------------------------------------- r=honzab ::: netwerk/cache/nsCacheEntry.cpp @@ +62,5 @@ > nsCacheStoragePolicy storagePolicy, > nsCacheDevice * device, > nsCacheEntry ** result) > { > + nsCacheEntry* entry = new nsCacheEntry(nsCString(key), Maybe the constructor could take just const char * key, but up to you. ::: netwerk/cache/nsCacheEntry.h @@ +208,5 @@ > void MarkInitialized() { mFlags |= eInitializedMask; } > void MarkActive() { mFlags |= eActiveMask; } > void MarkInactive() { mFlags &= ~eActiveMask; } > > + nsCString mKey; // 4 // XXX ask scc about const'ness "// 4 and so" should probably be removed.
Attachment #647912 -
Flags: review?(honzab.moz) → review+
Assignee | ||
Comment 2•12 years ago
|
||
(In reply to Honza Bambas (:mayhemer) from comment #1) > > { > > + nsCacheEntry* entry = new nsCacheEntry(nsCString(key), > > Maybe the constructor could take just const char * key, but up to you. Then mKey in nsCacheEntry and in nsCacheRequest wouldn't share the buffer when initialized at http://hg.mozilla.org/mozilla-central/annotate/f53c2abc69bd/netwerk/cache/nsCacheService.cpp#l2019, right? > > void MarkInactive() { mFlags &= ~eActiveMask; } > > > > + nsCString mKey; // 4 // XXX ask scc about const'ness > > "// 4 and so" should probably be removed. removed
Attachment #647912 -
Attachment is obsolete: true
Assignee | ||
Comment 3•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/db1308490142
Comment 4•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/db1308490142
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
You need to log in
before you can comment on or make changes to this bug.
Description
•