[SCM] WebKit Debian packaging branch, webkit-1.2, updated. upstream/1.1.90-6072-g9a69373

barraclough at apple.com barraclough at apple.com
Thu Apr 8 01:13:06 UTC 2010


The following commit has been merged in the webkit-1.2 branch:
commit 735ee66c42871d34641bd8cfb79b3a2a022069ac
Author: barraclough at apple.com <barraclough at apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date:   Mon Jan 18 19:27:50 2010 +0000

    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):
    
    
    
    git-svn-id: http://svn.webkit.org/repository/webkit/trunk@53416 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index 3278424..dfdba36 100644
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,32 @@
+2010-01-15  Gavin Barraclough  <barraclough at apple.com>
+
+        Reviewed by Darin Adler.
+
+        https://bugs.webkit.org/show_bug.cgi?id=33731
+        Remove uses of PtrAndFlags from WebCore::StringImpl.
+
+        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):
+
 2010-01-18  Chris Marrin  <cmarrin at apple.com>
 
         Reviewed by Darin Adler.
diff --git a/WebCore/WebCore.base.exp b/WebCore/WebCore.base.exp
index 7aeb6d0..2ddc861 100644
--- a/WebCore/WebCore.base.exp
+++ b/WebCore/WebCore.base.exp
@@ -142,7 +142,6 @@ __ZN7WebCore10StringImpl7replaceEtt
 __ZN7WebCore10StringImpl8endsWithEPS0_b
 __ZN7WebCore10StringImplD1Ev
 __ZN7WebCore10StringImplcvP8NSStringEv
-__ZN7WebCore10StringImpldlEPv
 __ZN7WebCore10handCursorEv
 __ZN7WebCore10setCookiesEPNS_8DocumentERKNS_4KURLERKNS_6StringE
 __ZN7WebCore11BitmapImageC1EP7CGImagePNS_13ImageObserverE
