Bug 175974

Summary: CacheQueryOptions::isolatedCopy() copies the cache name twice
Product: WebKit Reporter: Daniel Bates <dbates>
Component: WebCore Misc.Assignee: Daniel Bates <dbates>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, webkit-bug-importer, youennf
Priority: P2 Keywords: InRadar
Version: WebKit Local Build   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch
none
Patch none

Description Daniel Bates 2017-08-24 19:01:15 PDT
CacheQueryOptions::isolatedCopy() copies the cache name twice.
Comment 1 Daniel Bates 2017-08-24 19:03:49 PDT
Created attachment 319054 [details]
Patch
Comment 2 Daniel Bates 2017-08-24 19:06:52 PDT
I do not understand how we can came to the decision to have a user-defined constructor for CacheQueryOptions that makes an isolated copy of the passed string. I mean it seems weird that the constructor would charge for an isolated copy when a caller may not pass CacheQueryOptions to another thread. Or is the design that CacheQueryOptions is always passed to another thread?
Comment 3 youenn fablet 2017-08-24 19:21:41 PDT
Comment on attachment 319054 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=319054&action=review

> Source/WebCore/Modules/cache/CacheQueryOptions.h:-47
> -    , cacheName(cacheName.isolatedCopy())

Wouh, that is a strange constructor!
This constructor might be needed for some platforms since CacheQueryOptions is initializing some variables.
If this patch compiles on win, this patch is fine.
Otherwise, the constructor should be kept and take a String&&.

r=me, waiting for win bot
Comment 4 youenn fablet 2017-08-24 19:22:57 PDT
(In reply to Daniel Bates from comment #2)
> I do not understand how we can came to the decision to have a user-defined
> constructor for CacheQueryOptions that makes an isolated copy of the passed
> string. I mean it seems weird that the constructor would charge for an
> isolated copy when a caller may not pass CacheQueryOptions to another
> thread. Or is the design that CacheQueryOptions is always passed to another
> thread?

It is definitely a mistake.
Comment 5 Daniel Bates 2017-08-24 19:50:09 PDT
Created attachment 319061 [details]
Patch
Comment 6 Daniel Bates 2017-08-29 11:09:14 PDT
Committed r221306: <http://trac.webkit.org/changeset/221306>
Comment 7 Radar WebKit Bug Importer 2017-08-29 11:09:58 PDT
<rdar://problem/34136830>