Reduce use of String::characters
authordarin@apple.com <darin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 12 Jan 2014 23:23:44 +0000 (23:23 +0000)
committerdarin@apple.com <darin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 12 Jan 2014 23:23:44 +0000 (23:23 +0000)
https://bugs.webkit.org/show_bug.cgi?id=126854

Reviewed by Sam Weinig.

Source/JavaScriptCore:

* API/JSValueRef.cpp:
(JSValueMakeFromJSONString): Use characters16 instead of characters for 16-bit case.
Had to remove length check because an empty string could be either 8 bit or 16 bit.
Don't need a null string check before calling is8Bit because JSStringRef can't hold
a null string.

* bindings/ScriptValue.cpp:
(Deprecated::jsToInspectorValue): Use the existing string here instead of creating
a new one by calling characters and length on the old string. I think this may be
left over from when string types were not the same in JavaScriptCore and WebCore.
Also rewrite the property names loop to use modern for syntax and fewer locals.

* inspector/InspectorValues.cpp:
(Inspector::escapeChar): Changed to use appendLiteral instead of hard-coding string
lengths. Moved handling of "<" and ">" in here instead of at the call site.
(Inspector::doubleQuoteString): Simplify the code so there is no use of characters
and length. This is still an inefficient way of doing this job and could use a rethink.

* runtime/DatePrototype.cpp:
(JSC::formatLocaleDate): Use RetainPtr, createCFString, and the conversion from
CFStringRef to WTF::String to remove a lot of unneeded code.

* runtime/Identifier.h: Removed unneeded Identifier::characters function.

* runtime/JSStringBuilder.h:
(JSC::JSStringBuilder::append): Use characters16 instead of characters function here,
since we have already checked is8Bit above.

Source/WebCore:

* bindings/objc/WebScriptObject.mm:
(+[WebScriptObject _convertValueToObjcValue:JSC::originRootObject:rootObject:]):
Get rid of unneeded code to turn a WTF::String into an NSString, since that's
built into the WTF::String class.

* editing/CompositeEditCommand.cpp:
(WebCore::containsOnlyWhitespace): Use WTF::String's [] operator instead of
an explicit call to the characters function. Small performance cost.
* editing/TypingCommand.cpp:
(WebCore::TypingCommand::insertText): Ditto.

* editing/VisibleUnits.cpp:
(WebCore::startOfParagraph): Use reverseFind instead of writing our own loop.
(WebCore::endOfParagraph): Use find instead of writing our own loop.

* html/parser/HTMLParserIdioms.cpp:
(WebCore::stripLeadingAndTrailingHTMLSpaces): Use characters16 instead of
characters since we have already checked is8Bit.
(WebCore::parseHTMLNonNegativeInteger): Ditto. Replace the length check with
a check of isNull since a zero length string could be 8 bit, but it's not
safe to call is8Bit on a null WTF::String. (That should probably be fixed.)

* inspector/ContentSearchUtils.cpp:
(WebCore::ContentSearchUtils::createSearchRegexSource): Use StringBuilder
instead of String in code that builds up a string one character at a time.
The old way was ridiculously slow because it allocated a new string for every
character; the new way still isn't all that efficient, but it's better. Also
changed to use strchr instead of WTF::String::find for special characters.

* inspector/InspectorStyleSheet.cpp:
(WebCore::InspectorStyle::newLineAndWhitespaceDelimiters): Use WTF::String's
[] operator instead of an explicit call to the characters function.
* inspector/InspectorStyleTextEditor.cpp:
(WebCore::InspectorStyleTextEditor::insertProperty): Ditto.
(WebCore::InspectorStyleTextEditor::internalReplaceProperty): Ditto.
* platform/Length.cpp:
(WebCore::newCoordsArray): Ditto.

* platform/LinkHash.cpp:
(WebCore::visitedLinkHash): Use characters16 instead of characters since
we have already checked is8Bit. Replace the length check with a check of
isNull (as above in parseHTMLNonNegativeInteger).

* platform/graphics/Color.cpp:
(WebCore::Color::parseHexColor): Use characters16 since we already checked is8Bit.
(WebCore::Color::Color): Ditto.

* platform/graphics/TextRun.h:
(WebCore::TextRun::TextRun): Use characters16 instead of characters since
we have already checked is8Bit. Replace the length check with a check of
isNull (as above in parseHTMLNonNegativeInteger).

* platform/text/TextEncodingRegistry.cpp:
(WebCore::atomicCanonicalTextEncodingName): Use characters16 instead of
characters since we already checked is8Bit. Also use isEmpty instead of !length.

* rendering/RenderBlock.cpp:
(WebCore::RenderBlock::constructTextRun): Use characters16 instead of characters
since we have already checked is8Bit. Replace the length check with a check of
isNull (as above in parseHTMLNonNegativeInteger).

* rendering/RenderCombineText.cpp:
(WebCore::RenderCombineText::width): Check for zero length instead of checking
for null characters.

* svg/SVGFontElement.cpp:
(WebCore::SVGFontElement::registerLigaturesInGlyphCache): Rewrite ligatures
for loop and give local variables a better name. Construct the substring with
the substring function. Still a really inefficient approach -- allocating a new
string for every character is absurdly expensive.

