Commit 409f9a88 authored by andersca@apple.com's avatar andersca@apple.com

Get rid of OpaqueJSString::deprecatedCharacters()

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

Reviewed by Sam Weinig.

Handle OpaqueJSString::m_string being either 8-bit or 16-bit and add extra
code paths for the 8-bit cases.

Unfortunately, JSStringGetCharactersPtr is still expected to return a 16-bit character pointer.
Handle this by storing a separate 16-bit string and initializing it on demand when JSStringGetCharactersPtr
is called and the backing string is 8-bit.

This has the nice side effect of making JSStringGetCharactersPtr thread-safe when it wasn't before.
(In theory, someone could have a JSStringRef backed by an 8-bit string and call JSStringGetCharactersPtr on it
causing an unsafe upconversion to a 16-bit string).

* API/JSStringRef.cpp:
(JSStringGetCharactersPtr):
Call OpaqueJSString::characters.

(JSStringGetUTF8CString):
Add a code path that handles 8-bit strings.

(JSStringIsEqual):
Call OpaqueJSString::equal.

* API/JSStringRefCF.cpp:
(JSStringCreateWithCFString):
Reformat the code to use an early return instead of putting most of the code inside the body of an if statement.

(JSStringCopyCFString):
Create an 8-bit CFStringRef if possible.

* API/OpaqueJSString.cpp:
(OpaqueJSString::create):
Use nullptr.

(OpaqueJSString::~OpaqueJSString):
Free m_characters.

(OpaqueJSString::characters):
Do the up-conversion and store the result in m_characters.

(OpaqueJSString::equal):
New helper function.

* API/OpaqueJSString.h:
(OpaqueJSString::is8Bit):
New function that returns whether a string is 8-bit or not.

