Bug 100701 – AtomicString(CFStringRef) shouldn't unconditionally create a StringImpl

• Component: Web Template Framework
• Status: RESOLVED FIXED

[Anders Carlsson]

AtomicString(CFStringRef) shouldn't unconditionally create a StringImpl

[Anders Carlsson]

Created attachment 171316 [details]
Patch

[Anders Carlsson]

Created attachment 171319 [details]
Patch

[Anders Carlsson]

Created attachment 171322 [details]
Patch

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 on attachment 171322 [details]
Patch

Attachment 171322 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/14631283

[Anders Carlsson]

Committed r132858: <http://trac.webkit.org/changeset/132858>

[Benjamin Poulain]

<rdar://problem/12494603>

Thanks for doing this, it is a nice change.

[Csaba Osztrogonác]

(In reply to comment #6)
> Committed r132858: <http://trac.webkit.org/changeset/132858>

It broke the build on Qt Mac - https://bugs.webkit.org/show_bug.cgi?id=100727

[Darin Adler]

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]

(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]

> 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]

Anders and I checked, and I had remembered wrong. Code is fine.