* xml/XPathFunctions.cpp:
(WebCore::XPath::FunId::evaluate): Use String instead of StringBuilder. Still
not all that efficient, but better than before. Use substring to construct the
substring.

* xml/XPathNodeSet.h: Added begin and end functions so we can use a C++11 for
loop with this class.

Source/WTF:

* wtf/text/StringImpl.cpp:
(WTF::StringImpl::replace): Use characters16 here since is8Bit was already checked.
* wtf/text/WTFString.h:
(WTF::String::isAllSpecialCharacters): Use characters16 here since is8Bit was
already checked. Also renamed "len" to "length".

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

29 files changed:
Source/JavaScriptCore/API/JSValueRef.cpp
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/bindings/ScriptValue.cpp
Source/JavaScriptCore/inspector/InspectorValues.cpp
Source/JavaScriptCore/runtime/DatePrototype.cpp
Source/JavaScriptCore/runtime/Identifier.h
Source/JavaScriptCore/runtime/JSStringBuilder.h
Source/WTF/ChangeLog
Source/WTF/wtf/text/StringImpl.cpp
Source/WTF/wtf/text/WTFString.h
Source/WebCore/ChangeLog
Source/WebCore/bindings/objc/WebScriptObject.mm
Source/WebCore/editing/CompositeEditCommand.cpp
Source/WebCore/editing/TypingCommand.cpp
Source/WebCore/editing/VisibleUnits.cpp
Source/WebCore/html/parser/HTMLParserIdioms.cpp
Source/WebCore/inspector/ContentSearchUtils.cpp
Source/WebCore/inspector/InspectorStyleSheet.cpp
Source/WebCore/inspector/InspectorStyleTextEditor.cpp
Source/WebCore/platform/Length.cpp
Source/WebCore/platform/LinkHash.cpp
Source/WebCore/platform/graphics/Color.cpp
Source/WebCore/platform/graphics/TextRun.h
Source/WebCore/platform/text/TextEncodingRegistry.cpp
Source/WebCore/rendering/RenderBlock.cpp
Source/WebCore/rendering/RenderCombineText.cpp
Source/WebCore/svg/SVGFontElement.cpp
Source/WebCore/xml/XPathFunctions.cpp
Source/WebCore/xml/XPathNodeSet.h

index eb99f5d..26a7f86 100644 (file)
@@ -323,11 +323,11 @@ JSValueRef JSValueMakeFromJSONString(JSContextRef ctx, JSStringRef string)
     APIEntryShim entryShim(exec);
     String str = string->string();
     unsigned length = str.length();
-    if (length && str.is8Bit()) {
+    if (str.is8Bit()) {
         LiteralParser<LChar> parser(exec, str.characters8(), length, StrictJSON);
         return toRef(exec, parser.tryLiteralParse());
     }
-    LiteralParser<UChar> parser(exec, str.characters(), length, StrictJSON);
+    LiteralParser<UChar> parser(exec, str.characters16(), length, StrictJSON);
     return toRef(exec, parser.tryLiteralParse());
 }
 
index a0bd53d..f5cf253 100644 (file)
@@ -1,3 +1,38 @@
+2014-01-12  Darin Adler  <darin@apple.com>
+
+        Reduce use of String::characters
+        https://bugs.webkit.org/show_bug.cgi?id=126854
+
+        Reviewed by Sam Weinig.
+
+        * API/JSValueRef.cpp:
+        (JSValueMakeFromJSONString): Use characters16 instead of characters for 16-bit case.
+        Had to remove length check because an empty string could be either 8 bit or 16 bit.
+        Don't need a null string check before calling is8Bit because JSStringRef can't hold
+        a null string.
+
+        * bindings/ScriptValue.cpp:
+        (Deprecated::jsToInspectorValue): Use the existing string here instead of creating
+        a new one by calling characters and length on the old string. I think this may be
+        left over from when string types were not the same in JavaScriptCore and WebCore.
+        Also rewrite the property names loop to use modern for syntax and fewer locals.
+
+        * inspector/InspectorValues.cpp:
+        (Inspector::escapeChar): Changed to use appendLiteral instead of hard-coding string
+        lengths. Moved handling of "<" and ">" in here instead of at the call site.
+        (Inspector::doubleQuoteString): Simplify the code so there is no use of characters
+        and length. This is still an inefficient way of doing this job and could use a rethink.
+
+        * runtime/DatePrototype.cpp:
+        (JSC::formatLocaleDate): Use RetainPtr, createCFString, and the conversion from
+        CFStringRef to WTF::String to remove a lot of unneeded code.
+
+        * runtime/Identifier.h: Removed unneeded Identifier::characters function.
+
+        * runtime/JSStringBuilder.h:
+        (JSC::JSStringBuilder::append): Use characters16 instead of characters function here,
+        since we have already checked is8Bit above.
+
 2014-01-12  Andy Estes  <aestes@apple.com>
 
         [iOS] Enable the JSC Objective-C API
