[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 02:20:06 UTC 2010


The following commit has been merged in the webkit-1.2 branch:
commit 6671f13a77f9a603b3c2866a1732bf0a91cc100d
Author: barraclough at apple.com <barraclough at apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date:   Fri Mar 12 03:14:17 2010 +0000

    https://bugs.webkit.org/show_bug.cgi?id=36041
    Remove unnecessary differences in common code between WebCore::StringImpl & JSC::UStringImpl
    
    Reviewed by Oliver Hunt.
    
    Much of the code in WebCore::StringImpl and JSC::UStringImpl is now very similar,
    but has trivial and unnecessary formatting differences, such as the exact wording
    of comments, missing ASSERTs, functions implemented in the .h vs .cpp etc.
    
    JavaScriptCore:
    
    * runtime/Identifier.cpp:
    (JSC::Identifier::add): UStringImpl::empty() now automatically hashes, uas per WebCore strings.
    (JSC::Identifier::addSlowCase): UStringImpl::empty() now automatically hashes, uas per WebCore strings.
    * runtime/UStringImpl.cpp:
    (JSC::UStringImpl::~UStringImpl): Only call bufferOwnership() once, add missing ASSERTs.
    (JSC::UStringImpl::createUninitialized): Move from .h, not commonly called, no need to inline.
    (JSC::UStringImpl::create): Move from .h, not commonly called, no need to inline.
    (JSC::UStringImpl::sharedBuffer): Rewritten to more closely match WebCore implementation, remove need for separate baseSharedBuffer() method.
    * runtime/UStringImpl.h:
    (JSC::UStringImpl::UStringImpl): Automatically hash static strings, ASSERT m_data & m_length are non-null/non-zero in non-static strings.
    (JSC::UStringImpl::setHash): Add missing ASSERT.
    (JSC::UStringImpl::create): Moved to .cpp / added missing check for empty string creation.
    (JSC::UStringImpl::adopt): Vector.size() returns size_t, not unsigned.
    (JSC::UStringImpl::cost): Renamed m_bufferSubstring -> m_substringBuffer
    (JSC::UStringImpl::hash): Reordered in file.
    (JSC::UStringImpl::existingHash): Reordered in file.
    (JSC::UStringImpl::computeHash): Reordered in file, renamed parameter.
    (JSC::UStringImpl::checkConsistency): rewrote ASSERT.
    (JSC::UStringImpl::bufferOwnership): Return type should be BufferOwnership.
    (JSC::UStringImpl::): Moved friends to head of class.
    
    WebCore:
    
    * platform/text/StringImpl.cpp:
    (WebCore::StringImpl::empty): Reordered in file, made empty()->characters() return a non-null value to match JSC.
    (WebCore::StringImpl::createUninitialized): Added overflow check.
    (WebCore::StringImpl::create): Reordered in file.
    (WebCore::StringImpl::sharedBuffer): Reordered in file.
    * platform/text/StringImpl.h:
    (WebCore::StringImpl::): Remove ThreadGlobalData as friend, move SharableUChar & SharedUChar to WebCore namespace.
    (WebCore::StringImpl::StringImpl): Made static constructor method (used to create empty string) take arguments, to match JSC & prevent accidental use.
    (WebCore::StringImpl::setHash): Added missing ASSERT.
    (WebCore::StringImpl::adopt): Make adpot work with Vectors with a non-zero inline capacity.
    (WebCore::StringImpl::characters): Mark as const to match JSC.
    (WebCore::StringImpl::hash): Use !m_hash instead of m_hash == 0.
    (WebCore::StringImpl::computeHash): Remove redundant 'inline'.
    
    
    
    git-svn-id: http://svn.webkit.org/repository/webkit/trunk@55878 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/JavaScriptCore/ChangeLog b/JavaScriptCore/ChangeLog
index cac955c..550f772 100644
--- a/JavaScriptCore/ChangeLog
+++ b/JavaScriptCore/ChangeLog
@@ -1,3 +1,35 @@
+2010-03-11  Gavin Barraclough  <barraclough at apple.com>
+
+        Reviewed by Oliver Hunt.
+
+        https://bugs.webkit.org/show_bug.cgi?id=36041
+        Remove unnecessary differences in common code between WebCore::StringImpl & JSC::UStringImpl
+
+        Much of the code in WebCore::StringImpl and JSC::UStringImpl is now very similar,
+        but has trivial and unnecessary formatting differences, such as the exact wording
+        of comments, missing ASSERTs, functions implemented in the .h vs .cpp etc.
+
+        * runtime/Identifier.cpp:
+        (JSC::Identifier::add): UStringImpl::empty() now automatically hashes, uas per WebCore strings.
+        (JSC::Identifier::addSlowCase): UStringImpl::empty() now automatically hashes, uas per WebCore strings.
+        * runtime/UStringImpl.cpp:
+        (JSC::UStringImpl::~UStringImpl): Only call bufferOwnership() once, add missing ASSERTs.
+        (JSC::UStringImpl::createUninitialized): Move from .h, not commonly called, no need to inline.
+        (JSC::UStringImpl::create): Move from .h, not commonly called, no need to inline.
+        (JSC::UStringImpl::sharedBuffer): Rewritten to more closely match WebCore implementation, remove need for separate baseSharedBuffer() method.
+        * runtime/UStringImpl.h:
+        (JSC::UStringImpl::UStringImpl): Automatically hash static strings, ASSERT m_data & m_length are non-null/non-zero in non-static strings.
+        (JSC::UStringImpl::setHash): Add missing ASSERT.
+        (JSC::UStringImpl::create): Moved to .cpp / added missing check for empty string creation.
+        (JSC::UStringImpl::adopt): Vector.size() returns size_t, not unsigned.
+        (JSC::UStringImpl::cost): Renamed m_bufferSubstring -> m_substringBuffer
+        (JSC::UStringImpl::hash): Reordered in file.
+        (JSC::UStringImpl::existingHash): Reordered in file.
+        (JSC::UStringImpl::computeHash): Reordered in file, renamed parameter.
+        (JSC::UStringImpl::checkConsistency): rewrote ASSERT.
+        (JSC::UStringImpl::bufferOwnership): Return type should be BufferOwnership.
+        (JSC::UStringImpl::): Moved friends to head of class.
+
 2010-03-11  Mark Rowe  <mrowe at apple.com>
 
         Reviewed by David Kilzer.
diff --git a/JavaScriptCore/JavaScriptCore.exp b/JavaScriptCore/JavaScriptCore.exp
index 9e2493f..438912b 100644
--- a/JavaScriptCore/JavaScriptCore.exp
+++ b/JavaScriptCore/JavaScriptCore.exp
@@ -107,6 +107,7 @@ __ZN3JSC11JSByteArrayC1EPNS_9ExecStateEN3WTF17NonNullPassRefPtrINS_9StructureEEE
 __ZN3JSC11ParserArena5resetEv
 __ZN3JSC11UStringImpl12sharedBufferEv
 __ZN3JSC11UStringImpl5emptyEv
+__ZN3JSC11UStringImpl6createEN3WTF10PassRefPtrINS1_21CrossThreadRefCountedINS1_16OwnFastMallocPtrIKtEEEEEEPS5_j
 __ZN3JSC11UStringImplD1Ev
 __ZN3JSC11checkSyntaxEPNS_9ExecStateERKNS_10SourceCodeE
 __ZN3JSC12DateInstance4infoE
diff --git a/JavaScriptCore/runtime/Identifier.cpp b/JavaScriptCore/runtime/Identifier.cpp
index 4cc0e36..a361505 100644
--- a/JavaScriptCore/runtime/Identifier.cpp
+++ b/JavaScriptCore/runtime/Identifier.cpp
@@ -43,7 +43,7 @@ public:
         for (HashSet<UString::Rep*>::iterator iter = m_table.begin(); iter != end; ++iter)
             (*iter)->setIsIdentifier(false);
     }
