Changeset 53416 in webkit


Ignore:
Timestamp:
Jan 18, 2010 11:27:50 AM (14 years ago)
Author:
barraclough@apple.com
Message:

​https://bugs.webkit.org/show_bug.cgi?id=33731
Remove uses of PtrAndFlags from WebCore::StringImpl.

Reviewed by Darin Adler.

These break the OS X Leaks tool. Move the management of null-terminated copies
out from StringImpl to String, and use a bit stolen from the refCount to hold the
'InTable' flag.

  • platform/sql/SQLiteFileSystem.cpp:

(WebCore::SQLiteFileSystem::openDatabase):

  • platform/sql/SQLiteStatement.cpp:

(WebCore::SQLiteStatement::prepare):

  • platform/sql/SQLiteStatement.h:
  • platform/text/PlatformString.h:
  • platform/text/String.cpp:

(WebCore::String::copyWithNullTermination):

  • platform/text/StringImpl.cpp:

(WebCore::StringImpl::StringImpl):
(WebCore::StringImpl::~StringImpl):
(WebCore::StringImpl::create):
(WebCore::StringImpl::crossThreadString):
(WebCore::StringImpl::sharedBuffer):

  • platform/text/StringImpl.h:

(WebCore::StringImpl::inTable):
(WebCore::StringImpl::setInTable):

Location:
trunk/WebCore
Files:
11 edited

Legend:

