Use makeString and multi-argument StringBuilder::append instead of less efficient...
authordarin@apple.com <darin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 22 Aug 2019 16:26:04 +0000 (16:26 +0000)
committerdarin@apple.com <darin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 22 Aug 2019 16:26:04 +0000 (16:26 +0000)
https://bugs.webkit.org/show_bug.cgi?id=200862

Reviewed by Ryosuke Niwa.

Source/JavaScriptCore:

* runtime/ExceptionHelpers.cpp:
(JSC::createUndefinedVariableError): Got rid of unnecessary local variable.
(JSC::notAFunctionSourceAppender): Use single append instead of multiple.
Eliminate unneeded and unconventional use of makeString on a single string literal.
(JSC::invalidParameterInstanceofNotFunctionSourceAppender): Ditto.
(JSC::invalidParameterInstanceofhasInstanceValueNotFunctionSourceAppender): Ditto.
(JSC::createInvalidFunctionApplyParameterError): Ditto.
(JSC::createInvalidInParameterError): Ditto.
(JSC::createInvalidInstanceofParameterErrorNotFunction): Ditto.
(JSC::createInvalidInstanceofParameterErrorHasInstanceValueNotFunction): Ditto.

* runtime/FunctionConstructor.cpp:
(JSC::constructFunctionSkippingEvalEnabledCheck): Use single append instead of multiple.
* runtime/Options.cpp:
(JSC::Options::dumpOption): Ditto.
* runtime/TypeProfiler.cpp:
(JSC::TypeProfiler::typeInformationForExpressionAtOffset): Ditto.
* runtime/TypeSet.cpp:
(JSC::StructureShape::stringRepresentation): Ditto. Also use a modern for loop.

Source/WebCore:

* Modules/indexeddb/shared/IDBDatabaseInfo.cpp:
(WebCore::IDBDatabaseInfo::loggingString const): Use one append instead of multiple.
* Modules/indexeddb/shared/IDBObjectStoreInfo.cpp:
(WebCore::IDBObjectStoreInfo::loggingString const): Ditto.
* Modules/mediastream/libwebrtc/LibWebRTCUtils.cpp:
(WebCore::toRTCCodecParameters): Ditto.
* Modules/plugins/YouTubePluginReplacement.cpp:
(WebCore::YouTubePluginReplacement::youTubeURLFromAbsoluteURL): Ditto.
* Modules/webdatabase/DatabaseTracker.cpp:
(WebCore::generateDatabaseFileName): Ditto.
* Modules/websockets/WebSocketExtensionDispatcher.cpp:
(WebCore::WebSocketExtensionDispatcher::createHeaderValue const): Ditto.
(WebCore::WebSocketExtensionDispatcher::appendAcceptedExtension): Ditto.

* Modules/websockets/WebSocketHandshake.cpp:
(WebCore::WebSocketHandshake::clientLocation const): Use makeString instead of
StringBuilder.

* bindings/js/JSDOMExceptionHandling.cpp:
(WebCore::appendArgumentMustBe): Use one append instead of multiple.
(WebCore::throwArgumentMustBeEnumError): Ditto.
(WebCore::throwArgumentTypeError): Ditto.
* contentextensions/CombinedURLFilters.cpp:
(WebCore::ContentExtensions::recursivePrint): Ditto.
* css/CSSBasicShapes.cpp:
(WebCore::buildCircleString): Ditto.
(WebCore::buildEllipseString): Ditto.
(WebCore::buildPolygonString): Ditto.
(WebCore::buildInsetString): Ditto.

* css/CSSCalculationValue.cpp:
(WebCore::buildCssText): Deleted.
(WebCore::CSSCalcValue::customCSSText const): Use makeString.

* css/CSSComputedStyleDeclaration.cpp:
(WebCore::CSSComputedStyleDeclaration::cssText const): Use one append instead of multiple.

* css/CSSCrossfadeValue.cpp:
(WebCore::CSSCrossfadeValue::customCSSText const): Use makeString.
* css/CSSFilterImageValue.cpp:
(WebCore::CSSFilterImageValue::customCSSText const): Ditto.
* css/CSSFontFaceRule.cpp:
(WebCore::CSSFontFaceRule::cssText const): Ditto.
* css/CSSFontFaceSrcValue.cpp:
(WebCore::CSSFontFaceSrcValue::customCSSText const): Ditto.

* css/CSSGradientValue.cpp:
(WebCore::appendGradientStops): Moved code here from CSSLinearGradientValue::customCSSText
so it can be shared with CSSRadialGradientValue::customCSSText. Use one append per stop.
(WebCore::CSSLinearGradientValue::customCSSText const): Use one append instead of multiple.
(WebCore::CSSRadialGradientValue::customCSSText const): Ditto.
(WebCore::CSSConicGradientValue::customCSSText const): Ditto.
* css/CSSMediaRule.cpp:
(WebCore::CSSMediaRule::cssText const): Ditto.
* css/CSSNamespaceRule.cpp:
(WebCore::CSSNamespaceRule::cssText const): Ditto.

* css/CSSPageRule.cpp:
(WebCore::CSSPageRule::selectorText const): Use makeString.

* css/CSSPrimitiveValue.cpp:
(WebCore::CSSPrimitiveValue::formatNumberForCustomCSSText const):
Use one append instead of multiple.

* css/CSSPropertySourceData.cpp:
(WebCore::CSSPropertySourceData::CSSPropertySourceData): Initialize in the
structure definition instead of the constructor.
(WebCore::CSSPropertySourceData::toString const): Use makeString.
* css/CSSPropertySourceData.h: Initialize in the structure definition.

* css/CSSStyleRule.cpp:
(WebCore::CSSStyleRule::cssText const): Use makeString.

* css/parser/CSSParser.cpp:
(WebCore::CSSParser::parseFontFaceDescriptor): Use makeString.

* html/canvas/CanvasRenderingContext2D.cpp:
(WebCore::CanvasRenderingContext2D::font const): Use one append instead of multiple.

Source/WebKit:

* Shared/mac/AuxiliaryProcessMac.mm:
(WebKit::setAndSerializeSandboxParameters): Use one append instead of multiple.

Source/WTF:

* wtf/DateMath.cpp:
(WTF::makeRFC2822DateString): Use one append instead of multiple.
* wtf/JSONValues.cpp:
(WTF::appendDoubleQuotedString): Ditto.

Tools:

* WebKitTestRunner/InjectedBundle/TestRunner.cpp:
(WTR::TestRunner::statisticsDidRunTelemetryCallback): Use makeString.
* WebKitTestRunner/TestController.cpp:
(WTR::TestController::findAndDumpWebKitProcessIdentifiers): Ditto.
(WTR::TestController::downloadDidReceiveServerRedirectToURL): Ditto.
(WTR::TestController::downloadDidFail): Ditto.

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

41 files changed:
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/runtime/ExceptionHelpers.cpp
Source/JavaScriptCore/runtime/FunctionConstructor.cpp
Source/JavaScriptCore/runtime/Options.cpp
Source/JavaScriptCore/runtime/TypeProfiler.cpp
Source/JavaScriptCore/runtime/TypeSet.cpp
Source/WTF/ChangeLog
Source/WTF/wtf/DateMath.cpp
Source/WTF/wtf/JSONValues.cpp
Source/WebCore/ChangeLog
Source/WebCore/Modules/indexeddb/shared/IDBDatabaseInfo.cpp
Source/WebCore/Modules/indexeddb/shared/IDBObjectStoreInfo.cpp
Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCUtils.cpp
Source/WebCore/Modules/plugins/YouTubePluginReplacement.cpp
Source/WebCore/Modules/webdatabase/DatabaseTracker.cpp
Source/WebCore/Modules/websockets/WebSocketExtensionDispatcher.cpp
Source/WebCore/Modules/websockets/WebSocketHandshake.cpp
Source/WebCore/bindings/js/JSDOMExceptionHandling.cpp
Source/WebCore/contentextensions/CombinedURLFilters.cpp
Source/WebCore/css/CSSBasicShapes.cpp
Source/WebCore/css/CSSCalculationValue.cpp
Source/WebCore/css/CSSComputedStyleDeclaration.cpp
Source/WebCore/css/CSSCrossfadeValue.cpp
Source/WebCore/css/CSSFilterImageValue.cpp
Source/WebCore/css/CSSFontFaceRule.cpp
Source/WebCore/css/CSSFontFaceSrcValue.cpp
Source/WebCore/css/CSSGradientValue.cpp
Source/WebCore/css/CSSMediaRule.cpp
Source/WebCore/css/CSSNamespaceRule.cpp
Source/WebCore/css/CSSPageRule.cpp
Source/WebCore/css/CSSPrimitiveValue.cpp
Source/WebCore/css/CSSPropertySourceData.cpp
Source/WebCore/css/CSSPropertySourceData.h
Source/WebCore/css/CSSStyleRule.cpp
Source/WebCore/css/parser/CSSParser.cpp
Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp
Source/WebKit/ChangeLog
Source/WebKit/Shared/mac/AuxiliaryProcessMac.mm
Tools/ChangeLog
Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp
Tools/WebKitTestRunner/TestController.cpp