index 5c60b1f..b0a6911 100644 (file)
@@ -115,10 +115,8 @@ static PassRefPtr<InspectorValue> jsToInspectorValue(ExecState* scriptState, JSV
         return InspectorBasicValue::create(value.asBoolean());
     if (value.isNumber())
         return InspectorBasicValue::create(value.asNumber());
-    if (value.isString()) {
-        String s = value.getString(scriptState);
-        return InspectorString::create(String(s.characters(), s.length()));
-    }
+    if (value.isString())
+        return InspectorString::create(value.getString(scriptState));
 
     if (value.isObject()) {
         if (isJSArray(value)) {
@@ -138,13 +136,11 @@ static PassRefPtr<InspectorValue> jsToInspectorValue(ExecState* scriptState, JSV
         JSObject* object = value.getObject();
         PropertyNameArray propertyNames(scriptState);
         object->methodTable()->getOwnPropertyNames(object, scriptState, propertyNames, ExcludeDontEnumProperties);
-        for (size_t i = 0; i < propertyNames.size(); i++) {
-            const Identifier& name =  propertyNames[i];
-            JSValue propertyValue = object->get(scriptState, name);
-            RefPtr<InspectorValue> inspectorValue = jsToInspectorValue(scriptState, propertyValue, maxDepth);
+        for (auto& name : propertyNames) {
+            RefPtr<InspectorValue> inspectorValue = jsToInspectorValue(scriptState, object->get(scriptState, name), maxDepth);
             if (!inspectorValue)
                 return nullptr;
-            inspectorObject->setValue(String(name.characters(), name.length()), inspectorValue);
+            inspectorObject->setValue(name.string(), inspectorValue);
         }
         return inspectorObject;
     }
index 2419dea..8a8f2d7 100644 (file)
@@ -446,14 +446,17 @@ PassRefPtr<InspectorValue> buildValue(const UChar* start, const UChar* end, cons
 
 inline bool escapeChar(UChar c, StringBuilder* dst)
 {
+    // Must escape < and > to prevent script execution.
     switch (c) {
-    case '\b': dst->append("\\b", 2); break;
-    case '\f': dst->append("\\f", 2); break;
-    case '\n': dst->append("\\n", 2); break;
-    case '\r': dst->append("\\r", 2); break;
-    case '\t': dst->append("\\t", 2); break;
-    case '\\': dst->append("\\\\", 2); break;
-    case '"': dst->append("\\\"", 2); break;
+    case '\b': dst->appendLiteral("\\b"); break;
+    case '\f': dst->appendLiteral("\\f"); break;
+    case '\n': dst->appendLiteral("\\n"); break;
+    case '\r': dst->appendLiteral("\\r"); break;
+    case '\t': dst->appendLiteral("\\t"); break;
+    case '\\': dst->appendLiteral("\\\\"); break;
+    case '"': dst->appendLiteral("\\\""); break;
+    case '<': dst->appendLiteral("\\u003C"); break;
+    case '>': dst->appendLiteral("\\u003E"); break;
     default:
         return false;
     }
@@ -466,15 +469,13 @@ inline void doubleQuoteString(const String& str, StringBuilder* dst)
     for (unsigned i = 0; i < str.length(); ++i) {
         UChar c = str[i];
         if (!escapeChar(c, dst)) {
-            if (c < 32 || c > 126 || c == '<' || c == '>') {
-                // 1. Escaping <, > to prevent script execution.
-                // 2. Technically, we could also pass through c > 126 as UTF8, but this
-                //    is also optional.  It would also be a pain to implement here.
-                unsigned int symbol = static_cast<unsigned int>(c);
-                String symbolCode = String::format("\\u%04X", symbol);
-                dst->append(symbolCode.characters(), symbolCode.length());
-            } else
+            // We could format c > 126 as UTF-8 instead of escaping them.
+            if (c >= 32 || c <= 126)
                 dst->append(c);
+            else {
+                // FIXME: Way too slow to do this by creating and destroying a string each time.
+                dst->append(String::format("\\u%04X", static_cast<unsigned>(c)));
+            }
         }
     }
     dst->append('"');
index c2b8a0c..358f156 100644 (file)
@@ -162,33 +162,14 @@ static JSCell* formatLocaleDate(ExecState* exec, DateInstance*, double timeInMil
     else if (format != LocaleDate && !exec->argument(0).isUndefined())
         timeStyle = styleFromArgString(arg0String, timeStyle);
 
-    CFLocaleRef locale = CFLocaleCopyCurrent();
-    CFDateFormatterRef formatter = CFDateFormatterCreate(0, locale, dateStyle, timeStyle);
-    CFRelease(locale);
-
-    if (useCustomFormat) {
-        CFStringRef customFormatCFString = CFStringCreateWithCharacters(0, customFormatString.characters(), customFormatString.length());
-        CFDateFormatterSetFormat(formatter, customFormatCFString);
-        CFRelease(customFormatCFString);
-    }
-
-    CFStringRef string = CFDateFormatterCreateStringWithAbsoluteTime(0, formatter, floor(timeInMilliseconds / msPerSecond) - kCFAbsoluteTimeIntervalSince1970);
+    RetainPtr<CFDateFormatterRef> formatter = adoptCF(CFDateFormatterCreate(kCFAllocatorDefault, adoptCF(CFLocaleCopyCurrent()).get(), dateStyle, timeStyle));
 
-    CFRelease(formatter);
+    if (useCustomFormat)
+        CFDateFormatterSetFormat(formatter.get(), customFormatString.createCFString().get());
 
-    // We truncate the string returned from CFDateFormatter if it's absurdly long (> 200 characters).
-    // That's not great error handling, but it just won't happen so it doesn't matter.
-    UChar buffer[200];
-    const size_t bufferLength = WTF_ARRAY_LENGTH(buffer);
-    size_t length = CFStringGetLength(string);
-    ASSERT(length <= bufferLength);
-    if (length > bufferLength)
-        length = bufferLength;
-    CFStringGetCharacters(string, CFRangeMake(0, length), buffer);
+    RetainPtr<CFStringRef> string = adoptCF(CFDateFormatterCreateStringWithAbsoluteTime(kCFAllocatorDefault, formatter.get(), floor(timeInMilliseconds / msPerSecond) - kCFAbsoluteTimeIntervalSince1970));
 
-    CFRelease(string);
-
-    return jsNontrivialString(exec, String(buffer, length));
+    return jsNontrivialString(exec, string.get());
 }
 
 #elif USE(ICU_UNICODE) && !UCONFIG_NO_FORMATTING
index 0abb527..81d4b38 100644 (file)
@@ -55,7 +55,6 @@ namespace JSC {
         const String& string() const { return m_string; }
         StringImpl* impl() const { return m_string.impl(); }
         
-        const UChar* characters() const { return m_string.characters(); }
         int length() const { return m_string.length(); }
         
         CString ascii() const { return m_string.ascii(); }
index 5d4960e..61c957b 100644 (file)
@@ -105,7 +105,7 @@ public:
             }
             upConvert();
         }
-        m_okay &= buffer16.tryAppend(str.characters(), length);
+        m_okay &= buffer16.tryAppend(str.characters16(), length);
     }
 
     void upConvert()
index 5c7fb61..8dada98 100644 (file)
@@ -1,3 +1,16 @@
+2014-01-12  Darin Adler  <darin@apple.com>
+
+        Reduce use of String::characters
+        https://bugs.webkit.org/show_bug.cgi?id=126854
+
+        Reviewed by Sam Weinig.
+
+        * wtf/text/StringImpl.cpp:
+        (WTF::StringImpl::replace): Use characters16 here since is8Bit was already checked.
+        * wtf/text/WTFString.h:
+        (WTF::String::isAllSpecialCharacters): Use characters16 here since is8Bit was
+        already checked. Also renamed "len" to "length".
+
 2014-01-12  Anders Carlsson  <andersca@apple.com>
 
         Replace more uses of AtomicallyInitializedStatic with std::call_once
index 9ccfa94..8a549d4 100644 (file)
@@ -1489,7 +1489,7 @@ PassRef<StringImpl> StringImpl::replace(unsigned position, unsigned lengthToRepl
         for (unsigned i = 0; i < length() - position - lengthToReplace; ++i)
             data[i + position + lengthToInsert] = m_data8[i + position + lengthToReplace];
     } else {
-        memcpy(data + position + lengthToInsert, characters() + position + lengthToReplace,
+        memcpy(data + position + lengthToInsert, characters16() + position + lengthToReplace,
             (length() - position - lengthToReplace) * sizeof(UChar));
     }
     return newImpl;
index 17ca746..f5b8d36 100644 (file)
@@ -624,14 +624,14 @@ inline bool isAllSpecialCharacters(const CharacterType* characters, size_t lengt
 template<bool isSpecialCharacter(UChar)>
 inline bool String::isAllSpecialCharacters() const
 {
-    size_t len = length();
+    size_t length = this->length();
 
-    if (!len)
+    if (!length)
         return true;
 
     if (is8Bit())
-        return WTF::isAllSpecialCharacters<isSpecialCharacter, LChar>(characters8(), len);
-    return WTF::isAllSpecialCharacters<isSpecialCharacter, UChar>(characters(), len);
+        return WTF::isAllSpecialCharacters<isSpecialCharacter>(characters8(), length);
+    return WTF::isAllSpecialCharacters<isSpecialCharacter>(characters16(), length);
 }
 
 // StringHash is the default hash for String
index b9822e1..5b94293 100644 (file)
@@ -1,3 +1,89 @@
+2014-01-12  Darin Adler  <darin@apple.com>
+
+        Reduce use of String::characters
+        https://bugs.webkit.org/show_bug.cgi?id=126854
+
+        Reviewed by Sam Weinig.
+
+        * bindings/objc/WebScriptObject.mm:
+        (+[WebScriptObject _convertValueToObjcValue:JSC::originRootObject:rootObject:]):
+        Get rid of unneeded code to turn a WTF::String into an NSString, since that's
+        built into the WTF::String class.
+
+        * editing/CompositeEditCommand.cpp:
+        (WebCore::containsOnlyWhitespace): Use WTF::String's [] operator instead of
+        an explicit call to the characters function. Small performance cost.
+        * editing/TypingCommand.cpp:
+        (WebCore::TypingCommand::insertText): Ditto.
+
+        * editing/VisibleUnits.cpp:
+        (WebCore::startOfParagraph): Use reverseFind instead of writing our own loop.
+        (WebCore::endOfParagraph): Use find instead of writing our own loop.
+
+        * html/parser/HTMLParserIdioms.cpp:
+        (WebCore::stripLeadingAndTrailingHTMLSpaces): Use characters16 instead of
+        characters since we have already checked is8Bit.
+        (WebCore::parseHTMLNonNegativeInteger): Ditto. Replace the length check with
+        a check of isNull since a zero length string could be 8 bit, but it's not
+        safe to call is8Bit on a null WTF::String. (That should probably be fixed.)
+
+        * inspector/ContentSearchUtils.cpp:
+        (WebCore::ContentSearchUtils::createSearchRegexSource): Use StringBuilder
+        instead of String in code that builds up a string one character at a time.
+        The old way was ridiculously slow because it allocated a new string for every
+        character; the new way still isn't all that efficient, but it's better. Also
+        changed to use strchr instead of WTF::String::find for special characters.
+
+        * inspector/InspectorStyleSheet.cpp:
+        (WebCore::InspectorStyle::newLineAndWhitespaceDelimiters): Use WTF::String's
+        [] operator instead of an explicit call to the characters function.
+        * inspector/InspectorStyleTextEditor.cpp:
+        (WebCore::InspectorStyleTextEditor::insertProperty): Ditto.
+        (WebCore::InspectorStyleTextEditor::internalReplaceProperty): Ditto.
+        * platform/Length.cpp:
+        (WebCore::newCoordsArray): Ditto.
+
+        * platform/LinkHash.cpp:
+        (WebCore::visitedLinkHash): Use characters16 instead of characters since
+        we have already checked is8Bit. Replace the length check with a check of
+        isNull (as above in parseHTMLNonNegativeInteger).
+
+        * platform/graphics/Color.cpp:
+        (WebCore::Color::parseHexColor): Use characters16 since we already checked is8Bit.
+        (WebCore::Color::Color): Ditto.
+
+        * platform/graphics/TextRun.h:
+        (WebCore::TextRun::TextRun): Use characters16 instead of characters since
+        we have already checked is8Bit. Replace the length check with a check of
+        isNull (as above in parseHTMLNonNegativeInteger).
+
+        * platform/text/TextEncodingRegistry.cpp:
+        (WebCore::atomicCanonicalTextEncodingName): Use characters16 instead of
+        characters since we already checked is8Bit. Also use isEmpty instead of !length.
+
+        * rendering/RenderBlock.cpp:
+        (WebCore::RenderBlock::constructTextRun): Use characters16 instead of characters
+        since we have already checked is8Bit. Replace the length check with a check of
+        isNull (as above in parseHTMLNonNegativeInteger).
+
+        * rendering/RenderCombineText.cpp:
+        (WebCore::RenderCombineText::width): Check for zero length instead of checking
+        for null characters.
+
+        * svg/SVGFontElement.cpp:
+        (WebCore::SVGFontElement::registerLigaturesInGlyphCache): Rewrite ligatures
+        for loop and give local variables a better name. Construct the substring with
+        the substring function. Still a really inefficient approach -- allocating a new
+        string for every character is absurdly expensive.
+
+        * xml/XPathFunctions.cpp:
+        (WebCore::XPath::FunId::evaluate): Use String instead of StringBuilder. Still
+        not all that efficient, but better than before. Use substring to construct the
+        substring.
+
+        * xml/XPathNodeSet.h: Added begin and end functions so we can use a C++11 for
+        loop with this class.
+
 2014-01-12  Benjamin Poulain  <benjamin@webkit.org>
 
         Use the Selector Code Generator for matching in SelectorQuery
index 5b438f7..ff0536a 100644 (file)
@@ -548,11 +548,8 @@ static void getListFromNSArray(ExecState *exec, NSArray *array, RootObject* root
         return [WebScriptObject scriptObjectForJSObject:toRef(object) originRootObject:originRootObject rootObject:rootObject];
     }
 
-    if (value.isString()) {
-        ExecState* exec = rootObject->globalObject()->globalExec();
-        const String& u = asString(value)->value(exec);
-        return [NSString stringWithCharacters:u.characters() length:u.length()];
-    }
+    if (value.isString())
+        return asString(value)->value(rootObject->globalObject()->globalExec());
 
     if (value.isNumber())
         return [NSNumber numberWithDouble:value.asNumber()];
index 069d4d6..40e31a4 100644 (file)
@@ -667,10 +667,9 @@ void CompositeEditCommand::setNodeAttribute(PassRefPtr<Element> element, const Q
 static inline bool containsOnlyWhitespace(const String& text)
 {
     for (unsigned i = 0; i < text.length(); ++i) {
-        if (!isWhitespace(text.characters()[i]))
+        if (!isWhitespace(text[i]))
             return false;
     }
-    
     return true;
 }
 
index 1006849..ed3c72a 100644 (file)
@@ -152,7 +152,7 @@ void TypingCommand::insertText(Document& document, const String& text, Options o
     ASSERT(frame);
 
     if (!text.isEmpty())
-        frame->editor().updateMarkersForWordsAffectedByEditing(isSpaceOrNewline(text.characters()[0]));
+        frame->editor().updateMarkersForWordsAffectedByEditing(isSpaceOrNewline(text[0]));
     
     insertText(document, text, frame->selection().selection(), options, composition);
 }
index 780c699..a6f4e7e 100644 (file)
@@ -1146,15 +1146,10 @@ VisiblePosition startOfParagraph(const VisiblePosition& c, EditingBoundaryCrossi
             ASSERT_WITH_SECURITY_IMPLICATION(n->isTextNode());
             type = Position::PositionIsOffsetInAnchor;
             if (style.preserveNewline()) {
-                const UChar* chars = toRenderText(r)->characters();
-                int i = toRenderText(r)->textLength();
-                int o = offset;
-                if (n == startNode && o < i)
-                    i = std::max(0, o);
-                while (--i >= 0) {
-                    if (chars[i] == '\n')
-                        return VisiblePosition(Position(toText(n), i + 1), DOWNSTREAM);
-                }
+                unsigned startOffset = n == startNode ? std::max(0, offset) : std::numeric_limits<unsigned>::max();
+                size_t newlineOffset = toRenderText(r)->text()->reverseFind('\n', startOffset);
+                if (newlineOffset != notFound)
+                    return VisiblePosition(Position(toText(n), newlineOffset + 1), DOWNSTREAM);
             }
             node = n;
             offset = 0;
@@ -1226,15 +1221,11 @@ VisiblePosition endOfParagraph(const VisiblePosition &c, EditingBoundaryCrossing
         // FIXME: We avoid returning a position where the renderer can't accept the caret.
         if (r->isText() && toRenderText(r)->hasRenderedText()) {
             ASSERT_WITH_SECURITY_IMPLICATION(n->isTextNode());
-            int length = toRenderText(r)->textLength();
             type = Position::PositionIsOffsetInAnchor;
             if (style.preserveNewline()) {
-                const UChar* chars = toRenderText(r)->characters();
-                int o = n == startNode ? offset : 0;
-                for (int i = o; i < length; ++i) {
-                    if (chars[i] == '\n')
-                        return VisiblePosition(Position(toText(n), i), DOWNSTREAM);
-                }
+                size_t newlineOffset = toRenderText(r)->text()->find('\n', n == startNode ? offset : 0);
+                if (newlineOffset != notFound)
+                    return VisiblePosition(Position(toText(n), newlineOffset), DOWNSTREAM);
             }
             node = n;
             offset = r->caretMaxOffset();
index d4b7be9..2588daa 100644 (file)
@@ -71,7 +71,7 @@ String stripLeadingAndTrailingHTMLSpaces(const String& string)
     if (string.is8Bit())
         return stripLeadingAndTrailingHTMLSpaces(string, string.characters8(), length);
 
-    return stripLeadingAndTrailingHTMLSpaces(string, string.characters(), length);
+    return stripLeadingAndTrailingHTMLSpaces(string, string.characters16(), length);
 }
 
 String serializeForNumberType(const Decimal& number)
@@ -267,12 +267,12 @@ bool parseHTMLNonNegativeInteger(const String& input, unsigned& value)
     // Step 1
     // Step 2
     unsigned length = input.length();
-    if (length && input.is8Bit()) {
+    if (input.isNull() || input.is8Bit()) {
         const LChar* start = input.characters8();
         return parseHTMLNonNegativeIntegerInternal(start, start + length, value);
     }
     
-    const UChar* start = input.characters();
+    const UChar* start = input.characters16();
     return parseHTMLNonNegativeIntegerInternal(start, start + length, value);
 }
 
index e9dd070..2c2f880 100644 (file)
@@ -36,6 +36,7 @@
 #include <inspector/InspectorValues.h>
 #include <wtf/BumpPointerAllocator.h>
 #include <wtf/StdLibExtras.h>
+#include <wtf/text/StringBuilder.h>
 #include <yarr/Yarr.h>
 
 using namespace Inspector;
@@ -50,17 +51,13 @@ static const char regexSpecialCharacters[] = "[](){}+-*.,?\\^$|";
 
 static String createSearchRegexSource(const String& text)
 {
-    String result;
-    const UChar* characters = text.characters();
-    String specials(regexSpecialCharacters);
-
+    StringBuilder result;
     for (unsigned i = 0; i < text.length(); i++) {
-        if (specials.find(characters[i]) != notFound)
-            result.append("\\");
-        result.append(characters[i]);
+        if (isASCII(text[i]) && strchr(regexSpecialCharacters, text[i]))
+            result.append('\\');
+        result.append(text[i]);
     }
-
-    return result;
+    return result.toString();
 }
 
 static inline size_t sizetExtractor(const size_t* value)
index caa44ce..3e9a860 100644 (file)
@@ -723,14 +723,13 @@ NewLineAndWhitespace& InspectorStyle::newLineAndWhitespaceDelimiters() const
     int propertyIndex = 0;
     bool isFullPrefixScanned = false;
     bool lineFeedTerminated = false;
-    const UChar* characters = text.characters();
     while (propertyIndex < propertyCount) {
         const WebCore::CSSPropertySourceData& currentProperty = sourcePropertyData->at(propertyIndex++);
 
         bool processNextProperty = false;
         int scanEnd = currentProperty.range.start;
         for (int i = scanStart; i < scanEnd; ++i) {
-            UChar ch = characters[i];
+            UChar ch = text[i];
             bool isLineFeed = isHTMLLineBreak(ch);
             if (isLineFeed) {
                 if (!lineFeedTerminated)
index f3099a8..3ce5cb6 100644 (file)
@@ -82,12 +82,10 @@ void InspectorStyleTextEditor::insertProperty(unsigned index, const String& prop
     if (insertLast && !insertFirstInSource) {
         propertyStart = styleBodyLength;
         if (propertyStart && textToSet.length()) {
-            const UChar* characters = m_styleText.characters();
-
             long curPos = propertyStart - 1; // The last position of style declaration, since propertyStart points past one.
-            while (curPos && isHTMLSpace(characters[curPos]))
+            while (curPos && isHTMLSpace(m_styleText[curPos]))
                 --curPos;
-            if (curPos && characters[curPos] != ';') {
+            if (curPos && m_styleText[curPos] != ';') {
                 // Prepend a ";" to the property text if appending to a style declaration where
                 // the last property has no trailing ";".
                 textToSet.insert(";", 0);
@@ -230,7 +228,6 @@ void InspectorStyleTextEditor::internalReplaceProperty(const InspectorStylePrope
     const SourceRange& range = property.sourceData.range;
     long replaceRangeStart = range.start;
     long replaceRangeEnd = range.end;
-    const UChar* characters = m_styleText.characters();
     long newTextLength = newText.length();
     String finalNewText = newText;
 
@@ -241,14 +238,14 @@ void InspectorStyleTextEditor::internalReplaceProperty(const InspectorStylePrope
         if (replaceRangeStart >= fullPrefixLength && m_styleText.substring(replaceRangeStart - fullPrefixLength, fullPrefixLength) == fullPrefix)
             replaceRangeStart -= fullPrefixLength;
     } else if (newTextLength) {
-        if (isHTMLLineBreak(newText.characters()[newTextLength - 1])) {
+        if (isHTMLLineBreak(newText[newTextLength - 1])) {
             // Coalesce newlines of the original and new property values (to avoid a lot of blank lines while incrementally applying property values).
             bool foundNewline = false;
             bool isLastNewline = false;
             int i;
             int textLength = m_styleText.length();
-            for (i = replaceRangeEnd; i < textLength && isSpaceOrNewline(characters[i]); ++i) {
-                isLastNewline = isHTMLLineBreak(characters[i]);
+            for (i = replaceRangeEnd; i < textLength && isSpaceOrNewline(m_styleText[i]); ++i) {
+                isLastNewline = isHTMLLineBreak(m_styleText[i]);
                 if (isLastNewline)
                     foundNewline = true;
                 else if (foundNewline && !isLastNewline) {
index 19be2b8..3ff9d5d 100644 (file)
@@ -88,10 +88,9 @@ static int countCharacter(const UChar* data, unsigned length, UChar character)
 std::unique_ptr<Length[]> newCoordsArray(const String& string, int& len)
 {
     unsigned length = string.length();
-    const UChar* data = string.characters();
     StringBuffer<UChar> spacified(length);
     for (unsigned i = 0; i < length; i++) {
-        UChar cc = data[i];
+        UChar cc = string[i];
         if (cc > '9' || (cc < '0' && cc != '-' && cc != '*' && cc != '.'))
             spacified[i] = ' ';
         else
index 2b2ee20..2411660 100644 (file)
@@ -214,10 +214,9 @@ static ALWAYS_INLINE LinkHash visitedLinkHashInline(const CharacterType* url, un
 LinkHash visitedLinkHash(const String& url)
 {
     unsigned length = url.length();
-
-    if (length && url.is8Bit())
+    if (url.isNull() || url.is8Bit())
         return visitedLinkHashInline(url.characters8(), length);
-    return visitedLinkHashInline(url.characters(), length);
+    return visitedLinkHashInline(url.characters16(), length);
 }
 
 LinkHash visitedLinkHash(const UChar* url, unsigned length)
index 59d3759..9e3b5c1 100644 (file)
@@ -165,7 +165,7 @@ bool Color::parseHexColor(const String& name, RGBA32& rgb)
         return false;
     if (name.is8Bit())
         return parseHexColor(name.characters8(), name.length(), rgb);
-    return parseHexColor(name.characters(), name.length(), rgb);
+    return parseHexColor(name.characters16(), name.length(), rgb);
 }
 
 int differenceSquared(const Color& c1, const Color& c2)
@@ -182,7 +182,7 @@ Color::Color(const String& name)
         if (name.is8Bit())
             m_valid = parseHexColor(name.characters8() + 1, name.length() - 1, m_color);
         else
-            m_valid = parseHexColor(name.characters() + 1, name.length() - 1, m_color);
+            m_valid = parseHexColor(name.characters16() + 1, name.length() - 1, m_color);
     } else
         setNamedColor(name);
 }
index 170a1b6..4f4e9e5 100644 (file)
@@ -124,11 +124,11 @@ public:
         , m_tabSize(0)
     {
 #if ENABLE(8BIT_TEXTRUN)
-        if (m_charactersLength && s.is8Bit()) {
+        if (s.isNull() || s.is8Bit()) {
             m_data.characters8 = s.characters8();
             m_is8Bit = true;
         } else {
-            m_data.characters16 = s.characters();
+            m_data.characters16 = s.characters16();
             m_is8Bit = false;
         }
 #else
index b1fee35..f2e4cf9 100644 (file)
@@ -343,13 +343,13 @@ const char* atomicCanonicalTextEncodingName(const CharacterType* characters, siz
 
 const char* atomicCanonicalTextEncodingName(const String& alias)
 {
-    if (!alias.length())
-        return 0;
+    if (alias.isEmpty())
+        return nullptr;
 
     if (alias.is8Bit())
         return atomicCanonicalTextEncodingName<LChar>(alias.characters8(), alias.length());
 
-    return atomicCanonicalTextEncodingName<UChar>(alias.characters(), alias.length());
+    return atomicCanonicalTextEncodingName<UChar>(alias.characters16(), alias.length());
 }
 
 bool noExtendedTextEncodingNameUsed()
index cac1820..9456d4d 100644 (file)
@@ -5479,11 +5479,10 @@ TextRun RenderBlock::constructTextRun(RenderObject* context, const Font& font, c
 TextRun RenderBlock::constructTextRun(RenderObject* context, const Font& font, const String& string, const RenderStyle& style, TextRun::ExpansionBehavior expansion, TextRunFlags flags)
 {
     unsigned length = string.length();
-
 #if ENABLE(8BIT_TEXTRUN)
-    if (length && string.is8Bit())
+    if (string.isNull() || string.is8Bit())
         return constructTextRunInternal(context, font, string.characters8(), length, style, expansion, flags);
-    return constructTextRunInternal(context, font, string.characters(), length, style, expansion, flags);
+    return constructTextRunInternal(context, font, string.characters16(), length, style, expansion, flags);
 #else
     return constructTextRunInternal(context, font, string.characters(), length, style, expansion, flags);
 #endif
index 7b577b2..30e25e7 100644 (file)
@@ -60,7 +60,7 @@ void RenderCombineText::setTextInternal(const String& text)
 
 float RenderCombineText::width(unsigned from, unsigned length, const Font& font, float xPosition, HashSet<const SimpleFontData*>* fallbackFonts, GlyphOverflow* glyphOverflow) const
 {
-    if (!characters())
+    if (!textLength())
         return 0;
 
     if (m_isCombined)
index 8b96e49..908906a 100644 (file)
@@ -87,16 +87,13 @@ void SVGFontElement::registerLigaturesInGlyphCache(Vector<String>& ligatures)
     // will not be able to find a glyph for "f", but handles the fallback
     // character substitution properly through glyphDataForCharacter().
     Vector<SVGGlyph> glyphs;
-    size_t ligaturesSize = ligatures.size();
-    for (size_t i = 0; i < ligaturesSize; ++i) {
-        const String& unicode = ligatures[i];
+    for (auto& ligature : ligatures) {
+        unsigned ligatureLength = ligature.length();
+        ASSERT(ligatureLength > 1);
 
-        unsigned unicodeLength = unicode.length();
-        ASSERT(unicodeLength > 1);
-
-        const UChar* characters = unicode.characters();
-        for (unsigned i = 0; i < unicodeLength; ++i) {
-            String lookupString(characters + i, 1);
+        for (unsigned i = 0; i < ligatureLength; ++i) {
+            // FIXME: Is there a faster way to do this without allocating/deallocating a string for every character in every ligature?
+            String lookupString(ligature.substring(i, 1));
             m_glyphMap.collectGlyphsForString(lookupString, glyphs);
             if (!glyphs.isEmpty()) {
                 glyphs.clear();
index 37a276f..432440a 100644 (file)
@@ -307,18 +307,17 @@ Value FunPosition::evaluate() const
 Value FunId::evaluate() const
 {
     Value a = argument(0).evaluate();
-    StringBuilder idList; // A whitespace-separated list of IDs
 
+    String idList; // A whitespace-separated list of IDs
     if (a.isNodeSet()) {
-        const NodeSet& nodes = a.toNodeSet();
-        for (size_t i = 0; i < nodes.size(); ++i) {
-            String str = stringValue(nodes[i]);
-            idList.append(str);
-            idList.append(' ');
+        StringBuilder spaceSeparatedList;
+        for (auto& node : a.toNodeSet()) {
+            spaceSeparatedList.append(stringValue(node.get()));
+            spaceSeparatedList.append(' ');
         }
+        idList = spaceSeparatedList.toString();
     } else {
-        String str = a.toString();
-        idList.append(str);
+        idList = a.toString();
     }
     
     TreeScope& contextScope = evaluationContext().node->treeScope();
@@ -340,7 +339,7 @@ Value FunId::evaluate() const
 
         // If there are several nodes with the same id, id() should return the first one.
         // In WebKit, getElementById behaves so, too, although its behavior in this case is formally undefined.
-        Node* node = contextScope.getElementById(String(idList.characters() + startPos, endPos - startPos));
+        Node* node = contextScope.getElementById(idList.substring(startPos, endPos - startPos));
         if (node && resultSet.add(node).isNewEntry)
             result.append(node);
         
index 459fcf3..92b2f27 100644 (file)
@@ -44,6 +44,9 @@ namespace WebCore {
             void reserveCapacity(size_t newCapacity) { m_nodes.reserveCapacity(newCapacity); }
             void clear() { m_nodes.clear(); }
 
+            Vector<RefPtr<Node>>::const_iterator begin() const { return m_nodes.begin(); }
+            Vector<RefPtr<Node>>::const_iterator end() const { return m_nodes.end(); }
+
             // NodeSet itself does not verify that nodes in it are unique.
             void append(PassRefPtr<Node> node) { m_nodes.append(node); }
             void append(const NodeSet& nodeSet) { m_nodes.appendVector(nodeSet.m_nodes); }