-    
+
     std::pair<HashSet<UString::Rep*>::iterator, bool> add(UString::Rep* value)
     {
         std::pair<HashSet<UString::Rep*>::iterator, bool> result = m_table.add(value);
@@ -129,10 +129,8 @@ PassRefPtr<UString::Rep> Identifier::add(JSGlobalData* globalData, const char* c
         rep->hash();
         return rep;
     }
-    if (!c[0]) {
-        UString::Rep::empty()->hash();
+    if (!c[0])
         return UString::Rep::empty();
-    }
     if (!c[1])
         return add(globalData, globalData->smallStrings.singleCharacterStringRep(static_cast<unsigned char>(c[0])));
 
@@ -193,10 +191,8 @@ PassRefPtr<UString::Rep> Identifier::add(JSGlobalData* globalData, const UChar*
         if (c <= 0xFF)
             return add(globalData, globalData->smallStrings.singleCharacterStringRep(c));
     }
-    if (!length) {
-        UString::Rep::empty()->hash();
+    if (!length)
         return UString::Rep::empty();
-    }
     UCharBuffer buf = {s, length}; 
     pair<HashSet<UString::Rep*>::iterator, bool> addResult = globalData->identifierTable->add<UCharBuffer, UCharBufferTranslator>(buf);
 
@@ -224,10 +220,8 @@ PassRefPtr<UString::Rep> Identifier::addSlowCase(JSGlobalData* globalData, UStri
                 return r;
             }
     }
-    if (!r->length()) {
-        UString::Rep::empty()->hash();
+    if (!r->length())
         return UString::Rep::empty();
-    }
     return *globalData->identifierTable->add(r).first;
 }
 
