[webkit-changes] [219521] trunk

Fri, 14 Jul 2017 13:27:26 -0700

Title: [219521] trunk
Revision
219521
Author
commit-qu...@webkit.org
Date
2017-07-14 13:27:05 -0700 (Fri, 14 Jul 2017)

Log Message

Unreviewed, rolling out r219510.
https://bugs.webkit.org/show_bug.cgi?id=174525

Need to revert length() == 0 check for null string (Requested
by yusukesuzuki on #webkit).

Reverted changeset:

"[WTF] Newly added AtomicStringImpl should use BufferInternal
static string if StringImpl is static"
https://bugs.webkit.org/show_bug.cgi?id=174501
http://trac.webkit.org/changeset/219510

Modified Paths

Diff

Modified: trunk/Source/WTF/ChangeLog (219520 => 219521)


--- trunk/Source/WTF/ChangeLog	2017-07-14 20:17:18 UTC (rev 219520)
+++ trunk/Source/WTF/ChangeLog	2017-07-14 20:27:05 UTC (rev 219521)
@@ -1,3 +1,18 @@
+2017-07-14  Commit Queue  <commit-qu...@webkit.org>
+
+        Unreviewed, rolling out r219510.
+        https://bugs.webkit.org/show_bug.cgi?id=174525
+
+        Need to revert length() == 0 check for null string (Requested
+        by yusukesuzuki on #webkit).
+
+        Reverted changeset:
+
+        "[WTF] Newly added AtomicStringImpl should use BufferInternal
+        static string if StringImpl is static"
+        https://bugs.webkit.org/show_bug.cgi?id=174501
+        http://trac.webkit.org/changeset/219510
+
 2017-07-14  Yusuke Suzuki  <utatane....@gmail.com>
 
         [WTF] Newly added AtomicStringImpl should use BufferInternal static string if StringImpl is static

Modified: trunk/Source/WTF/wtf/text/AtomicStringImpl.cpp (219520 => 219521)


--- trunk/Source/WTF/wtf/text/AtomicStringImpl.cpp	2017-07-14 20:17:18 UTC (rev 219520)
+++ trunk/Source/WTF/wtf/text/AtomicStringImpl.cpp	2017-07-14 20:27:05 UTC (rev 219521)
@@ -95,70 +95,55 @@
 }
 
 struct CStringTranslator {
-    static unsigned hash(const LChar* characters)
+    static unsigned hash(const LChar* c)
     {
-        return StringHasher::computeHashAndMaskTop8Bits(characters);
+        return StringHasher::computeHashAndMaskTop8Bits(c);
     }
 
-    static inline bool equal(StringImpl* str, const LChar* characters)
+    static inline bool equal(StringImpl* r, const LChar* s)
     {
-        return WTF::equal(str, characters);
+        return WTF::equal(r, s);
     }
 
-    static void translate(StringImpl*& location, const LChar* const& characters, unsigned hash)
+    static void translate(StringImpl*& location, const LChar* const& c, unsigned hash)
     {
-        location = &StringImpl::create(characters).leakRef();
+        location = &StringImpl::create(c).leakRef();
         location->setHash(hash);
         location->setIsAtomic(true);
     }
 };
 
-RefPtr<AtomicStringImpl> AtomicStringImpl::add(const LChar* characters)
+RefPtr<AtomicStringImpl> AtomicStringImpl::add(const LChar* c)
 {
-    if (!characters)
+    if (!c)
         return nullptr;
-    if (!*characters)
+    if (!*c)
         return static_cast<AtomicStringImpl*>(StringImpl::empty());
 
-    return addToStringTable<const LChar*, CStringTranslator>(characters);
+    return addToStringTable<const LChar*, CStringTranslator>(c);
 }
 
 template<typename CharacterType>
 struct HashTranslatorCharBuffer {
-    const CharacterType* characters;
+    const CharacterType* s;
     unsigned length;
-    unsigned hash;
-
-    HashTranslatorCharBuffer(const CharacterType* characters, unsigned length)
-        : characters(characters)
-        , length(length)
-        , hash(StringHasher::computeHashAndMaskTop8Bits(characters, length))
-    {
-    }
-
-    HashTranslatorCharBuffer(const CharacterType* characters, unsigned length, unsigned hash)
-        : characters(characters)
-        , length(length)
-        , hash(hash)
-    {
-    }
 };
 
-using UCharBuffer = HashTranslatorCharBuffer<UChar>;
+typedef HashTranslatorCharBuffer<UChar> UCharBuffer;
 struct UCharBufferTranslator {
     static unsigned hash(const UCharBuffer& buf)
     {
-        return buf.hash;
+        return StringHasher::computeHashAndMaskTop8Bits(buf.s, buf.length);
     }
 
     static bool equal(StringImpl* const& str, const UCharBuffer& buf)
     {
-        return WTF::equal(str, buf.characters, buf.length);
+        return WTF::equal(str, buf.s, buf.length);
     }
 
     static void translate(StringImpl*& location, const UCharBuffer& buf, unsigned hash)
     {
-        location = &StringImpl::create8BitIfPossible(buf.characters, buf.length).leakRef();
+        location = &StringImpl::create8BitIfPossible(buf.s, buf.length).leakRef();
         location->setHash(hash);
         location->setIsAtomic(true);
     }
@@ -232,31 +217,31 @@
     }
 };
 
