Bug 100701

Summary: AtomicString(CFStringRef) shouldn't unconditionally create a StringImpl
Product: WebKit Reporter: Anders Carlsson <andersca>
Component: Web Template FrameworkAssignee: Anders Carlsson <andersca>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, darin, msaboff, ossy, webkit.review.bot
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 100727    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Patch mitz: review+, buildbot: commit-queue-

Anders Carlsson
Reported 2012-10-29 14:46:05 PDT
AtomicString(CFStringRef) shouldn't unconditionally create a StringImpl
Attachments
Patch (13.27 KB, patch)
2012-10-29 14:54 PDT, Anders Carlsson
no flags
Patch (13.28 KB, patch)
2012-10-29 15:04 PDT, Anders Carlsson
no flags
Patch (13.31 KB, patch)
2012-10-29 15:22 PDT, Anders Carlsson
mitz: review+
buildbot: commit-queue-
Anders Carlsson
Comment 1 2012-10-29 14:54:34 PDT
Anders Carlsson
Comment 2 2012-10-29 15:04:32 PDT
Anders Carlsson
Comment 3 2012-10-29 15:22:34 PDT
mitz
Comment 4 2012-10-29 15:42:38 PDT
Comment on attachment 171322 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=171322&action=review > Source/WebCore/ChangeLog:9 > + Add __ZN3WTF12AtomicString3addEPK10__CFString. You can use the demangled name here. > Source/WebCore/platform/text/cf/AtomicStringCF.cpp:2 > + * Copyright (C) 2010, 2011, 2012 Apple Inc. All rights reserved. I think this is all from 2012.
Build Bot
Comment 5 2012-10-29 16:07:54 PDT
Anders Carlsson
Comment 6 2012-10-29 16:22:52 PDT
Benjamin Poulain
Comment 7 2012-10-29 17:00:39 PDT
<rdar://problem/12494603> Thanks for doing this, it is a nice change.
Csaba Osztrogonác
Comment 8 2012-10-29 23:02:32 PDT
Darin Adler
Comment 9 2013-01-14 09:49:57 PST
Comment on attachment 171322 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=171322&action=review > Source/WebCore/platform/text/cf/AtomicStringCF.cpp:39 > + if (const LChar* ptr = reinterpret_cast<const LChar*>(CFStringGetCStringPtr(string, kCFStringEncodingISOLatin1))) Anders, I seem to recall something that Michael Saboff had learned, where CFStringGetCStringPtr returns a non-null pointer far more often when passed ASCII as the encoding rather than ISO Latin-1 on newer versions of Foundation. Am I remembering that right? Do you know about that?
Anders Carlsson
Comment 10 2013-01-14 10:25:09 PST
(In reply to comment #9) > (From update of attachment 171322 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=171322&action=review > > > Source/WebCore/platform/text/cf/AtomicStringCF.cpp:39 > > + if (const LChar* ptr = reinterpret_cast<const LChar*>(CFStringGetCStringPtr(string, kCFStringEncodingISOLatin1))) > > Anders, I seem to recall something that Michael Saboff had learned, where CFStringGetCStringPtr returns a non-null pointer far more often when passed ASCII as the encoding rather than ISO Latin-1 on newer versions of Foundation. Am I remembering that right? Do you know about that? Hmm, that doesn't ring a bell. Should be easy to test though.
Benjamin Poulain
Comment 11 2013-01-14 13:25:45 PST
> Anders, I seem to recall something that Michael Saboff had learned, where CFStringGetCStringPtr returns a non-null pointer far more often when passed ASCII as the encoding rather than ISO Latin-1 on newer versions of Foundation. Am I remembering that right? Do you know about that? (We should probably look if we should fix Foundation instead of WebKit.)
Darin Adler
Comment 12 2013-01-15 10:15:08 PST
Anders and I checked, and I had remembered wrong. Code is fine.
Note You need to log in before you can comment on or make changes to this bug.