[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:21:30 UTC 2010
The following commit has been merged in the webkit-1.2 branch:
commit c24293a16251a4ec94ab326bea5c88c3018b21a7
Author: barraclough at apple.com <barraclough at apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Fri Mar 12 23:49:05 2010 +0000
Bug 36075 - Clean up screwyness re static string impls & Identifiers.
Reviewed by Oliver Hunt.
JavaScriptCore:
* API/JSClassRef.cpp:
(OpaqueJSClass::~OpaqueJSClass): Classname may be null/empty, and these are an identifer. This is okay, since the null/empty strings are shared across all threads.
* JavaScriptCore.exp:
* runtime/Identifier.cpp:
(JSC::Identifier::add): No need to explicitly hash null reps, this is done in the ststic UStringImpl constructor.
(JSC::Identifier::addSlowCase): UStringImpl::empty() handled & checkCurrentIdentifierTable now called in the header.
(JSC::Identifier::checkCurrentIdentifierTable): Replaces checkSameIdentifierTable (this no longer checked the rep since the identifierTable pointer was removed from UString::Rep long ago).
* runtime/Identifier.h:
(JSC::Identifier::add): Replace call to checkSameIdentifierTable with call to checkCurrentIdentifierTable at head of function.
* runtime/UStringImpl.cpp:
(JSC::UStringImpl::~UStringImpl): Remove call to checkConsistency - this function no longer checks anything interesting.
* runtime/UStringImpl.h:
(JSC::UStringOrRopeImpl::UStringOrRopeImpl): Set s_refCountFlagIsIdentifier in static constructor.
(JSC::UStringImpl::UStringImpl): remove calls to checkConsistency (see above), add new ASSERT to substring constructor.
(JSC::UStringImpl::setHash): ASSERT not static (static strings set the hash in their constructor, should not reach this code path).
(JSC::UStringImpl::create): Add missing ASSERT.
(JSC::UStringImpl::setIsIdentifier): ASSERT !isStatic() (static strings hash set in constructor).
WebCore:
* platform/text/StringImpl.cpp:
(WebCore::StringImpl::~StringImpl): Add ASSERT
(WebCore::StringImpl::sharedBuffer): Add ASSERT
* platform/text/StringImpl.h:
(WebCore::StringImpl::setHash): Add ASSERT
(WebCore::StringImpl::isStatic): added.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@55943 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/JavaScriptCore/API/JSClassRef.cpp b/JavaScriptCore/API/JSClassRef.cpp
index f4a6c2a..3c2133d 100644
--- a/JavaScriptCore/API/JSClassRef.cpp
+++ b/JavaScriptCore/API/JSClassRef.cpp
@@ -111,7 +111,8 @@ OpaqueJSClass::OpaqueJSClass(const JSClassDefinition* definition, OpaqueJSClass*
OpaqueJSClass::~OpaqueJSClass()
{
- ASSERT(!m_className.rep()->isIdentifier());
+ // The empty string is shared across threads & is an identifier, in all other cases we should have done a deep copy in className(), below.
+ ASSERT(!m_className.size() || !m_className.rep()->isIdentifier());
if (m_staticValues) {
OpaqueJSClassStaticValuesTable::const_iterator end = m_staticValues->end();
diff --git a/JavaScriptCore/ChangeLog b/JavaScriptCore/ChangeLog
index 75e447a..a4a6818 100644
--- a/JavaScriptCore/ChangeLog
+++ b/JavaScriptCore/ChangeLog
@@ -1,3 +1,27 @@
+2010-03-11 Gavin Barraclough <barraclough at apple.com>
+
+ Reviewed by Oliver Hunt.
+
+ Bug 36075 - Clean up screwyness re static string impls & Identifiers.
+
+ * API/JSClassRef.cpp:
+ (OpaqueJSClass::~OpaqueJSClass): Classname may be null/empty, and these are an identifer. This is okay, since the null/empty strings are shared across all threads.
+ * JavaScriptCore.exp:
+ * runtime/Identifier.cpp:
+ (JSC::Identifier::add): No need to explicitly hash null reps, this is done in the ststic UStringImpl constructor.
+ (JSC::Identifier::addSlowCase): UStringImpl::empty() handled & checkCurrentIdentifierTable now called in the header.
+ (JSC::Identifier::checkCurrentIdentifierTable): Replaces checkSameIdentifierTable (this no longer checked the rep since the identifierTable pointer was removed from UString::Rep long ago).
+ * runtime/Identifier.h:
+ (JSC::Identifier::add): Replace call to checkSameIdentifierTable with call to checkCurrentIdentifierTable at head of function.
+ * runtime/UStringImpl.cpp:
+ (JSC::UStringImpl::~UStringImpl): Remove call to checkConsistency - this function no longer checks anything interesting.
+ * runtime/UStringImpl.h:
+ (JSC::UStringOrRopeImpl::UStringOrRopeImpl): Set s_refCountFlagIsIdentifier in static constructor.
+ (JSC::UStringImpl::UStringImpl): remove calls to checkConsistency (see above), add new ASSERT to substring constructor.
+ (JSC::UStringImpl::setHash): ASSERT not static (static strings set the hash in their constructor, should not reach this code path).
+ (JSC::UStringImpl::create): Add missing ASSERT.
+ (JSC::UStringImpl::setIsIdentifier): ASSERT !isStatic() (static strings hash set in constructor).
+
2010-03-12 Peter Varga <pvarga at inf.u-szeged.hu>
Reviewed by David Levin.
diff --git a/JavaScriptCore/JavaScriptCore.exp b/JavaScriptCore/JavaScriptCore.exp
index 438912b..938c7ff 100644
--- a/JavaScriptCore/JavaScriptCore.exp
+++ b/JavaScriptCore/JavaScriptCore.exp
@@ -92,8 +92,8 @@ __Z15jsRegExpExecutePK8JSRegExpPKtiiPii
__ZN14OpaqueJSString6createERKN3JSC7UStringE
__ZN3JSC10Identifier11addSlowCaseEPNS_12JSGlobalDataEPNS_11UStringImplE
__ZN3JSC10Identifier11addSlowCaseEPNS_9ExecStateEPNS_11UStringImplE
-__ZN3JSC10Identifier24checkSameIdentifierTableEPNS_12JSGlobalDataEPNS_11UStringImplE
-__ZN3JSC10Identifier24checkSameIdentifierTableEPNS_9ExecStateEPNS_11UStringImplE
+__ZN3JSC10Identifier27checkCurrentIdentifierTableEPNS_12JSGlobalDataE
+__ZN3JSC10Identifier27checkCurrentIdentifierTableEPNS_9ExecStateE
__ZN3JSC10Identifier3addEPNS_9ExecStateEPKc
__ZN3JSC10Identifier4fromEPNS_9ExecStateEi
__ZN3JSC10Identifier4fromEPNS_9ExecStateEj
diff --git a/JavaScriptCore/runtime/Identifier.cpp b/JavaScriptCore/runtime/Identifier.cpp
index a361505..9acccf0 100644
--- a/JavaScriptCore/runtime/Identifier.cpp
+++ b/JavaScriptCore/runtime/Identifier.cpp
@@ -124,11 +124,8 @@ struct CStringTranslator {
PassRefPtr<UString::Rep> Identifier::add(JSGlobalData* globalData, const char* c)
{
- if (!c) {
- UString::Rep* rep = UString::null().rep();
- rep->hash();
- return rep;
- }
+ if (!c)
+ return UString::null().rep();
if (!c[0])
return UString::Rep::empty();
if (!c[1])
@@ -209,19 +206,18 @@ PassRefPtr<UString::Rep> Identifier::add(ExecState* exec, const UChar* s, int le
PassRefPtr<UString::Rep> Identifier::addSlowCase(JSGlobalData* globalData, UString::Rep* r)
{
ASSERT(!r->isIdentifier());
+ // The empty & null strings are static singletons, and static strings are handled
+ // in ::add() in the header, so we should never get here with a zero length string.
+ ASSERT(r->length());
+
if (r->length() == 1) {
UChar c = r->characters()[0];
if (c <= 0xFF)
r = globalData->smallStrings.singleCharacterStringRep(c);
- if (r->isIdentifier()) {
-#ifndef NDEBUG
- checkSameIdentifierTable(globalData, r);
-#endif
+ if (r->isIdentifier())
return r;
- }
}
- if (!r->length())
- return UString::Rep::empty();
+
return *globalData->identifierTable->add(r).first;
}
@@ -252,25 +248,24 @@ Identifier Identifier::from(ExecState* exec, double value)
#ifndef NDEBUG
-void Identifier::checkSameIdentifierTable(ExecState* exec, UString::Rep*)
+void Identifier::checkCurrentIdentifierTable(JSGlobalData* globalData)
{
- ASSERT_UNUSED(exec, exec->globalData().identifierTable == currentIdentifierTable());
+ // Check the identifier table accessible through the threadspecific matches the
+ // globalData's identifier table.
+ ASSERT_UNUSED(globalData, globalData->identifierTable == currentIdentifierTable());
}
-void Identifier::checkSameIdentifierTable(JSGlobalData* globalData, UString::Rep*)
+void Identifier::checkCurrentIdentifierTable(ExecState* exec)
{
- ASSERT_UNUSED(globalData, globalData->identifierTable == currentIdentifierTable());
+ checkCurrentIdentifierTable(&exec->globalData());
}
#else
-void Identifier::checkSameIdentifierTable(ExecState*, UString::Rep*)
-{
-}
-
-void Identifier::checkSameIdentifierTable(JSGlobalData*, UString::Rep*)
-{
-}
+// These only exists so that our exports are the same for debug and release builds.
+// This would be an ASSERT_NOT_REACHED(), but we're in NDEBUG only code here!
+void Identifier::checkCurrentIdentifierTable(JSGlobalData*) { CRASH(); }
+void Identifier::checkCurrentIdentifierTable(ExecState*) { CRASH(); }
#endif
diff --git a/JavaScriptCore/runtime/Identifier.h b/JavaScriptCore/runtime/Identifier.h
index 253f3c1..48b1da1 100644
--- a/JavaScriptCore/runtime/Identifier.h
+++ b/JavaScriptCore/runtime/Identifier.h
@@ -93,30 +93,28 @@ namespace JSC {
static PassRefPtr<UString::Rep> add(ExecState* exec, UString::Rep* r)
{
- if (r->isIdentifier()) {
#ifndef NDEBUG
- checkSameIdentifierTable(exec, r);
+ checkCurrentIdentifierTable(exec);
#endif
+ if (r->isIdentifier())
return r;
- }
return addSlowCase(exec, r);
}
static PassRefPtr<UString::Rep> add(JSGlobalData* globalData, UString::Rep* r)
{
- if (r->isIdentifier()) {
#ifndef NDEBUG
- checkSameIdentifierTable(globalData, r);
+ checkCurrentIdentifierTable(globalData);
#endif
+ if (r->isIdentifier())
return r;
- }
return addSlowCase(globalData, r);
}
static PassRefPtr<UString::Rep> addSlowCase(ExecState*, UString::Rep* r);
static PassRefPtr<UString::Rep> addSlowCase(JSGlobalData*, UString::Rep* r);
- static void checkSameIdentifierTable(ExecState*, UString::Rep*);
- static void checkSameIdentifierTable(JSGlobalData*, UString::Rep*);
+ static void checkCurrentIdentifierTable(ExecState*);
+ static void checkCurrentIdentifierTable(JSGlobalData*);
};
inline bool operator==(const Identifier& a, const Identifier& b)
diff --git a/JavaScriptCore/runtime/UStringImpl.cpp b/JavaScriptCore/runtime/UStringImpl.cpp
index acf7f1c..dd3eb51 100644
--- a/JavaScriptCore/runtime/UStringImpl.cpp
+++ b/JavaScriptCore/runtime/UStringImpl.cpp
@@ -41,7 +41,6 @@ static const unsigned minLengthToShare = 20;
UStringImpl::~UStringImpl()
{
ASSERT(!isStatic());
- checkConsistency();
if (isIdentifier())
Identifier::remove(this);
diff --git a/JavaScriptCore/runtime/UStringImpl.h b/JavaScriptCore/runtime/UStringImpl.h
index eab297b..4dccb25 100644
--- a/JavaScriptCore/runtime/UStringImpl.h
+++ b/JavaScriptCore/runtime/UStringImpl.h
@@ -72,7 +72,7 @@ protected:
enum StaticStringConstructType { ConstructStaticString };
UStringOrRopeImpl(unsigned length, StaticStringConstructType)
- : m_refCountAndFlags(s_refCountFlagStatic | BufferOwned)
+ : m_refCountAndFlags(s_refCountFlagStatic | s_refCountFlagIsIdentifier | BufferOwned)
, m_length(length)
{
ASSERT(!isRope());
@@ -125,7 +125,6 @@ private:
, m_hash(0)
{
hash();
- checkConsistency();
}
// Create a normal string with internal storage (BufferInternal)
@@ -137,7 +136,6 @@ private:
{
ASSERT(m_data);
ASSERT(m_length);
- checkConsistency();
}
// Create a UStringImpl adopting ownership of the provided buffer (BufferOwned)
@@ -149,7 +147,6 @@ private:
{
ASSERT(m_data);
ASSERT(m_length);
- checkConsistency();
}
// Used to create new strings that are a substring of an existing UStringImpl (BufferSubstring)
@@ -161,7 +158,7 @@ private:
{
ASSERT(m_data);
ASSERT(m_length);
- checkConsistency();
+ ASSERT(m_substringBuffer->bufferOwnership() != BufferSubstring);
}
// Used to construct new strings sharing an existing SharedUChar (BufferShared)
@@ -173,12 +170,12 @@ private:
{
ASSERT(m_data);
ASSERT(m_length);
- checkConsistency();
}
// For use only by Identifier's XXXTranslator helpers.
void setHash(unsigned hash)
{
+ ASSERT(!isStatic());
ASSERT(!m_hash);
ASSERT(hash == computeHash(m_data, m_length));
m_hash = hash;
@@ -193,10 +190,12 @@ public:
static PassRefPtr<UStringImpl> create(PassRefPtr<SharedUChar>, const UChar*, unsigned length);
static PassRefPtr<UStringImpl> create(PassRefPtr<UStringImpl> rep, unsigned offset, unsigned length)
{
+ ASSERT(rep);
+ ASSERT(length <= rep->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));
}
@@ -247,6 +246,7 @@ public:
bool isIdentifier() const { return m_refCountAndFlags & s_refCountFlagIsIdentifier; }
void setIsIdentifier(bool isIdentifier)
{
+ ASSERT(!isStatic());
if (isIdentifier)
m_refCountAndFlags |= s_refCountFlagIsIdentifier;
else
@@ -272,14 +272,6 @@ public:
memcpy(destination, source, numCharacters * sizeof(UChar));
}
- ALWAYS_INLINE void checkConsistency() const
- {
- // There is no recursion of substrings.
- ASSERT((bufferOwnership() != BufferSubstring) || (m_substringBuffer->bufferOwnership() != BufferSubstring));
- // Static strings cannot be put in identifier tables, because they are globally shared.
- ASSERT(!isStatic() || !isIdentifier());
- }
-
private:
// This number must be at least 2 to avoid sharing empty, null as well as 1 character strings from SmallStrings.
static const unsigned s_copyCharsInlineCutOff = 20;
@@ -287,7 +279,6 @@ private:
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;
diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index 931cf1d..44e11c1 100644
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,16 @@
+2010-03-11 Gavin Barraclough <barraclough at apple.com>
+
+ Reviewed by Oliver Hunt.
+
+ Bug 36075 - Clean up screwyness re static string impls & Identifiers.
+
+ * platform/text/StringImpl.cpp:
+ (WebCore::StringImpl::~StringImpl): Add ASSERT
+ (WebCore::StringImpl::sharedBuffer): Add ASSERT
+ * platform/text/StringImpl.h:
+ (WebCore::StringImpl::setHash): Add ASSERT
+ (WebCore::StringImpl::isStatic): added.
+
2010-03-12 Enrica Casucci <enrica at apple.com>
Reviewed by Simon Fraser.
diff --git a/WebCore/platform/text/StringImpl.cpp b/WebCore/platform/text/StringImpl.cpp
index f11dc53..bd04a51 100644
--- a/WebCore/platform/text/StringImpl.cpp
+++ b/WebCore/platform/text/StringImpl.cpp
@@ -48,6 +48,8 @@ static const unsigned minLengthToShare = 20;
StringImpl::~StringImpl()
{
+ ASSERT(!isStatic());
+
if (inTable())
AtomicString::remove(this);
@@ -129,6 +131,8 @@ SharedUChar* StringImpl::sharedBuffer()
{
if (m_length < minLengthToShare)
return 0;
+ // All static strings are smaller that the minimim length to share.
+ ASSERT(!isStatic());
BufferOwnership ownership = bufferOwnership();
diff --git a/WebCore/platform/text/StringImpl.h b/WebCore/platform/text/StringImpl.h
index d0a79ff..af9d650 100644
--- a/WebCore/platform/text/StringImpl.h
+++ b/WebCore/platform/text/StringImpl.h
@@ -127,6 +127,7 @@ private:
// For use only by AtomicString's XXXTranslator helpers.
void setHash(unsigned hash)
{
+ ASSERT(!isStatic());
ASSERT(!m_hash);
ASSERT(hash == computeHash(m_data, m_length));
m_hash = hash;
@@ -252,6 +253,7 @@ private:
static PassRefPtr<StringImpl> createStrippingNullCharactersSlowCase(const UChar*, unsigned length);
BufferOwnership bufferOwnership() const { return static_cast<BufferOwnership>(m_refCountAndFlags & s_refCountMaskBufferOwnership); }
+ bool isStatic() const { return m_refCountAndFlags & s_refCountFlagStatic; }
static const unsigned s_refCountMask = 0xFFFFFFE0;
static const unsigned s_refCountIncrement = 0x20;
--
WebKit Debian packaging
More information about the Pkg-webkit-commits
mailing list