diff --git a/WebCore/platform/sql/SQLiteFileSystem.cpp b/WebCore/platform/sql/SQLiteFileSystem.cpp
index 8cd7e80..c094847 100644
--- a/WebCore/platform/sql/SQLiteFileSystem.cpp
+++ b/WebCore/platform/sql/SQLiteFileSystem.cpp
@@ -49,8 +49,9 @@ void SQLiteFileSystem::registerSQLiteVFS()
 int SQLiteFileSystem::openDatabase(const String& fileName, sqlite3** database)
 {
     // SQLite expects a null terminator on its UTF-16 strings.
-    String path = fileName;
-    return sqlite3_open16(path.charactersWithNullTermination(), database);
+    OwnArrayPtr<const UChar> path;
+    fileName.copyWithNullTermination(path);
+    return sqlite3_open16(path.get(), database);
 }
 
 String SQLiteFileSystem::getFileNameForNewDatabase(const String& dbDir, const String&,
diff --git a/WebCore/platform/sql/SQLiteStatement.cpp b/WebCore/platform/sql/SQLiteStatement.cpp
index ac96034..a488d03 100644
--- a/WebCore/platform/sql/SQLiteStatement.cpp
+++ b/WebCore/platform/sql/SQLiteStatement.cpp
@@ -63,7 +63,9 @@ int SQLiteStatement::prepare()
     ASSERT(!m_isPrepared);
     const void* tail;
     LOG(SQLDatabase, "SQL - prepare - %s", m_query.ascii().data());
-    int error = sqlite3_prepare16_v2(m_database.sqlite3Handle(), m_query.charactersWithNullTermination(), -1, &m_statement, &tail);
+    if (!m_queryWithNullTermination)
+        m_query.copyWithNullTermination(m_queryWithNullTermination);
+    int error = sqlite3_prepare16_v2(m_database.sqlite3Handle(), m_queryWithNullTermination.get(), -1, &m_statement, &tail);
     if (error != SQLITE_OK)
         LOG(SQLDatabase, "sqlite3_prepare16 failed (%i)\n%s\n%s", error, m_query.ascii().data(), sqlite3_errmsg(m_database.sqlite3Handle()));
 #ifndef NDEBUG
diff --git a/WebCore/platform/sql/SQLiteStatement.h b/WebCore/platform/sql/SQLiteStatement.h
index e62b4f0..87dba54 100644
--- a/WebCore/platform/sql/SQLiteStatement.h
+++ b/WebCore/platform/sql/SQLiteStatement.h
@@ -91,6 +91,7 @@ public:
 private:
     SQLiteDatabase& m_database;
     String m_query;
+    OwnArrayPtr<const UChar> m_queryWithNullTermination;
     sqlite3_stmt* m_statement;
 #ifndef NDEBUG
     bool m_isPrepared;
diff --git a/WebCore/platform/sql/chromium/SQLiteFileSystemChromium.cpp b/WebCore/platform/sql/chromium/SQLiteFileSystemChromium.cpp
index 752c613..a66ac9f 100644
--- a/WebCore/platform/sql/chromium/SQLiteFileSystemChromium.cpp
+++ b/WebCore/platform/sql/chromium/SQLiteFileSystemChromium.cpp
@@ -51,8 +51,9 @@ SQLiteFileSystem::SQLiteFileSystem()
 int SQLiteFileSystem::openDatabase(const String& fileName, sqlite3** database)
 {
     if (!ChromiumBridge::sandboxEnabled()) {
-        String path = fileName;
-        return sqlite3_open16(path.charactersWithNullTermination(), database);
+        OwnArrayPtr<const UChar> path;
+        fileName.copyWithNullTermination(path);
+        return sqlite3_open16(path.get(), database);
     }
 
     // open databases using the default VFS
diff --git a/WebCore/platform/text/PlatformString.h b/WebCore/platform/text/PlatformString.h
index 8a379be..2f74c25 100644
--- a/WebCore/platform/text/PlatformString.h
+++ b/WebCore/platform/text/PlatformString.h
@@ -26,6 +26,7 @@
 // on systems without case-sensitive file systems.
 
 #include "StringImpl.h"
+#include <wtf/OwnArrayPtr.h>
 
 #ifdef __OBJC__
 #include <objc/objc.h>
@@ -93,7 +94,7 @@ public:
 
     unsigned length() const;
     const UChar* characters() const;
-    const UChar* charactersWithNullTermination();
+    void copyWithNullTermination(OwnArrayPtr<const UChar>&) const;
     
     UChar operator[](unsigned i) const; // if i >= length(), returns 0    
     UChar32 characterStartingAt(unsigned) const; // Ditto.
diff --git a/WebCore/platform/text/String.cpp b/WebCore/platform/text/String.cpp
index 04b04ab..3eb75ec 100644
--- a/WebCore/platform/text/String.cpp
+++ b/WebCore/platform/text/String.cpp
@@ -329,14 +329,15 @@ const UChar* String::characters() const
     return m_impl->characters();
 }
 
-const UChar* String::charactersWithNullTermination()
+void String::copyWithNullTermination(OwnArrayPtr<const UChar>& result) const
 {
     if (!m_impl)
-        return 0;
-    if (m_impl->hasTerminatingNullCharacter())
-        return m_impl->characters();
-    m_impl = StringImpl::createWithTerminatingNullCharacter(*m_impl);
-    return m_impl->characters();
+        CRASH();
+    unsigned length = m_impl->length();
+    UChar* buffer = new UChar[length + 1];
+    memcpy(buffer, m_impl->characters(), length * sizeof(UChar));
+    buffer[length] = 0;
+    result.set(buffer);
 }
 
 String String::format(const char *format, ...)
diff --git a/WebCore/platform/text/StringImpl.cpp b/WebCore/platform/text/StringImpl.cpp
index 3b61a0b..f40d96b 100644
--- a/WebCore/platform/text/StringImpl.cpp
+++ b/WebCore/platform/text/StringImpl.cpp
@@ -57,31 +57,12 @@ static inline void deleteUCharVector(const UChar* p)
     fastFree(const_cast<UChar*>(p));
 }
 
-// Some of the factory methods create buffers using fastMalloc.
-// We must ensure that all allocations of StringImpl are allocated using
-// fastMalloc so that we don't have mis-matched frees. We accomplish 
-// this by overriding the new and delete operators.
-void* StringImpl::operator new(size_t size, void* address)
-{
-    if (address)
-        return address;  // Allocating using an internal buffer
-    return fastMalloc(size);
-}
-
-void* StringImpl::operator new(size_t size)
-{
-    return fastMalloc(size);
-}
-
-void StringImpl::operator delete(void* address)
-{
-    fastFree(address);
-}
-
 // This constructor is used only to create the empty string.
 StringImpl::StringImpl()
     : m_data(0)
+    , m_sharedBuffer(0)
     , m_length(0)