-RefPtr<AtomicStringImpl> AtomicStringImpl::add(const UChar* characters, unsigned length)
+RefPtr<AtomicStringImpl> AtomicStringImpl::add(const UChar* s, unsigned length)
 {
-    if (!characters)
+    if (!s)
         return nullptr;
 
     if (!length)
         return static_cast<AtomicStringImpl*>(StringImpl::empty());
 
-    UCharBuffer buffer { characters, length };
+    UCharBuffer buffer = { s, length };
     return addToStringTable<UCharBuffer, UCharBufferTranslator>(buffer);
 }
 
-RefPtr<AtomicStringImpl> AtomicStringImpl::add(const UChar* characters)
+RefPtr<AtomicStringImpl> AtomicStringImpl::add(const UChar* s)
 {
-    if (!characters)
+    if (!s)
         return nullptr;
 
     unsigned length = 0;
-    while (characters[length] != UChar(0))
+    while (s[length] != UChar(0))
         ++length;
 
     if (!length)
         return static_cast<AtomicStringImpl*>(StringImpl::empty());
 
-    UCharBuffer buffer { characters, length };
+    UCharBuffer buffer = { s, length };
     return addToStringTable<UCharBuffer, UCharBufferTranslator>(buffer);
 }
 
@@ -320,56 +305,55 @@
     return addToStringTable<SubstringLocation, SubstringTranslator16>(buffer);
 }
     
-using LCharBuffer = HashTranslatorCharBuffer<LChar>;
+typedef HashTranslatorCharBuffer<LChar> LCharBuffer;
 struct LCharBufferTranslator {
     static unsigned hash(const LCharBuffer& buf)
     {
-        return buf.hash;
+        return StringHasher::computeHashAndMaskTop8Bits(buf.s, buf.length);
     }
 
     static bool equal(StringImpl* const& str, const LCharBuffer& buf)
     {
-        return WTF::equal(str, buf.characters, buf.length);
+        return WTF::equal(str, buf.s, buf.length);
     }
 
     static void translate(StringImpl*& location, const LCharBuffer& buf, unsigned hash)
     {
-        location = &StringImpl::create(buf.characters, buf.length).leakRef();
+        location = &StringImpl::create(buf.s, buf.length).leakRef();
         location->setHash(hash);
         location->setIsAtomic(true);
     }
 };
 
-template<typename CharType>
-struct BufferFromStaticDataTranslator {
-    using Buffer = HashTranslatorCharBuffer<CharType>;
-    static unsigned hash(const Buffer& buf)
+typedef HashTranslatorCharBuffer<char> CharBuffer;
+struct CharBufferFromLiteralDataTranslator {
+    static unsigned hash(const CharBuffer& buf)
     {
-        return buf.hash;
+        return StringHasher::computeHashAndMaskTop8Bits(reinterpret_cast<const LChar*>(buf.s), buf.length);
     }
 
-    static bool equal(StringImpl* const& str, const Buffer& buf)
+    static bool equal(StringImpl* const& str, const CharBuffer& buf)
     {
-        return WTF::equal(str, buf.characters, buf.length);
+        return WTF::equal(str, buf.s, buf.length);
     }
 
-    static void translate(StringImpl*& location, const Buffer& buf, unsigned hash)
+    static void translate(StringImpl*& location, const CharBuffer& buf, unsigned hash)
     {
-        location = &StringImpl::createWithoutCopying(buf.characters, buf.length).leakRef();
+        location = &StringImpl::createFromLiteral(buf.s, buf.length).leakRef();
         location->setHash(hash);
         location->setIsAtomic(true);
     }
 };
 
-RefPtr<AtomicStringImpl> AtomicStringImpl::add(const LChar* characters, unsigned length)
+RefPtr<AtomicStringImpl> AtomicStringImpl::add(const LChar* s, unsigned length)
 {
-    if (!characters)
+    if (!s)
         return nullptr;
 
     if (!length)
         return static_cast<AtomicStringImpl*>(StringImpl::empty());
 
-    LCharBuffer buffer { characters, length };
+    LCharBuffer buffer = { s, length };
     return addToStringTable<LCharBuffer, LCharBufferTranslator>(buffer);
 }
 
@@ -378,14 +362,14 @@
     ASSERT(characters);
     ASSERT(length);
 
-    LCharBuffer buffer { reinterpret_cast<const LChar*>(characters), length };
-    return addToStringTable<LCharBuffer, BufferFromStaticDataTranslator<LChar>>(buffer);
+    CharBuffer buffer = { characters, length };
+    return addToStringTable<CharBuffer, CharBufferFromLiteralDataTranslator>(buffer);
 }
 