index 8a7bfe3..5adce80 100644 (file)
@@ -1,3 +1,30 @@
+2019-08-17  Darin Adler  <darin@apple.com>
+
+        Use makeString and multi-argument StringBuilder::append instead of less efficient multiple appends
+        https://bugs.webkit.org/show_bug.cgi?id=200862
+
+        Reviewed by Ryosuke Niwa.
+
+        * runtime/ExceptionHelpers.cpp:
+        (JSC::createUndefinedVariableError): Got rid of unnecessary local variable.
+        (JSC::notAFunctionSourceAppender): Use single append instead of multiple.
+        Eliminate unneeded and unconventional use of makeString on a single string literal.
+        (JSC::invalidParameterInstanceofNotFunctionSourceAppender): Ditto.
+        (JSC::invalidParameterInstanceofhasInstanceValueNotFunctionSourceAppender): Ditto.
+        (JSC::createInvalidFunctionApplyParameterError): Ditto.
+        (JSC::createInvalidInParameterError): Ditto.
+        (JSC::createInvalidInstanceofParameterErrorNotFunction): Ditto.
+        (JSC::createInvalidInstanceofParameterErrorHasInstanceValueNotFunction): Ditto.
+
+        * runtime/FunctionConstructor.cpp:
+        (JSC::constructFunctionSkippingEvalEnabledCheck): Use single append instead of multiple.
+        * runtime/Options.cpp:
+        (JSC::Options::dumpOption): Ditto.
+        * runtime/TypeProfiler.cpp:
+        (JSC::TypeProfiler::typeInformationForExpressionAtOffset): Ditto.
+        * runtime/TypeSet.cpp:
+        (JSC::StructureShape::stringRepresentation): Ditto. Also use a modern for loop.
+
 2019-08-21  Mark Lam  <mark.lam@apple.com>
 
         Wasm::FunctionParser is failing to enforce maxFunctionLocals.
