Title: [125809] trunk/Source/WebCore
Revision
125809
Author
[email protected]
Date
2012-08-16 13:48:02 -0700 (Thu, 16 Aug 2012)

Log Message

Do not perform 8 to 16bits characters conversion when converting a WTFString to NSString/CFString
https://bugs.webkit.org/show_bug.cgi?id=90720

Patch by Benjamin Poulain <[email protected]> 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.

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (125808 => 125809)


--- trunk/Source/WebCore/ChangeLog	2012-08-16 20:47:18 UTC (rev 125808)
+++ trunk/Source/WebCore/ChangeLog	2012-08-16 20:48:02 UTC (rev 125809)
@@ -1,3 +1,30 @@
+2012-08-16  Benjamin Poulain  <[email protected]>
+
+        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  <[email protected]>
 
         DirectoryEntry should use Dictionary rather than custom bindings code

Modified: trunk/Source/WebCore/platform/text/cf/StringCF.cpp (125808 => 125809)


--- trunk/Source/WebCore/platform/text/cf/StringCF.cpp	2012-08-16 20:47:18 UTC (rev 125808)
+++ trunk/Source/WebCore/platform/text/cf/StringCF.cpp	2012-08-16 20:48:02 UTC (rev 125809)
@@ -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 @@
 CFStringRef String::createCFString() const
 {
     if (!m_impl)
-        return static_cast<CFStringRef>(CFRetain(CFSTR("")));
+        return CFSTR("");
 
     return m_impl->createCFString();
 }

Modified: trunk/Source/WebCore/platform/text/cf/StringImplCF.cpp (125808 => 125809)


--- trunk/Source/WebCore/platform/text/cf/StringImplCF.cpp	2012-08-16 20:47:18 UTC (rev 125808)
+++ trunk/Source/WebCore/platform/text/cf/StringImplCF.cpp	2012-08-16 20:48:02 UTC (rev 125809)
@@ -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 @@
 
 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;
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to