Use a Vector for the buffer in StringBuilder.

third_party/WebKit/Source/wtf/text/StringBuilder.h

Index: third_party/WebKit/Source/wtf/text/StringBuilder.h
diff --git a/third_party/WebKit/Source/wtf/text/StringBuilder.h b/third_party/WebKit/Source/wtf/text/StringBuilder.h
index c967fa6082696ec2c0f232172ddd0837ee13a327..783fdbaf8c8b4254a99af444239f488fd9c7ac13 100644
--- a/third_party/WebKit/Source/wtf/text/StringBuilder.h
+++ b/third_party/WebKit/Source/wtf/text/StringBuilder.h
@@ -35,19 +35,17 @@
 namespace WTF {
 
 class WTF_EXPORT StringBuilder {
-    // Disallow copying since it's expensive and we don't want code to do it by accident.
     WTF_MAKE_NONCOPYABLE(StringBuilder);
-
 public:
     StringBuilder()
-        : m_bufferCharacters8(0)
+        : m_buffer(nullptr)
         , m_length(0)
-        , m_is8Bit(true)
-    {
-    }
+        , m_is8Bit(true) {}
 
-    void append(const UChar*, unsigned);
-    void append(const LChar*, unsigned);
+    ~StringBuilder() { clear(); }
+
+    void append(const UChar*, unsigned length);
+    void append(const LChar*, unsigned length);
 
     ALWAYS_INLINE void append(const char* characters, unsigned length) { append(reinterpret_cast<const LChar*>(characters), length); }
 
@@ -56,11 +54,10 @@ public:
         if (!other.m_length)
             return;
 
-        // If we're appending to an empty string, and there is not a buffer (reserveCapacity has not been called)
-        // then just retain the string.
-        if (!m_length && !m_buffer && !other.m_string.isNull()) {
+        if (!m_length && !hasBuffer() && !other.m_string.isNull()) {
             m_string = other.m_string;
-            m_length = other.m_length;
+            m_length = other.m_string.length();
+            m_is8Bit = other.m_string.is8Bit();
             return;
         }
 
@@ -99,7 +96,7 @@ public:
         // inside the parser in the common case when flushing buffered text to
         // a Text node.
         StringImpl* impl = string.sharedImpl();
-        if (!m_length && !m_buffer && impl) {
+        if (!m_length && !hasBuffer() && impl) {
             m_string = impl;
             m_length = impl->length();
             m_is8Bit = impl->is8Bit();
@@ -114,30 +111,26 @@ public:
 
     void append(UChar c)
     {
-        if (m_buffer && m_length < m_buffer->length() && m_string.isNull()) {
-            if (!m_is8Bit) {
-                m_bufferCharacters16[m_length++] = c;
-                return;
-            }
-
-            if (!(c & ~0xff)) {
-                m_bufferCharacters8[m_length++] = static_cast<LChar>(c);
-                return;
-            }
+        if (m_is8Bit && c <= 0xFF) {
+            append(static_cast<LChar>(c));
+            return;
         }
-        append(&c, 1);
+        ensureBuffer16();
+        m_string = String();
+        m_buffer16->append(c);
+        ++m_length;
     }
 
     void append(LChar c)
     {
-        if (m_buffer && m_length < m_buffer->length() && m_string.isNull()) {
-            if (m_is8Bit)
-                m_bufferCharacters8[m_length++] = c;
-            else
-                m_bufferCharacters16[m_length++] = c;
-        } else {
-            append(&c, 1);
+        if (!m_is8Bit) {
+            append(static_cast<UChar>(c));
+            return;
         }
+        ensureBuffer8();
+        m_string = String();
+        m_buffer8->append(c);
+        ++m_length;
     }
 
     void append(char c)
@@ -163,62 +156,19 @@ public:
     void appendNumber(unsigned long long);
     void appendNumber(double, unsigned precision = 6, TrailingZerosTruncatingPolicy = TruncateTrailingZeros);
 
-    String toString()
-    {
-        shrinkToFit();
-        if (m_string.isNull())
-            reifyString();
-        return m_string;
-    }
-
-    String substring(unsigned position, unsigned length) const
-    {
-        if (!m_length)
-            return emptyString();
-        if (!m_string.isNull())
-            return m_string.substring(position, length);
-        return reifySubstring(position, length);
-    }
-
-    AtomicString toAtomicString() const
-    {
-        if (!m_length)
-            return emptyAtom;
-
-        // If the buffer is sufficiently over-allocated, make a new AtomicString from a copy so its buffer is not so large.
-        if (canShrink()) {
-            if (is8Bit())
-                return AtomicString(characters8(), length());
-            return AtomicString(characters16(), length());
-        }
-
-        if (!m_string.isNull())
-            return AtomicString(m_string);
-
-        DCHECK(m_buffer);
-        return AtomicString(m_buffer.get(), 0, m_length);
-    }
-
-    unsigned length() const
-    {
-        return m_length;
-    }
+    String toString();
+    AtomicString toAtomicString();
+    String substring(unsigned start, unsigned length) const;
 
+    unsigned length() const { return m_length; }
     bool isEmpty() const { return !m_length; }
 
+    unsigned capacity() const;
     void reserveCapacity(unsigned newCapacity);
 
-    unsigned capacity() const
-    {
-        return m_buffer ? m_buffer->length() : m_length;
-    }
-
+    // TODO(esprehn): Rename to shrink().
     void resize(unsigned newSize);
 
-    bool canShrink() const;
-
-    void shrinkToFit();
-
     UChar operator[](unsigned i) const
     {
         ASSERT_WITH_SECURITY_IMPLICATION(i < m_length);
@@ -230,84 +180,62 @@ public:
     const LChar* characters8() const
     {
         DCHECK(m_is8Bit);
-        if (!m_length)
-            return 0;
+        if (!length())
+            return nullptr;
         if (!m_string.isNull())
             return m_string.characters8();
-        DCHECK(m_buffer);
-        return m_buffer->characters8();
+        DCHECK(m_buffer8);
+        return m_buffer8->data();
     }
 
     const UChar* characters16() const
     {
         DCHECK(!m_is8Bit);
-        if (!m_length)
-            return 0;
+        if (!length())
+            return nullptr;
         if (!m_string.isNull())
             return m_string.characters16();
-        DCHECK(m_buffer);
-        return m_buffer->characters16();
+        DCHECK(m_buffer16);
+        return m_buffer16->data();
     }
 
     bool is8Bit() const { return m_is8Bit; }
 
-    void clear()
+    void clear();
+    void swap(StringBuilder&);
+
+private:
+    typedef Vector<LChar, 16> Buffer8;
+    typedef Vector<UChar, 16> Buffer16;

[haraken, 2016/06/27 06:36:01]

I don't mind specifying a larger inline capacity for this because most
StringBuilders are used on stack. (Honestly speaking, I want to make
StringBuilders STACK_ALLOCATED.) It's very rare that StringBuilders are
long-lived and the overallocated capacity becomes a problem.

[esprehn, 2016/06/27 23:36:40]

Yeah we can do that, there's a bunch of StringBuilder m_* in the codebase right
now, FetchDataLoaderAsString is an example of one where I'm a bit nervous making
this too big. I'd like to set a large capacity in another patch.

+
+    void ensureBuffer8()
     {
-        m_length = 0;
-        m_string = String();
-        m_buffer = nullptr;
-        m_bufferCharacters8 = 0;
-        m_is8Bit = true;
+        DCHECK(m_is8Bit);
+        if (!hasBuffer())
+            createBuffer8();
     }
 
-    void swap(StringBuilder& stringBuilder)
+    void ensureBuffer16()
     {
-        std::swap(m_length, stringBuilder.m_length);
-        m_string.swap(stringBuilder.m_string);
-        m_buffer.swap(stringBuilder.m_buffer);
-        std::swap(m_is8Bit, stringBuilder.m_is8Bit);
-        std::swap(m_bufferCharacters8, stringBuilder.m_bufferCharacters8);
+        if (m_is8Bit || !hasBuffer())

[haraken, 2016/06/27 06:36:01]

Shouldn't this be:

  DCHECK(!m_is8Bit);
  if (!hasBuffer())
    ...

?

[esprehn, 2016/06/27 23:36:40]

No, this is called whenever we need a 16 bit buffer. We might have already
converted to a 16bit buffer though.

ex.

append(UChar) called twice will invoke ensireBuffer16() twice, the first time we
get here we have m_is8Bit true, the second time we have it false.
createBuffer16() sets it.

+            createBuffer16();
     }
 
-private:
-    void allocateBuffer(const LChar* currentCharacters, unsigned requiredLength);
-    void allocateBuffer(const UChar* currentCharacters, unsigned requiredLength);
-    void allocateBufferUpConvert(const LChar* currentCharacters, unsigned requiredLength);
-    template <typename CharType>
-    void reallocateBuffer(unsigned requiredLength);
-    template <typename CharType>
-    ALWAYS_INLINE CharType* appendUninitialized(unsigned length);
-    template <typename CharType>
-    CharType* appendUninitializedSlow(unsigned length);
-    template <typename CharType>
-    ALWAYS_INLINE CharType * getBufferCharacters();
-    void reifyString();
-    String reifySubstring(unsigned position, unsigned length) const;
-
-    String m_string; // Pointers first: crbug.com/232031
-    RefPtr<StringImpl> m_buffer;
+    void createBuffer8();
+    void createBuffer16();
+
+    bool hasBuffer() const { return m_buffer; }
+
+    String m_string;
     union {
-        LChar* m_bufferCharacters8;
-        UChar* m_bufferCharacters16;
+        Buffer8* m_buffer8;
+        Buffer16* m_buffer16;
+        void* m_buffer;

[hajimehoshi, 2016/06/27 06:13:20]

Where is |m_buffer| used now?

[esprehn, 2016/06/27 06:15:15]

hasBuffer() uses it since it doesn't care about the type. I guess it could just
check either of the other two pointers

     };
     unsigned m_length;
     bool m_is8Bit;
 };
 
-template <>
-ALWAYS_INLINE LChar* StringBuilder::getBufferCharacters<LChar>()
-{
-    DCHECK(m_is8Bit);
-    return m_bufferCharacters8;
-}
-
-template <>
-ALWAYS_INLINE UChar* StringBuilder::getBufferCharacters<UChar>()
-{
-    DCHECK(!m_is8Bit);
-    return m_bufferCharacters16;
-}
-
 template <typename CharType>
 bool equal(const StringBuilder& s, const CharType* buffer, unsigned length)
 {