Changeset 162205 in webkit


Ignore:
Timestamp:
Jan 17, 2014 9:44:49 AM (10 years ago)
Author:
andersca@apple.com
Message:

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.

Location:
trunk/Source/JavaScriptCore
Files:
5 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/JavaScriptCore/API/JSStringRef.cpp

    r162192 r162205  
    8484const JSChar* JSStringGetCharactersPtr(JSStringRef string)
    8585{
    86     return string->deprecatedCharacters();
     86    return string->characters();
    8787}
    8888
    … …  
    9898        return 0;
    9999
    100     char* p = buffer;
    101     const UChar* d = string->deprecatedCharacters();
    102     ConversionResult result = convertUTF16ToUTF8(&d, d + string->length(), &p, p + bufferSize - 1, true);
    103     *p++ = '\0';
     100    char* destination = buffer;
     101    ConversionResult result;
     102    if (string->is8Bit()) {
     103        const LChar* source = string->characters8();
     104        result = convertLatin1ToUTF8(&source, source + string->length(), &destination, destination + bufferSize - 1);
     105    } else {
     106        const UChar* source = string->characters16();
     107        result = convertUTF16ToUTF8(&source, source + string->length(), &destination, destination + bufferSize - 1, true);
     108    }
     109
     110    *destination++ = '\0';
    104111    if (result != conversionOK && result != targetExhausted)
    105112        return 0;
    106113
    107     return p - buffer;
     114    return destination - buffer;
    108115}
    109116
    110117bool JSStringIsEqual(JSStringRef a, JSStringRef b)
    111118{
    112     unsigned len = a->length();
    113     return len == b->length() && 0 == memcmp(a->deprecatedCharacters(), b->deprecatedCharacters(), len * sizeof(UChar));
     119    return OpaqueJSString::equal(a, b);
    114120}
    115121

  • trunk/Source/JavaScriptCore/API/JSStringRefCF.cpp

    r162192 r162205  
    4141    // it can hold.  (<rdar://problem/6806478>)
    4242    size_t length = CFStringGetLength(string);
    43     if (length) {
    44         Vector<LChar, 1024> lcharBuffer(length);
    45         CFIndex usedBufferLength;
    46         CFIndex convertedSize = CFStringGetBytes(string, CFRangeMake(0, length), kCFStringEncodingISOLatin1, 0, false, lcharBuffer.data(), length, &usedBufferLength);
    47         if (static_cast<size_t>(convertedSize) == length && static_cast<size_t>(usedBufferLength) == length)
    48             return OpaqueJSString::create(lcharBuffer.data(), length).leakRef();
     43    if (!length)
     44        return OpaqueJSString::create(reinterpret_cast<const LChar*>(""), 0).leakRef();
    4945
    50         auto buffer = std::make_unique<UniChar[]>(length);
    51         CFStringGetCharacters(string, CFRangeMake(0, length), buffer.get());
    52         COMPILE_ASSERT(sizeof(UniChar) == sizeof(UChar), unichar_and_uchar_must_be_same_size);
    53         return OpaqueJSString::create(reinterpret_cast<UChar*>(buffer.get()), length).leakRef();
    54     }
    55    
    56     return OpaqueJSString::create(reinterpret_cast<const LChar*>(""), 0).leakRef();
     46    Vector<LChar, 1024> lcharBuffer(length);
     47    CFIndex usedBufferLength;
     48    CFIndex convertedSize = CFStringGetBytes(string, CFRangeMake(0, length), kCFStringEncodingISOLatin1, 0, false, lcharBuffer.data(), length, &usedBufferLength);
     49    if (static_cast<size_t>(convertedSize) == length && static_cast<size_t>(usedBufferLength) == length)
     50        return OpaqueJSString::create(lcharBuffer.data(), length).leakRef();
     51
     52    auto buffer = std::make_unique<UniChar[]>(length);
     53    CFStringGetCharacters(string, CFRangeMake(0, length), buffer.get());
     54    static_assert(sizeof(UniChar) == sizeof(UChar), "UniChar and UChar must be same size");
     55    return OpaqueJSString::create(reinterpret_cast<UChar*>(buffer.get()), length).leakRef();
    5756}
    5857
    59 CFStringRef JSStringCopyCFString(CFAllocatorRef alloc, JSStringRef string)
     58CFStringRef JSStringCopyCFString(CFAllocatorRef allocator, JSStringRef string)
    6059{
    6160    if (!string->length())
    6261        return CFSTR("");
    6362
    64     return CFStringCreateWithCharacters(alloc, reinterpret_cast<const UniChar*>(string->deprecatedCharacters()), string->length());
     63    if (string->is8Bit())
     64        return CFStringCreateWithBytes(allocator, reinterpret_cast<const UInt8*>(string->characters8()), string->length(), kCFStringEncodingISOLatin1, false);
     65
     66    return CFStringCreateWithCharacters(allocator, reinterpret_cast<const UniChar*>(string->characters16()), string->length());
    6567}

  • trunk/Source/JavaScriptCore/API/OpaqueJSString.cpp

    r162192 r162205  
    3535PassRefPtr<OpaqueJSString> OpaqueJSString::create(const String& string)
    3636{
    37     if (!string.isNull())
    38         return adoptRef(new OpaqueJSString(string));
    39     return 0;
     37    if (string.isNull())
     38        return nullptr;
     39
     40    return adoptRef(new OpaqueJSString(string));
     41}
     42
     43OpaqueJSString::~OpaqueJSString()
     44{
     45    // m_characters is put in a local here to avoid an extra atomic load.
     46    const UChar* characters = m_characters;
     47    if (!characters)
     48        return;
     49
     50    if (!m_string.is8Bit() && m_string.characters() == characters)
     51        return;
     52
     53    fastFree(const_cast<void*>(static_cast<const void*>(characters)));
    4054}
    4155
    … …  
    6276    return Identifier(vm, m_string.characters16(), m_string.length());
    6377}
     78
     79const UChar* OpaqueJSString::characters()
     80{
     81    if (!this)
     82        return nullptr;
     83
     84    // m_characters is put in a local here to avoid an extra atomic load.
     85    const UChar* characters = m_characters;
     86    if (characters)
     87        return characters;
     88
     89    if (m_string.isNull())
     90        return nullptr;
     91
     92    unsigned length = m_string.length();
     93    UChar* newCharacters = static_cast<UChar*>(fastMalloc(length * sizeof(UChar)));
     94
     95    if (m_string.is8Bit()) {
     96        for (size_t i = 0; i < length; ++i)
     97            newCharacters[i] = m_string.characters8()[i];
     98    } else
     99        memcpy(newCharacters, m_string.characters16(), length * sizeof(UChar));
     100
     101    if (!m_characters.compare_exchange_strong(characters, newCharacters)) {
     102        fastFree(newCharacters);
     103        return characters;
     104    }
     105
     106    return newCharacters;
     107}
     108
     109bool OpaqueJSString::equal(const OpaqueJSString* a, const OpaqueJSString* b)
     110{
     111    if (a == b)
     112        return true;
     113
     114    if (!a || !b)
     115        return false;
     116
     117    return a->m_string == b->m_string;
     118}

  • trunk/Source/JavaScriptCore/API/OpaqueJSString.h

    r162192 r162205  
    2727#define OpaqueJSString_h
    2828
     29#include <atomic>
    2930#include <wtf/ThreadSafeRefCounted.h>
    3031#include <wtf/text/WTFString.h>
    … …  
    3637
    3738struct OpaqueJSString : public ThreadSafeRefCounted<OpaqueJSString> {
    38 
    39     static PassRefPtr<OpaqueJSString> create() // null
     39    static PassRefPtr<OpaqueJSString> create()
    4040    {
    4141        return adoptRef(new OpaqueJSString);
    … …  
    5454    JS_EXPORT_PRIVATE static PassRefPtr<OpaqueJSString> create(const String&);
    5555
    56     const UChar* characters() { return deprecatedCharacters(); } // FIXME: Delete this.
    57     const UChar* deprecatedCharacters() { return this ? m_string.deprecatedCharacters() : nullptr; }
     56    JS_EXPORT_PRIVATE ~OpaqueJSString();
     57
     58    bool is8Bit() { return this ? m_string.is8Bit() : false; }
     59    const LChar* characters8() { return this ? m_string.characters8() : nullptr; }
     60    const UChar* characters16() { return this ? m_string.characters16() : nullptr; }
    5861    unsigned length() { return this ? m_string.length() : 0; }
     62
     63    const UChar* characters();
    5964
    6065    JS_EXPORT_PRIVATE String string() const;
    6166    JSC::Identifier identifier(JSC::VM*) const;
     67
     68    static bool equal(const OpaqueJSString*, const OpaqueJSString*);
    6269
    6370private:
    … …  
    6572
    6673    OpaqueJSString()
     74        : m_characters(static_cast<const UChar*>(nullptr))
    6775    {
    6876    }
    … …  
    7078    OpaqueJSString(const String& string)
    7179        : m_string(string.isolatedCopy())
     80        , m_characters(m_string.is8Bit() ? nullptr : m_string.characters16())
    7281    {
    7382    }
    … …  
    7584    OpaqueJSString(const LChar* characters, unsigned length)
    7685        : m_string(characters, length)
     86        , m_characters(nullptr)
    7787    {
    7888    }
    … …  
    8090    OpaqueJSString(const UChar* characters, unsigned length)
    8191        : m_string(characters, length)
     92        , m_characters(m_string.is8Bit() ? nullptr : m_string.characters16())
    8293    {
    8394    }
    8495
    8596    String m_string;
     97
     98    // This will be initialized on demand when characters() is called.
     99    std::atomic<const UChar*> m_characters;
    86100};
    87101

  • trunk/Source/JavaScriptCore/ChangeLog

    r162198 r162205  
     12014-01-17  Anders Carlsson  <andersca@apple.com>
     2
     3        Get rid of OpaqueJSString::deprecatedCharacters()
     4        https://bugs.webkit.org/show_bug.cgi?id=127161
     5
     6        Reviewed by Sam Weinig.
     7
     8        Handle OpaqueJSString::m_string being either 8-bit or 16-bit and add extra
     9        code paths for the 8-bit cases.
     10       
     11        Unfortunately, JSStringGetCharactersPtr is still expected to return a 16-bit character pointer.
     12        Handle this by storing a separate 16-bit string and initializing it on demand when JSStringGetCharactersPtr
     13        is called and the backing string is 8-bit.
     14       
     15        This has the nice side effect of making JSStringGetCharactersPtr thread-safe when it wasn't before.
     16        (In theory, someone could have a JSStringRef backed by an 8-bit string and call JSStringGetCharactersPtr on it
     17        causing an unsafe upconversion to a 16-bit string).
     18
     19        * API/JSStringRef.cpp:
     20        (JSStringGetCharactersPtr):
     21        Call OpaqueJSString::characters.
     22
     23        (JSStringGetUTF8CString):
     24        Add a code path that handles 8-bit strings.
     25
     26        (JSStringIsEqual):
     27        Call OpaqueJSString::equal.
     28
     29        * API/JSStringRefCF.cpp:
     30        (JSStringCreateWithCFString):
     31        Reformat the code to use an early return instead of putting most of the code inside the body of an if statement.
     32
     33        (JSStringCopyCFString):
     34        Create an 8-bit CFStringRef if possible.
     35
     36        * API/OpaqueJSString.cpp:
     37        (OpaqueJSString::create):
     38        Use nullptr.
     39
     40        (OpaqueJSString::~OpaqueJSString):
     41        Free m_characters.
     42
     43        (OpaqueJSString::characters):
     44        Do the up-conversion and store the result in m_characters.
     45
     46        (OpaqueJSString::equal):
     47        New helper function.
     48
     49        * API/OpaqueJSString.h:
     50        (OpaqueJSString::is8Bit):
     51        New function that returns whether a string is 8-bit or not.
     52
     53        (OpaqueJSString::characters8):
     54        (OpaqueJSString::characters16):
     55        Add getters.
     56
    1572014-01-17  Peter Molnar  <pmolnar.u-szeged@partner.samsung.com>
    258
Note: See TracChangeset for help on using the changeset viewer.