- Revision
- 102298
- Author
- msab...@apple.com
- Date
- 2011-12-07 18:19:59 -0800 (Wed, 07 Dec 2011)
Log Message
StringBuilderTest.Append and StringBuilderTest.ToStringPreserveCapacity are failing.
https://bugs.webkit.org/show_bug.cgi?id=73995
Source/_javascript_Core:
Reviewed by Geoffrey Garen.
Problem was that a call to characters on an StringImpl associated
with a StringBuilder that is being appended to gets stale.
Added a new m_valid16BitShadowlen that keeps the length of
the 16 bit shadow that has been upconverted or will be up converted
with the first getCharacters(). When StringBuilder::characters or
::reifyString is called, further characters are upconverted if
we have a shadow16bit copy and the m_valid16BitShadowlen is updated.
* _javascript_Core.exp:
* wtf/text/StringBuilder.cpp:
(WTF::StringBuilder::reifyString):
* wtf/text/StringBuilder.h:
(WTF::StringBuilder::StringBuilder):
(WTF::StringBuilder::characters):
(WTF::StringBuilder::clear): Cleaned up as part of the change.
* wtf/text/StringImpl.cpp:
(WTF::StringImpl::getData16SlowCase):
(WTF::StringImpl::upconvertCharacters):
* wtf/text/StringImpl.h:
Tools:
Reenabled failing tests that the code part of the patch fixes.
Reviewed by Geoffrey Garen.
* TestWebKitAPI/Tests/WTF/StringBuilder.cpp:
(TestWebKitAPI::TEST):
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (102297 => 102298)
--- trunk/Source/_javascript_Core/ChangeLog 2011-12-08 02:11:09 UTC (rev 102297)
+++ trunk/Source/_javascript_Core/ChangeLog 2011-12-08 02:19:59 UTC (rev 102298)
@@ -1,3 +1,30 @@
+2011-12-07 Michael Saboff <msab...@apple.com>
+
+ StringBuilderTest.Append and StringBuilderTest.ToStringPreserveCapacity are failing.
+ https://bugs.webkit.org/show_bug.cgi?id=73995
+
+ Reviewed by Geoffrey Garen.
+
+ Problem was that a call to characters on an StringImpl associated
+ with a StringBuilder that is being appended to gets stale.
+ Added a new m_valid16BitShadowlen that keeps the length of
+ the 16 bit shadow that has been upconverted or will be up converted
+ with the first getCharacters(). When StringBuilder::characters or
+ ::reifyString is called, further characters are upconverted if
+ we have a shadow16bit copy and the m_valid16BitShadowlen is updated.
+
+ * _javascript_Core.exp:
+ * wtf/text/StringBuilder.cpp:
+ (WTF::StringBuilder::reifyString):
+ * wtf/text/StringBuilder.h:
+ (WTF::StringBuilder::StringBuilder):
+ (WTF::StringBuilder::characters):
+ (WTF::StringBuilder::clear): Cleaned up as part of the change.
+ * wtf/text/StringImpl.cpp:
+ (WTF::StringImpl::getData16SlowCase):
+ (WTF::StringImpl::upconvertCharacters):
+ * wtf/text/StringImpl.h:
+
2011-12-07 Filip Pizlo <fpi...@apple.com>
Compare and Swap should be enabled on ARMv7
Modified: trunk/Source/_javascript_Core/_javascript_Core.exp (102297 => 102298)
--- trunk/Source/_javascript_Core/_javascript_Core.exp 2011-12-08 02:11:09 UTC (rev 102297)
+++ trunk/Source/_javascript_Core/_javascript_Core.exp 2011-12-08 02:19:59 UTC (rev 102298)
@@ -583,6 +583,7 @@
__ZNK3JSC9HashTable11createTableEPNS_12JSGlobalDataE
__ZNK3JSC9HashTable11deleteTableEv
__ZNK3WTF10StringImpl17getData16SlowCaseEv
+__ZNK3WTF10StringImpl19upconvertCharactersEjj
__ZNK3WTF12AtomicString5lowerEv
__ZNK3WTF13DecimalNumber15toStringDecimalEPtj
__ZNK3WTF13DecimalNumber19toStringExponentialEPtj
Modified: trunk/Source/_javascript_Core/wtf/text/StringBuilder.cpp (102297 => 102298)
--- trunk/Source/_javascript_Core/wtf/text/StringBuilder.cpp 2011-12-08 02:11:09 UTC (rev 102297)
+++ trunk/Source/_javascript_Core/wtf/text/StringBuilder.cpp 2011-12-08 02:19:59 UTC (rev 102298)
@@ -51,6 +51,11 @@
m_string = (m_length == m_buffer->length())
? m_buffer.get()
: StringImpl::create(m_buffer, 0, m_length);
+
+ if (m_buffer->has16BitShadow() && m_valid16BitShadowLength < m_length)
+ m_buffer->upconvertCharacters(m_valid16BitShadowLength, m_length);
+
+ m_valid16BitShadowLength = m_length;
}
void StringBuilder::resize(unsigned newSize)
Modified: trunk/Source/_javascript_Core/wtf/text/StringBuilder.h (102297 => 102298)
--- trunk/Source/_javascript_Core/wtf/text/StringBuilder.h 2011-12-08 02:11:09 UTC (rev 102297)
+++ trunk/Source/_javascript_Core/wtf/text/StringBuilder.h 2011-12-08 02:19:59 UTC (rev 102298)
@@ -36,6 +36,7 @@
StringBuilder()
: m_length(0)
, m_is8Bit(true)
+ , m_valid16BitShadowLength(0)
, m_bufferCharacters8(0)
{
}
@@ -169,6 +170,11 @@
if (!m_string.isNull())
return m_string.characters();
ASSERT(m_buffer);
+ if (m_buffer->has16BitShadow() && m_valid16BitShadowLength < m_length)
+ m_buffer->upconvertCharacters(m_valid16BitShadowLength, m_length);
+
+ m_valid16BitShadowLength = m_length;
+
return m_buffer->characters();
}
@@ -177,6 +183,9 @@
m_length = 0;
m_string = String();
m_buffer = 0;
+ m_bufferCharacters8 = 0;
+ m_is8Bit = true;
+ m_valid16BitShadowLength = 0;
}
private:
@@ -197,6 +206,7 @@
String m_string;
RefPtr<StringImpl> m_buffer;
bool m_is8Bit;
+ mutable unsigned m_valid16BitShadowLength;
union {
LChar* m_bufferCharacters8;
UChar* m_bufferCharacters16;
Modified: trunk/Source/_javascript_Core/wtf/text/StringImpl.cpp (102297 => 102298)
--- trunk/Source/_javascript_Core/wtf/text/StringImpl.cpp 2011-12-08 02:11:09 UTC (rev 102297)
+++ trunk/Source/_javascript_Core/wtf/text/StringImpl.cpp 2011-12-08 02:19:59 UTC (rev 102298)
@@ -205,19 +205,23 @@
m_copyData16 = static_cast<UChar*>(fastMalloc(len * sizeof(UChar)));
- if (hasTerminatingNullCharacter()) {
- len--;
- m_copyData16[len] = '\0';
- }
+ m_hashAndFlags |= s_hashFlagHas16BitShadow;
- for (size_t i = 0; i < len; i++)
- m_copyData16[i] = m_data8[i];
+ upconvertCharacters(0, len);
- m_hashAndFlags |= s_hashFlagHas16BitShadow;
-
return m_copyData16;
}
+void StringImpl::upconvertCharacters(unsigned start, unsigned end) const
+{
+ ASSERT(is8Bit());
+ ASSERT(has16BitShadow());
+
+ for (size_t i = start; i < end; i++)
+ m_copyData16[i] = m_data8[i];
+}
+
+
bool StringImpl::containsOnlyWhitespace()
{
// FIXME: The definition of whitespace here includes a number of characters
Modified: trunk/Source/_javascript_Core/wtf/text/StringImpl.h (102297 => 102298)
--- trunk/Source/_javascript_Core/wtf/text/StringImpl.h 2011-12-08 02:11:09 UTC (rev 102297)
+++ trunk/Source/_javascript_Core/wtf/text/StringImpl.h 2011-12-08 02:19:59 UTC (rev 102298)
@@ -309,6 +309,7 @@
}
bool has16BitShadow() const { return m_hashAndFlags & s_hashFlagHas16BitShadow; }
+ void upconvertCharacters(unsigned, unsigned) const;
bool isIdentifier() const { return m_hashAndFlags & s_hashFlagIsIdentifier; }
void setIsIdentifier(bool isIdentifier)
{
Modified: trunk/Tools/ChangeLog (102297 => 102298)
--- trunk/Tools/ChangeLog 2011-12-08 02:11:09 UTC (rev 102297)
+++ trunk/Tools/ChangeLog 2011-12-08 02:19:59 UTC (rev 102298)
@@ -1,3 +1,15 @@
+2011-12-07 Michael Saboff <msab...@apple.com>
+
+ StringBuilderTest.Append and StringBuilderTest.ToStringPreserveCapacity are failing.
+ https://bugs.webkit.org/show_bug.cgi?id=73995
+
+ Reenabled failing tests that the code part of the patch fixes.
+
+ Reviewed by Geoffrey Garen.
+
+ * TestWebKitAPI/Tests/WTF/StringBuilder.cpp:
+ (TestWebKitAPI::TEST):
+
2011-12-07 MORITA Hajime <morr...@google.com>
[filter-build-webkit] should not emit reset color when --no-color is given
Modified: trunk/Tools/TestWebKitAPI/Tests/WTF/StringBuilder.cpp (102297 => 102298)
--- trunk/Tools/TestWebKitAPI/Tests/WTF/StringBuilder.cpp 2011-12-08 02:11:09 UTC (rev 102297)
+++ trunk/Tools/TestWebKitAPI/Tests/WTF/StringBuilder.cpp 2011-12-08 02:19:59 UTC (rev 102298)
@@ -66,7 +66,7 @@
expectEmpty(builder);
}
-TEST(StringBuilderTest, DISABLED_Append)
+TEST(StringBuilderTest, Append)
{
StringBuilder builder;
builder.append(String("0123456789"));
@@ -123,7 +123,7 @@
ASSERT_EQ(String("0123456789abcdefghijklmnopqrstuvwxyzABC"), string1);
}
-TEST(StringBuilderTest, DISABLED_ToStringPreserveCapacity)
+TEST(StringBuilderTest, ToStringPreserveCapacity)
{
StringBuilder builder;
builder.append("0123456789");
