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)

11 Branch
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla17

People

(Reporter: michal, Assigned: michal)

Details

Attachments

(1 file, 1 obsolete file)

Attached patch fix (obsolete) — 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 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+
(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
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.

Attachment

General

Created:
Updated:
Size: