[SCM] WebKit Debian packaging branch, webkit-1.1, updated. upstream/1.1.21-584-g1e41756
barraclough at apple.com
barraclough at apple.com
Fri Feb 26 22:19:02 UTC 2010
The following commit has been merged in the webkit-1.1 branch:
commit 2d0c104bf060069aac17bc7f7c9f52d25990c4cd
Author: barraclough at apple.com <barraclough at apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Fri Feb 12 22:22:09 2010 +0000
https://bugs.webkit.org/show_bug.cgi?id=33731
Remove uses of PtrAndFlags from WebCore::StringImpl.
Reviewed by Sam Weinig.
These break the OS X Leaks tool. Use a bits stolen from the refCount to hold the
'InTable' and 'HasTerminatingNullCharacter' flags, along with the string type
(fixes a leak where the string data is allocated at the address (this + 1), and is
misinterpreted as being an internal buffer).
* WebCore.base.exp:
* platform/text/StringImpl.cpp:
(WebCore::StringImpl::StringImpl):
(WebCore::StringImpl::~StringImpl):
(WebCore::StringImpl::create):
(WebCore::StringImpl::createWithTerminatingNullCharacter):
(WebCore::StringImpl::crossThreadString):
(WebCore::StringImpl::sharedBuffer):
* platform/text/StringImpl.h:
(WebCore::StringImpl::):
(WebCore::StringImpl::hasTerminatingNullCharacter):
(WebCore::StringImpl::inTable):
(WebCore::StringImpl::setInTable):
(WebCore::StringImpl::ref):
(WebCore::StringImpl::deref):
(WebCore::StringImpl::hasOneRef):
(WebCore::StringImpl::operator new):
(WebCore::StringImpl::bufferOwnership):
* storage/OriginUsageRecord.cpp:
(WebCore::OriginUsageRecord::addDatabase):
(WebCore::OriginUsageRecord::markDatabase):
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@54736 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index 3c421a2..24deafc 100644
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,37 @@
+2010-02-12 Gavin Barraclough <barraclough at apple.com>
+
+ Reviewed by Sam Weinig.
+
+ https://bugs.webkit.org/show_bug.cgi?id=33731
+ Remove uses of PtrAndFlags from WebCore::StringImpl.
+
+ These break the OS X Leaks tool. Use a bits stolen from the refCount to hold the
+ 'InTable' and 'HasTerminatingNullCharacter' flags, along with the string type
+ (fixes a leak where the string data is allocated at the address (this + 1), and is
+ misinterpreted as being an internal buffer).
+
+ * WebCore.base.exp:
+ * platform/text/StringImpl.cpp:
+ (WebCore::StringImpl::StringImpl):
+ (WebCore::StringImpl::~StringImpl):
+ (WebCore::StringImpl::create):
+ (WebCore::StringImpl::createWithTerminatingNullCharacter):
+ (WebCore::StringImpl::crossThreadString):
+ (WebCore::StringImpl::sharedBuffer):
+ * platform/text/StringImpl.h:
+ (WebCore::StringImpl::):
+ (WebCore::StringImpl::hasTerminatingNullCharacter):
+ (WebCore::StringImpl::inTable):
+ (WebCore::StringImpl::setInTable):
+ (WebCore::StringImpl::ref):
+ (WebCore::StringImpl::deref):
+ (WebCore::StringImpl::hasOneRef):
+ (WebCore::StringImpl::operator new):
+ (WebCore::StringImpl::bufferOwnership):
+ * storage/OriginUsageRecord.cpp:
+ (WebCore::OriginUsageRecord::addDatabase):
+ (WebCore::OriginUsageRecord::markDatabase):
+
2010-02-12 Nikolas Zimmermann <nzimmermann at rim.com>
Reviewed by Dirk Schulze.
diff --git a/WebCore/WebCore.base.exp b/WebCore/WebCore.base.exp
index a0a5262..59e03f5 100644
--- a/WebCore/WebCore.base.exp
+++ b/WebCore/WebCore.base.exp
@@ -144,7 +144,6 @@ __ZN7WebCore10StringImpl7ustringEv
__ZN7WebCore10StringImpl8endsWithEPS0_b
__ZN7WebCore10StringImplD1Ev
__ZN7WebCore10StringImplcvP8NSStringEv
-__ZN7WebCore10StringImpldlEPv
__ZN7WebCore10handCursorEv
__ZN7WebCore10setCookiesEPNS_8DocumentERKNS_4KURLERKNS_6StringE
__ZN7WebCore11BitmapImageC1EP7CGImagePNS_13ImageObserverE
diff --git a/WebCore/platform/text/StringImpl.cpp b/WebCore/platform/text/StringImpl.cpp
index db6152d..3704c4e 100644
--- a/WebCore/platform/text/StringImpl.cpp
+++ b/WebCore/platform/text/StringImpl.cpp
@@ -52,36 +52,12 @@ static inline UChar* newUCharVector(unsigned n)
return static_cast<UChar*>(fastMalloc(sizeof(UChar) * n));
}
-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 | BufferInternal)
, m_hash(0)
{
// Ensure that the hash is computed so that AtomicStringHash can call existingHash()
@@ -90,35 +66,55 @@ StringImpl::StringImpl()
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(characters);
- ASSERT(length);
- ASSERT(!bufferIsInternal());
+ ASSERT(m_data);
+ ASSERT(m_length);
}
-inline StringImpl::StringImpl(unsigned length)
- : m_data(reinterpret_cast<const UChar*>(this + 1))
+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(length);
- ASSERT(bufferIsInternal());
+ ASSERT(m_data);
+ ASSERT(m_length);
}
StringImpl::~StringImpl()
{
if (inTable())
AtomicString::remove(this);
- if (!bufferIsInternal()) {
- SharedUChar* sharedBuffer = m_sharedBufferAndFlags.get();
- if (sharedBuffer)
- sharedBuffer->deref();
- else
- deleteUCharVector(m_data);
+
+ BufferOwnership ownership = bufferOwnership();
+ if (ownership != BufferInternal) {
+ if (ownership == BufferOwned) {
+ ASSERT(!m_sharedBuffer);
+ ASSERT(m_data);
+ fastFree(const_cast<UChar*>(m_data));
+ } else {
+ ASSERT(ownership == BufferShared);
+ ASSERT(m_sharedBuffer);
+ m_sharedBuffer->deref();
+ }
}
}
@@ -976,13 +972,8 @@ PassRefPtr<StringImpl> StringImpl::create(const char* string)
#if USE(JSC)
PassRefPtr<StringImpl> StringImpl::create(const JSC::UString& str)
{
- SharedUChar* sharedBuffer = const_cast<JSC::UString*>(&str)->rep()->sharedBuffer();
- if (sharedBuffer) {
- PassRefPtr<StringImpl> impl = adoptRef(new StringImpl(str.data(), str.size()));
- sharedBuffer->ref();
- impl->m_sharedBufferAndFlags.set(sharedBuffer);
- return impl;
- }
+ if (SharedUChar* sharedBuffer = const_cast<JSC::UString*>(&str)->rep()->sharedBuffer())
+ return adoptRef(new StringImpl(str.data(), str.size(), sharedBuffer));
return StringImpl::create(str.data(), str.size());
}
@@ -1007,7 +998,7 @@ PassRefPtr<StringImpl> StringImpl::createWithTerminatingNullCharacter(const Stri
data[length] = 0;
terminatedString->m_length--;
terminatedString->m_hash = string.m_hash;
- terminatedString->m_sharedBufferAndFlags.setFlag(HasTerminatingNullCharacter);
+ terminatedString->m_refCountAndFlags |= s_refCountFlagHasTerminatingNullCharacter;
return terminatedString.release();
}
@@ -1021,12 +1012,8 @@ PassRefPtr<StringImpl> StringImpl::threadsafeCopy() const
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());
- return impl.release();
- }
+ if (SharedUChar* sharedBuffer = this->sharedBuffer())
+ return adoptRef(new StringImpl(m_data, m_length, sharedBuffer->crossThreadCopy()));
// If no shared buffer is available, create a copy.
return threadsafeCopy();
@@ -1034,13 +1021,23 @@ PassRefPtr<StringImpl> StringImpl::crossThreadString()
StringImpl::SharedUChar* StringImpl::sharedBuffer()
{
- if (m_length < minLengthToShare || bufferIsInternal())
+ if (m_length < minLengthToShare)
return 0;
- if (!m_sharedBufferAndFlags.get())
- m_sharedBufferAndFlags.set(SharedUChar::create(new OwnFastMallocPtr<UChar>(const_cast<UChar*>(m_data))).releaseRef());
- return m_sharedBufferAndFlags.get();
-}
+ BufferOwnership ownership = bufferOwnership();
+ if (ownership == BufferInternal)
+ return 0;
+
+ if (ownership == BufferOwned) {
+ ASSERT(!m_sharedBuffer);
+ m_sharedBuffer = SharedUChar::create(new OwnFastMallocPtr<UChar>(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 21f936d..65848bb 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,25 +58,33 @@ 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;
private:
friend class ThreadGlobalData;
- StringImpl();
-
- // This constructor adopts the UChar* without copying the buffer.
- StringImpl(const UChar*, unsigned length);
- // This constructor assumes that 'this' was allocated with a UChar buffer of size 'length' at the end.
+ enum BufferOwnership {
+ BufferInternal,
+ BufferOwned,
+ BufferShared,
+ };
+
+ typedef CrossThreadRefCounted<OwnFastMallocPtr<UChar> > SharedUChar;
+
+ // 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>);
// For use only by AtomicString's XXXTranslator helpers.
void setHash(unsigned hash) { ASSERT(!m_hash); m_hash = hash; }
- typedef CrossThreadRefCounted<OwnFastMallocPtr<UChar> > SharedUChar;
-
public:
~StringImpl();
@@ -99,16 +107,20 @@ public:
const UChar* characters() { return m_data; }
unsigned length() { return m_length; }
- bool hasTerminatingNullCharacter() const { return m_sharedBufferAndFlags.isFlagSet(HasTerminatingNullCharacter); }
+ bool hasTerminatingNullCharacter() const { return m_refCountAndFlags & s_refCountFlagHasTerminatingNullCharacter; }
- 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
@@ -178,31 +190,27 @@ 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);
// 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.
- bool bufferIsInternal() { return m_data == reinterpret_cast<const UChar*>(this + 1); }
+ BufferOwnership bufferOwnership() const { return static_cast<BufferOwnership>(m_refCountAndFlags & s_refCountMaskBufferOwnership); }
- enum StringImplFlags {
- HasTerminatingNullCharacter,
- InTable,
- };
+ static const unsigned s_refCountMask = 0xFFFFFFF0;
+ static const unsigned s_refCountIncrement = 0x10;
+ static const unsigned s_refCountFlagHasTerminatingNullCharacter = 0x8;
+ static const unsigned s_refCountFlagInTable = 0x4;
+ static const unsigned s_refCountMaskBufferOwnership = 0x3;
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.
};
bool equal(StringImpl*, StringImpl*);
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