(OpaqueJSString::characters8):
(OpaqueJSString::characters16):
Add getters.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@162205 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent 654e35c8
......@@ -83,7 +83,7 @@ size_t JSStringGetLength(JSStringRef string)
const JSChar* JSStringGetCharactersPtr(JSStringRef string)
{
return string->deprecatedCharacters();
return string->characters();
}
size_t JSStringGetMaximumUTF8CStringSize(JSStringRef string)
......@@ -97,20 +97,26 @@ size_t JSStringGetUTF8CString(JSStringRef string, char* buffer, size_t bufferSiz
if (!bufferSize)
return 0;
char* p = buffer;
const UChar* d = string->deprecatedCharacters();
ConversionResult result = convertUTF16ToUTF8(&d, d + string->length(), &p, p + bufferSize - 1, true);
*p++ = '\0';
char* destination = buffer;
ConversionResult result;
if (string->is8Bit()) {
const LChar* source = string->characters8();
result = convertLatin1ToUTF8(&source, source + string->length(), &destination, destination + bufferSize - 1);
} else {
const UChar* source = string->characters16();
result = convertUTF16ToUTF8(&source, source + string->length(), &destination, destination + bufferSize - 1, true);
}
*destination++ = '\0';
if (result != conversionOK && result != targetExhausted)
return 0;
return p - buffer;
return destination - buffer;
}
bool JSStringIsEqual(JSStringRef a, JSStringRef b)
{
unsigned len = a->length();
return len == b->length() && 0 == memcmp(a->deprecatedCharacters(), b->deprecatedCharacters(), len * sizeof(UChar));
return OpaqueJSString::equal(a, b);
}
bool JSStringIsEqualToUTF8CString(JSStringRef a, const char* b)
......
......@@ -40,26 +40,28 @@ JSStringRef JSStringCreateWithCFString(CFStringRef string)
// We cannot use CFIndex here since CFStringGetLength can return values larger than
// it can hold. (<rdar://problem/6806478>)
size_t length = CFStringGetLength(string);
if (length) {
Vector<LChar, 1024> lcharBuffer(length);
CFIndex usedBufferLength;
CFIndex convertedSize = CFStringGetBytes(string, CFRangeMake(0, length), kCFStringEncodingISOLatin1, 0, false, lcharBuffer.data(), length, &usedBufferLength);
if (static_cast<size_t>(convertedSize) == length && static_cast<size_t>(usedBufferLength) == length)
return OpaqueJSString::create(lcharBuffer.data(), length).leakRef();
if (!length)
return OpaqueJSString::create(reinterpret_cast<const LChar*>(""), 0).leakRef();
auto buffer = std::make_unique<UniChar[]>(length);
CFStringGetCharacters(string, CFRangeMake(0, length), buffer.get());
COMPILE_ASSERT(sizeof(UniChar) == sizeof(UChar), unichar_and_uchar_must_be_same_size);
return OpaqueJSString::create(reinterpret_cast<UChar*>(buffer.get()), length).leakRef();
}
return OpaqueJSString::create(reinterpret_cast<const LChar*>(""), 0).leakRef();
Vector<LChar, 1024> lcharBuffer(length);
CFIndex usedBufferLength;
CFIndex convertedSize = CFStringGetBytes(string, CFRangeMake(0, length), kCFStringEncodingISOLatin1, 0, false, lcharBuffer.data(), length, &usedBufferLength);
if (static_cast<size_t>(convertedSize) == length && static_cast<size_t>(usedBufferLength) == length)
return OpaqueJSString::create(lcharBuffer.data(), length).leakRef();
auto buffer = std::make_unique<UniChar[]>(length);
CFStringGetCharacters(string, CFRangeMake(0, length), buffer.get());
static_assert(sizeof(UniChar) == sizeof(UChar), "UniChar and UChar must be same size");
return OpaqueJSString::create(reinterpret_cast<UChar*>(buffer.get()), length).leakRef();
}
CFStringRef JSStringCopyCFString(CFAllocatorRef alloc, JSStringRef string)
CFStringRef JSStringCopyCFString(CFAllocatorRef allocator, JSStringRef string)
{
if (!string->length())
return CFSTR("");
return CFStringCreateWithCharacters(alloc, reinterpret_cast<const UniChar*>(string->deprecatedCharacters()), string->length());
if (string->is8Bit())
return CFStringCreateWithBytes(allocator, reinterpret_cast<const UInt8*>(string->characters8()), string->length(), kCFStringEncodingISOLatin1, false);
return CFStringCreateWithCharacters(allocator, reinterpret_cast<const UniChar*>(string->characters16()), string->length());
}
......@@ -34,9 +34,23 @@ using namespace JSC;
PassRefPtr<OpaqueJSString> OpaqueJSString::create(const String& string)
{
if (!string.isNull())
return adoptRef(new OpaqueJSString(string));
return 0;
if (string.isNull())
return nullptr;
return adoptRef(new OpaqueJSString(string));
}
OpaqueJSString::~OpaqueJSString()
{
// m_characters is put in a local here to avoid an extra atomic load.
const UChar* characters = m_characters;
if (!characters)
return;
if (!m_string.is8Bit() && m_string.characters() == characters)
return;
fastFree(const_cast<void*>(static_cast<const void*>(characters)));
}
String OpaqueJSString::string() const
......@@ -61,3 +75,44 @@ Identifier OpaqueJSString::identifier(VM* vm) const
return Identifier(vm, m_string.characters16(), m_string.length());
}
const UChar* OpaqueJSString::characters()
{
if (!this)
return nullptr;
// m_characters is put in a local here to avoid an extra atomic load.
const UChar* characters = m_characters;
if (characters)
return characters;
if (m_string.isNull())
return nullptr;
unsigned length = m_string.length();
UChar* newCharacters = static_cast<UChar*>(fastMalloc(length * sizeof(UChar)));
if (m_string.is8Bit()) {
for (size_t i = 0; i < length; ++i)
newCharacters[i] = m_string.characters8()[i];
} else
memcpy(newCharacters, m_string.characters16(), length * sizeof(UChar));
if (!m_characters.compare_exchange_strong(characters, newCharacters)) {
fastFree(newCharacters);
return characters;
}
return newCharacters;
}
bool OpaqueJSString::equal(const OpaqueJSString* a, const OpaqueJSString* b)
{
if (a == b)
return true;
if (!a || !b)
return false;
return a->m_string == b->m_string;
}
......@@ -26,6 +26,7 @@
#ifndef OpaqueJSString_h
#define OpaqueJSString_h
#include <atomic>
#include <wtf/ThreadSafeRefCounted.h>
#include <wtf/text/WTFString.h>
......@@ -35,8 +36,7 @@ namespace JSC {
}
struct OpaqueJSString : public ThreadSafeRefCounted<OpaqueJSString> {
static PassRefPtr<OpaqueJSString> create() // null
static PassRefPtr<OpaqueJSString> create()
{
return adoptRef(new OpaqueJSString);
}
......@@ -53,36 +53,50 @@ struct OpaqueJSString : public ThreadSafeRefCounted<OpaqueJSString> {
JS_EXPORT_PRIVATE static PassRefPtr<OpaqueJSString> create(const String&);
const UChar* characters() { return deprecatedCharacters(); } // FIXME: Delete this.
const UChar* deprecatedCharacters() { return this ? m_string.deprecatedCharacters() : nullptr; }
JS_EXPORT_PRIVATE ~OpaqueJSString();
bool is8Bit() { return this ? m_string.is8Bit() : false; }
const LChar* characters8() { return this ? m_string.characters8() : nullptr; }
const UChar* characters16() { return this ? m_string.characters16() : nullptr; }
unsigned length() { return this ? m_string.length() : 0; }
const UChar* characters();
JS_EXPORT_PRIVATE String string() const;
JSC::Identifier identifier(JSC::VM*) const;
static bool equal(const OpaqueJSString*, const OpaqueJSString*);
private:
friend class WTF::ThreadSafeRefCounted<OpaqueJSString>;
OpaqueJSString()
: m_characters(static_cast<const UChar*>(nullptr))
{
}
OpaqueJSString(const String& string)
: m_string(string.isolatedCopy())
, m_characters(m_string.is8Bit() ? nullptr : m_string.characters16())
{
}
OpaqueJSString(const LChar* characters, unsigned length)
: m_string(characters, length)
, m_characters(nullptr)
{
}
OpaqueJSString(const UChar* characters, unsigned length)
: m_string(characters, length)
, m_characters(m_string.is8Bit() ? nullptr : m_string.characters16())
{
}
String m_string;
// This will be initialized on demand when characters() is called.
std::atomic<const UChar*> m_characters;
};
#endif
2014-01-17 Anders Carlsson <andersca@apple.com>
Get rid of OpaqueJSString::deprecatedCharacters()
https://bugs.webkit.org/show_bug.cgi?id=127161
Reviewed by Sam Weinig.
Handle OpaqueJSString::m_string being either 8-bit or 16-bit and add extra
code paths for the 8-bit cases.
Unfortunately, JSStringGetCharactersPtr is still expected to return a 16-bit character pointer.
Handle this by storing a separate 16-bit string and initializing it on demand when JSStringGetCharactersPtr
is called and the backing string is 8-bit.
This has the nice side effect of making JSStringGetCharactersPtr thread-safe when it wasn't before.
(In theory, someone could have a JSStringRef backed by an 8-bit string and call JSStringGetCharactersPtr on it
causing an unsafe upconversion to a 16-bit string).
* API/JSStringRef.cpp:
(JSStringGetCharactersPtr):
Call OpaqueJSString::characters.
(JSStringGetUTF8CString):
Add a code path that handles 8-bit strings.
(JSStringIsEqual):
Call OpaqueJSString::equal.
* API/JSStringRefCF.cpp:
(JSStringCreateWithCFString):
Reformat the code to use an early return instead of putting most of the code inside the body of an if statement.
(JSStringCopyCFString):
Create an 8-bit CFStringRef if possible.
* API/OpaqueJSString.cpp:
(OpaqueJSString::create):
Use nullptr.
(OpaqueJSString::~OpaqueJSString):
Free m_characters.
(OpaqueJSString::characters):
Do the up-conversion and store the result in m_characters.
(OpaqueJSString::equal):
New helper function.
* API/OpaqueJSString.h:
(OpaqueJSString::is8Bit):
New function that returns whether a string is 8-bit or not.
(OpaqueJSString::characters8):
(OpaqueJSString::characters16):
Add getters.
2014-01-17 Peter Molnar <pmolnar.u-szeged@partner.samsung.com>
Remove workaround for compilers not supporting deleted functions
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment