Changeset 130190 in webkit

Timestamp:

Oct 2, 2012, 11:41:05 AM (13 years ago)

Author:

msaboff@apple.com

Message:

HTMLConstructionSite::insertTextNode isn't optimized for 8 bit strings
​https://bugs.webkit.org/show_bug.cgi?id=97740

Reviewed by Darin Adler.

Source/WebCore:

Changed parserAppendData to take a string instead of a UChar*. Also added an optional offset
argument to handle string+offset. The actual string append now uses the appropriate string
size.

• dom/CharacterData.cpp:

(WebCore::CharacterData::parserAppendData):

• dom/CharacterData.h:

(CharacterData):

• dom/Text.cpp:

(WebCore::Text::createWithLengthLimit):

• html/parser/HTMLConstructionSite.cpp:

(WebCore::HTMLConstructionSite::insertTextNode):

Source/WTF:

Added an append method that takes an LChar source. Made both the UChar and LChar versions optimally handle
the appendee and appendend string bitness.

• wtf/text/WTFString.cpp:

(WTF::String::append):

• wtf/text/WTFString.h:

(String):

Location:

trunk/Source

Files:

• WTF/ChangeLog (modified) (1 diff)
• WTF/wtf/text/WTFString.cpp (modified) (2 diffs)
• WTF/wtf/text/WTFString.h (modified) (1 diff)
• WebCore/ChangeLog (modified) (1 diff)
• WebCore/dom/CharacterData.cpp (modified) (2 diffs)
• WebCore/dom/CharacterData.h (modified) (1 diff)
• WebCore/dom/Text.cpp (modified) (1 diff)
• WebCore/html/parser/HTMLConstructionSite.cpp (modified) (1 diff)

trunk/Source/WTF/ChangeLog

@@ +1 @@
+2012-10-02  Michael Saboff  <msaboff@apple.com>
+
+        HTMLConstructionSite::insertTextNode isn't optimized for 8 bit strings
+        https://bugs.webkit.org/show_bug.cgi?id=97740
+
+        Reviewed by Darin Adler.
+
+        Added an append method that takes an LChar source.  Made both the UChar and LChar versions optimally handle
+        the appendee and appendend string bitness.
+
+        * wtf/text/WTFString.cpp:
+        (WTF::String::append):
+        * wtf/text/WTFString.h:
+        (String):
+
 2012-10-02  Ilya Tikhonovsky  <loislo@chromium.org>
 

trunk/Source/WTF/wtf/text/WTFString.cpp

@@ -168 +168 @@
 }
 
-void String::append(const UChar* charactersToAppend, unsigned lengthToAppend)
+void String::append(const LChar* charactersToAppend, unsigned lengthToAppend)
 {
     if (!m_impl) {
@@ -181 +181 @@
 
     ASSERT(charactersToAppend);
+
+    unsigned strLength = m_impl->length();
+
+    if (m_impl->is8Bit()) {
+        if (lengthToAppend > numeric_limits<unsigned>::max() - strLength)
+            CRASH();
+        LChar* data;
+        RefPtr<StringImpl> newImpl = StringImpl::createUninitialized(strLength + lengthToAppend, data);
+        StringImpl::copyChars(data, m_impl->characters8(), strLength);
+        StringImpl::copyChars(data + strLength, charactersToAppend, lengthToAppend);
+        m_impl = newImpl.release();
+        return;
+    }
+
+    if (lengthToAppend > numeric_limits<unsigned>::max() - strLength)
+        CRASH();
     UChar* data;
-    if (lengthToAppend > numeric_limits<unsigned>::max() - length())
+    RefPtr<StringImpl> newImpl = StringImpl::createUninitialized(length() + lengthToAppend, data);
+    StringImpl::copyChars(data, m_impl->characters16(), strLength);
+    StringImpl::copyChars(data + strLength, charactersToAppend, lengthToAppend);
+    m_impl = newImpl.release();
+}
+
+void String::append(const UChar* charactersToAppend, unsigned lengthToAppend)
+{
+    if (!m_impl) {
+        if (!charactersToAppend)
+            return;
+        m_impl = StringImpl::create(charactersToAppend, lengthToAppend);
+        return;
+    }
+
+    if (!lengthToAppend)
+        return;
+
+    unsigned strLength = m_impl->length();
+    
+    ASSERT(charactersToAppend);
+    if (lengthToAppend > numeric_limits<unsigned>::max() - strLength)
         CRASH();
-    RefPtr<StringImpl> newImpl = StringImpl::createUninitialized(length() + lengthToAppend, data);
-    memcpy(data, characters(), length() * sizeof(UChar));
-    memcpy(data + length(), charactersToAppend, lengthToAppend * sizeof(UChar));
+    UChar* data;
+    RefPtr<StringImpl> newImpl = StringImpl::createUninitialized(strLength + lengthToAppend, data);
+    if (m_impl->is8Bit())
+        StringImpl::copyChars(data, characters8(), strLength);
+    else
+        StringImpl::copyChars(data, characters16(), strLength);
+    StringImpl::copyChars(data + strLength, charactersToAppend, lengthToAppend);
     m_impl = newImpl.release();
 }
+
 
 void String::insert(const UChar* charactersToInsert, unsigned lengthToInsert, unsigned position)

trunk/Source/WTF/wtf/text/WTFString.h

@@ -303 +303 @@
     void append(char c) { append(static_cast<LChar>(c)); };
     WTF_EXPORT_STRING_API void append(UChar);
+    WTF_EXPORT_STRING_API void append(const LChar*, unsigned length);
     WTF_EXPORT_STRING_API void append(const UChar*, unsigned length);
     WTF_EXPORT_STRING_API void insert(const String&, unsigned pos);

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2012-10-02  Michael Saboff  <msaboff@apple.com>
+
+        HTMLConstructionSite::insertTextNode isn't optimized for 8 bit strings
+        https://bugs.webkit.org/show_bug.cgi?id=97740
+
+        Reviewed by Darin Adler.
+
+        Changed parserAppendData to take a string instead of a UChar*.  Also added an optional offset
+        argument to handle string+offset.  The actual string append now uses the appropriate string
+        size.
+
+        * dom/CharacterData.cpp:
+        (WebCore::CharacterData::parserAppendData):
+        * dom/CharacterData.h:
+        (CharacterData):
+        * dom/Text.cpp:
+        (WebCore::Text::createWithLengthLimit):
+        * html/parser/HTMLConstructionSite.cpp:
+        (WebCore::HTMLConstructionSite::insertTextNode):
+
 2012-10-02  Ojan Vafai  <ojan@chromium.org>
 

trunk/Source/WebCore/dom/CharacterData.cpp

@@ -64 +64 @@
 }
 