-static Ref<AtomicStringImpl> addSymbol(AtomicStringTableLocker& locker, StringTableImpl& atomicStringTable, StringImpl& base)
+static inline Ref<AtomicStringImpl> addSubstring(AtomicStringTableLocker& locker, StringTableImpl& atomicStringTable, StringImpl& base)
 {
     ASSERT(base.length());
-    ASSERT(base.isSymbol());
+    ASSERT(base.isSymbol() || base.isStatic());
 
     SubstringLocation buffer = { &base, 0, base.length() };
     if (base.is8Bit())
@@ -393,41 +377,20 @@
     return addToStringTable<SubstringLocation, SubstringTranslator16>(locker, atomicStringTable, buffer);
 }
 
-static inline Ref<AtomicStringImpl> addSymbol(StringImpl& base)
+static inline Ref<AtomicStringImpl> addSubstring(StringImpl& base)
 {
     AtomicStringTableLocker locker;
-    return addSymbol(locker, stringTable(), base);
+    return addSubstring(locker, stringTable(), base);
 }
 
-static Ref<AtomicStringImpl> addStatic(AtomicStringTableLocker& locker, StringTableImpl& atomicStringTable, StringImpl& base)
-{
-    ASSERT(base.length());
-    ASSERT(base.isStatic());
-
-    if (base.is8Bit()) {
-        LCharBuffer buffer { base.characters8(), base.length(), base.hash() };
-        return addToStringTable<LCharBuffer, BufferFromStaticDataTranslator<LChar>>(locker, atomicStringTable, buffer);
-    }
-    UCharBuffer buffer { base.characters16(), base.length(), base.hash() };
-    return addToStringTable<UCharBuffer, BufferFromStaticDataTranslator<UChar>>(locker, atomicStringTable, buffer);
-}
-
-static inline Ref<AtomicStringImpl> addStatic(StringImpl& base)
-{
-    AtomicStringTableLocker locker;
-    return addStatic(locker, stringTable(), base);
-}
-
 Ref<AtomicStringImpl> AtomicStringImpl::addSlowCase(StringImpl& string)
 {
-    ASSERT_WITH_MESSAGE(string.length(), "Empty string should be already handled.");
+    if (!string.length())
+        return *static_cast<AtomicStringImpl*>(StringImpl::empty());
 
-    if (string.isStatic())
-        return addStatic(string);
+    if (string.isSymbol() || string.isStatic())
+        return addSubstring(string);
 
-    if (string.isSymbol())
-        return addSymbol(string);
-
     ASSERT_WITH_MESSAGE(!string.isAtomic(), "AtomicStringImpl should not hit the slow case if the string is already atomic.");
 
     AtomicStringTableLocker locker;
@@ -443,18 +406,14 @@
 
 Ref<AtomicStringImpl> AtomicStringImpl::addSlowCase(AtomicStringTable& stringTable, StringImpl& string)
 {
-    ASSERT_WITH_MESSAGE(string.length(), "Empty string should be already handled.");
+    if (!string.length())
+        return *static_cast<AtomicStringImpl*>(StringImpl::empty());
 
-    if (string.isStatic()) {
+    if (string.isSymbol() || string.isStatic()) {
         AtomicStringTableLocker locker;
-        return addStatic(locker, stringTable.table(), string);
+        return addSubstring(locker, stringTable.table(), string);
     }
 
-    if (string.isSymbol()) {
-        AtomicStringTableLocker locker;
-        return addSymbol(locker, stringTable.table(), string);
-    }
-
     ASSERT_WITH_MESSAGE(!string.isAtomic(), "AtomicStringImpl should not hit the slow case if the string is already atomic.");
 
     AtomicStringTableLocker locker;
@@ -523,7 +482,7 @@
     AtomicStringTableLocker locker;
     auto& table = stringTable();
 
-    UCharBuffer buffer { characters, length };
+    UCharBuffer buffer = { characters, length };
     auto iterator = table.find<UCharBufferTranslator>(buffer);
     if (iterator != table.end())
         return static_cast<AtomicStringImpl*>(*iterator);

Modified: trunk/Source/WTF/wtf/text/StringImpl.h (219520 => 219521)


--- trunk/Source/WTF/wtf/text/StringImpl.h	2017-07-14 20:17:18 UTC (rev 219520)
+++ trunk/Source/WTF/wtf/text/StringImpl.h	2017-07-14 20:27:05 UTC (rev 219521)
@@ -55,7 +55,7 @@
 class SymbolRegistry;
 
 struct CStringTranslator;
-template<typename> struct BufferFromStaticDataTranslator;
+struct CharBufferFromLiteralDataTranslator;
 struct HashAndUTF8CharactersTranslator;
 struct LCharBufferTranslator;
 struct StringHash;
@@ -182,13 +182,12 @@
     friend struct WTF::CStringTranslator;
     template<typename CharacterType> friend struct WTF::HashAndCharactersTranslator;
     friend struct WTF::HashAndUTF8CharactersTranslator;
-    template<typename CharacterType> friend struct WTF::BufferFromStaticDataTranslator;
+    friend struct WTF::CharBufferFromLiteralDataTranslator;
     friend struct WTF::LCharBufferTranslator;
     friend struct WTF::SubstringTranslator;
     friend struct WTF::UCharBufferTranslator;
     friend class JSC::LLInt::Data;
     friend class JSC::LLIntOffsetsExtractor;
-    friend class AtomicStringImpl;
     friend class SymbolImpl;
     friend class RegisteredSymbolImpl;

Modified: trunk/Tools/ChangeLog (219520 => 219521)


--- trunk/Tools/ChangeLog	2017-07-14 20:17:18 UTC (rev 219520)
+++ trunk/Tools/ChangeLog	2017-07-14 20:27:05 UTC (rev 219521)
@@ -1,3 +1,18 @@
+2017-07-14  Commit Queue  <commit-qu...@webkit.org>
+
+        Unreviewed, rolling out r219510.
+        https://bugs.webkit.org/show_bug.cgi?id=174525
+
+        Need to revert length() == 0 check for null string (Requested
+        by yusukesuzuki on #webkit).
+
+        Reverted changeset:
+
+        "[WTF] Newly added AtomicStringImpl should use BufferInternal
+        static string if StringImpl is static"
+        https://bugs.webkit.org/show_bug.cgi?id=174501
+        http://trac.webkit.org/changeset/219510
+
 2017-07-14  Jer Noble  <jer.no...@apple.com>
 
         [MSE] Removing samples when presentation order does not match decode order can cause bad behavior.

Modified: trunk/Tools/TestWebKitAPI/Tests/WTF/StringImpl.cpp (219520 => 219521)


--- trunk/Tools/TestWebKitAPI/Tests/WTF/StringImpl.cpp	2017-07-14 20:17:18 UTC (rev 219520)
+++ trunk/Tools/TestWebKitAPI/Tests/WTF/StringImpl.cpp	2017-07-14 20:27:05 UTC (rev 219521)
@@ -55,17 +55,6 @@
     ASSERT_TRUE(equal(programmaticStringNoLength.get(), stringWithoutLengthLiteral));
     ASSERT_EQ(stringWithoutLengthLiteral, reinterpret_cast<const char*>(programmaticStringNoLength->characters8()));
     ASSERT_TRUE(programmaticStringNoLength->is8Bit());
-
-    // AtomicStringImpl from createFromLiteral should use the same underlying string.
-    auto atomicStringWithTemplate = AtomicStringImpl::add(stringWithTemplate.ptr());
-    ASSERT_TRUE(atomicStringWithTemplate->is8Bit());
-    ASSERT_EQ(atomicStringWithTemplate->characters8(), stringWithTemplate->characters8());
-    auto atomicProgrammaticString = AtomicStringImpl::add(programmaticString.ptr());
-    ASSERT_TRUE(atomicProgrammaticString->is8Bit());
-    ASSERT_EQ(atomicProgrammaticString->characters8(), programmaticString->characters8());
-    auto atomicProgrammaticStringNoLength = AtomicStringImpl::add(programmaticStringNoLength.ptr());
-    ASSERT_TRUE(atomicProgrammaticStringNoLength->is8Bit());
-    ASSERT_EQ(atomicProgrammaticStringNoLength->characters8(), programmaticStringNoLength->characters8());
 }
 
 TEST(WTF, StringImplReplaceWithLiteral)
@@ -620,12 +609,8 @@
     ASSERT_FALSE(original.isAtomic());
     ASSERT_TRUE(original.isStatic());
 
-    ASSERT_TRUE(atomic->is8Bit());
-    ASSERT_EQ(atomic->characters8(), original.characters8());
-
     auto result2 = AtomicStringImpl::lookUp(&original);
     ASSERT_TRUE(result2);
-    ASSERT_EQ(atomic, result2);
 }
 
 TEST(WTF, StringImplConstexprHasher)

_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to