Make StringImpl's content immutable

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

Index: third_party/WebKit/Source/wtf/text/StringBuffer.h
diff --git a/third_party/WebKit/Source/wtf/text/StringBuffer.h b/third_party/WebKit/Source/wtf/text/StringBuffer.h
index 5e780e63d8090594318186c60f3d38fd1bacda0e..019002ac667db54ecceda098326311e091996a42 100644
--- a/third_party/WebKit/Source/wtf/text/StringBuffer.h
+++ b/third_party/WebKit/Source/wtf/text/StringBuffer.h
@@ -39,12 +39,16 @@ template <typename CharType>
 class StringBuffer {
     WTF_MAKE_NONCOPYABLE(StringBuffer);
 public:
-    StringBuffer() { }
+    StringBuffer()
+        : m_length(0)
+    {
+    }
 
     explicit StringBuffer(unsigned length)
     {
         CharType* characters;
         m_data = StringImpl::createUninitialized(length, characters);
+        m_length = m_data->length();
     }
 
     ~StringBuffer()
@@ -53,7 +57,7 @@ public:
 
     void shrink(unsigned newLength);
 
-    unsigned length() const { return m_data ? m_data->length() : 0; }
+    unsigned length() const { return m_length; }
     CharType* characters() { return length() ? const_cast<CharType*>(m_data->getCharacters<CharType>()) : 0; }
 
     CharType& operator[](unsigned i) { ASSERT_WITH_SECURITY_IMPLICATION(i < length()); return characters()[i]; }
@@ -62,15 +66,15 @@ public:
 
 private:
     RefPtr<StringImpl> m_data;
+    unsigned m_length;

[haraken, 2015/10/13 07:40:29]

Why do we need to cache m_length in StringBuffer? m_data->length() is not
enough?

[hajimehoshi, 2015/10/13 07:51:18]

This is just for avoiding StringImpl::substring.

 };
 
 template <typename CharType>
 void StringBuffer<CharType>::shrink(unsigned newLength)
 {
     ASSERT(m_data);
-    if (m_data->length() == newLength)

[haraken, 2015/10/13 07:40:29]

Is it safe to change this condition to ASSERT?

Sounds nice to try but it would be better to try in a separate CL.

[hajimehoshi, 2015/10/13 07:51:18]

Yes, it's safe: truncateAssumingIsolated had a same assertion. (newLength <=
m_data->length()).

-        return;
-    m_data->truncateAssumingIsolated(newLength);
+    ASSERT(newLength <= m_length);
+    m_length = newLength;

[haraken, 2015/10/13 07:40:29]

Don't we need to create a substring(0, newLength)? Otherwise we cannot shrink
the memory usage when StringBuffer::shrink is called.

[hajimehoshi, 2015/10/13 07:51:18]

truncateAssumingIsolated doesn't reallocate Paritition alloc and this doesn't
change the performance in terms of memory usage (substring would make it
slightly better anyway, but it would make the speed bad).

[haraken, 2015/10/13 07:54:05]

Not related to this CL, but what's the benefit of calling the shrink() method?
If it doesn't shrink the backing storage, it looks useless.

 }
 
 } // namespace WTF