-unsigned CharacterData::parserAppendData(const UChar* data, unsigned dataLength, unsigned lengthLimit)
+unsigned CharacterData::parserAppendData(const String& string, unsigned offset, unsigned lengthLimit)
 {
     unsigned oldLength = m_data.length();
 
-    unsigned end = min(dataLength, lengthLimit - oldLength);
+    ASSERT(lengthLimit >= oldLength);
+
+    unsigned characterLength = string.length() - offset;
+    unsigned characterLengthLimit = min(characterLength, lengthLimit - oldLength);
 
     // Check that we are not on an unbreakable boundary.
-    // Some text break iterator implementations work best if the passed buffer is as small as possible, 
-    // see <https://bugs.webkit.org/show_bug.cgi?id=29092>. 
+    // Some text break iterator implementations work best if the passed buffer is as small as possible,
+    // see <https://bugs.webkit.org/show_bug.cgi?id=29092>.
     // We need at least two characters look-ahead to account for UTF-16 surrogates.
-    if (end < dataLength) {
-        NonSharedCharacterBreakIterator it(data, (end + 2 > dataLength) ? dataLength : end + 2);
-        if (!isTextBreak(it, end))
-            end = textBreakPreceding(it, end);
-    }
-    
-    if (!end)
+    if (characterLengthLimit < characterLength) {
+        NonSharedCharacterBreakIterator it(string.characters() + offset, (characterLengthLimit + 2 > characterLength) ? characterLength : characterLengthLimit + 2);
+        if (!isTextBreak(it, characterLengthLimit))
+            characterLengthLimit = textBreakPreceding(it, characterLengthLimit);
+    }
+
+    if (!characterLengthLimit)
         return 0;
 
-    m_data.append(data, end);
+    if (string.is8Bit())
+        m_data.append(string.characters8() + offset, characterLengthLimit);
+    else
+        m_data.append(string.characters16() + offset, characterLengthLimit);
 
     updateRenderer(oldLength, 0);
@@ -91 +97 @@
     if (parentNode())
         parentNode()->childrenChanged();
-    
-    return end;
+
+    return characterLengthLimit;
 }
 

trunk/Source/WebCore/dom/CharacterData.h

@@ -46 +46 @@
     // Like appendData, but optimized for the parser (e.g., no mutation events).
     // Returns how much could be added before length limit was met.
-    unsigned parserAppendData(const UChar*, unsigned dataLength, unsigned lengthLimit);
+    unsigned parserAppendData(const String& string, unsigned offset, unsigned lengthLimit);
 
     virtual void reportMemoryUsage(MemoryObjectInfo*) const;

trunk/Source/WebCore/dom/Text.cpp

@@ -289 +289 @@
 
     RefPtr<Text> result = Text::create(document, String());
-    result->parserAppendData(data.characters() + start, dataLength - start, maxChars);
+    result->parserAppendData(data, start, maxChars);
 
     return result;

trunk/Source/WebCore/html/parser/HTMLConstructionSite.cpp

@@ -369 +369 @@
         // was the last text node inserted by the parser.
         CharacterData* textNode = static_cast<CharacterData*>(previousChild);
-        currentPosition = textNode->parserAppendData(characters.characters(), characters.length(), Text::defaultLengthLimit);
+        currentPosition = textNode->parserAppendData(characters, 0, Text::defaultLengthLimit);
     }