- Revision
- 127112
- Author
- [email protected]
- Date
- 2012-08-30 01:20:03 -0700 (Thu, 30 Aug 2012)
Log Message
Replace uses of WTF::String::operator+= with StringBuilder
https://bugs.webkit.org/show_bug.cgi?id=95416
Reviewed by Benjamin Poulain.
WTF::String::operator+= appears to be a sandtrap for contributors. The
vast majority of the callers are using very inefficient string
patterns. This patch removes the use of operator+= in favor of
StringBuilder. Eventually, I'd like to remove operator+= so that more
code doesn't fall into this trap.
* Modules/websockets/WebSocketHandshake.cpp:
(WebCore::resourceName):
* html/HTMLAnchorElement.cpp:
(WebCore::appendServerMapMousePosition):
(WebCore::HTMLAnchorElement::handleClick):
* html/canvas/CanvasRenderingContext2D.cpp:
(WebCore::CanvasRenderingContext2D::font):
* platform/network/ResourceRequestBase.cpp:
(WebCore::ResourceRequestBase::addHTTPHeaderField):
* rendering/RenderLayerBacking.cpp:
(WebCore::RenderLayerBacking::nameForLayer):
* rendering/RenderTreeAsText.cpp:
(WebCore::RenderTreeAsText::writeRenderObject):
(WebCore::nodePosition):
* rendering/style/RenderStyle.cpp:
(WebCore::RenderStyle::setContent):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (127111 => 127112)
--- trunk/Source/WebCore/ChangeLog 2012-08-30 08:06:28 UTC (rev 127111)
+++ trunk/Source/WebCore/ChangeLog 2012-08-30 08:20:03 UTC (rev 127112)
@@ -1,3 +1,33 @@
+2012-08-30 Adam Barth <[email protected]>
+
+ Replace uses of WTF::String::operator+= with StringBuilder
+ https://bugs.webkit.org/show_bug.cgi?id=95416
+
+ Reviewed by Benjamin Poulain.
+
+ WTF::String::operator+= appears to be a sandtrap for contributors. The
+ vast majority of the callers are using very inefficient string
+ patterns. This patch removes the use of operator+= in favor of
+ StringBuilder. Eventually, I'd like to remove operator+= so that more
+ code doesn't fall into this trap.
+
+ * Modules/websockets/WebSocketHandshake.cpp:
+ (WebCore::resourceName):
+ * html/HTMLAnchorElement.cpp:
+ (WebCore::appendServerMapMousePosition):
+ (WebCore::HTMLAnchorElement::handleClick):
+ * html/canvas/CanvasRenderingContext2D.cpp:
+ (WebCore::CanvasRenderingContext2D::font):
+ * platform/network/ResourceRequestBase.cpp:
+ (WebCore::ResourceRequestBase::addHTTPHeaderField):
+ * rendering/RenderLayerBacking.cpp:
+ (WebCore::RenderLayerBacking::nameForLayer):
+ * rendering/RenderTreeAsText.cpp:
+ (WebCore::RenderTreeAsText::writeRenderObject):
+ (WebCore::nodePosition):
+ * rendering/style/RenderStyle.cpp:
+ (WebCore::RenderStyle::setContent):
+
2012-08-30 Shinya Kawanaka <[email protected]>
AuthorShadowDOM support for textarea element.
Modified: trunk/Source/WebCore/Modules/websockets/WebSocketHandshake.cpp (127111 => 127112)
--- trunk/Source/WebCore/Modules/websockets/WebSocketHandshake.cpp 2012-08-30 08:06:28 UTC (rev 127111)
+++ trunk/Source/WebCore/Modules/websockets/WebSocketHandshake.cpp 2012-08-30 08:20:03 UTC (rev 127112)
@@ -64,14 +64,18 @@
static String resourceName(const KURL& url)
{
- String name = url.path();
+ StringBuilder name;
+ name.append(url.path());
if (name.isEmpty())
- name = "/";
- if (!url.query().isNull())
- name += "?" + url.query();
- ASSERT(!name.isEmpty());
- ASSERT(!name.contains(' '));
- return name;
+ name.append('/');
+ if (!url.query().isNull()) {
+ name.append('?');
+ name.append(url.query());
+ }
+ String result = name.toString();
+ ASSERT(!result.isEmpty());
+ ASSERT(!result.contains(' '));
+ return result;
}
static String hostName(const KURL& url, bool secure)
Modified: trunk/Source/WebCore/html/HTMLAnchorElement.cpp (127111 => 127112)
--- trunk/Source/WebCore/html/HTMLAnchorElement.cpp 2012-08-30 08:06:28 UTC (rev 127111)
+++ trunk/Source/WebCore/html/HTMLAnchorElement.cpp 2012-08-30 08:20:03 UTC (rev 127112)
@@ -41,6 +41,7 @@
#include "SecurityOrigin.h"
#include "SecurityPolicy.h"
#include "Settings.h"
+#include <wtf/text/StringBuilder.h>
namespace WebCore {
@@ -120,7 +121,7 @@
return hasNonEmptyBoundingBox();
}
-static void appendServerMapMousePosition(String& url, Event* event)
+static void appendServerMapMousePosition(StringBuilder& url, Event* event)
{
if (!event->isMouseEvent())
return;
@@ -143,10 +144,10 @@
FloatPoint absolutePosition = renderer->absoluteToLocal(FloatPoint(static_cast<MouseEvent*>(event)->pageX(), static_cast<MouseEvent*>(event)->pageY()));
int x = absolutePosition.x();
int y = absolutePosition.y();
- url += "?";
- url += String::number(x);
- url += ",";
- url += String::number(y);
+ url.append('?');
+ url.append(String::number(x));
+ url.append(',');
+ url.append(String::number(y));
}
void HTMLAnchorElement::defaultEventHandler(Event* event)
@@ -507,9 +508,10 @@
if (!frame)
return;
- String url = ""
+ StringBuilder url;
+ url.append(stripLeadingAndTrailingHTMLSpaces(fastGetAttribute(hrefAttr)));
appendServerMapMousePosition(url, event);
- KURL kurl = document()->completeURL(url);
+ KURL kurl = document()->completeURL(url.toString());
#if ENABLE(DOWNLOAD_ATTRIBUTE)
if (hasAttribute(downloadAttr)) {
Modified: trunk/Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp (127111 => 127112)
--- trunk/Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp 2012-08-30 08:06:28 UTC (rev 127111)
+++ trunk/Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp 2012-08-30 08:20:03 UTC (rev 127112)
@@ -72,6 +72,7 @@
#include <wtf/OwnPtr.h>
#include <wtf/Uint8ClampedArray.h>
#include <wtf/UnusedParam.h>
+#include <wtf/text/StringBuilder.h>
#if USE(CG)
#include <ApplicationServices/ApplicationServices.h>
@@ -1992,31 +1993,34 @@
if (!state().m_realizedFont)
return defaultFont;
- String serializedFont;
+ StringBuilder serializedFont;
const FontDescription& fontDescription = state().m_font.fontDescription();
if (fontDescription.italic())
- serializedFont += "italic ";
+ serializedFont.appendLiteral("italic ");
if (fontDescription.smallCaps() == FontSmallCapsOn)
- serializedFont += "small-caps ";
+ serializedFont.appendLiteral("small-caps ");
- serializedFont += String::number(fontDescription.computedPixelSize()) + "px";
+ serializedFont.append(String::number(fontDescription.computedPixelSize()));
+ serializedFont.appendLiteral("px");
const FontFamily& firstFontFamily = fontDescription.family();
for (const FontFamily* fontFamily = &firstFontFamily; fontFamily; fontFamily = fontFamily->next()) {
if (fontFamily != &firstFontFamily)
- serializedFont += ",";
+ serializedFont.append(',');
+ // FIXME: We should append family directly to serializedFont rather than building a temporary string.
String family = fontFamily->family();
if (family.startsWith("-webkit-"))
family = family.substring(8);
if (family.contains(' '))
family = makeString('"', family, '"');
- serializedFont += " " + family;
+ serializedFont.append(' ');
+ serializedFont.append(family);
}
- return serializedFont;
+ return serializedFont.toString();
}
void CanvasRenderingContext2D::setFont(const String& newFont)
Modified: trunk/Source/WebCore/platform/network/ResourceRequestBase.cpp (127111 => 127112)
--- trunk/Source/WebCore/platform/network/ResourceRequestBase.cpp 2012-08-30 08:06:28 UTC (rev 127111)
+++ trunk/Source/WebCore/platform/network/ResourceRequestBase.cpp 2012-08-30 08:20:03 UTC (rev 127112)
@@ -374,7 +374,7 @@
updateResourceRequest();
HTTPHeaderMap::AddResult result = m_httpHeaderFields.add(name, value);
if (!result.isNewEntry)
- result.iterator->second += "," + value;
+ result.iterator->second = result.iterator->second + ',' + value;
if (url().protocolIsInHTTPFamily())
m_platformRequestUpdated = false;
Modified: trunk/Source/WebCore/rendering/RenderLayerBacking.cpp (127111 => 127112)
--- trunk/Source/WebCore/rendering/RenderLayerBacking.cpp 2012-08-30 08:06:28 UTC (rev 127111)
+++ trunk/Source/WebCore/rendering/RenderLayerBacking.cpp 2012-08-30 08:20:03 UTC (rev 127112)
@@ -55,8 +55,8 @@
#include "Settings.h"
#include "StyleResolver.h"
#include "TiledBacking.h"
-
#include <wtf/CurrentTime.h>
+#include <wtf/text/StringBuilder.h>
#if ENABLE(CSS_FILTERS)
#include "FilterEffectRenderer.h"
@@ -1710,29 +1710,35 @@
String RenderLayerBacking::nameForLayer() const
{
- String name = renderer()->renderName();
+ StringBuilder name;
+ name.append(renderer()->renderName());
if (Node* node = renderer()->node()) {
- if (node->isElementNode())
- name += " " + static_cast<Element*>(node)->tagName();
- if (node->hasID())
- name += " id=\'" + static_cast<Element*>(node)->getIdAttribute() + "\'";
+ if (node->isElementNode()) {
+ name.append(' ');
+ name.append(static_cast<Element*>(node)->tagName());
+ }
+ if (node->hasID()) {
+ name.appendLiteral(" id=\'");
+ name.append(static_cast<Element*>(node)->getIdAttribute());
+ name.append('\'');
+ }
if (node->hasClass()) {
+ name.appendLiteral(" class=\'");
StyledElement* styledElement = static_cast<StyledElement*>(node);
- String classes;
for (size_t i = 0; i < styledElement->classNames().size(); ++i) {
if (i > 0)
- classes += " ";
- classes += styledElement->classNames()[i];
+ name.append(' ');
+ name.append(styledElement->classNames()[i]);
}
- name += " class=\'" + classes + "\'";
+ name.append('\'');
}
}
if (m_owningLayer->isReflection())
- name += " (reflection)";
+ name.appendLiteral(" (reflection)");
- return name;
+ return name.toString();
}
CompositingLayerType RenderLayerBacking::compositingLayerType() const
Modified: trunk/Source/WebCore/rendering/RenderTreeAsText.cpp (127111 => 127112)
--- trunk/Source/WebCore/rendering/RenderTreeAsText.cpp 2012-08-30 08:06:28 UTC (rev 127111)
+++ trunk/Source/WebCore/rendering/RenderTreeAsText.cpp 2012-08-30 08:20:03 UTC (rev 127112)
@@ -445,14 +445,14 @@
ts << " id=\"" + static_cast<Element*>(node)->getIdAttribute() + "\"";
if (node->hasClass()) {
+ ts << " class=\"";
StyledElement* styledElement = static_cast<StyledElement*>(node);
- String classes;
for (size_t i = 0; i < styledElement->classNames().size(); ++i) {
if (i > 0)
- classes += " ";
- classes += styledElement->classNames()[i];
+ ts << " ";
+ ts << styledElement->classNames()[i];
}
- ts << " class=\"" + classes + "\"";
+ ts << "\"";
}
}
}
@@ -789,29 +789,36 @@
static String nodePosition(Node* node)
{
- String result;
+ StringBuilder result;
Element* body = node->document()->body();
Node* parent;
for (Node* n = node; n; n = parent) {
parent = n->parentOrHostNode();
if (n != node)
- result += " of ";
+ result.appendLiteral(" of ");
if (parent) {
if (body && n == body) {
// We don't care what offset body may be in the document.
- result += "body";
+ result.appendLiteral("body");
break;
}
- if (n->isShadowRoot())
- result += "{" + getTagName(n) + "}";
- else
- result += "child " + String::number(n->nodeIndex()) + " {" + getTagName(n) + "}";
+ if (n->isShadowRoot()) {
+ result.append('{');
+ result.append(getTagName(n));
+ result.append('}');
+ } else {
+ result.appendLiteral("child ");
+ result.append(String::number(n->nodeIndex()));
+ result.appendLiteral(" {");
+ result.append(getTagName(n));
+ result.append('}');
+ }
} else
- result += "document";
+ result.appendLiteral("document");
}
- return result;
+ return result.toString();
}
static void writeSelection(TextStream& ts, const RenderObject* o)
Modified: trunk/Source/WebCore/rendering/style/RenderStyle.cpp (127111 => 127112)
--- trunk/Source/WebCore/rendering/style/RenderStyle.cpp 2012-08-30 08:06:28 UTC (rev 127111)
+++ trunk/Source/WebCore/rendering/style/RenderStyle.cpp 2012-08-30 08:20:03 UTC (rev 127112)
@@ -778,9 +778,7 @@
// We attempt to merge with the last ContentData if possible.
if (lastContent->isText()) {
TextContentData* textContent = static_cast<TextContentData*>(lastContent);
- String text = textContent->text();
- text += string;
- textContent->setText(text);
+ textContent->setText(textContent->text() + string);
} else
lastContent->setNext(ContentData::create(string));