+    , m_refCountAndFlags(s_refCountIncrement)
     , m_hash(0)
 {
     // Ensure that the hash is computed so that AtomicStringHash can call existingHash()
@@ -92,7 +73,9 @@ StringImpl::StringImpl()
 
 inline StringImpl::StringImpl(const UChar* characters, unsigned length)
     : m_data(characters)
+    , m_sharedBuffer(0)
     , m_length(length)
+    , m_refCountAndFlags(s_refCountIncrement)
     , m_hash(0)
 {
     ASSERT(characters);
@@ -104,7 +87,7 @@ StringImpl::~StringImpl()
     if (inTable())
         AtomicString::remove(this);
     if (!bufferIsInternal()) {
-        SharedUChar* sharedBuffer = m_sharedBufferAndFlags.get();
+        SharedUChar* sharedBuffer = m_sharedBuffer;
         if (sharedBuffer)
             sharedBuffer->deref();
         else
@@ -970,7 +953,7 @@ PassRefPtr<StringImpl> StringImpl::create(const JSC::UString& str)
     if (sharedBuffer) {
         PassRefPtr<StringImpl> impl = adoptRef(new StringImpl(str.data(), str.size()));
         sharedBuffer->ref();
-        impl->m_sharedBufferAndFlags.set(sharedBuffer);
+        impl->m_sharedBuffer = sharedBuffer;
         return impl;
     }
     return StringImpl::create(str.data(), str.size());
@@ -986,21 +969,6 @@ JSC::UString StringImpl::ustring()
 }
 #endif
 
-PassRefPtr<StringImpl> StringImpl::createWithTerminatingNullCharacter(const StringImpl& string)
-{
-    // Use createUninitialized instead of 'new StringImpl' so that the string and its buffer
-    // get allocated in a single malloc block.
-    UChar* data;
-    int length = string.m_length;
-    RefPtr<StringImpl> terminatedString = createUninitialized(length + 1, data);
-    memcpy(data, string.m_data, length * sizeof(UChar));
-    data[length] = 0;
-    terminatedString->m_length--;
-    terminatedString->m_hash = string.m_hash;
-    terminatedString->m_sharedBufferAndFlags.setFlag(HasTerminatingNullCharacter);
-    return terminatedString.release();
-}
-
 PassRefPtr<StringImpl> StringImpl::threadsafeCopy() const
 {
     // Special-case empty strings to make sure that per-thread empty string instance isn't returned.
@@ -1014,7 +982,7 @@ PassRefPtr<StringImpl> StringImpl::crossThreadString()
     SharedUChar* shared = sharedBuffer();
     if (shared) {
         RefPtr<StringImpl> impl = adoptRef(new StringImpl(m_data, m_length));
-        impl->m_sharedBufferAndFlags.set(shared->crossThreadCopy().releaseRef());
+        impl->m_sharedBuffer = shared->crossThreadCopy().releaseRef();
         return impl.release();
     }
 
@@ -1027,9 +995,9 @@ StringImpl::SharedUChar* StringImpl::sharedBuffer()
     if (m_length < minLengthToShare || bufferIsInternal())
         return 0;
 
-    if (!m_sharedBufferAndFlags.get())
-        m_sharedBufferAndFlags.set(SharedUChar::create(new OwnFastMallocPtr<UChar>(const_cast<UChar*>(m_data))).releaseRef());
-    return m_sharedBufferAndFlags.get();
+    if (!m_sharedBuffer)
+        m_sharedBuffer = SharedUChar::create(new OwnFastMallocPtr<UChar>(const_cast<UChar*>(m_data))).releaseRef();
+    return m_sharedBuffer;
 }
 
 
diff --git a/WebCore/platform/text/StringImpl.h b/WebCore/platform/text/StringImpl.h
index f7a9d06..219c73b 100644
--- a/WebCore/platform/text/StringImpl.h
+++ b/WebCore/platform/text/StringImpl.h
@@ -26,8 +26,8 @@
 #include <limits.h>
 #include <wtf/ASCIICType.h>
 #include <wtf/CrossThreadRefCounted.h>
+#include <wtf/Noncopyable.h>
 #include <wtf/OwnFastMallocPtr.h>
-#include <wtf/PtrAndFlags.h>
 #include <wtf/RefCounted.h>
 #include <wtf/StringHashFunctions.h>
 #include <wtf/Vector.h>
@@ -58,7 +58,7 @@ enum TextCaseSensitivity { TextCaseSensitive, TextCaseInsensitive };
 
 typedef bool (*CharacterMatchFunctionPtr)(UChar);
 
-class StringImpl : public RefCounted<StringImpl> {
+class StringImpl : public Noncopyable {
     friend struct CStringTranslator;
     friend struct HashAndCharactersTranslator;
     friend struct UCharBufferTranslator;
@@ -82,8 +82,6 @@ public:
     static PassRefPtr<StringImpl> create(const char*);
     static PassRefPtr<StringImpl> createUninitialized(unsigned length, UChar*& data);
 
-    static PassRefPtr<StringImpl> createWithTerminatingNullCharacter(const StringImpl&);
-
     static PassRefPtr<StringImpl> createStrippingNullCharacters(const UChar*, unsigned length);
     static PassRefPtr<StringImpl> adopt(StringBuffer&);
     static PassRefPtr<StringImpl> adopt(Vector<UChar>&);
@@ -96,16 +94,18 @@ public:
     const UChar* characters() { return m_data; }
     unsigned length() { return m_length; }
 
-    bool hasTerminatingNullCharacter() const { return m_sharedBufferAndFlags.isFlagSet(HasTerminatingNullCharacter); }
-
-    bool inTable() const { return m_sharedBufferAndFlags.isFlagSet(InTable); }
-    void setInTable() { return m_sharedBufferAndFlags.setFlag(InTable); }
+    bool inTable() const { return m_refCountAndFlags & s_refCountFlagInTable; }
+    void setInTable() { m_refCountAndFlags |= s_refCountFlagInTable; }
 
     unsigned hash() { if (m_hash == 0) m_hash = computeHash(m_data, m_length); return m_hash; }
     unsigned existingHash() const { ASSERT(m_hash); return m_hash; }
     inline static unsigned computeHash(const UChar* data, unsigned length) { return WTF::stringHash(data, length); }
     inline static unsigned computeHash(const char* data) { return WTF::stringHash(data); }
     
+    StringImpl* ref() { m_refCountAndFlags += s_refCountIncrement; return this; }
+    ALWAYS_INLINE void deref() { m_refCountAndFlags -= s_refCountIncrement; if (!(m_refCountAndFlags & s_refCountMask)) delete this; }
+    ALWAYS_INLINE bool hasOneRef() const { return (m_refCountAndFlags & s_refCountMask) == s_refCountIncrement; }
+
     // Returns a StringImpl suitable for use on another thread.
     PassRefPtr<StringImpl> crossThreadString();
     // Makes a deep copy. Helpful only if you need to use a String on another thread
@@ -175,13 +175,9 @@ public:
     operator NSString*();
 #endif
 
-    void operator delete(void*);
-
 private:
-    // Allocation from a custom buffer is only allowed internally to avoid
-    // mismatched allocators. Callers should use create().
-    void* operator new(size_t size);
-    void* operator new(size_t size, void* address);
+    using Noncopyable::operator new;
+    void* operator new(size_t, void* inPlace) { ASSERT(inPlace); return inPlace; }
 
     static PassRefPtr<StringImpl> createStrippingNullCharactersSlowCase(const UChar*, unsigned length);
     
@@ -189,15 +185,15 @@ private:
     // In this case, the m_data pointer is an "internal buffer", and does not need to be deallocated.
     bool bufferIsInternal() { return m_data == reinterpret_cast<const UChar*>(this + 1); }
 
-    enum StringImplFlags {
-        HasTerminatingNullCharacter,
-        InTable,
-    };
+    static const unsigned s_refCountMask = 0xFFFFFFFE;
+    static const unsigned s_refCountIncrement = 0x2;
+    static const unsigned s_refCountFlagInTable = 0x1;
 
     const UChar* m_data;
+    SharedUChar* m_sharedBuffer;
     unsigned m_length;
+    unsigned m_refCountAndFlags;
     mutable unsigned m_hash;
-    PtrAndFlags<SharedUChar, StringImplFlags> m_sharedBufferAndFlags;
     // There is a fictitious variable-length UChar array at the end, which is used
     // as the internal buffer by the createUninitialized and create methods.
 };
diff --git a/WebCore/storage/OriginUsageRecord.cpp b/WebCore/storage/OriginUsageRecord.cpp
index 684df53..8128a1b 100644
--- a/WebCore/storage/OriginUsageRecord.cpp
+++ b/WebCore/storage/OriginUsageRecord.cpp
@@ -42,8 +42,8 @@ OriginUsageRecord::OriginUsageRecord()
 void OriginUsageRecord::addDatabase(const String& identifier, const String& fullPath)
 {
     ASSERT(!m_databaseMap.contains(identifier));
-    ASSERT_ARG(identifier, identifier.impl()->refCount() == 1);
-    ASSERT_ARG(fullPath, fullPath.impl()->refCount() == 1);
+    ASSERT_ARG(identifier, identifier.impl()->hasOneRef());
+    ASSERT_ARG(fullPath, fullPath.impl()->hasOneRef());
 
     m_databaseMap.set(identifier, DatabaseEntry(fullPath));
     m_unknownSet.add(identifier);
@@ -63,7 +63,7 @@ void OriginUsageRecord::removeDatabase(const String& identifier)
 void OriginUsageRecord::markDatabase(const String& identifier)
 {
     ASSERT(m_databaseMap.contains(identifier));
-    ASSERT_ARG(identifier, identifier.impl()->refCount() == 1);
+    ASSERT_ARG(identifier, identifier.impl()->hasOneRef());
 
     m_unknownSet.add(identifier);
     m_cachedDiskUsageIsValid = false;

-- 
WebKit Debian packaging



More information about the Pkg-webkit-commits mailing list