Unmodified
Added
Removed

  • trunk/WebCore/ChangeLog

    r53415 r53416  
     12010-01-15  Gavin Barraclough  <barraclough@apple.com>
     2
     3        Reviewed by Darin Adler.
     4
     5        https://bugs.webkit.org/show_bug.cgi?id=33731
     6        Remove uses of PtrAndFlags from WebCore::StringImpl.
     7
     8        These break the OS X Leaks tool.  Move the management of null-terminated copies
     9        out from StringImpl to String, and use a bit stolen from the refCount to hold the
     10        'InTable' flag.
     11
     12        * platform/sql/SQLiteFileSystem.cpp:
     13        (WebCore::SQLiteFileSystem::openDatabase):
     14        * platform/sql/SQLiteStatement.cpp:
     15        (WebCore::SQLiteStatement::prepare):
     16        * platform/sql/SQLiteStatement.h:
     17        * platform/text/PlatformString.h:
     18        * platform/text/String.cpp:
     19        (WebCore::String::copyWithNullTermination):
     20        * platform/text/StringImpl.cpp:
     21        (WebCore::StringImpl::StringImpl):
     22        (WebCore::StringImpl::~StringImpl):
     23        (WebCore::StringImpl::create):
     24        (WebCore::StringImpl::crossThreadString):
     25        (WebCore::StringImpl::sharedBuffer):
     26        * platform/text/StringImpl.h:
     27        (WebCore::StringImpl::inTable):
     28        (WebCore::StringImpl::setInTable):
     29
    1302010-01-18  Chris Marrin  <cmarrin@apple.com>
    231

  • trunk/WebCore/WebCore.base.exp

    r53371 r53416  
    143143__ZN7WebCore10StringImplD1Ev
    144144__ZN7WebCore10StringImplcvP8NSStringEv
    145 __ZN7WebCore10StringImpldlEPv
    146145__ZN7WebCore10handCursorEv
    147146__ZN7WebCore10setCookiesEPNS_8DocumentERKNS_4KURLERKNS_6StringE

  • trunk/WebCore/platform/sql/SQLiteFileSystem.cpp

    r45487 r53416  
    5050{
    5151    // SQLite expects a null terminator on its UTF-16 strings.
    52     String path = fileName;
    53     return sqlite3_open16(path.charactersWithNullTermination(), database);
     52    OwnArrayPtr<const UChar> path;
     53    fileName.copyWithNullTermination(path);
     54    return sqlite3_open16(path.get(), database);
    5455}
    5556

  • trunk/WebCore/platform/sql/SQLiteStatement.cpp

    r30087 r53416  
    6464    const void* tail;
    6565    LOG(SQLDatabase, "SQL - prepare - %s", m_query.ascii().data());
    66     int error = sqlite3_prepare16_v2(m_database.sqlite3Handle(), m_query.charactersWithNullTermination(), -1, &m_statement, &tail);
     66    if (!m_queryWithNullTermination)
     67        m_query.copyWithNullTermination(m_queryWithNullTermination);
     68    int error = sqlite3_prepare16_v2(m_database.sqlite3Handle(), m_queryWithNullTermination.get(), -1, &m_statement, &tail);
    6769    if (error != SQLITE_OK)
    6870        LOG(SQLDatabase, "sqlite3_prepare16 failed (%i)\n%s\n%s", error, m_query.ascii().data(), sqlite3_errmsg(m_database.sqlite3Handle()));

  • trunk/WebCore/platform/sql/SQLiteStatement.h

    r32920 r53416  
    9292    SQLiteDatabase& m_database;
    9393    String m_query;
     94    OwnArrayPtr<const UChar> m_queryWithNullTermination;
    9495    sqlite3_stmt* m_statement;
    9596#ifndef NDEBUG

  • trunk/WebCore/platform/sql/chromium/SQLiteFileSystemChromium.cpp

    r50954 r53416  
    5252{
    5353    if (!ChromiumBridge::sandboxEnabled()) {
    54         String path = fileName;
    55         return sqlite3_open16(path.charactersWithNullTermination(), database);
     54        OwnArrayPtr<const UChar> path;
     55        fileName.copyWithNullTermination(path);
     56        return sqlite3_open16(path.get(), database);
    5657    }
    5758

  • trunk/WebCore/platform/text/PlatformString.h

    r52278 r53416  
    2727
    2828#include "StringImpl.h"
     29#include <wtf/OwnArrayPtr.h>
    2930
    3031#ifdef __OBJC__
    … …  
    9495    unsigned length() const;
    9596    const UChar* characters() const;
    96     const UChar* charactersWithNullTermination();
     97    void copyWithNullTermination(OwnArrayPtr<const UChar>&) const;
    9798   
    9899    UChar operator[](unsigned i) const; // if i >= length(), returns 0   

  • trunk/WebCore/platform/text/String.cpp

    r52791 r53416  
    330330}
    331331
    332 const UChar* String::charactersWithNullTermination()
    333 {
    334     if (!m_impl)
    335         return 0;
    336     if (m_impl->hasTerminatingNullCharacter())
    337         return m_impl->characters();
    338     m_impl = StringImpl::createWithTerminatingNullCharacter(*m_impl);
    339     return m_impl->characters();
     332void String::copyWithNullTermination(OwnArrayPtr<const UChar>& result) const
     333{
     334    if (!m_impl)
     335        CRASH();
     336    unsigned length = m_impl->length();
     337    UChar* buffer = new UChar[length + 1];
     338    memcpy(buffer, m_impl->characters(), length * sizeof(UChar));
     339    buffer[length] = 0;
     340    result.set(buffer);
    340341}
    341342

  • trunk/WebCore/platform/text/StringImpl.cpp

    r52463 r53416  
    5858}
    5959
    60 // Some of the factory methods create buffers using fastMalloc.
    61 // We must ensure that all allocations of StringImpl are allocated using
    62 // fastMalloc so that we don't have mis-matched frees. We accomplish
    63 // this by overriding the new and delete operators.
    64 void* StringImpl::operator new(size_t size, void* address)
    65 {
    66     if (address)
    67         return address;  // Allocating using an internal buffer
    68     return fastMalloc(size);
    69 }
    70 
    71 void* StringImpl::operator new(size_t size)
    72 {
    73     return fastMalloc(size);
    74 }
    75 
    76 void StringImpl::operator delete(void* address)
    77 {
    78     fastFree(address);
    79 }
    80 
    8160// This constructor is used only to create the empty string.
    8261StringImpl::StringImpl()
    8362    : m_data(0)
     63    , m_sharedBuffer(0)
    8464    , m_length(0)
     65    , m_refCountAndFlags(s_refCountIncrement)
    8566    , m_hash(0)
    8667{
    … …  
    9374inline StringImpl::StringImpl(const UChar* characters, unsigned length)
    9475    : m_data(characters)
     76    , m_sharedBuffer(0)
    9577    , m_length(length)
     78    , m_refCountAndFlags(s_refCountIncrement)
    9679    , m_hash(0)
    9780{
    … …  
    10588        AtomicString::remove(this);
    10689    if (!bufferIsInternal()) {
    107         SharedUChar* sharedBuffer = m_sharedBufferAndFlags.get();
     90        SharedUChar* sharedBuffer = m_sharedBuffer;
    10891        if (sharedBuffer)
    10992            sharedBuffer->deref();
    … …  
    971954        PassRefPtr<StringImpl> impl = adoptRef(new StringImpl(str.data(), str.size()));
    972955        sharedBuffer->ref();
    973         impl->m_sharedBufferAndFlags.set(sharedBuffer);
     956        impl->m_sharedBuffer = sharedBuffer;
    974957        return impl;
    975958    }
    … …  
    986969}
    987970#endif
    988 
    989 PassRefPtr<StringImpl> StringImpl::createWithTerminatingNullCharacter(const StringImpl& string)
    990 {
    991     // Use createUninitialized instead of 'new StringImpl' so that the string and its buffer
    992     // get allocated in a single malloc block.
    993     UChar* data;
    994     int length = string.m_length;
    995     RefPtr<StringImpl> terminatedString = createUninitialized(length + 1, data);
    996     memcpy(data, string.m_data, length * sizeof(UChar));
    997     data[length] = 0;
    998     terminatedString->m_length--;
    999     terminatedString->m_hash = string.m_hash;
    1000     terminatedString->m_sharedBufferAndFlags.setFlag(HasTerminatingNullCharacter);
    1001     return terminatedString.release();
    1002 }
    1003971
    1004972PassRefPtr<StringImpl> StringImpl::threadsafeCopy() const
    … …  
    1015983    if (shared) {
    1016984        RefPtr<StringImpl> impl = adoptRef(new StringImpl(m_data, m_length));
    1017         impl->m_sharedBufferAndFlags.set(shared->crossThreadCopy().releaseRef());
     985        impl->m_sharedBuffer = shared->crossThreadCopy().releaseRef();
    1018986        return impl.release();
    1019987    }
    … …  
    1028996        return 0;
    1029997
    1030     if (!m_sharedBufferAndFlags.get())
    1031         m_sharedBufferAndFlags.set(SharedUChar::create(new OwnFastMallocPtr<UChar>(const_cast<UChar*>(m_data))).releaseRef());
    1032     return m_sharedBufferAndFlags.get();
     998    if (!m_sharedBuffer)
     999        m_sharedBuffer = SharedUChar::create(new OwnFastMallocPtr<UChar>(const_cast<UChar*>(m_data))).releaseRef();
     1000    return m_sharedBuffer;
    10331001}
    10341002

  • trunk/WebCore/platform/text/StringImpl.h

    r52776 r53416  
    2727#include <wtf/ASCIICType.h>
    2828#include <wtf/CrossThreadRefCounted.h>
     29#include <wtf/Noncopyable.h>
    2930#include <wtf/OwnFastMallocPtr.h>
    30 #include <wtf/PtrAndFlags.h>
    3131#include <wtf/RefCounted.h>
    3232#include <wtf/StringHashFunctions.h>
    … …  
    5959typedef bool (*CharacterMatchFunctionPtr)(UChar);
    6060
    61 class StringImpl : public RefCounted<StringImpl> {
     61class StringImpl : public Noncopyable {
    6262    friend struct CStringTranslator;
    6363    friend struct HashAndCharactersTranslator;
    … …  
    8383    static PassRefPtr<StringImpl> createUninitialized(unsigned length, UChar*& data);
    8484
    85     static PassRefPtr<StringImpl> createWithTerminatingNullCharacter(const StringImpl&);
    86 
    8785    static PassRefPtr<StringImpl> createStrippingNullCharacters(const UChar*, unsigned length);
    8886    static PassRefPtr<StringImpl> adopt(StringBuffer&);
    … …  
    9795    unsigned length() { return m_length; }
    9896
    99     bool hasTerminatingNullCharacter() const { return m_sharedBufferAndFlags.isFlagSet(HasTerminatingNullCharacter); }
    100 
    101     bool inTable() const { return m_sharedBufferAndFlags.isFlagSet(InTable); }
    102     void setInTable() { return m_sharedBufferAndFlags.setFlag(InTable); }
     97    bool inTable() const { return m_refCountAndFlags & s_refCountFlagInTable; }
     98    void setInTable() { m_refCountAndFlags |= s_refCountFlagInTable; }
    10399
    104100    unsigned hash() { if (m_hash == 0) m_hash = computeHash(m_data, m_length); return m_hash; }
    … …  
    107103    inline static unsigned computeHash(const char* data) { return WTF::stringHash(data); }
    108104   
     105    StringImpl* ref() { m_refCountAndFlags += s_refCountIncrement; return this; }
     106    ALWAYS_INLINE void deref() { m_refCountAndFlags -= s_refCountIncrement; if (!(m_refCountAndFlags & s_refCountMask)) delete this; }
     107    ALWAYS_INLINE bool hasOneRef() const { return (m_refCountAndFlags & s_refCountMask) == s_refCountIncrement; }
     108
    109109    // Returns a StringImpl suitable for use on another thread.
    110110    PassRefPtr<StringImpl> crossThreadString();
    … …  
    176176#endif
    177177
    178     void operator delete(void*);
    179 
    180178private:
    181     // Allocation from a custom buffer is only allowed internally to avoid
    182     // mismatched allocators. Callers should use create().
    183     void* operator new(size_t size);
    184     void* operator new(size_t size, void* address);
     179    using Noncopyable::operator new;
     180    void* operator new(size_t, void* inPlace) { ASSERT(inPlace); return inPlace; }
    185181
    186182    static PassRefPtr<StringImpl> createStrippingNullCharactersSlowCase(const UChar*, unsigned length);
    … …  
    190186    bool bufferIsInternal() { return m_data == reinterpret_cast<const UChar*>(this + 1); }
    191187
    192     enum StringImplFlags {
    193         HasTerminatingNullCharacter,
    194         InTable,
    195     };
     188    static const unsigned s_refCountMask = 0xFFFFFFFE;
     189    static const unsigned s_refCountIncrement = 0x2;
     190    static const unsigned s_refCountFlagInTable = 0x1;
    196191
    197192    const UChar* m_data;
     193    SharedUChar* m_sharedBuffer;
    198194    unsigned m_length;
     195    unsigned m_refCountAndFlags;
    199196    mutable unsigned m_hash;
    200     PtrAndFlags<SharedUChar, StringImplFlags> m_sharedBufferAndFlags;
    201197    // There is a fictitious variable-length UChar array at the end, which is used
    202198    // as the internal buffer by the createUninitialized and create methods.

  • trunk/WebCore/storage/OriginUsageRecord.cpp

    r47808 r53416  
    4343{
    4444    ASSERT(!m_databaseMap.contains(identifier));
    45     ASSERT_ARG(identifier, identifier.impl()->refCount() == 1);
    46     ASSERT_ARG(fullPath, fullPath.impl()->refCount() == 1);
     45    ASSERT_ARG(identifier, identifier.impl()->hasOneRef());
     46    ASSERT_ARG(fullPath, fullPath.impl()->hasOneRef());
    4747
    4848    m_databaseMap.set(identifier, DatabaseEntry(fullPath));
    … …  
    6464{
    6565    ASSERT(m_databaseMap.contains(identifier));
    66     ASSERT_ARG(identifier, identifier.impl()->refCount() == 1);
     66    ASSERT_ARG(identifier, identifier.impl()->hasOneRef());
    6767
    6868    m_unknownSet.add(identifier);
Note: See TracChangeset for help on using the changeset viewer.