Skip to content
Browse files

Do not perform 8 to 16bits characters conversion when converting a WT…

…FString to NSString/CFString


https://bugs.webkit.org/show_bug.cgi?id=90720

Patch by Benjamin Poulain <bpoulain@apple.com> on 2012-08-16
Reviewed by Geoffrey Garen.

In most String to CFString conversion, we should be able to use the "NoCopy" constructor and have
a relatively cheap conversion from WTF::String to CFString.

When the String is 8 bits, it was converted to 16 bits by getData16SlowCase() because of the call
to String::characters().

This patch adds a path for creating a CFString from a 8bits string using CFStringCreateWithBytes.

This is covered by existing tests.

* platform/text/cf/StringCF.cpp:
(WTF::String::createCFString): CFSTR() create static CFString, it is unecessary to retain it.
* platform/text/cf/StringImplCF.cpp:
(WTF::StringImpl::createCFString): The logic to avoid the StringWrapperCFAllocator has also been simplified.
The allocator creation is now closer to where it is useful.

The function CFStringCreateWithBytesNoCopy() does not necessarilly allocate a new string, it can reuse
existing strings. In those cases, the allocator is not used. For that reason, the assertion regarding
currentString is moved to the branch that always allocate new strings.


git-svn-id: http://svn.webkit.org/repository/webkit/trunk@125809 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information...
1 parent 5b6af83 commit 2e16523dc623ed83849b21f1762a2b004336490c @BenjaminPoulain BenjaminPoulain committed Aug 16, 2012
View
27 Source/WebCore/ChangeLog
@@ -1,3 +1,30 @@
+2012-08-16 Benjamin Poulain <bpoulain@apple.com>
+
+ Do not perform 8 to 16bits characters conversion when converting a WTFString to NSString/CFString
+ https://bugs.webkit.org/show_bug.cgi?id=90720
+
+ Reviewed by Geoffrey Garen.
+
+ In most String to CFString conversion, we should be able to use the "NoCopy" constructor and have
+ a relatively cheap conversion from WTF::String to CFString.
+
+ When the String is 8 bits, it was converted to 16 bits by getData16SlowCase() because of the call
+ to String::characters().
+
+ This patch adds a path for creating a CFString from a 8bits string using CFStringCreateWithBytes.
+
+ This is covered by existing tests.
+
+ * platform/text/cf/StringCF.cpp:
+ (WTF::String::createCFString): CFSTR() create static CFString, it is unecessary to retain it.
+ * platform/text/cf/StringImplCF.cpp:
+ (WTF::StringImpl::createCFString): The logic to avoid the StringWrapperCFAllocator has also been simplified.
+ The allocator creation is now closer to where it is useful.
+
+ The function CFStringCreateWithBytesNoCopy() does not necessarilly allocate a new string, it can reuse
+ existing strings. In those cases, the allocator is not used. For that reason, the assertion regarding
+ currentString is moved to the branch that always allocate new strings.
+
2012-08-16 Adam Barth <abarth@webkit.org>
DirectoryEntry should use Dictionary rather than custom bindings code
View
4 Source/WebCore/platform/text/cf/StringCF.cpp
@@ -1,5 +1,5 @@
/**
- * Copyright (C) 2006 Apple Computer, Inc.
+ * Copyright (C) 2006, 2012 Apple Computer, Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Library General Public
@@ -45,7 +45,7 @@ String::String(CFStringRef str)
CFStringRef String::createCFString() const
{
if (!m_impl)
- return static_cast<CFStringRef>(CFRetain(CFSTR("")));
+ return CFSTR("");
return m_impl->createCFString();
}
View
21 Source/WebCore/platform/text/cf/StringImplCF.cpp
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2006, 2009 Apple Inc. All rights reserved.
+ * Copyright (C) 2006, 2009, 2012 Apple Inc. All rights reserved.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Library General Public
@@ -134,18 +134,23 @@ namespace StringWrapperCFAllocator {
CFStringRef StringImpl::createCFString()
{
- CFAllocatorRef allocator = (m_length && isMainThread()) ? StringWrapperCFAllocator::allocator() : 0;
- if (!allocator)
- return CFStringCreateWithCharacters(0, reinterpret_cast<const UniChar*>(characters()), m_length);
+ if (!m_length || !isMainThread()) {
+ if (is8Bit())
+ return CFStringCreateWithBytes(0, reinterpret_cast<const UInt8*>(characters8()), m_length, kCFStringEncodingISOLatin1, false);
+ return CFStringCreateWithCharacters(0, reinterpret_cast<const UniChar*>(characters16()), m_length);
+ }
+ CFAllocatorRef allocator = StringWrapperCFAllocator::allocator();
// Put pointer to the StringImpl in a global so the allocator can store it with the CFString.
ASSERT(!StringWrapperCFAllocator::currentString);
StringWrapperCFAllocator::currentString = this;
- CFStringRef string = CFStringCreateWithCharactersNoCopy(allocator, reinterpret_cast<const UniChar*>(characters()), m_length, kCFAllocatorNull);
-
- // The allocator cleared the global when it read it, but also clear it here just in case.
- ASSERT(!StringWrapperCFAllocator::currentString);
+ CFStringRef string;
+ if (is8Bit())
+ string = CFStringCreateWithBytesNoCopy(allocator, reinterpret_cast<const UInt8*>(characters8()), m_length, kCFStringEncodingISOLatin1, false, kCFAllocatorNull);
+ else
+ string = CFStringCreateWithCharactersNoCopy(allocator, reinterpret_cast<const UniChar*>(characters16()), m_length, kCFAllocatorNull);
+ // CoreFoundation might not have to allocate anything, we clear currentString in case we did not execute allocate().
StringWrapperCFAllocator::currentString = 0;
return string;

0 comments on commit 2e16523

Please sign in to comment.
Something went wrong with that request. Please try again.