Reviewed by Darin.

        http://bugs.webkit.org/show_bug.cgi?id=10818
        String::append does 2 full copies instead of 1 (or zero!)

        No change in functionality, thus no test.

        * platform/String.cpp:
        (WebCore::String::append): Rewrote to copy once. Also removed an ancient
        FIXME that doesn't seem to make any sense. Note that append() behavior doesn't
        match documented String behavior ("modifications to one instance will
        also modify all others"), but there are a lot of methods that don't.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@27300 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index 644dbe16d252fb14f7e5a608a36787c4999419da..619b9b01f2ade126181bc586cfed3d693d34bb8a 100644 (file)
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,18 @@
+2007-10-31  Alexey Proskuryakov  <ap@webkit.org>
+
+        Reviewed by Darin.
+
+        http://bugs.webkit.org/show_bug.cgi?id=10818
+        String::append does 2 full copies instead of 1 (or zero!)
+
+        No change in functionality, thus no test.
+
+        * platform/String.cpp:
+        (WebCore::String::append): Rewrote to copy once. Also removed an ancient
+        FIXME that doesn't seem to make any sense. Note that append() behavior doesn't 
+        match documented String behavior ("modifications to one instance will
+        also modify all others"), but there are a lot of methods that don't.
+
 2007-10-31  Adam Roben  <aroben@apple.com>
 
         Windows build fix

diff --git a/WebCore/platform/String.cpp b/WebCore/platform/String.cpp
index ef330de7019d67cb0dd680c0598f5da78a1f23c5..a4a33f80292df8528585c4055786209512529493 100644 (file)
--- a/WebCore/platform/String.cpp
+++ b/WebCore/platform/String.cpp
@@ -99,34 +99,36 @@ String::String(const char* str, unsigned length)
 void String::append(const String &str)
 {
     if (str.m_impl) {
-        if (!m_impl) {
-            // ### FIXME!!!
+        if (m_impl) {
+            Vector<UChar> newCharacters(m_impl->length() + str.length());
+            memcpy(newCharacters.data(), m_impl->characters(), m_impl->length() * sizeof(UChar));
+            memcpy(newCharacters.data() + m_impl->length(), str.characters(), str.length() * sizeof(UChar));
+            m_impl = StringImpl::adopt(newCharacters);
+        } else
             m_impl = str.m_impl;
-            return;
-        }
-        m_impl = m_impl->copy();
-        m_impl->append(str.m_impl.get());
     }
 }
 
 void String::append(char c)
 {
-    if (!m_impl)
+    if (m_impl) {
+        Vector<UChar> newCharacters(m_impl->length() + 1);
+        memcpy(newCharacters.data(), m_impl->characters(), m_impl->length() * sizeof(UChar));
+        newCharacters[m_impl->length()] = c;
+        m_impl = StringImpl::adopt(newCharacters);
+    } else
         m_impl = new StringImpl(&c, 1);
-    else {
-        m_impl = m_impl->copy();
-        m_impl->append(c);
-    }
 }
 
 void String::append(UChar c)
 {
-    if (!m_impl)
+    if (m_impl) {
+        Vector<UChar> newCharacters(m_impl->length() + 1);
+        memcpy(newCharacters.data(), m_impl->characters(), m_impl->length() * sizeof(UChar));
+        newCharacters[m_impl->length()] = c;
+        m_impl = StringImpl::adopt(newCharacters);
+    } else
         m_impl = new StringImpl(&c, 1);
-    else {
-        m_impl = m_impl->copy();
-        m_impl->append(c);
-    }
 }
 
 String operator+(const String& a, const String& b)