Diff
Modified: trunk/Source/WTF/ChangeLog (243117 => 243118)
--- trunk/Source/WTF/ChangeLog 2019-03-19 00:10:30 UTC (rev 243117)
+++ trunk/Source/WTF/ChangeLog 2019-03-19 00:27:10 UTC (rev 243118)
@@ -1,3 +1,15 @@
+2019-03-18 Darin Adler <[email protected]>
+
+ Cut down on use of StringBuffer, possibly leading toward removing it entirely
+ https://bugs.webkit.org/show_bug.cgi?id=195870
+
+ Reviewed by Daniel Bates.
+
+ * wtf/URL.cpp: Remove a now-inaccurate comment mentioning StringBuffer.
+
+ * wtf/text/StringView.cpp:
+ (WTF::convertASCIICase): Use createUninitialized instead of StringBuffer.
+
2019-03-18 Xan Lopez <[email protected]>
[WTF] Remove redundant std::move in StringConcatenate
Modified: trunk/Source/WTF/wtf/URL.cpp (243117 => 243118)
--- trunk/Source/WTF/wtf/URL.cpp 2019-03-19 00:10:30 UTC (rev 243117)
+++ trunk/Source/WTF/wtf/URL.cpp 2019-03-19 00:27:10 UTC (rev 243118)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2004, 2007-2008, 2011-2013, 2015-2016 Apple Inc. All rights reserved.
+ * Copyright (C) 2004-2019 Apple Inc. All rights reserved.
* Copyright (C) 2012 Research In Motion Limited. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
@@ -40,11 +40,6 @@
#include <wtf/text/StringHash.h>
#include <wtf/text/TextStream.h>
-// FIXME: This file makes too much use of the + operator on String.
-// We either have to optimize that operator so it doesn't involve
-// so many allocations, or change this to use StringBuffer instead.
-
-
namespace WTF {
typedef Vector<char, 512> CharBuffer;
Modified: trunk/Source/WTF/wtf/text/StringView.cpp (243117 => 243118)
--- trunk/Source/WTF/wtf/text/StringView.cpp 2019-03-19 00:10:30 UTC (rev 243117)
+++ trunk/Source/WTF/wtf/text/StringView.cpp 2019-03-19 00:27:10 UTC (rev 243118)
@@ -34,7 +34,6 @@
#include <wtf/Lock.h>
#include <wtf/NeverDestroyed.h>
#include <wtf/Optional.h>
-#include <wtf/text/StringBuffer.h>
#include <wtf/text/TextBreakIterator.h>
#include <wtf/unicode/UTF8Conversion.h>
@@ -220,11 +219,11 @@
if (!input)
return { };
- StringBuffer<CharacterType> buffer(length);
- CharacterType* characters = buffer.characters();
+ CharacterType* characters;
+ auto result = String::createUninitialized(length, characters);
for (unsigned i = 0; i < length; ++i)
characters[i] = type == ASCIICase::Lower ? toASCIILower(input[i]) : toASCIIUpper(input[i]);
- return String::adopt(WTFMove(buffer));
+ return result;
}
String StringView::convertToASCIILowercase() const
Modified: trunk/Source/WebCore/ChangeLog (243117 => 243118)
--- trunk/Source/WebCore/ChangeLog 2019-03-19 00:10:30 UTC (rev 243117)
+++ trunk/Source/WebCore/ChangeLog 2019-03-19 00:27:10 UTC (rev 243118)
@@ -1,3 +1,37 @@
+2019-03-18 Darin Adler <[email protected]>
+
+ Cut down on use of StringBuffer, possibly leading toward removing it entirely
+ https://bugs.webkit.org/show_bug.cgi?id=195870
+
+ Reviewed by Daniel Bates.
+
+ * dom/Document.cpp:
+ (WebCore::canonicalizedTitle): Fixed all the problems mentioned in "FIXME".
+ Made this a single function rather than a function template. Switch to
+ StringBuilder instead of StringBuffer. Return the original string if the
+ canonicalize operation doesn't change anything.
+ (WebCore::Document::updateTitle): Updated for the change above.
+
+ * platform/Length.cpp:
+ (WebCore::newCoordsArray): Use createUninitialized instead of StringBuffer.
+ Also got rid of unneeded use of upconvertedCharacters on a temporary string
+ that we explicitly created with 16-bit characters. The performance of this
+ function could be considerably simplified by not copying the original string
+ at all, but didn't do that at this time.
+
+ * platform/text/TextCodecUTF16.cpp:
+ (WebCore::TextCodecUTF16::decode): Use createUninitialized instead of
+ StringBuffer. Also renamed numChars to numCodeUnits to both switch to complete
+ words and to be slightly more accurate.
+
+ * rendering/RenderText.cpp:
+ (WebCore::convertNoBreakSpace): Added.
+ (WebCore::capitalize): Use Vector instead of StringBuffer. Simplify code by
+ using convertNoBreakSpace function. Removed code that was using StringImpl
+ directly for a tiny speed boost; if we want to optimize the performance of
+ this function we would need to do more than that. Return the original string
+ if it happens to already be capitalized.
+
2019-03-18 Timothy Hatcher <[email protected]>
WKWebView.GetContentsShouldReturnAttributedString is crashing on iOS Simulator.
Modified: trunk/Source/WebCore/dom/Document.cpp (243117 => 243118)
--- trunk/Source/WebCore/dom/Document.cpp 2019-03-19 00:10:30 UTC (rev 243117)
+++ trunk/Source/WebCore/dom/Document.cpp 2019-03-19 00:27:10 UTC (rev 243118)
@@ -3,7 +3,7 @@
* (C) 1999 Antti Koivisto ([email protected])
* (C) 2001 Dirk Mueller ([email protected])
* (C) 2006 Alexey Proskuryakov ([email protected])
- * Copyright (C) 2004-2018 Apple Inc. All rights reserved.
+ * Copyright (C) 2004-2019 Apple Inc. All rights reserved.
* Copyright (C) 2008, 2009 Torch Mobile Inc. All rights reserved. (http://www.torchmobile.com/)
* Copyright (C) 2008, 2009, 2011, 2012 Google Inc. All rights reserved.
* Copyright (C) 2010 Nokia Corporation and/or its subsidiary(-ies)
@@ -1503,44 +1503,32 @@
return body();
}
-template<typename CharacterType> static inline String canonicalizedTitle(Document& document, const String& title)
+static String canonicalizedTitle(Document& document, const String& title)
{
- // FIXME: Compiling a separate copy of this for LChar and UChar is likely unnecessary.
- // FIXME: Missing an optimized case for when title is fine as-is. This unnecessarily allocates
- // and keeps around a new copy, and it's even the less optimal type of StringImpl with a separate buffer.
- // Could probably just use StringBuilder instead.
+ // Collapse runs of HTML spaces into single space characters.
+ // Strip leading and trailing spaces.
+ // Replace backslashes with currency symbols.
- auto* characters = title.characters<CharacterType>();
- unsigned length = title.length();
+ StringBuilder builder;
- StringBuffer<CharacterType> buffer { length };
- unsigned bufferLength = 0;
-
auto* decoder = document.decoder();
auto backslashAsCurrencySymbol = decoder ? decoder->encoding().backslashAsCurrencySymbol() : '\\';
- // Collapse runs of HTML spaces into single space characters.
- // Strip leading and trailing spaces.
- // Replace backslashes with currency symbols.
bool previousCharacterWasHTMLSpace = false;
- for (unsigned i = 0; i < length; ++i) {
- auto character = characters[i];
+ for (auto character : StringView { title }.codeUnits()) {
if (isHTMLSpace(character))
previousCharacterWasHTMLSpace = true;
else {
if (character == '\\')
character = backslashAsCurrencySymbol;
- if (previousCharacterWasHTMLSpace && bufferLength)
- buffer[bufferLength++] = ' ';
- buffer[bufferLength++] = character;
+ if (previousCharacterWasHTMLSpace && !builder.isEmpty())
+ builder.append(' ');
+ builder.append(character);
previousCharacterWasHTMLSpace = false;
}
}
- if (!bufferLength)
- return { };
- buffer.shrink(bufferLength);
- return String::adopt(WTFMove(buffer));
+ return builder == title ? title : builder.toString();
}
void Document::updateTitle(const StringWithDirection& title)
@@ -1549,14 +1537,9 @@
return;
m_rawTitle = title;
- m_title = title;
- if (!m_title.string.isEmpty()) {
- if (m_title.string.is8Bit())
- m_title.string = canonicalizedTitle<LChar>(*this, m_title.string);
- else
- m_title.string = canonicalizedTitle<UChar>(*this, m_title.string);
- }
+ m_title.string = canonicalizedTitle(*this, title.string);
+ m_title.direction = title.direction;
if (auto* loader = this->loader())
loader->setTitle(m_title);
Modified: trunk/Source/WebCore/platform/Length.cpp (243117 => 243118)
--- trunk/Source/WebCore/platform/Length.cpp 2019-03-19 00:10:30 UTC (rev 243117)
+++ trunk/Source/WebCore/platform/Length.cpp 2019-03-19 00:27:10 UTC (rev 243118)
@@ -2,7 +2,7 @@
* Copyright (C) 1999 Lars Knoll ([email protected])
* (C) 1999 Antti Koivisto ([email protected])
* (C) 2001 Dirk Mueller ( [email protected] )
- * Copyright (C) 2003, 2004, 2005, 2006, 2007, 2008, 2014 Apple Inc. All rights reserved.
+ * Copyright (C) 2003-2019 Apple Inc. All rights reserved.
* Copyright (C) 2006 Andrew Wellington ([email protected])
*
* This library is free software; you can redistribute it and/or
@@ -31,11 +31,9 @@
#include <wtf/MallocPtr.h>
#include <wtf/NeverDestroyed.h>
#include <wtf/StdLibExtras.h>
-#include <wtf/text/StringBuffer.h>
#include <wtf/text/StringView.h>
#include <wtf/text/TextStream.h>
-
namespace WebCore {
static Length parseLength(const UChar* data, unsigned length)
@@ -91,7 +89,8 @@
UniqueArray<Length> newCoordsArray(const String& string, int& len)
{
unsigned length = string.length();
- StringBuffer<UChar> spacified(length);
+ UChar* spacified;
+ auto str = StringImpl::createUninitialized(length, spacified);
for (unsigned i = 0; i < length; i++) {
UChar cc = string[i];
if (cc > '9' || (cc < '0' && cc != '-' && cc != '*' && cc != '.'))
@@ -99,11 +98,10 @@
else
spacified[i] = cc;
}
- RefPtr<StringImpl> str = StringImpl::adopt(WTFMove(spacified));
str = str->simplifyWhiteSpace();
- len = countCharacter(*str, ' ') + 1;
+ len = countCharacter(str, ' ') + 1;
auto r = makeUniqueArray<Length>(len);
int i = 0;
@@ -110,12 +108,11 @@
unsigned pos = 0;
size_t pos2;
- auto upconvertedCharacters = StringView(str.get()).upconvertedCharacters();
while ((pos2 = str->find(' ', pos)) != notFound) {
- r[i++] = parseLength(upconvertedCharacters + pos, pos2 - pos);
+ r[i++] = parseLength(str->characters16() + pos, pos2 - pos);
pos = pos2+1;
}
- r[i] = parseLength(upconvertedCharacters + pos, str->length() - pos);
+ r[i] = parseLength(str->characters16() + pos, str->length() - pos);
ASSERT(i == len - 1);
Modified: trunk/Source/WebCore/platform/text/TextCodecUTF16.cpp (243117 => 243118)
--- trunk/Source/WebCore/platform/text/TextCodecUTF16.cpp 2019-03-19 00:10:30 UTC (rev 243117)
+++ trunk/Source/WebCore/platform/text/TextCodecUTF16.cpp 2019-03-19 00:27:10 UTC (rev 243118)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2004-2017 Apple Inc. All rights reserved.
+ * Copyright (C) 2004-2019 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -27,7 +27,6 @@
#include "TextCodecUTF16.h"
#include <wtf/text/CString.h>
-#include <wtf/text/StringBuffer.h>
#include <wtf/text/WTFString.h>
namespace WebCore {
@@ -71,10 +70,11 @@
const unsigned char* p = reinterpret_cast<const unsigned char*>(bytes);
size_t numBytes = length + m_haveBufferedByte;
- size_t numChars = numBytes / 2;
+ size_t numCodeUnits = numBytes / 2;
+ RELEASE_ASSERT(numCodeUnits <= std::numeric_limits<unsigned>::max());
- StringBuffer<UChar> buffer(numChars);
- UChar* q = buffer.characters();
+ UChar* q;
+ auto result = String::createUninitialized(numCodeUnits, q);
if (m_haveBufferedByte) {
UChar c;
@@ -85,17 +85,17 @@
*q++ = c;
m_haveBufferedByte = false;
p += 1;
- numChars -= 1;
+ numCodeUnits -= 1;
}
if (m_littleEndian) {
- for (size_t i = 0; i < numChars; ++i) {
+ for (size_t i = 0; i < numCodeUnits; ++i) {
UChar c = p[0] | (p[1] << 8);
p += 2;
*q++ = c;
}
} else {
- for (size_t i = 0; i < numChars; ++i) {
+ for (size_t i = 0; i < numCodeUnits; ++i) {
UChar c = (p[0] << 8) | p[1];
p += 2;
*q++ = c;
@@ -108,9 +108,7 @@
m_bufferedByte = p[0];
}
- buffer.shrink(q - buffer.characters());
-
- return String::adopt(WTFMove(buffer));
+ return result;
}
Vector<uint8_t> TextCodecUTF16::encode(StringView string, UnencodableHandling)
Modified: trunk/Source/WebCore/rendering/RenderText.cpp (243117 => 243118)
--- trunk/Source/WebCore/rendering/RenderText.cpp 2019-03-19 00:10:30 UTC (rev 243117)
+++ trunk/Source/WebCore/rendering/RenderText.cpp 2019-03-19 00:27:10 UTC (rev 243118)
@@ -1,7 +1,7 @@
/*
* (C) 1999 Lars Knoll ([email protected])
* (C) 2000 Dirk Mueller ([email protected])
- * Copyright (C) 2004-2007, 2013-2015 Apple Inc. All rights reserved.
+ * Copyright (C) 2004-2019 Apple Inc. All rights reserved.
* Copyright (C) 2006 Andrew Wellington ([email protected])
* Copyright (C) 2006 Graham Dennis ([email protected])
*
@@ -51,7 +51,6 @@
#include "VisiblePosition.h"
#include <wtf/IsoMallocInlines.h>
#include <wtf/NeverDestroyed.h>
-#include <wtf/text/StringBuffer.h>
#include <wtf/text/StringBuilder.h>
#include <wtf/text/TextBreakIterator.h>
#include <wtf/unicode/CharacterNames.h>
@@ -141,30 +140,25 @@
return map;
}
+static inline UChar convertNoBreakSpace(UChar character)
+{
+ return character == noBreakSpace ? ' ' : character;
+}
+
String capitalize(const String& string, UChar previousCharacter)
{
- // FIXME: Need to change this to use u_strToTitle instead of u_totitle and to consider locale.
+ // FIXME: Change this to use u_strToTitle instead of u_totitle and to consider locale.
- if (string.isNull())
- return string;
-
unsigned length = string.length();
- auto& stringImpl = *string.impl();
- if (length >= std::numeric_limits<unsigned>::max())
- CRASH();
+ // Prepend the previous character, and convert NO BREAK SPACE to SPACE so ICU will see a word separator.
+ Vector<UChar> wordBreakCharacters;
+ wordBreakCharacters.grow(length + 1);
+ wordBreakCharacters[0] = convertNoBreakSpace(previousCharacter);
+ for (unsigned i = 1; i < length + 1; i++)
+ wordBreakCharacters[i] = convertNoBreakSpace(string[i - 1]);
- StringBuffer<UChar> stringWithPrevious(length + 1);
- stringWithPrevious[0] = previousCharacter == noBreakSpace ? ' ' : previousCharacter;
- for (unsigned i = 1; i < length + 1; i++) {
- // Replace NO BREAK SPACE with a real space since ICU does not treat it as a word separator.
- if (stringImpl[i - 1] == noBreakSpace)
- stringWithPrevious[i] = ' ';
- else
- stringWithPrevious[i] = stringImpl[i - 1];
- }
-
- auto* boundary = wordBreakIterator(StringView(stringWithPrevious.characters(), length + 1));
+ auto* boundary = wordBreakIterator(StringView { wordBreakCharacters.data(), length + 1 });
if (!boundary)
return string;
@@ -175,12 +169,12 @@
int32_t startOfWord = ubrk_first(boundary);
for (endOfWord = ubrk_next(boundary); endOfWord != UBRK_DONE; startOfWord = endOfWord, endOfWord = ubrk_next(boundary)) {
if (startOfWord) // Ignore first char of previous string
- result.append(stringImpl[startOfWord - 1] == noBreakSpace ? noBreakSpace : u_totitle(stringWithPrevious[startOfWord]));
+ result.append(string[startOfWord - 1] == noBreakSpace ? noBreakSpace : u_totitle(wordBreakCharacters[startOfWord]));
for (int i = startOfWord + 1; i < endOfWord; i++)
- result.append(stringImpl[i - 1]);
+ result.append(string[i - 1]);
}
- return result.toString();
+ return result == string ? string : result.toString();
}
inline RenderText::RenderText(Node& node, const String& text)
