Make StringImpl's content immutable

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

 /*
  * Copyright (C) 2008, 2010 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  *
  * 1.  Redistributions of source code must retain the above copyright
  *     notice, this list of conditions and the following disclaimer.
  * 2.  Redistributions in binary form must reproduce the above copyright
@@ skipping 21 matching lines @@
 #include "wtf/Assertions.h"
 #include "wtf/text/StringImpl.h"
 #include "wtf/text/Unicode.h"
 
 namespace WTF {
 
 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()
     {
     }
 
     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]; }
 
     PassRefPtr<StringImpl> release() { return m_data.release(); }
 
 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
 
 using WTF::StringBuffer;
 
 #endif // StringBuffer_h