index 9e75892..dc392f0 100644 (file)
@@ -81,12 +81,9 @@ JSObject* createStackOverflowError(ExecState* exec, JSGlobalObject* globalObject
 
 JSObject* createUndefinedVariableError(ExecState* exec, const Identifier& ident)
 {
-    if (ident.isPrivateName()) {
-        String message(makeString("Can't find private variable: PrivateSymbol.", ident.string()));
-        return createReferenceError(exec, message);
-    }
-    String message(makeString("Can't find variable: ", ident.string()));
-    return createReferenceError(exec, message);
+    if (ident.isPrivateName())
+        return createReferenceError(exec, makeString("Can't find private variable: PrivateSymbol.", ident.string()));
+    return createReferenceError(exec, makeString("Can't find variable: ", ident.string()));
 }
     
 String errorDescriptionForValue(ExecState* exec, JSValue v)
@@ -203,12 +200,7 @@ static String notAFunctionSourceAppender(const String& originalMessage, const St
     if (!base)
         return defaultApproximateSourceError(originalMessage, sourceText);
     StringBuilder builder(StringBuilder::OverflowHandler::RecordOverflow);
-    builder.append(base);
-    builder.appendLiteral(" is not a function. (In '");
-    builder.append(sourceText);
-    builder.appendLiteral("', '");
-    builder.append(base);
-    builder.appendLiteral("' is ");
+    builder.append(base, " is not a function. (In '", sourceText, "', '", base, "' is ");
     if (type == TypeSymbol)
         builder.appendLiteral("a Symbol");
     else {
@@ -219,7 +211,7 @@ static String notAFunctionSourceAppender(const String& originalMessage, const St
     builder.append(')');
 
     if (builder.hasOverflowed())
-        return makeString("object is not a function."_s);
+        return "object is not a function."_s;
 
     return builder.toString();
 }
@@ -265,12 +257,12 @@ inline String invalidParameterInstanceofSourceAppender(const String& content, co
 
 static String invalidParameterInstanceofNotFunctionSourceAppender(const String& originalMessage, const String& sourceText, RuntimeType runtimeType, ErrorInstance::SourceTextWhereErrorOccurred occurrence)
 {
-    return invalidParameterInstanceofSourceAppender(WTF::makeString(" is not a function"), originalMessage, sourceText, runtimeType, occurrence);
+    return invalidParameterInstanceofSourceAppender(" is not a function"_s, originalMessage, sourceText, runtimeType, occurrence);
 }
 
 static String invalidParameterInstanceofhasInstanceValueNotFunctionSourceAppender(const String& originalMessage, const String& sourceText, RuntimeType runtimeType, ErrorInstance::SourceTextWhereErrorOccurred occurrence)
 {
-    return invalidParameterInstanceofSourceAppender(WTF::makeString("[Symbol.hasInstance] is not a function, undefined, or null"), originalMessage, sourceText, runtimeType, occurrence);
+    return invalidParameterInstanceofSourceAppender("[Symbol.hasInstance] is not a function, undefined, or null"_s, originalMessage, sourceText, runtimeType, occurrence);
 }
 
 JSObject* createError(ExecState* exec, JSValue value, const String& message, ErrorInstance::SourceAppender appender)
@@ -296,25 +288,22 @@ JSObject* createError(ExecState* exec, JSValue value, const String& message, Err
 
 JSObject* createInvalidFunctionApplyParameterError(ExecState* exec, JSValue value)
 {
-    VM& vm = exec->vm();
-    JSObject* exception = createTypeError(exec, makeString("second argument to Function.prototype.apply must be an Array-like object"), defaultSourceAppender, runtimeTypeForValue(vm, value));
-    ASSERT(exception->isErrorInstance());
-    return exception;
+    return createTypeError(exec, "second argument to Function.prototype.apply must be an Array-like object"_s, defaultSourceAppender, runtimeTypeForValue(exec->vm(), value));
 }
 
 JSObject* createInvalidInParameterError(ExecState* exec, JSValue value)
 {
-    return createError(exec, value, makeString("is not an Object."), invalidParameterInSourceAppender);
+    return createError(exec, value, "is not an Object."_s, invalidParameterInSourceAppender);
 }
 
 JSObject* createInvalidInstanceofParameterErrorNotFunction(ExecState* exec, JSValue value)
 {
-    return createError(exec, value, makeString(" is not a function"), invalidParameterInstanceofNotFunctionSourceAppender);
+    return createError(exec, value, " is not a function"_s, invalidParameterInstanceofNotFunctionSourceAppender);
 }
 
 JSObject* createInvalidInstanceofParameterErrorHasInstanceValueNotFunction(ExecState* exec, JSValue value)
 {
-    return createError(exec, value, makeString("[Symbol.hasInstance] is not a function, undefined, or null"), invalidParameterInstanceofhasInstanceValueNotFunctionSourceAppender);
+    return createError(exec, value, "[Symbol.hasInstance] is not a function, undefined, or null"_s, invalidParameterInstanceofhasInstanceValueNotFunctionSourceAppender);
 }
 
 JSObject* createNotAConstructorError(ExecState* exec, JSValue value)
index a2c0ad9..b61e776 100644 (file)
@@ -112,18 +112,15 @@ JSObject* constructFunctionSkippingEvalEnabledCheck(
         program = makeString(prefix, functionName.string(), "() {\n", body, "\n}");
     } else {
         StringBuilder builder(StringBuilder::OverflowHandler::RecordOverflow);
-        builder.append(prefix);
-        builder.append(functionName.string());
+        builder.append(prefix, functionName.string(), '(');
 
-        builder.append('(');
         auto viewWithString = args.at(0).toString(exec)->viewWithUnderlyingString(exec);
         RETURN_IF_EXCEPTION(scope, nullptr);
         builder.append(viewWithString.view);
         for (size_t i = 1; !builder.hasOverflowed() && i < args.size() - 1; i++) {
-            builder.appendLiteral(", ");
             auto viewWithString = args.at(i).toString(exec)->viewWithUnderlyingString(exec);
             RETURN_IF_EXCEPTION(scope, nullptr);
-            builder.append(viewWithString.view);
+            builder.append(", ", viewWithString.view);
         }
         if (builder.hasOverflowed()) {
             throwOutOfMemoryError(exec, scope);
@@ -131,12 +128,10 @@ JSObject* constructFunctionSkippingEvalEnabledCheck(
         }
 
         functionConstructorParametersEndPosition = builder.length() + 1;
-        builder.appendLiteral(") {\n");
 
         auto body = args.at(args.size() - 1).toString(exec)->viewWithUnderlyingString(exec);
         RETURN_IF_EXCEPTION(scope, nullptr);
-        builder.append(body.view);
-        builder.appendLiteral("\n}");
+        builder.append(") {\n", body.view, "\n}");
         if (builder.hasOverflowed()) {
             throwOutOfMemoryError(exec, scope);
             return nullptr;
index 1a6f9b7..ea996be 100644 (file)
@@ -866,8 +866,7 @@ void Options::dumpOption(StringBuilder& builder, DumpLevel level, Options::ID id
 
     if (header)
         builder.append(header);
-    builder.append(option.name());
-    builder.append('=');
+    builder.append(option.name(), '=');
     option.dump(builder);
 
     if (wasOverridden && (dumpDefaultsOption == DumpDefaults)) {
@@ -876,10 +875,8 @@ void Options::dumpOption(StringBuilder& builder, DumpLevel level, Options::ID id
         builder.appendLiteral(")");
     }
 
-    if (needsDescription) {
-        builder.appendLiteral("   ... ");
-        builder.append(option.description());
-    }
+    if (needsDescription)
+        builder.append("   ... ", option.description());
 
     builder.append(footer);
 }
index 8a6d5c0..086294f 100644 (file)
@@ -93,9 +93,7 @@ String TypeProfiler::typeInformationForExpressionAtOffset(TypeProfilerSearchDesc
         json.appendLiteral("null");
     json.append(',');
 
-    json.appendLiteral("\"instructionTypeSet\":");
-    json.append(location->m_instructionTypeSet->toJSONString());
-    json.append(',');
+    json.append("\"instructionTypeSet\":", location->m_instructionTypeSet->toJSONString(), ',');
 
     json.appendLiteral("\"isOverflown\":");
     if (location->m_instructionTypeSet->isOverflown() || (location->m_globalTypeSet && location->m_globalTypeSet->isOverflown()))
@@ -104,7 +102,7 @@ String TypeProfiler::typeInformationForExpressionAtOffset(TypeProfilerSearchDesc
         json.appendLiteral("false");
 
     json.append('}');
-    
+
     return json.toString();
 }
 
index cb9c1cc..b9d0ffd 100644 (file)
@@ -412,18 +412,10 @@ String StructureShape::stringRepresentation()
 
     representation.append('{');
     while (curShape) {
-        for (auto it = curShape->m_fields.begin(), end = curShape->m_fields.end(); it != end; ++it) {
-            String prop((*it).get());
-            representation.append(prop);
-            representation.appendLiteral(", ");
-        }
-
-        if (curShape->m_proto) {
-            representation.appendLiteral("__proto__ [");
-            representation.append(curShape->m_proto->m_constructorName);
-            representation.appendLiteral("], ");
-        }
-
+        for (auto& field : curShape->m_fields)
+            representation.append(StringView { field.get() }, ", ");
+        if (curShape->m_proto)
+            representation.append("__proto__ [", curShape->m_proto->m_constructorName, "], ");
         curShape = curShape->m_proto;
     }
 
index c7051a0..3a1afd3 100644 (file)
@@ -1,3 +1,15 @@
+2019-08-17  Darin Adler  <darin@apple.com>
+
+        Use makeString and multi-argument StringBuilder::append instead of less efficient multiple appends
+        https://bugs.webkit.org/show_bug.cgi?id=200862
+
+        Reviewed by Ryosuke Niwa.
+
+        * wtf/DateMath.cpp:
+        (WTF::makeRFC2822DateString): Use one append instead of multiple.
+        * wtf/JSONValues.cpp:
+        (WTF::appendDoubleQuotedString): Ditto.
+
 2019-08-21  Mark Lam  <mark.lam@apple.com>
 
         Fix infinite recursion in WTFCrashWithInfo() after r248930.
index af31a71..2725507 100644 (file)
@@ -1170,14 +1170,7 @@ double timeClip(double t)
 String makeRFC2822DateString(unsigned dayOfWeek, unsigned day, unsigned month, unsigned year, unsigned hours, unsigned minutes, unsigned seconds, int utcOffset)
 {
     StringBuilder stringBuilder;
-    stringBuilder.append(weekdayName[dayOfWeek]);
-    stringBuilder.appendLiteral(", ");
-    stringBuilder.appendNumber(day);
-    stringBuilder.append(' ');
-    stringBuilder.append(monthName[month]);
-    stringBuilder.append(' ');
-    stringBuilder.appendNumber(year);
-    stringBuilder.append(' ');
+    stringBuilder.append(weekdayName[dayOfWeek], ", ", day, ' ', monthName[month], ' ', year, ' ');
 
     appendTwoDigitNumber(stringBuilder, hours);
     stringBuilder.append(':');
index 8681a88..5d5e6ed 100644 (file)
@@ -485,11 +485,9 @@ inline void appendDoubleQuotedString(StringBuilder& builder, StringView string)
         // We could encode characters >= 127 as UTF-8 instead of \u escape sequences.
         // We could handle surrogates here if callers wanted that; for now we just
         // write them out as a \u sequence, so a surrogate pair appears as two of them.
-        builder.appendLiteral("\\u");
-        builder.append(upperNibbleToASCIIHexDigit(codeUnit >> 8));
-        builder.append(lowerNibbleToASCIIHexDigit(codeUnit >> 8));
-        builder.append(upperNibbleToASCIIHexDigit(codeUnit));
-        builder.append(lowerNibbleToASCIIHexDigit(codeUnit));
+        builder.append("\\u",
+            upperNibbleToASCIIHexDigit(codeUnit >> 8), lowerNibbleToASCIIHexDigit(codeUnit >> 8),
+            upperNibbleToASCIIHexDigit(codeUnit), lowerNibbleToASCIIHexDigit(codeUnit));
     }
     builder.append('"');
 }
index e31a829..3a38757 100644 (file)
@@ -1,3 +1,89 @@
+2019-08-17  Darin Adler  <darin@apple.com>
+
+        Use makeString and multi-argument StringBuilder::append instead of less efficient multiple appends
+        https://bugs.webkit.org/show_bug.cgi?id=200862
+
+        Reviewed by Ryosuke Niwa.
+
+        * Modules/indexeddb/shared/IDBDatabaseInfo.cpp:
+        (WebCore::IDBDatabaseInfo::loggingString const): Use one append instead of multiple.
+        * Modules/indexeddb/shared/IDBObjectStoreInfo.cpp:
+        (WebCore::IDBObjectStoreInfo::loggingString const): Ditto.
+        * Modules/mediastream/libwebrtc/LibWebRTCUtils.cpp:
+        (WebCore::toRTCCodecParameters): Ditto.
+        * Modules/plugins/YouTubePluginReplacement.cpp:
+        (WebCore::YouTubePluginReplacement::youTubeURLFromAbsoluteURL): Ditto.
+        * Modules/webdatabase/DatabaseTracker.cpp:
+        (WebCore::generateDatabaseFileName): Ditto.
+        * Modules/websockets/WebSocketExtensionDispatcher.cpp:
+        (WebCore::WebSocketExtensionDispatcher::createHeaderValue const): Ditto.
+        (WebCore::WebSocketExtensionDispatcher::appendAcceptedExtension): Ditto.
+
+        * Modules/websockets/WebSocketHandshake.cpp:
+        (WebCore::WebSocketHandshake::clientLocation const): Use makeString instead of
+        StringBuilder.
+
+        * bindings/js/JSDOMExceptionHandling.cpp:
+        (WebCore::appendArgumentMustBe): Use one append instead of multiple.
+        (WebCore::throwArgumentMustBeEnumError): Ditto.
+        (WebCore::throwArgumentTypeError): Ditto.
+        * contentextensions/CombinedURLFilters.cpp:
+        (WebCore::ContentExtensions::recursivePrint): Ditto.
+        * css/CSSBasicShapes.cpp:
+        (WebCore::buildCircleString): Ditto.
+        (WebCore::buildEllipseString): Ditto.
+        (WebCore::buildPolygonString): Ditto.
+        (WebCore::buildInsetString): Ditto.
+
+        * css/CSSCalculationValue.cpp:
+        (WebCore::buildCssText): Deleted.
+        (WebCore::CSSCalcValue::customCSSText const): Use makeString.
+
+        * css/CSSComputedStyleDeclaration.cpp:
+        (WebCore::CSSComputedStyleDeclaration::cssText const): Use one append instead of multiple.
+
+        * css/CSSCrossfadeValue.cpp:
+        (WebCore::CSSCrossfadeValue::customCSSText const): Use makeString.
+        * css/CSSFilterImageValue.cpp:
+        (WebCore::CSSFilterImageValue::customCSSText const): Ditto.
+        * css/CSSFontFaceRule.cpp:
+        (WebCore::CSSFontFaceRule::cssText const): Ditto.
+        * css/CSSFontFaceSrcValue.cpp:
+        (WebCore::CSSFontFaceSrcValue::customCSSText const): Ditto.
+
+        * css/CSSGradientValue.cpp:
+        (WebCore::appendGradientStops): Moved code here from CSSLinearGradientValue::customCSSText
+        so it can be shared with CSSRadialGradientValue::customCSSText. Use one append per stop.
+        (WebCore::CSSLinearGradientValue::customCSSText const): Use one append instead of multiple.
+        (WebCore::CSSRadialGradientValue::customCSSText const): Ditto.
+        (WebCore::CSSConicGradientValue::customCSSText const): Ditto.
+        * css/CSSMediaRule.cpp:
+        (WebCore::CSSMediaRule::cssText const): Ditto.
+        * css/CSSNamespaceRule.cpp:
+        (WebCore::CSSNamespaceRule::cssText const): Ditto.
+
+        * css/CSSPageRule.cpp:
+        (WebCore::CSSPageRule::selectorText const): Use makeString.
+
+        * css/CSSPrimitiveValue.cpp:
+        (WebCore::CSSPrimitiveValue::formatNumberForCustomCSSText const):
+        Use one append instead of multiple.
+
+        * css/CSSPropertySourceData.cpp:
+        (WebCore::CSSPropertySourceData::CSSPropertySourceData): Initialize in the
+        structure definition instead of the constructor.
+        (WebCore::CSSPropertySourceData::toString const): Use makeString.
+        * css/CSSPropertySourceData.h: Initialize in the structure definition.
+
+        * css/CSSStyleRule.cpp:
+        (WebCore::CSSStyleRule::cssText const): Use makeString.
+
+        * css/parser/CSSParser.cpp:
+        (WebCore::CSSParser::parseFontFaceDescriptor): Use makeString.
+
+        * html/canvas/CanvasRenderingContext2D.cpp:
+        (WebCore::CanvasRenderingContext2D::font const): Use one append instead of multiple.
+
 2019-08-22  Kai Ninomiya  <kainino@chromium.org>
 
         Pass conformance/attribs WebGL conformance tests
index 6aea141..ea312cc 100644 (file)
@@ -156,21 +156,16 @@ void IDBDatabaseInfo::deleteObjectStore(uint64_t objectStoreIdentifier)
 }
 
 #if !LOG_DISABLED
+
 String IDBDatabaseInfo::loggingString() const
 {
     StringBuilder builder;
-    builder.appendLiteral("Database:");
-    builder.append(m_name);
-    builder.appendLiteral(" version ");
-    builder.appendNumber(m_version);
-    builder.append('\n');
-    for (const auto& objectStore : m_objectStoreMap.values()) {
-        builder.append(objectStore.loggingString(1));
-        builder.append('\n');
-    }
-
+    builder.append("Database:", m_name, " version ", m_version, '\n');
+    for (auto& objectStore : m_objectStoreMap.values())
+        builder.append(objectStore.loggingString(1), '\n');
     return builder.toString();
 }
+
 #endif
 
 } // namespace WebCore
index 2938e93..2a051f3 100644 (file)
@@ -134,20 +134,15 @@ void IDBObjectStoreInfo::deleteIndex(uint64_t indexIdentifier)
 }
 
 #if !LOG_DISABLED
+
 String IDBObjectStoreInfo::loggingString(int indent) const
 {
     StringBuilder builder;
     for (int i = 0; i < indent; ++i)
         builder.append(' ');
-
-    builder.appendLiteral("Object store: ");
-    builder.append(m_name);
-    builder.appendNumber(m_identifier);
-    for (auto index : m_indexMap.values()) {
-        builder.append(index.loggingString(indent + 1));
-        builder.append('\n');
-    }
-
+    builder.append("Object store: ", m_name, m_identifier);
+    for (auto index : m_indexMap.values())
+        builder.append(index.loggingString(indent + 1), '\n');
     return builder.toString();
 }
 
index fa91dc6..f530578 100644 (file)
@@ -104,9 +104,7 @@ static inline RTCRtpCodecParameters toRTCCodecParameters(const webrtc::RtpCodecP
         parameters.channels = *rtcParameters.num_channels;
 
     StringBuilder sdpFmtpLineBuilder;
-    sdpFmtpLineBuilder.appendLiteral("a=fmtp:");
-    sdpFmtpLineBuilder.appendNumber(parameters.payloadType);
-    sdpFmtpLineBuilder.append(' ');
+    sdpFmtpLineBuilder.append("a=fmtp:", parameters.payloadType, ' ');
 
     bool isFirst = true;
     for (auto& keyValue : rtcParameters.parameters) {
@@ -114,10 +112,7 @@ static inline RTCRtpCodecParameters toRTCCodecParameters(const webrtc::RtpCodecP
             sdpFmtpLineBuilder.append(';');
         else
             isFirst = false;
-
-        sdpFmtpLineBuilder.append(StringView { keyValue.first.c_str() });
-        sdpFmtpLineBuilder.append('=');
-        sdpFmtpLineBuilder.append(StringView { keyValue.second.c_str() });
+        sdpFmtpLineBuilder.append(keyValue.first.c_str(), '=', keyValue.second.c_str());
     }
     parameters.sdpFmtpLine = sdpFmtpLineBuilder.toString();
 
index ba95bf5..5e86bf4 100644 (file)
@@ -327,12 +327,9 @@ String YouTubePluginReplacement::youTubeURLFromAbsoluteURL(const URL& srcURL, co
         finalURL.appendLiteral("http://www.youtube.com");
     else
         finalURL.append(srcURLPrefix);
-    finalURL.appendLiteral("/embed/");
-    finalURL.append(videoID);
-    if (!query.isEmpty()) {
-        finalURL.append('?');
-        finalURL.append(query);
-    }
+    finalURL.append("/embed/", videoID);
+    if (!query.isEmpty())
+        finalURL.append('?', query);
     return finalURL.toString();
 }
     
index c28a96f..69c3b2e 100644 (file)
@@ -308,12 +308,7 @@ String DatabaseTracker::originPath(const SecurityOriginData& origin) const
 
 static String generateDatabaseFileName()
 {
-    StringBuilder stringBuilder;
-
-    stringBuilder.append(createCanonicalUUIDString());
-    stringBuilder.appendLiteral(".db");
-
-    return stringBuilder.toString();
+    return makeString(createCanonicalUUIDString(), ".db");
 }
 
 String DatabaseTracker::fullPathForDatabaseNoLock(const SecurityOriginData& origin, const String& name, bool createIfNotExists)
index fcc9dc3..0066638 100644 (file)
@@ -64,10 +64,8 @@ const String WebSocketExtensionDispatcher::createHeaderValue() const
 
     StringBuilder builder;
     builder.append(m_processors[0]->handshakeString());
-    for (size_t i = 1; i < numProcessors; ++i) {
-        builder.appendLiteral(", ");
-        builder.append(m_processors[i]->handshakeString());
-    }
+    for (size_t i = 1; i < numProcessors; ++i)
+        builder.append(", ", m_processors[i]->handshakeString());
     return builder.toString();
 }
 
@@ -78,12 +76,9 @@ void WebSocketExtensionDispatcher::appendAcceptedExtension(const String& extensi
     m_acceptedExtensionsBuilder.append(extensionToken);
     // FIXME: Should use ListHashSet to keep the order of the parameters.
     for (auto& parameter : extensionParameters) {
-        m_acceptedExtensionsBuilder.appendLiteral("; ");
-        m_acceptedExtensionsBuilder.append(parameter.key);
-        if (!parameter.value.isNull()) {
-            m_acceptedExtensionsBuilder.append('=');
-            m_acceptedExtensionsBuilder.append(parameter.value);
-        }
+        m_acceptedExtensionsBuilder.append("; ", parameter.key);
+        if (!parameter.value.isNull())
+            m_acceptedExtensionsBuilder.append('=', parameter.value);
     }
 }
 
index f37ced1..4240b95 100644 (file)
@@ -166,12 +166,7 @@ bool WebSocketHandshake::secure() const
 
 String WebSocketHandshake::clientLocation() const
 {
-    StringBuilder builder;
-    builder.append(m_secure ? "wss" : "ws");
-    builder.appendLiteral("://");
-    builder.append(hostName(m_url, m_secure));
-    builder.append(resourceName(m_url));
-    return builder.toString();
+    return makeString(m_secure ? "wss" : "ws", "://", hostName(m_url, m_secure), resourceName(m_url));
 }
 
 CString WebSocketHandshake::clientHandshakeMessage() const
index 7002453..a500641 100644 (file)
@@ -170,20 +170,11 @@ static EncodedJSValue throwTypeError(JSC::ExecState& state, JSC::ThrowScope& sco
 
 static void appendArgumentMustBe(StringBuilder& builder, unsigned argumentIndex, const char* argumentName, const char* interfaceName, const char* functionName)
 {
-    builder.appendLiteral("Argument ");
-    builder.appendNumber(argumentIndex + 1);
-    builder.appendLiteral(" ('");
-    builder.append(argumentName);
-    builder.appendLiteral("') to ");
-    if (!functionName) {
-        builder.appendLiteral("the ");
-        builder.append(interfaceName);
-        builder.appendLiteral(" constructor");
-    } else {
-        builder.append(interfaceName);
-        builder.append('.');
-        builder.append(functionName);
-    }
+    builder.append("Argument ", argumentIndex + 1, " ('", argumentName, "') to ");
+    if (!functionName)
+        builder.append("the ", interfaceName, " constructor");
+    else
+        builder.append(interfaceName, '.', functionName);
     builder.appendLiteral(" must be ");
 }
 
@@ -209,8 +200,7 @@ JSC::EncodedJSValue throwArgumentMustBeEnumError(JSC::ExecState& state, JSC::Thr
 {
     StringBuilder builder;
     appendArgumentMustBe(builder, argumentIndex, argumentName, functionInterfaceName, functionName);
-    builder.appendLiteral("one of: ");
-    builder.append(expectedValues);
+    builder.append("one of: ", expectedValues);
     return throwVMTypeError(&state, scope, builder.toString());
 }
 
@@ -226,8 +216,7 @@ JSC::EncodedJSValue throwArgumentTypeError(JSC::ExecState& state, JSC::ThrowScop
 {
     StringBuilder builder;
     appendArgumentMustBe(builder, argumentIndex, argumentName, functionInterfaceName, functionName);
-    builder.appendLiteral("an instance of ");
-    builder.append(expectedType);
+    builder.append("an instance of ", expectedType);
     return throwVMTypeError(&state, scope, builder.toString());
 }
 
index f171a58..e87a854 100644 (file)
@@ -119,9 +119,7 @@ static void recursivePrint(const PrefixTreeVertex& vertex, const HashMap<const P
         StringBuilder builder;
         for (unsigned i = 0; i < depth * 2; ++i)
             builder.append(' ');
-        builder.appendLiteral("vertex edge: ");
-        builder.append(edge.term->toString());
-        builder.append('\n');
+        builder.append("vertex edge: ", edge.term->toString(), '\n');
         dataLogF("%s", builder.toString().utf8().data());
         ASSERT(edge.child);
         recursivePrint(*edge.child.get(), actions, depth + 1);
index 35fcef2..9b851e3 100644 (file)
@@ -86,24 +86,16 @@ static Ref<CSSPrimitiveValue> buildSerializablePositionOffset(CSSPrimitiveValue*
 
 static String buildCircleString(const String& radius, const String& centerX, const String& centerY)
 {
-    char opening[] = "circle(";
-    char at[] = "at";
-    char separator[] = " ";
     StringBuilder result;
-    result.appendLiteral(opening);
+    result.appendLiteral("circle(");
     if (!radius.isNull())
         result.append(radius);
-
     if (!centerX.isNull() || !centerY.isNull()) {
         if (!radius.isNull())
-            result.appendLiteral(separator);
-        result.appendLiteral(at);
-        result.appendLiteral(separator);
-        result.append(centerX);
-        result.appendLiteral(separator);
-        result.append(centerY);
+            result.append(' ');
+        result.append("at ", centerX, ' ', centerY);
     }
-    result.appendLiteral(")");
+    result.append(')');
     return result.toString();
 }
 
@@ -134,11 +126,8 @@ bool CSSBasicShapeCircle::equals(const CSSBasicShape& shape) const
 
 static String buildEllipseString(const String& radiusX, const String& radiusY, const String& centerX, const String& centerY)
 {
-    char opening[] = "ellipse(";
-    char at[] = "at";
-    char separator[] = " ";
     StringBuilder result;
-    result.appendLiteral(opening);
+    result.appendLiteral("ellipse(");
     bool needsSeparator = false;
     if (!radiusX.isNull()) {
         result.append(radiusX);
@@ -146,21 +135,16 @@ static String buildEllipseString(const String& radiusX, const String& radiusY, c
     }
     if (!radiusY.isNull()) {
         if (needsSeparator)
-            result.appendLiteral(separator);
+            result.append(' ');
         result.append(radiusY);
         needsSeparator = true;
     }
-
     if (!centerX.isNull() || !centerY.isNull()) {
         if (needsSeparator)
-            result.appendLiteral(separator);
-        result.appendLiteral(at);
-        result.appendLiteral(separator);
-        result.append(centerX);
-        result.appendLiteral(separator);
-        result.append(centerY);
+            result.append(' ');
+        result.append("at ", centerX, ' ', centerY);
     }
-    result.appendLiteral(")");
+    result.append(')');
     return result.toString();
 }
 
@@ -270,9 +254,7 @@ static String buildPolygonString(const WindRule& windRule, const Vector<String>&
     for (size_t i = 0; i < points.size(); i += 2) {
         if (i)
             result.appendLiteral(commaSeparator);
-        result.append(points[i]);
-        result.append(' ');
-        result.append(points[i + 1]);
+        result.append(points[i], ' ', points[i + 1]);
     }
 
     result.append(')');
@@ -322,28 +304,18 @@ static String buildInsetString(const String& top, const String& right, const Str
     const String& bottomRightRadiusWidth, const String& bottomRightRadiusHeight,
     const String& bottomLeftRadiusWidth, const String& bottomLeftRadiusHeight)
 {
-    char opening[] = "inset(";
-    char separator[] = " ";
-    char cornersSeparator[] = "round";
     StringBuilder result;
-    result.appendLiteral(opening);
-    result.append(top);
+    result.append("inset(", top);
 
     bool showLeftArg = !left.isNull() && left != right;
     bool showBottomArg = !bottom.isNull() && (bottom != top || showLeftArg);
     bool showRightArg = !right.isNull() && (right != top || showBottomArg);
-    if (showRightArg) {
-        result.appendLiteral(separator);
-        result.append(right);
-    }
-    if (showBottomArg) {
-        result.appendLiteral(separator);
-        result.append(bottom);
-    }
-    if (showLeftArg) {
-        result.appendLiteral(separator);
-        result.append(left);
-    }
+    if (showRightArg)
+        result.append(' ', right);
+    if (showBottomArg)
+        result.append(' ', bottom);
+    if (showLeftArg)
+        result.append(' ', left);
 
     if (!topLeftRadiusWidth.isNull() && !topLeftRadiusHeight.isNull()) {
         Vector<String> horizontalRadii;
@@ -353,23 +325,16 @@ static String buildInsetString(const String& top, const String& right, const Str
         areDefaultCornerRadii &= buildInsetRadii(verticalRadii, topLeftRadiusHeight, topRightRadiusHeight, bottomRightRadiusHeight, bottomLeftRadiusHeight);
 
         if (!areDefaultCornerRadii) {
-            result.appendLiteral(separator);
-            result.appendLiteral(cornersSeparator);
+            result.appendLiteral(" round");
 
-            for (size_t i = 0; i < horizontalRadii.size(); ++i) {
-                result.appendLiteral(separator);
-                result.append(horizontalRadii[i]);
-            }
+            for (auto& radius : horizontalRadii)
+                result.append(' ', radius);
 
             if (verticalRadii.size() != horizontalRadii.size()
                 || !WTF::VectorComparer<false, String>::compare(verticalRadii.data(), horizontalRadii.data(), verticalRadii.size())) {
-                result.appendLiteral(separator);
-                result.appendLiteral("/");
-
-                for (size_t i = 0; i < verticalRadii.size(); ++i) {
-                    result.appendLiteral(separator);
-                    result.append(verticalRadii[i]);
-                }
+                result.appendLiteral(" /");
+                for (auto& radius : verticalRadii)
+                    result.append(' ', radius);
             }
         }
     }
index a42f564..9be751d 100644 (file)
@@ -149,22 +149,12 @@ static bool hasDoubleValue(CSSPrimitiveValue::UnitType type)
     return false;
 }
 
-static String buildCssText(const String& expression)
-{
-    StringBuilder result;
-    result.appendLiteral("calc");
-    bool expressionHasSingleTerm = expression[0] != '(';
-    if (expressionHasSingleTerm)
-        result.append('(');
-    result.append(expression);
-    if (expressionHasSingleTerm)
-        result.append(')');
-    return result.toString();
-}
-
 String CSSCalcValue::customCSSText() const
 {
-    return buildCssText(m_expression->customCSSText());
+    auto expression = m_expression->customCSSText();
+    if (expression[0] == '(')
+        return makeString("calc", expression);
+    return makeString("calc(", expression, ')');
 }
 
 bool CSSCalcValue::equals(const CSSCalcValue& other) const
index b36e03e..60b6a52 100644 (file)
@@ -1691,16 +1691,11 @@ void CSSComputedStyleDeclaration::deref()
 String CSSComputedStyleDeclaration::cssText() const
 {
     StringBuilder result;
-
     for (unsigned i = 0; i < numComputedProperties; i++) {
         if (i)
             result.append(' ');
-        result.append(getPropertyName(computedProperties[i]));
-        result.appendLiteral(": ");
-        result.append(getPropertyValue(computedProperties[i]));
-        result.append(';');
+        result.append(getPropertyName(computedProperties[i]), ": ", getPropertyValue(computedProperties[i]), ';');
     }
-
     return result.toString();
 }
 
index 172c945..cd016cc 100644 (file)
@@ -91,18 +91,7 @@ CSSCrossfadeValue::~CSSCrossfadeValue()
 
 String CSSCrossfadeValue::customCSSText() const
 {
-    StringBuilder result;
-    if (m_isPrefixed)
-        result.appendLiteral("-webkit-cross-fade(");
-    else
-        result.appendLiteral("cross-fade(");
-    result.append(m_fromValue->cssText());
-    result.appendLiteral(", ");
-    result.append(m_toValue->cssText());
-    result.appendLiteral(", ");
-    result.append(m_percentageValue->cssText());
-    result.append(')');
-    return result.toString();
+    return makeString(m_isPrefixed ? "-webkit-" : "", "cross-fade(", m_fromValue->cssText(), ", ", m_toValue->cssText(), ", ", m_percentageValue->cssText(), ')');
 }
 
 FloatSize CSSCrossfadeValue::fixedSize(const RenderElement& renderer)
index 7348282..0b3b9f0 100644 (file)
@@ -47,13 +47,7 @@ CSSFilterImageValue::~CSSFilterImageValue()
 
 String CSSFilterImageValue::customCSSText() const
 {
-    StringBuilder result;
-    result.appendLiteral("filter(");
-    result.append(m_imageValue->cssText());
-    result.appendLiteral(", ");
-    result.append(m_filterValue->cssText());
-    result.append(')');
-    return result.toString();
+    return makeString("filter(", m_imageValue->cssText(), ", ", m_filterValue->cssText(), ')');
 }
 
 FloatSize CSSFilterImageValue::fixedSize(const RenderElement* renderer)
index 0dda1b2..de459dd 100644 (file)
@@ -50,14 +50,10 @@ CSSStyleDeclaration& CSSFontFaceRule::style()
 
 String CSSFontFaceRule::cssText() const
 {
-    StringBuilder result;
-    result.appendLiteral("@font-face { ");
-    String descs = m_fontFaceRule->properties().asText();
-    result.append(descs);
-    if (!descs.isEmpty())
-        result.append(' ');
-    result.append('}');
-    return result.toString();
+    String declarations = m_fontFaceRule->properties().asText();
+    if (declarations.isEmpty())
+        return "@font-face { }"_s;
+    return makeString("@font-face { ", declarations, " }");
 }
 
 void CSSFontFaceRule::reattach(StyleRuleBase& rule)
index 0fbcdc9..a430e93 100644 (file)
@@ -69,19 +69,10 @@ bool CSSFontFaceSrcValue::isSupportedFormat() const
 
 String CSSFontFaceSrcValue::customCSSText() const
 {
-    StringBuilder result;
-    if (isLocal())
-        result.appendLiteral("local(");
-    else
-        result.appendLiteral("url(");
-    result.append(m_resource);
-    result.append(')');
-    if (!m_format.isEmpty()) {
-        result.appendLiteral(" format(");
-        result.append(m_format);
-        result.append(')');
-    }
-    return result.toString();
+    const char* prefix = isLocal() ? "local(" : "url(";
+    if (m_format.isEmpty())
+        return makeString(prefix, m_resource, ')');
+    return makeString(prefix, m_resource, ')', " format(", m_format, ')');
 }
 
 bool CSSFontFaceSrcValue::traverseSubresources(const WTF::Function<bool (const CachedResource&)>& handler) const
index f6bf249..705c48f 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2008 Apple Inc.  All rights reserved.
+ * Copyright (C) 2008 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -230,7 +230,6 @@ private:
 
 class ConicGradientAdapter {
 public:
-    explicit ConicGradientAdapter() { }
     float gradientLength() const { return 1; }
     float maxExtent(float, float) const { return 1; }
 
@@ -667,38 +666,25 @@ bool CSSGradientValue::knownToBeOpaque(const RenderElement& renderer) const
     return true;
 }
 
+static void appendGradientStops(StringBuilder& builder, const Vector<CSSGradientColorStop, 2>& stops)
+{
+    for (auto& stop : stops) {
+        double position = stop.m_position->doubleValue(CSSPrimitiveValue::CSS_NUMBER);
+        if (!position)
+            builder.append(", from(", stop.m_color->cssText(), ')');
+        else if (position == 1)
+            builder.append(", to(", stop.m_color->cssText(), ')');
+        else
+            builder.append(", color-stop(", FormattedNumber::fixedPrecision(position), ", ", stop.m_color->cssText(), ')');
+    }
+}
+
 String CSSLinearGradientValue::customCSSText() const
 {
     StringBuilder result;
     if (m_gradientType == CSSDeprecatedLinearGradient) {
-        result.appendLiteral("-webkit-gradient(linear, ");
-        result.append(m_firstX->cssText());
-        result.append(' ');
-        result.append(m_firstY->cssText());
-        result.appendLiteral(", ");
-        result.append(m_secondX->cssText());
-        result.append(' ');
-        result.append(m_secondY->cssText());
-
-        for (auto& stop : m_stops) {
-            result.appendLiteral(", ");
-            auto position = stop.m_position->doubleValue(CSSPrimitiveValue::CSS_NUMBER);
-            if (!position) {
-                result.appendLiteral("from(");
-                result.append(stop.m_color->cssText());
-                result.append(')');
-            } else if (position == 1) {
-                result.appendLiteral("to(");
-                result.append(stop.m_color->cssText());
-                result.append(')');
-            } else {
-                result.appendLiteral("color-stop(");
-                result.appendFixedPrecisionNumber(position);
-                result.appendLiteral(", ");
-                result.append(stop.m_color->cssText());
-                result.append(')');
-            }
-        }
+        result.append("-webkit-gradient(linear, ", m_firstX->cssText(), ' ', m_firstY->cssText(), ", ", m_secondX->cssText(), ' ', m_secondY->cssText());
+        appendGradientStops(result, m_stops);
     } else if (m_gradientType == CSSPrefixedLinearGradient) {
         if (m_repeating)
             result.appendLiteral("-webkit-repeating-linear-gradient(");
@@ -708,27 +694,18 @@ String CSSLinearGradientValue::customCSSText() const
         if (m_angle)
             result.append(m_angle->cssText());
         else {
-            if (m_firstX && m_firstY) {
+            if (m_firstX && m_firstY)
+                result.append(m_firstX->cssText(), ' ', m_firstY->cssText());
+            else if (m_firstX)
                 result.append(m_firstX->cssText());
-                result.append(' ');
+            else if (m_firstY)
                 result.append(m_firstY->cssText());
-            } else if (m_firstX || m_firstY) {
-                if (m_firstX)
-                    result.append(m_firstX->cssText());
-
-                if (m_firstY)
-                    result.append(m_firstY->cssText());
-            }
         }
 
-        for (unsigned i = 0; i < m_stops.size(); i++) {
-            auto& stop = m_stops[i];
-            result.appendLiteral(", ");
-            result.append(stop.m_color->cssText());
-            if (stop.m_position) {
-                result.append(' ');
-                result.append(stop.m_position->cssText());
-            }
+        for (auto& stop : m_stops) {
+            result.append(", ", stop.m_color->cssText());
+            if (stop.m_position)
+                result.append(' ', stop.m_position->cssText());
         }
     } else {
         if (m_repeating)
@@ -743,11 +720,9 @@ String CSSLinearGradientValue::customCSSText() const
             wroteSomething = true;
         } else if ((m_firstX || m_firstY) && !(!m_firstX && m_firstY && m_firstY->valueID() == CSSValueBottom)) {
             result.appendLiteral("to ");
-            if (m_firstX && m_firstY) {
-                result.append(m_firstX->cssText());
-                result.append(' ');
-                result.append(m_firstY->cssText());
-            } else if (m_firstX)
+            if (m_firstX && m_firstY)
+                result.append(m_firstX->cssText(), ' ', m_firstY->cssText());
+            else if (m_firstX)
                 result.append(m_firstX->cssText());
             else
                 result.append(m_firstY->cssText());
@@ -757,10 +732,11 @@ String CSSLinearGradientValue::customCSSText() const
         if (wroteSomething)
             result.appendLiteral(", ");
 
-        for (unsigned i = 0; i < m_stops.size(); i++) {
-            const CSSGradientColorStop& stop = m_stops[i];
-            if (i)
+        bool wroteFirstStop = false;
+        for (auto& stop : m_stops) {
+            if (wroteFirstStop)
                 result.appendLiteral(", ");
+            wroteFirstStop = true;
             if (!stop.isMidpoint)
                 result.append(stop.m_color->cssText());
             if (stop.m_position) {
@@ -769,7 +745,6 @@ String CSSLinearGradientValue::customCSSText() const
                 result.append(stop.m_position->cssText());
             }
         }
-        
     }
 
     result.append(')');
@@ -947,84 +922,41 @@ String CSSRadialGradientValue::customCSSText() const
     StringBuilder result;
 
     if (m_gradientType == CSSDeprecatedRadialGradient) {
-        result.appendLiteral("-webkit-gradient(radial, ");
-        result.append(m_firstX->cssText());
-        result.append(' ');
-        result.append(m_firstY->cssText());
-        result.appendLiteral(", ");
-        result.append(m_firstRadius->cssText());
-        result.appendLiteral(", ");
-        result.append(m_secondX->cssText());
-        result.append(' ');
-        result.append(m_secondY->cssText());
-        result.appendLiteral(", ");
-        result.append(m_secondRadius->cssText());
-
-        // FIXME: share?
-        for (auto& stop : m_stops) {
-            result.appendLiteral(", ");
-            auto position = stop.m_position->doubleValue(CSSPrimitiveValue::CSS_NUMBER);
-            if (!position) {
-                result.appendLiteral("from(");
-                result.append(stop.m_color->cssText());
-                result.append(')');
-            } else if (position == 1) {
-                result.appendLiteral("to(");
-                result.append(stop.m_color->cssText());
-                result.append(')');
-            } else {
-                result.appendLiteral("color-stop(");
-                result.appendFixedPrecisionNumber(position);
-                result.appendLiteral(", ");
-                result.append(stop.m_color->cssText());
-                result.append(')');
-            }
-        }
+        result.append("-webkit-gradient(radial, ", m_firstX->cssText(), ' ', m_firstY->cssText(), ", ", m_firstRadius->cssText(),
+            ", ", m_secondX->cssText(), ' ', m_secondY->cssText(), ", ", m_secondRadius->cssText());
+        appendGradientStops(result, m_stops);
     } else if (m_gradientType == CSSPrefixedRadialGradient) {
         if (m_repeating)
             result.appendLiteral("-webkit-repeating-radial-gradient(");
         else
             result.appendLiteral("-webkit-radial-gradient(");
 
-        if (m_firstX && m_firstY) {
+        if (m_firstX && m_firstY)
+            result.append(m_firstX->cssText(), ' ', m_firstY->cssText());
+        else if (m_firstX)
             result.append(m_firstX->cssText());
-            result.append(' ');
-            result.append(m_firstY->cssText());
-        } else if (m_firstX)
-            result.append(m_firstX->cssText());
-         else if (m_firstY)
+        else if (m_firstY)
             result.append(m_firstY->cssText());
         else
             result.appendLiteral("center");
 
         if (m_shape || m_sizingBehavior) {
             result.appendLiteral(", ");
-            if (m_shape) {
-                result.append(m_shape->cssText());
-                result.append(' ');
-            } else
+            if (m_shape)
+                result.append(m_shape->cssText(), ' ');
+            else
                 result.appendLiteral("ellipse ");
-
             if (m_sizingBehavior)
                 result.append(m_sizingBehavior->cssText());
             else
                 result.appendLiteral("cover");
+        } else if (m_endHorizontalSize && m_endVerticalSize)
+            result.append(", ", m_endHorizontalSize->cssText(), ' ', m_endVerticalSize->cssText());
 
-        } else if (m_endHorizontalSize && m_endVerticalSize) {
-            result.appendLiteral(", ");
-            result.append(m_endHorizontalSize->cssText());
-            result.append(' ');
-            result.append(m_endVerticalSize->cssText());
-        }
-
-        for (unsigned i = 0; i < m_stops.size(); i++) {
-            const CSSGradientColorStop& stop = m_stops[i];
-            result.appendLiteral(", ");
-            result.append(stop.m_color->cssText());
-            if (stop.m_position) {
-                result.append(' ');
-                result.append(stop.m_position->cssText());
-            }
+        for (auto& stop : m_stops) {
+            result.append(", ", stop.m_color->cssText());
+            if (stop.m_position)
+                result.append(' ', stop.m_position->cssText());
         }
     } else {
         if (m_repeating)
@@ -1050,10 +982,8 @@ String CSSRadialGradientValue::customCSSText() const
             if (wroteSomething)
                 result.append(' ');
             result.append(m_endHorizontalSize->cssText());
-            if (m_endVerticalSize) {
-                result.append(' ');
-                result.append(m_endVerticalSize->cssText());
-            }
+            if (m_endVerticalSize)
+                result.append(' ', m_endVerticalSize->cssText());
             wroteSomething = true;
         }
 
@@ -1061,11 +991,9 @@ String CSSRadialGradientValue::customCSSText() const
             if (wroteSomething)
                 result.append(' ');
             result.appendLiteral("at ");
-            if (m_firstX && m_firstY) {
-                result.append(m_firstX->cssText());
-                result.append(' ');
-                result.append(m_firstY->cssText());
-            } else if (m_firstX)
+            if (m_firstX && m_firstY)
+                result.append(m_firstX->cssText(), ' ', m_firstY->cssText());
+            else if (m_firstX)
                 result.append(m_firstX->cssText());
             else
                 result.append(m_firstY->cssText());
@@ -1097,13 +1025,12 @@ String CSSRadialGradientValue::customCSSText() const
 float CSSRadialGradientValue::resolveRadius(CSSPrimitiveValue& radius, const CSSToLengthConversionData& conversionData, float* widthOrHeight)
 {
     float result = 0;
-    if (radius.isNumber()) // Can the radius be a percentage?
+    if (radius.isNumber())
         result = radius.floatValue() * conversionData.zoom();
     else if (widthOrHeight && radius.isPercentage())
         result = *widthOrHeight * radius.floatValue() / 100;
     else
         result = radius.computeLength<float>(conversionData);
-
     return result;
 }
 
@@ -1386,18 +1313,14 @@ String CSSConicGradientValue::customCSSText() const
     bool wroteSomething = false;
 
     if (m_angle) {
-        result.appendLiteral("from ");
-        result.append(m_angle->cssText());
+        result.append("from ", m_angle->cssText());
         wroteSomething = true;
     }
 
     if (m_firstX && m_firstY) {
         if (wroteSomething)
-            result.appendLiteral(" ");
-        result.appendLiteral("at ");
-        result.append(m_firstX->cssText());
-        result.append(' ');
-        result.append(m_firstY->cssText());
+            result.append(' ');
+        result.append("at ", m_firstX->cssText(), ' ', m_firstY->cssText());
         wroteSomething = true;
     }
 
index b094edd..15e9dc0 100644 (file)
@@ -51,8 +51,7 @@ String CSSMediaRule::cssText() const
     StringBuilder result;
     result.appendLiteral("@media ");
     if (mediaQueries()) {
-        result.append(mediaQueries()->mediaText());
-        result.append(' ');
+        result.append(mediaQueries()->mediaText(), ' ');
     }
     result.appendLiteral("{ \n");
     appendCssTextForItems(result);
index 3b933b8..783c2e8 100644 (file)
@@ -57,9 +57,7 @@ String CSSNamespaceRule::cssText() const
     serializeIdentifier(prefix(), result);
     if (!prefix().isEmpty())
         result.append(' ');
-    result.append("url(");
-    result.append(serializeString(namespaceURI()));
-    result.append(");");
+    result.append("url(", serializeString(namespaceURI()), ");");
     return result.toString();
 }
 
index 2242136..1c22b90 100644 (file)
@@ -54,17 +54,12 @@ CSSStyleDeclaration& CSSPageRule::style()
 
 String CSSPageRule::selectorText() const
 {
-    StringBuilder text;
-    text.appendLiteral("@page");
-    const CSSSelector* selector = m_pageRule->selector();
-    if (selector) {
+    if (auto* selector = m_pageRule->selector()) {
         String pageSpecification = selector->selectorText();
-        if (!pageSpecification.isEmpty() && pageSpecification != starAtom()) {
-            text.append(' ');
-            text.append(pageSpecification);
-        }
+        if (!pageSpecification.isEmpty() && pageSpecification != starAtom())
+            return makeString("@page ", pageSpecification);
     }
-    return text.toString();
+    return "@page"_s;
 }
 
 void CSSPageRule::setSelectorText(const String& selectorText)
index 91168e7..9aa6f76 100644 (file)
@@ -1003,10 +1003,8 @@ ALWAYS_INLINE String CSSPrimitiveValue::formatNumberForCustomCSSText() const
             serializeString(separator, result);
         }
         String listStyle = m_value.counter->listStyle();
-        if (!listStyle.isEmpty()) {
-            result.appendLiteral(", ");
-            result.append(listStyle);
-        }
+        if (!listStyle.isEmpty())
+            result.append(", ", listStyle);
         result.append(')');
 
         return result.toString();
index 75b7990..508c1bd 100644 (file)
@@ -77,10 +77,6 @@ CSSPropertySourceData::CSSPropertySourceData(const CSSPropertySourceData& other)
 CSSPropertySourceData::CSSPropertySourceData()
     : name(emptyString())
     , value(emptyString())
-    , important(false)
-    , disabled(false)
-    , parsedOk(false)
-    , range(SourceRange(0, 0))
 {
 }
 
@@ -88,15 +84,7 @@ String CSSPropertySourceData::toString() const
 {
     if (!name && value == "e")
         return String();
-
-    StringBuilder result;
-    result.append(name);
-    result.appendLiteral(": ");
-    result.append(value);
-    if (important)
-        result.appendLiteral(" !important");
-    result.append(';');
-    return result.toString();
+    return makeString(name, ": ", value, important ? " !important" : "", ';');
 }
 
 unsigned CSSPropertySourceData::hash() const
index 89e28aa..8c93eef 100644 (file)
@@ -60,9 +60,9 @@ struct CSSPropertySourceData {
 
     String name;
     String value;
-    bool important;
-    bool disabled;
-    bool parsedOk;
+    bool important { false };
+    bool disabled { false };
+    bool parsedOk { false };
     SourceRange range;
 };
 
index ed11f75..5724a94 100644 (file)
@@ -112,15 +112,10 @@ void CSSStyleRule::setSelectorText(const String& selectorText)
 
 String CSSStyleRule::cssText() const
 {
-    StringBuilder result;
-    result.append(selectorText());
-    result.appendLiteral(" { ");
-    String decls = m_styleRule->properties().asText();
-    result.append(decls);
-    if (!decls.isEmpty())
-        result.append(' ');
-    result.append('}');
-    return result.toString();
+    String declarations = m_styleRule->properties().asText();
+    if (declarations.isEmpty())
+        return makeString(selectorText(), " { }");
+    return makeString(selectorText(), " { ", declarations, " }");
 }
 
 void CSSStyleRule::reattach(StyleRuleBase& rule)
index 05e100b..a68d55b 100644 (file)
@@ -246,13 +246,8 @@ std::unique_ptr<Vector<double>> CSSParser::parseKeyframeKeyList(const String& se
 
 RefPtr<CSSValue> CSSParser::parseFontFaceDescriptor(CSSPropertyID propertyID, const String& propertyValue, const CSSParserContext& context)
 {
-    StringBuilder builder;
-    builder.appendLiteral("@font-face { ");
-    builder.append(getPropertyNameString(propertyID));
-    builder.appendLiteral(" : ");
-    builder.append(propertyValue);
-    builder.appendLiteral("; }");
-    RefPtr<StyleRuleBase> rule = parseRule(context, nullptr, builder.toString());
+    String string = makeString("@font-face { ", getPropertyNameString(propertyID), " : ", propertyValue, "; }");
+    RefPtr<StyleRuleBase> rule = parseRule(context, nullptr, string);
     if (!rule || !rule->isFontFaceRule())
         return nullptr;
     return downcast<StyleRuleFontFace>(*rule.get()).properties().getPropertyCSSValue(propertyID);
index fca9a5a..9ebf12f 100644 (file)
@@ -111,6 +111,7 @@ String CanvasRenderingContext2D::font() const
     for (unsigned i = 0; i < fontDescription.familyCount(); ++i) {
         if (i)
             serializedFont.append(',');
+
         // FIXME: We should append family directly to serializedFont rather than building a temporary string.
         String family = fontDescription.familyAt(i);
         if (family.startsWith("-webkit-"))
@@ -118,8 +119,7 @@ String CanvasRenderingContext2D::font() const
         if (family.contains(' '))
             family = makeString('"', family, '"');
 
-        serializedFont.append(' ');
-        serializedFont.append(family);
+        serializedFont.append(' ', family);
     }
 
     return serializedFont.toString();
index 95e287a..eb93403 100644 (file)
@@ -1,3 +1,13 @@
+2019-08-17  Darin Adler  <darin@apple.com>
+
+        Use makeString and multi-argument StringBuilder::append instead of less efficient multiple appends
+        https://bugs.webkit.org/show_bug.cgi?id=200862
+
+        Reviewed by Ryosuke Niwa.
+
+        * Shared/mac/AuxiliaryProcessMac.mm:
+        (WebKit::setAndSerializeSandboxParameters): Use one append instead of multiple.
+
 2019-08-22  Wenson Hsieh  <wenson_hsieh@apple.com>
 
         Typing Korean in title field after typing in the body inserts extraneous characters on blog.naver.com
index 1bc143b..3675271 100644 (file)
@@ -234,10 +234,7 @@ static Optional<CString> setAndSerializeSandboxParameters(const SandboxInitializ
             WTFLogAlways("%s: Could not set sandbox parameter: %s\n", getprogname(), strerror(errno));
             CRASH();
         }
-        builder.append(name);
-        builder.append(':');
-        builder.append(value);
-        builder.append(':');
+        builder.append(name, ':', value, ':');
     }
     if (isProfilePath) {
         auto contents = fileContents(profileOrProfilePath);
index da65376..d7742aa 100644 (file)
@@ -1,3 +1,17 @@
+2019-08-17  Darin Adler  <darin@apple.com>
+
+        Use makeString and multi-argument StringBuilder::append instead of less efficient multiple appends
+        https://bugs.webkit.org/show_bug.cgi?id=200862
+
+        Reviewed by Ryosuke Niwa.
+
+        * WebKitTestRunner/InjectedBundle/TestRunner.cpp:
+        (WTR::TestRunner::statisticsDidRunTelemetryCallback): Use makeString.
+        * WebKitTestRunner/TestController.cpp:
+        (WTR::TestController::findAndDumpWebKitProcessIdentifiers): Ditto.
+        (WTR::TestController::downloadDidReceiveServerRedirectToURL): Ditto.
+        (WTR::TestController::downloadDidFail): Ditto.
+
 2019-08-22  clopez@igalia.com  <clopez@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
 
         [GTK][WPE] Support for command "--version" on the MiniBrowser (follow-up fix)
index fa1bc1d..d3ef461 100644 (file)
@@ -1906,16 +1906,9 @@ void TestRunner::statisticsDidRunTelemetryCallback(unsigned totalPrevalentResour
     WKBundleFrameRef mainFrame = WKBundlePageGetMainFrame(InjectedBundle::singleton().page()->page());
     JSContextRef context = WKBundleFrameGetJavaScriptContext(mainFrame);
     
-    StringBuilder stringBuilder;
-    stringBuilder.appendLiteral("{ \"totalPrevalentResources\" : ");
-    stringBuilder.appendNumber(totalPrevalentResources);
-    stringBuilder.appendLiteral(", \"totalPrevalentResourcesWithUserInteraction\" : ");
-    stringBuilder.appendNumber(totalPrevalentResourcesWithUserInteraction);
-    stringBuilder.appendLiteral(", \"top3SubframeUnderTopFrameOrigins\" : ");
-    stringBuilder.appendNumber(top3SubframeUnderTopFrameOrigins);
-    stringBuilder.appendLiteral(" }");
+    String string = makeString("{ \"totalPrevalentResources\" : ", totalPrevalentResources, ", \"totalPrevalentResourcesWithUserInteraction\" : ", totalPrevalentResourcesWithUserInteraction, ", \"top3SubframeUnderTopFrameOrigins\" : ", top3SubframeUnderTopFrameOrigins, " }");
     
-    JSValueRef result = JSValueMakeFromJSONString(context, adopt(JSStringCreateWithUTF8CString(stringBuilder.toString().utf8().data())).get());
+    JSValueRef result = JSValueMakeFromJSONString(context, adopt(JSStringCreateWithUTF8CString(string.utf8().data())).get());
 
     callTestRunnerCallback(StatisticsDidRunTelemetryCallbackID, 1, &result);
 }
index b925956..7262f95 100644 (file)
@@ -1094,25 +1094,14 @@ void TestController::dumpResponse(const String& result)
 
 void TestController::findAndDumpWebKitProcessIdentifiers()
 {
-    StringBuilder builder;
-
 #if PLATFORM(COCOA)
-    builder.append(TestController::webProcessName());
-    builder.appendLiteral(": ");
-    pid_t webContentPID = WKPageGetProcessIdentifier(TestController::singleton().mainWebView()->page());
-    builder.appendNumber(webContentPID);
-    builder.append('\n');
-
-    builder.append(TestController::networkProcessName());
-    builder.appendLiteral(": ");
-    pid_t networkingPID = WKContextGetNetworkProcessIdentifier(m_context.get());
-    builder.appendNumber(networkingPID);
-    builder.append('\n');
+    dumpResponse(makeString(TestController::webProcessName(), ": ",
+        WKPageGetProcessIdentifier(TestController::singleton().mainWebView()->page()), '\n',
+        TestController::networkProcessName(), ": ",
+        WKContextGetNetworkProcessIdentifier(m_context.get()), '\n'));
 #else
-    builder.append('\n');
+    dumpResponse("\n"_s);
 #endif
-
-    dumpResponse(builder.toString());
 }
 
 void TestController::findAndDumpWorldLeaks()
@@ -2450,14 +2439,8 @@ void TestController::downloadDidFinish(WKContextRef, WKDownloadRef)
 
 void TestController::downloadDidReceiveServerRedirectToURL(WKContextRef, WKDownloadRef, WKURLRef url)
 {
-    if (m_shouldLogDownloadCallbacks) {
-        StringBuilder builder;
-        builder.appendLiteral("Download was redirected to \"");
-        WKRetainPtr<WKStringRef> urlStringWK = adoptWK(WKURLCopyString(url));
-        builder.append(toSTD(urlStringWK).c_str());
-        builder.appendLiteral("\".\n");
-        m_currentInvocation->outputText(builder.toString());
-    }
+    if (m_shouldLogDownloadCallbacks)
+        m_currentInvocation->outputText(makeString("Download was redirected to \"", toWTFString(adoptWK(WKURLCopyString(url))), "\".\n"));
 }
 
 void TestController::downloadDidFail(WKContextRef, WKDownloadRef, WKErrorRef error)
@@ -2465,20 +2448,11 @@ void TestController::downloadDidFail(WKContextRef, WKDownloadRef, WKErrorRef err
     if (m_shouldLogDownloadCallbacks) {
         m_currentInvocation->outputText("Download failed.\n"_s);
 
-        WKRetainPtr<WKStringRef> errorDomain = adoptWK(WKErrorCopyDomain(error));
-        WKRetainPtr<WKStringRef> errorDescription = adoptWK(WKErrorCopyLocalizedDescription(error));
-        int errorCode = WKErrorGetErrorCode(error);
-
-        StringBuilder errorBuilder;
-        errorBuilder.append("Failed: ");
-        errorBuilder.append(toWTFString(errorDomain));
-        errorBuilder.append(", code=");
-        errorBuilder.appendNumber(errorCode);
-        errorBuilder.append(", description=");
-        errorBuilder.append(toWTFString(errorDescription));
-        errorBuilder.append("\n");
+        auto domain = toWTFString(adoptWK(WKErrorCopyDomain(error)));
+        auto description = toWTFString(adoptWK(WKErrorCopyLocalizedDescription(error)));
+        int code = WKErrorGetErrorCode(error);
 
-        m_currentInvocation->outputText(errorBuilder.toString());
+        m_currentInvocation->outputText(makeString("Failed: ", domain, ", code=", code, ", description=", description, "\n"));
     }
     m_currentInvocation->notifyDownloadDone();
 }