diff --git a/JavaScriptCore/runtime/UStringImpl.cpp b/JavaScriptCore/runtime/UStringImpl.cpp
index 3ba32de..acf7f1c 100644
--- a/JavaScriptCore/runtime/UStringImpl.cpp
+++ b/JavaScriptCore/runtime/UStringImpl.cpp
@@ -36,6 +36,33 @@ using namespace std;
 
 namespace JSC {
 
+static const unsigned minLengthToShare = 20;
+
+UStringImpl::~UStringImpl()
+{
+    ASSERT(!isStatic());
+    checkConsistency();
+
+    if (isIdentifier())
+        Identifier::remove(this);
+
+    BufferOwnership ownership = bufferOwnership();
+    if (ownership != BufferInternal) {
+        if (ownership == BufferOwned) {
+            ASSERT(!m_sharedBuffer);
+            ASSERT(m_data);
+            fastFree(const_cast<UChar*>(m_data));
+        } else if (ownership == BufferSubstring) {
+            ASSERT(m_substringBuffer);
+            m_substringBuffer->deref();
+        } else {
+            ASSERT(ownership == BufferShared);
+            ASSERT(m_sharedBuffer);
+            m_sharedBuffer->deref();
+        }
+    }
+}
+
 UStringImpl* UStringImpl::empty()
 {
     // A non-null pointer at an invalid address (in page zero) so that if it were to be accessed we
@@ -45,6 +72,25 @@ UStringImpl* UStringImpl::empty()
     return &emptyString;
 }
 
+PassRefPtr<UStringImpl> UStringImpl::createUninitialized(unsigned length, UChar*& data)
+{
+    if (!length) {
+        data = 0;
+        return empty();
+    }
+
+    // Allocate a single buffer large enough to contain the StringImpl
+    // struct as well as the data which it contains. This removes one 
+    // heap allocation from this call.
+    if (length > ((std::numeric_limits<size_t>::max() - sizeof(UStringImpl)) / sizeof(UChar)))
+        CRASH();
+    size_t size = sizeof(UStringImpl) + length * sizeof(UChar);
+    UStringImpl* string = static_cast<UStringImpl*>(fastMalloc(size));
+
+    data = reinterpret_cast<UChar*>(string + 1);
+    return adoptRef(new (string) UStringImpl(length));
+}
+
 PassRefPtr<UStringImpl> UStringImpl::create(const UChar* characters, unsigned length)
 {
     if (!characters || !length)
@@ -77,50 +123,35 @@ PassRefPtr<UStringImpl> UStringImpl::create(const char* string)
     return create(string, strlen(string));
 }
 
-SharedUChar* UStringImpl::baseSharedBuffer()
+PassRefPtr<UStringImpl> UStringImpl::create(PassRefPtr<SharedUChar> sharedBuffer, const UChar* buffer, unsigned length)
 {
-    ASSERT((bufferOwnership() == BufferShared)
-        || ((bufferOwnership() == BufferOwned) && !m_buffer));
-
-    if (bufferOwnership() != BufferShared) {
-        m_refCountAndFlags = (m_refCountAndFlags & ~s_refCountMaskBufferOwnership) | BufferShared;
-        m_bufferShared = SharedUChar::create(new SharableUChar(m_data)).releaseRef();
-    }
-
-    return m_bufferShared;
+    if (!length)
+        return empty();
+    return adoptRef(new UStringImpl(buffer, length, sharedBuffer));
 }
 
 SharedUChar* UStringImpl::sharedBuffer()
 {
-    if (m_length < s_minLengthToShare)
+    if (m_length < minLengthToShare)
         return 0;
+    // All static strings are smaller that the minimim length to share.
     ASSERT(!isStatic());
 
-    UStringImpl* owner = bufferOwnerString();
-    if (owner->bufferOwnership() == BufferInternal)
-        return 0;
+    BufferOwnership ownership = bufferOwnership();
 
-    return owner->baseSharedBuffer();
-}
-
-UStringImpl::~UStringImpl()
-{
-    ASSERT(!isStatic());
-    checkConsistency();
-
-    if (isIdentifier())
-        Identifier::remove(this);
-
-    if (bufferOwnership() != BufferInternal) {
-        if (bufferOwnership() == BufferOwned)
-            fastFree(const_cast<UChar*>(m_data));
-        else if (bufferOwnership() == BufferSubstring)
-            m_bufferSubstring->deref();
-        else {
-            ASSERT(bufferOwnership() == BufferShared);
-            m_bufferShared->deref();
-        }
+    if (ownership == BufferInternal)
+        return 0;
+    if (ownership == BufferSubstring)
+        return m_substringBuffer->sharedBuffer();
+    if (ownership == BufferOwned) {
+        ASSERT(!m_sharedBuffer);
+        m_sharedBuffer = SharedUChar::create(new SharableUChar(m_data)).releaseRef();
+        m_refCountAndFlags = (m_refCountAndFlags & ~s_refCountMaskBufferOwnership) | BufferShared;
     }
+
+    ASSERT(bufferOwnership() == BufferShared);
+    ASSERT(m_sharedBuffer);
+    return m_sharedBuffer;
 }
 
 void URopeImpl::derefFibersNonRecursive(Vector<URopeImpl*, 32>& workQueue)
diff --git a/JavaScriptCore/runtime/UStringImpl.h b/JavaScriptCore/runtime/UStringImpl.h
index 77490cd..026970e 100644
--- a/JavaScriptCore/runtime/UStringImpl.h
+++ b/JavaScriptCore/runtime/UStringImpl.h
@@ -105,47 +105,103 @@ protected:
 };
 
 class UStringImpl : public UStringOrRopeImpl {
-public:
-    template<size_t inlineCapacity>
-    static PassRefPtr<UStringImpl> adopt(Vector<UChar, inlineCapacity>& vector)
+    friend class CStringTranslator;
+    friend class UCharBufferTranslator;
+    friend class JIT;
+    friend class SmallStringsStorage;
+    friend class UStringOrRopeImpl;
+    friend void initializeUString();
+private:
+    // For SmallStringStorage, which allocates an array and uses an in-place new.
+    UStringImpl() { }
+
+    // Used to construct static strings, which have an special refCount that can never hit zero.
+    // This means that the static string will never be destroyed, which is important because
+    // static strings will be shared across threads & ref-counted in a non-threadsafe manner.
+    UStringImpl(const UChar* characters, unsigned length, StaticStringConstructType)
+        : UStringOrRopeImpl(length, ConstructStaticString)
+        , m_data(characters)
+        , m_buffer(0)
+        , m_hash(0)
     {
-        if (unsigned length = vector.size()) {
-            ASSERT(vector.data());
-            return adoptRef(new UStringImpl(vector.releaseBuffer(), length));
-        }
-        return empty();
+        hash();
+        checkConsistency();
     }
 
-    static PassRefPtr<UStringImpl> create(const UChar* buffer, unsigned length);
-    static PassRefPtr<UStringImpl> create(const char* c, unsigned length);
-    static PassRefPtr<UStringImpl> create(const char* c);
+    // Create a normal string with internal storage (BufferInternal)
+    UStringImpl(unsigned length)
+        : UStringOrRopeImpl(length, BufferInternal)
+        , m_data(reinterpret_cast<UChar*>(this + 1))
+        , m_buffer(0)
+        , m_hash(0)
+    {
+        ASSERT(m_data);
+        ASSERT(m_length);
+        checkConsistency();
+    }
 
-    static PassRefPtr<UStringImpl> create(PassRefPtr<UStringImpl> rep, unsigned offset, unsigned length)
+    // Create a UStringImpl adopting ownership of the provided buffer (BufferOwned)
+    UStringImpl(const UChar* characters, unsigned length)
+        : UStringOrRopeImpl(length, BufferOwned)
+        , m_data(characters)
+        , m_buffer(0)
+        , m_hash(0)
     {
-        ASSERT(rep);
-        rep->checkConsistency();
-        return adoptRef(new UStringImpl(rep->m_data + offset, length, rep->bufferOwnerString()));
+        ASSERT(m_data);
+        ASSERT(m_length);
+        checkConsistency();
     }
 
-    static PassRefPtr<UStringImpl> create(PassRefPtr<SharedUChar> sharedBuffer, const UChar* buffer, unsigned length)
+    // Used to create new strings that are a substring of an existing UStringImpl (BufferSubstring)
+    UStringImpl(const UChar* characters, unsigned length, PassRefPtr<UStringImpl> base)
+        : UStringOrRopeImpl(length, BufferSubstring)
+        , m_data(characters)
+        , m_substringBuffer(base.releaseRef())
+        , m_hash(0)
     {
-        return adoptRef(new UStringImpl(buffer, length, sharedBuffer));
+        ASSERT(m_data);
+        ASSERT(m_length);
+        checkConsistency();
     }
 
-    static PassRefPtr<UStringImpl> createUninitialized(unsigned length, UChar*& output)
+    // Used to construct new strings sharing an existing SharedUChar (BufferShared)
+    UStringImpl(const UChar* characters, unsigned length, PassRefPtr<SharedUChar> sharedBuffer)
+        : UStringOrRopeImpl(length, BufferShared)
+        , m_data(characters)
+        , m_sharedBuffer(sharedBuffer.releaseRef())
+        , m_hash(0)
     {
-        if (!length) {
-            output = 0;
-            return empty();
-        }
+        ASSERT(m_data);
+        ASSERT(m_length);
+        checkConsistency();
+    }
 
-        if (length > ((std::numeric_limits<size_t>::max() - sizeof(UStringImpl)) / sizeof(UChar)))
-            CRASH();
-        UStringImpl* resultImpl = static_cast<UStringImpl*>(fastMalloc(sizeof(UChar) * length + sizeof(UStringImpl)));
-        output = reinterpret_cast<UChar*>(resultImpl + 1);
-        return adoptRef(new(resultImpl) UStringImpl(length));
+    // For use only by Identifier's XXXTranslator helpers.
+    void setHash(unsigned hash)
+    {
+        ASSERT(!m_hash);
+        ASSERT(hash == computeHash(m_data, m_length));
+        m_hash = hash;
+    }
+
+public:
+    ~UStringImpl();
+
+    static PassRefPtr<UStringImpl> create(const UChar*, unsigned length);
+    static PassRefPtr<UStringImpl> create(const char*, unsigned length);
+    static PassRefPtr<UStringImpl> create(const char*);
+    static PassRefPtr<UStringImpl> create(PassRefPtr<SharedUChar>, const UChar*, unsigned length);
+    static PassRefPtr<UStringImpl> create(PassRefPtr<UStringImpl> rep, unsigned offset, unsigned length)
+    {
+        if (!length)
+            return empty();
+        ASSERT(rep);
+        rep->checkConsistency();
+        UStringImpl* ownerRep = (rep->bufferOwnership() == BufferSubstring) ? rep->m_substringBuffer : rep.get();
+        return adoptRef(new UStringImpl(rep->m_data + offset, length, ownerRep));
     }
 
+    static PassRefPtr<UStringImpl> createUninitialized(unsigned length, UChar*& output);
     static PassRefPtr<UStringImpl> tryCreateUninitialized(unsigned length, UChar*& output)
     {
         if (!length) {
@@ -162,13 +218,24 @@ public:
         return adoptRef(new(resultImpl) UStringImpl(length));
     }
 
+    template<size_t inlineCapacity>
+    static PassRefPtr<UStringImpl> adopt(Vector<UChar, inlineCapacity>& vector)
+    {
+        if (size_t size = vector.size()) {
+            ASSERT(vector.data());
+            return adoptRef(new UStringImpl(vector.releaseBuffer(), size));
+        }
+        return empty();
+    }
+
     SharedUChar* sharedBuffer();
     const UChar* characters() const { return m_data; }
+
     size_t cost()
     {
         // For substrings, return the cost of the base string.
         if (bufferOwnership() == BufferSubstring)
-            return m_bufferSubstring->cost();
+            return m_substringBuffer->cost();
 
         if (m_refCountAndFlags & s_refCountFlagShouldReportedCost) {
             m_refCountAndFlags &= ~s_refCountFlagShouldReportedCost;
@@ -176,9 +243,7 @@ public:
         }
         return 0;
     }
-    unsigned hash() const { if (!m_hash) m_hash = computeHash(m_data, m_length); return m_hash; }
-    unsigned existingHash() const { ASSERT(m_hash); return m_hash; } // fast path for Identifiers
-    void setHash(unsigned hash) { ASSERT(hash == computeHash(m_data, m_length)); m_hash = hash; } // fast path for Identifiers
+
     bool isIdentifier() const { return m_refCountAndFlags & s_refCountFlagIsIdentifier; }
     void setIsIdentifier(bool isIdentifier)
     {
@@ -188,8 +253,22 @@ public:
             m_refCountAndFlags &= ~s_refCountFlagIsIdentifier;
     }
 
+// SYNC SYNC SYNC
+// SYNC SYNC SYNC
+// SYNC SYNC SYNC
+// SYNC SYNC SYNC
+// SYNC SYNC SYNC
+
+    unsigned hash() const { if (!m_hash) m_hash = computeHash(m_data, m_length); return m_hash; }
+    unsigned existingHash() const { ASSERT(m_hash); return m_hash; }
+    static unsigned computeHash(const UChar* data, unsigned length) { return WTF::stringHash(data, length); }
+    static unsigned computeHash(const char* data, unsigned length) { return WTF::stringHash(data, length); }
+    static unsigned computeHash(const char* data) { return WTF::stringHash(data); }
+
     ALWAYS_INLINE void deref() { m_refCountAndFlags -= s_refCountIncrement; if (!(m_refCountAndFlags & (s_refCountMask | s_refCountFlagStatic))) delete this; }
 
+    static UStringImpl* empty();
+
     static void copyChars(UChar* destination, const UChar* source, unsigned numCharacters)
     {
         if (numCharacters <= s_copyCharsInlineCutOff) {
@@ -199,109 +278,33 @@ public:
             memcpy(destination, source, numCharacters * sizeof(UChar));
     }
 
-    static unsigned computeHash(const UChar* s, unsigned length) { return WTF::stringHash(s, length); }
-    static unsigned computeHash(const char* s, unsigned length) { return WTF::stringHash(s, length); }
-    static unsigned computeHash(const char* s) { return WTF::stringHash(s); }
-
-    static UStringImpl* empty();
-
     ALWAYS_INLINE void checkConsistency() const
     {
         // There is no recursion of substrings.
-        ASSERT(bufferOwnerString()->bufferOwnership() != BufferSubstring);
+        ASSERT((bufferOwnership() != BufferSubstring) || (m_substringBuffer->bufferOwnership() != BufferSubstring));
         // Static strings cannot be put in identifier tables, because they are globally shared.
         ASSERT(!isStatic() || !isIdentifier());
     }
 
 private:
-    // For SmallStringStorage, which allocates an array and uses an in-place new.
-    UStringImpl() { }
-
-    // Used to construct normal strings with an internal buffer.
-    UStringImpl(unsigned length)
-        : UStringOrRopeImpl(length, BufferInternal)
-        , m_data(reinterpret_cast<UChar*>(this + 1))
-        , m_buffer(0)
-        , m_hash(0)
-    {
-        checkConsistency();
-    }
-
-    // Used to construct normal strings with an external buffer.
-    UStringImpl(const UChar* data, unsigned length)
-        : UStringOrRopeImpl(length, BufferOwned)
-        , m_data(data)
-        , m_buffer(0)
-        , m_hash(0)
-    {
-        checkConsistency();
-    }
-
-    // Used to construct static strings, which have an special refCount that can never hit zero.
-    // This means that the static string will never be destroyed, which is important because
-    // static strings will be shared across threads & ref-counted in a non-threadsafe manner.
-    UStringImpl(const UChar* data, unsigned length, StaticStringConstructType)
-        : UStringOrRopeImpl(length, ConstructStaticString)
-        , m_data(data)
-        , m_buffer(0)
-        , m_hash(0)
-    {
-        checkConsistency();
-    }
-
-    // Used to create new strings that are a substring of an existing string.
-    UStringImpl(const UChar* data, unsigned length, PassRefPtr<UStringImpl> base)
-        : UStringOrRopeImpl(length, BufferSubstring)
-        , m_data(data)
-        , m_bufferSubstring(base.releaseRef())
-        , m_hash(0)
-    {
-        // Do use static strings as a base for substrings; UntypedPtrAndBitfield assumes
-        // that all pointers will be at least 8-byte aligned, we cannot guarantee that of
-        // UStringImpls that are not heap allocated.
-        ASSERT(m_bufferSubstring->length());
-        ASSERT(!m_bufferSubstring->isStatic());
-        checkConsistency();
-    }
-
-    // Used to construct new strings sharing an existing shared buffer.
-    UStringImpl(const UChar* data, unsigned length, PassRefPtr<SharedUChar> sharedBuffer)
-        : UStringOrRopeImpl(length, BufferShared)
-        , m_data(data)
-        , m_bufferShared(sharedBuffer.releaseRef())
-        , m_hash(0)
-    {
-        checkConsistency();
-    }
-
-    ~UStringImpl();
-
     // This number must be at least 2 to avoid sharing empty, null as well as 1 character strings from SmallStrings.
-    static const unsigned s_minLengthToShare = 10;
     static const unsigned s_copyCharsInlineCutOff = 20;
 
-    UStringImpl* bufferOwnerString() { return (bufferOwnership() == BufferSubstring) ? m_bufferSubstring :  this; }
-    const UStringImpl* bufferOwnerString() const { return (bufferOwnership() == BufferSubstring) ? m_bufferSubstring :  this; }
-    SharedUChar* baseSharedBuffer();
-    unsigned bufferOwnership() const { return m_refCountAndFlags & s_refCountMaskBufferOwnership; }
+    BufferOwnership bufferOwnership() const { return static_cast<BufferOwnership>(m_refCountAndFlags & s_refCountMaskBufferOwnership); }
     bool isStatic() const { return m_refCountAndFlags & s_refCountFlagStatic; }
 
     // unshared data
     const UChar* m_data;
     union {
         void* m_buffer;
-        UStringImpl* m_bufferSubstring;
-        SharedUChar* m_bufferShared;
+        UStringImpl* m_substringBuffer;
+        SharedUChar* m_sharedBuffer;
     };
     mutable unsigned m_hash;
-
-    friend class JIT;
-    friend class SmallStringsStorage;
-    friend class UStringOrRopeImpl;
-    friend void initializeUString();
 };
 
 class URopeImpl : public UStringOrRopeImpl {
+    friend class UStringOrRopeImpl;
 public:
     // A URopeImpl is composed from a set of smaller strings called Fibers.
     // Each Fiber in a rope is either UStringImpl or another URopeImpl.
@@ -339,8 +342,6 @@ private:
 
     unsigned m_fiberCount;
     Fiber m_fibers[1];
-
-    friend class UStringOrRopeImpl;
 };
 
 inline void UStringOrRopeImpl::deref()
diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index 3690b23..e799786 100644
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,28 @@
+2010-03-11  Gavin Barraclough  <barraclough at apple.com>
+
+        Reviewed by Oliver Hunt.
+
+        https://bugs.webkit.org/show_bug.cgi?id=36041
+        Remove unnecessary differences in common code between WebCore::StringImpl & JSC::UStringImpl
+
+        Much of the code in WebCore::StringImpl and JSC::UStringImpl is now very similar,
+        but has trivial and unnecessary formatting differences, such as the exact wording
+        of comments, missing ASSERTs, functions implemented in the .h vs .cpp etc.
+
+        * platform/text/StringImpl.cpp:
+        (WebCore::StringImpl::empty): Reordered in file, made empty()->characters() return a non-null value to match JSC.
+        (WebCore::StringImpl::createUninitialized): Added overflow check.
+        (WebCore::StringImpl::create): Reordered in file.
+        (WebCore::StringImpl::sharedBuffer): Reordered in file.
+        * platform/text/StringImpl.h:
+        (WebCore::StringImpl::): Remove ThreadGlobalData as friend, move SharableUChar & SharedUChar to WebCore namespace.
+        (WebCore::StringImpl::StringImpl): Made static constructor method (used to create empty string) take arguments, to match JSC & prevent accidental use.
+        (WebCore::StringImpl::setHash): Added missing ASSERT.
+        (WebCore::StringImpl::adopt): Make adpot work with Vectors with a non-zero inline capacity.
+        (WebCore::StringImpl::characters): Mark as const to match JSC.
+        (WebCore::StringImpl::hash): Use !m_hash instead of m_hash == 0.
+        (WebCore::StringImpl::computeHash): Remove redundant 'inline'.
+
 2010-03-11  Mark Rowe  <mrowe at apple.com>
 
         Reviewed by David Kilzer.
diff --git a/WebCore/platform/text/StringImpl.cpp b/WebCore/platform/text/StringImpl.cpp
index edddd28..f11dc53 100644
--- a/WebCore/platform/text/StringImpl.cpp
+++ b/WebCore/platform/text/StringImpl.cpp
@@ -33,7 +33,6 @@
 #include "StringHash.h"
 #include "TextBreakIterator.h"
 #include "TextEncoding.h"
-#include "ThreadGlobalData.h"
 #include <runtime/UString.h>
 #include <wtf/dtoa.h>
 #include <wtf/Assertions.h>
@@ -47,58 +46,6 @@ namespace WebCore {
 
 static const unsigned minLengthToShare = 20;
 
-static inline UChar* newUCharVector(unsigned n)
-{
-    return static_cast<UChar*>(fastMalloc(sizeof(UChar) * n));
-}
-
-// This constructor is used only to create the empty string.
-StringImpl::StringImpl()
-    : m_data(0)
-    , m_sharedBuffer(0)
-    , m_length(0)
-    , m_refCountAndFlags(s_refCountFlagStatic | BufferInternal)
-    , m_hash(0)
-{
-    // Ensure that the hash is computed so that AtomicStringHash can call existingHash()
-    // with impunity. The empty string is special because it is never entered into
-    // AtomicString's HashKey, but still needs to compare correctly.
-    hash();
-}
-
-inline StringImpl::StringImpl(unsigned length)
-    : m_data(reinterpret_cast<const UChar*>(this + 1))
-    , m_sharedBuffer(0)
-    , m_length(length)
-    , m_refCountAndFlags(s_refCountIncrement | BufferInternal)
-    , m_hash(0)
-{
-    ASSERT(m_data);
-    ASSERT(m_length);
-}
-
-inline StringImpl::StringImpl(const UChar* characters, unsigned length)
-    : m_data(characters)
-    , m_sharedBuffer(0)
-    , m_length(length)
-    , m_refCountAndFlags(s_refCountIncrement | BufferOwned)
-    , m_hash(0)
-{
-    ASSERT(m_data);
-    ASSERT(m_length);
-}
-
-inline StringImpl::StringImpl(const UChar* characters, unsigned length, PassRefPtr<SharedUChar> sharedBuffer)
-    : m_data(characters)
-    , m_sharedBuffer(sharedBuffer.releaseRef())
-    , m_length(length)
-    , m_refCountAndFlags(s_refCountIncrement | BufferShared)
-    , m_hash(0)
-{
-    ASSERT(m_data);
-    ASSERT(m_length);
-}
-
 StringImpl::~StringImpl()
 {
     if (inTable())
@@ -120,10 +67,84 @@ StringImpl::~StringImpl()
 
 StringImpl* StringImpl::empty()
 {
-    DEFINE_STATIC_LOCAL(StringImpl, emptyString, ());
+    // A non-null pointer at an invalid address (in page zero) so that if it were to be accessed we
+    // should catch the error with fault (however it should be impossible to access, since length is zero).
+    static const UChar* invalidNonNullUCharPtr = reinterpret_cast<UChar*>(static_cast<intptr_t>(1));
+    DEFINE_STATIC_LOCAL(StringImpl, emptyString, (invalidNonNullUCharPtr, 0, ConstructStaticString));
     return &emptyString;
 }
 
+PassRefPtr<StringImpl> StringImpl::createUninitialized(unsigned length, UChar*& data)
+{
+    if (!length) {
+        data = 0;
+        return empty();
+    }
+
+    // Allocate a single buffer large enough to contain the StringImpl
+    // struct as well as the data which it contains. This removes one 
+    // heap allocation from this call.
+    if (length > ((std::numeric_limits<size_t>::max() - sizeof(StringImpl)) / sizeof(UChar)))
+        CRASH();
+    size_t size = sizeof(StringImpl) + length * sizeof(UChar);
+    StringImpl* string = static_cast<StringImpl*>(fastMalloc(size));
+
+    data = reinterpret_cast<UChar*>(string + 1);
+    return adoptRef(new (string) StringImpl(length));
+}
+
+PassRefPtr<StringImpl> StringImpl::create(const UChar* characters, unsigned length)
+{
+    if (!characters || !length)
+        return empty();
+
+    UChar* data;
+    PassRefPtr<StringImpl> string = createUninitialized(length, data);
+    memcpy(data, characters, length * sizeof(UChar));
+    return string;
+}
+
+PassRefPtr<StringImpl> StringImpl::create(const char* characters, unsigned length)
+{
+    if (!characters || !length)
+        return empty();
+
+    UChar* data;
+    PassRefPtr<StringImpl> string = createUninitialized(length, data);
+    for (unsigned i = 0; i != length; ++i) {
+        unsigned char c = characters[i];
+        data[i] = c;
+    }
+    return string;
+}
+
+PassRefPtr<StringImpl> StringImpl::create(const char* string)
+{
+    if (!string)
+        return empty();
+    return create(string, strlen(string));
+}
+
+SharedUChar* StringImpl::sharedBuffer()
+{
+    if (m_length < minLengthToShare)
+        return 0;
+
+    BufferOwnership ownership = bufferOwnership();
+
+    if (ownership == BufferInternal)
+        return 0;
+    if (ownership == BufferOwned) {
+        ASSERT(!m_sharedBuffer);
+        m_sharedBuffer = SharedUChar::create(new SharableUChar(m_data)).releaseRef();
+        m_refCountAndFlags = (m_refCountAndFlags & ~s_refCountMaskBufferOwnership) | BufferShared;
+    }
+
+    ASSERT(bufferOwnership() == BufferShared);
+    ASSERT(m_sharedBuffer);
+    return m_sharedBuffer;
+}
+
 bool StringImpl::containsOnlyWhitespace()
 {
     // FIXME: The definition of whitespace here includes a number of characters
@@ -913,63 +934,6 @@ PassRefPtr<StringImpl> StringImpl::adopt(StringBuffer& buffer)
     return adoptRef(new StringImpl(buffer.release(), length));
 }
 
-PassRefPtr<StringImpl> StringImpl::adopt(Vector<UChar>& vector)
-{
-    size_t size = vector.size();
-    if (size == 0)
-        return empty();
-    return adoptRef(new StringImpl(vector.releaseBuffer(), size));
-}
-
-PassRefPtr<StringImpl> StringImpl::createUninitialized(unsigned length, UChar*& data)
-{
-    if (!length) {
-        data = 0;
-        return empty();
-    }
-
-    // Allocate a single buffer large enough to contain the StringImpl
-    // struct as well as the data which it contains. This removes one 
-    // heap allocation from this call.
-    size_t size = sizeof(StringImpl) + length * sizeof(UChar);
-    StringImpl* string = static_cast<StringImpl*>(fastMalloc(size));
-    data = reinterpret_cast<UChar*>(string + 1);
-    string = new (string) StringImpl(length);
-    return adoptRef(string);
-}
-
-PassRefPtr<StringImpl> StringImpl::create(const UChar* characters, unsigned length)
-{
-    if (!characters || !length)
-        return empty();
-
-    UChar* data;
-    PassRefPtr<StringImpl> string = createUninitialized(length, data);
-    memcpy(data, characters, length * sizeof(UChar));
-    return string;
-}
-
-PassRefPtr<StringImpl> StringImpl::create(const char* characters, unsigned length)
-{
-    if (!characters || !length)
-        return empty();
-
-    UChar* data;
-    PassRefPtr<StringImpl> string = createUninitialized(length, data);
-    for (unsigned i = 0; i != length; ++i) {
-        unsigned char c = characters[i];
-        data[i] = c;
-    }
-    return string;
-}
-
-PassRefPtr<StringImpl> StringImpl::create(const char* string)
-{
-    if (!string)
-        return empty();
-    return create(string, strlen(string));
-}
-
 #if USE(JSC)
 PassRefPtr<StringImpl> StringImpl::create(const JSC::UString& str)
 {
@@ -1017,25 +981,4 @@ PassRefPtr<StringImpl> StringImpl::crossThreadString()
     return threadsafeCopy();
 }
 
-StringImpl::SharedUChar* StringImpl::sharedBuffer()
-{
-    if (m_length < minLengthToShare)
-        return 0;
-
-    BufferOwnership ownership = bufferOwnership();
-
-    if (ownership == BufferInternal)
-        return 0;
-
-    if (ownership == BufferOwned) {
-        ASSERT(!m_sharedBuffer);
-        m_sharedBuffer = SharedUChar::create(new SharableUChar(const_cast<UChar*>(m_data))).releaseRef();
-        m_refCountAndFlags = (m_refCountAndFlags & ~s_refCountMaskBufferOwnership) | BufferShared;
-    }
-
-    ASSERT(bufferOwnership() == BufferShared);
-    ASSERT(m_sharedBuffer);
-    return m_sharedBuffer;
-}
-
 } // namespace WebCore
diff --git a/WebCore/platform/text/StringImpl.h b/WebCore/platform/text/StringImpl.h
index 5c86ffb..27497c0 100644
--- a/WebCore/platform/text/StringImpl.h
+++ b/WebCore/platform/text/StringImpl.h
@@ -56,6 +56,8 @@ struct UCharBufferTranslator;
 
 enum TextCaseSensitivity { TextCaseSensitive, TextCaseInsensitive };
 
+typedef OwnFastMallocPtr<const UChar> SharableUChar;
+typedef CrossThreadRefCounted<SharableUChar> SharedUChar;
 typedef bool (*CharacterMatchFunctionPtr)(UChar);
 
 class StringImpl : public Noncopyable {
@@ -63,49 +65,101 @@ class StringImpl : public Noncopyable {
     friend struct HashAndCharactersTranslator;
     friend struct UCharBufferTranslator;
 private:
-    friend class ThreadGlobalData;
-
     enum BufferOwnership {
         BufferInternal,
         BufferOwned,
         BufferShared,
     };
 
-    typedef OwnFastMallocPtr<const UChar> SharableUChar;
-    typedef CrossThreadRefCounted<SharableUChar> SharedUChar;
+    // Used to construct static strings, which have an special refCount that can never hit zero.
+    // This means that the static string will never be destroyed, which is important because
+    // static strings will be shared across threads & ref-counted in a non-threadsafe manner.
+    enum StaticStringConstructType { ConstructStaticString };
+    StringImpl(const UChar* characters, unsigned length, StaticStringConstructType)
+        : m_data(characters)
+        , m_sharedBuffer(0)
+        , m_length(length)
+        , m_refCountAndFlags(s_refCountFlagStatic | BufferInternal)
+        , m_hash(0)
+    {
+        // Ensure that the hash is computed so that AtomicStringHash can call existingHash()
+        // with impunity. The empty string is special because it is never entered into
+        // AtomicString's HashKey, but still needs to compare correctly.
+        hash();
+    }
+
+    // Create a normal string with internal storage (BufferInternal)
+    StringImpl(unsigned length)
+        : m_data(reinterpret_cast<const UChar*>(this + 1))
+        , m_sharedBuffer(0)
+        , m_length(length)
+        , m_refCountAndFlags(s_refCountIncrement | BufferInternal)
+        , m_hash(0)
+    {
+        ASSERT(m_data);
+        ASSERT(m_length);
+    }
 
-    // Used to create the empty string (""), automatically hashes.
-    StringImpl();
-    // Create a StringImpl with internal storage (BufferInternal)
-    StringImpl(unsigned length);
     // Create a StringImpl adopting ownership of the provided buffer (BufferOwned)
-    StringImpl(const UChar*, unsigned length);
-    // Create a StringImpl using a shared buffer (BufferShared)
-    StringImpl(const UChar*, unsigned length, PassRefPtr<SharedUChar>);
+    StringImpl(const UChar* characters, unsigned length)
+        : m_data(characters)
+        , m_sharedBuffer(0)
+        , m_length(length)
+        , m_refCountAndFlags(s_refCountIncrement | BufferOwned)
+        , m_hash(0)
+    {
+        ASSERT(m_data);
+        ASSERT(m_length);
+    }
+
+    // Used to construct new strings sharing an existing SharedUChar (BufferShared)
+    StringImpl(const UChar* characters, unsigned length, PassRefPtr<SharedUChar> sharedBuffer)
+        : m_data(characters)
+        , m_sharedBuffer(sharedBuffer.releaseRef())
+        , m_length(length)
+        , m_refCountAndFlags(s_refCountIncrement | BufferShared)
+        , m_hash(0)
+    {
+        ASSERT(m_data);
+        ASSERT(m_length);
+    }
 
     // For use only by AtomicString's XXXTranslator helpers.
-    void setHash(unsigned hash) { ASSERT(!m_hash); m_hash = hash; }
-    
+    void setHash(unsigned hash)
+    {
+        ASSERT(!m_hash);
+        ASSERT(hash == computeHash(m_data, m_length));
+        m_hash = hash;
+    }
+
 public:
     ~StringImpl();
 
     static PassRefPtr<StringImpl> create(const UChar*, unsigned length);
     static PassRefPtr<StringImpl> create(const char*, unsigned length);
     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>&);
 #if USE(JSC)
     static PassRefPtr<StringImpl> create(const JSC::UString&);
     JSC::UString ustring();
 #endif
 
+    static PassRefPtr<StringImpl> createUninitialized(unsigned length, UChar*& data);
+    static PassRefPtr<StringImpl> createWithTerminatingNullCharacter(const StringImpl&);
+    static PassRefPtr<StringImpl> createStrippingNullCharacters(const UChar*, unsigned length);
+
+    template<size_t inlineCapacity>
+    static PassRefPtr<StringImpl> adopt(Vector<UChar, inlineCapacity>& vector)
+    {
+        if (size_t size = vector.size()) {
+            ASSERT(vector.data());
+            return adoptRef(new StringImpl(vector.releaseBuffer(), size));
+        }
+        return empty();
+    }
+    static PassRefPtr<StringImpl> adopt(StringBuffer&);
+
     SharedUChar* sharedBuffer();
-    const UChar* characters() { return m_data; }
+    const UChar* characters() const { return m_data; }
     unsigned length() { return m_length; }
 
     bool hasTerminatingNullCharacter() const { return m_refCountAndFlags & s_refCountFlagHasTerminatingNullCharacter; }
@@ -113,15 +167,23 @@ public:
     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; }
+// SYNC SYNC SYNC
+// SYNC SYNC SYNC
+// SYNC SYNC SYNC
+// SYNC SYNC SYNC
+// SYNC SYNC SYNC
+
+    unsigned hash() const { if (!m_hash) 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); }
+    static unsigned computeHash(const UChar* data, unsigned length) { return WTF::stringHash(data, length); }
+    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 | s_refCountFlagStatic))) delete this; }
     ALWAYS_INLINE bool hasOneRef() const { return (m_refCountAndFlags & (s_refCountMask | s_refCountFlagStatic)) == s_refCountIncrement; }
 
+    static StringImpl* empty();
+
     // 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
@@ -178,8 +240,6 @@ public:
     PassRefPtr<StringImpl> replace(StringImpl*, StringImpl*);
     PassRefPtr<StringImpl> replace(unsigned index, unsigned len, StringImpl*);
 
-    static StringImpl* empty();
-
     Vector<char> ascii();
 
     WTF::Unicode::Direction defaultWritingDirection();
@@ -197,8 +257,6 @@ private:
 
     static PassRefPtr<StringImpl> createStrippingNullCharactersSlowCase(const UChar*, unsigned length);
     
-    // The StringImpl struct and its data may be allocated within a single heap block.
-    // In this case, the m_data pointer is an "internal buffer", and does not need to be deallocated.
     BufferOwnership bufferOwnership() const { return static_cast<BufferOwnership>(m_refCountAndFlags & s_refCountMaskBufferOwnership); }
 
     static const unsigned s_refCountMask = 0xFFFFFFE0;

-- 
WebKit Debian packaging



More information about the Pkg-webkit-commits mailing list