Use even more "shortest form" formatting, and less "fixed precision" and "fixed width"
authordarin@apple.com <darin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 14 Jan 2020 17:16:03 +0000 (17:16 +0000)
committerdarin@apple.com <darin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 14 Jan 2020 17:16:03 +0000 (17:16 +0000)
https://bugs.webkit.org/show_bug.cgi?id=198918

Reviewed by Sam Weinig.

Source/JavaScriptCore:

* API/tests/ExecutionTimeLimitTest.cpp:
(testExecutionTimeLimit): Rewrote the string creation code to use makeString instead
of StringBuilder and no longer use any fixed precision.

* runtime/Options.cpp:
(JSC::OptionReader::Option::dump const): Dump doubles with shortest form instead of
fixed precision.

Source/WebCore:

The places left untouched are the ones where changing behavior has some kind of unwanted
observable effect for one of two reasons. Otherwise, switched almost all call sites.

1) Substantial number of test results that depend on the current behavior.

2) Poor rounding resulting in conversion from float to double and back (or similar) that
   results in values with tiny fractional residue like "6.000001".

* accessibility/AccessibilityNodeObject.cpp:
(WebCore::AccessibilityNodeObject::changeValueByStep): Use shortest instead of fixed.
(WebCore::AccessibilityNodeObject::changeValueByPercent): Ditto.
* css/CSSAspectRatioValue.cpp:
(WebCore::CSSAspectRatioValue::customCSSText const): Ditto.
* css/CSSFontVariationValue.cpp:
(WebCore::CSSFontVariationValue::customCSSText const): Ditto. Also use makeString instead of
StringBuilder for better efficiency.
* css/CSSGradientValue.cpp:
(WebCore::appendGradientStops): Ditto.
* css/CSSKeyframeRule.cpp:
(WebCore::StyleRuleKeyframe::keyText const): Ditto.
* css/CSSTimingFunctionValue.cpp:
(WebCore::CSSCubicBezierTimingFunctionValue::customCSSText const): Ditto.
* css/MediaQueryEvaluator.cpp:
(WebCore::aspectRatioValueAsString): Ditto.

* css/TransformFunctions.h: Removed unnneeded forward declarations.

* css/parser/CSSParserToken.cpp:
(WebCore::CSSParserToken::serialize const): Use shortest instead of fixed.
* html/HTMLImageElement.cpp:
(WebCore::HTMLImageElement::completeURLsInAttributeValue const): Ditto.

* html/track/VTTCue.cpp:
(WebCore::VTTCueBox::applyCSSProperties): Use shortest instead of fixed.
Also wrote a FIXME abot this strange code that uses "calc()" to do math on two numbers,
math that could instead be done by the code converting the numbers to a style string.

* inspector/InspectorOverlay.cpp:
(WebCore::InspectorOverlay::drawRulers): Use shortest instead of fixed.
* page/CaptionUserPreferencesMediaAF.cpp:
(WebCore::CaptionUserPreferencesMediaAF::windowRoundedCornerRadiusCSS const): Ditto.
* page/scrolling/AxisScrollSnapOffsets.cpp:
(WebCore::snapOffsetsToString): Ditto.
(WebCore::snapOffsetRangesToString): Ditto.
(WebCore::snapPortOrAreaToString): Ditto.

* platform/graphics/Color.cpp:
(WebCore::decimalDigit): Added.
(WebCore::serializedFractionDigitsForFractionalAlphaValue): Added.
(WebCore::Color::cssText const): Rewrote to generate the same results using
makeString rather than StringBuilder, and integer math rather than converting from
integer to floating point and then doing floating point math.

* platform/graphics/ExtendedColor.cpp:
(WebCore::ExtendedColor::cssText const): Use shortest instead of fixed.
* rendering/RenderLayerCompositor.cpp:
(WebCore::RenderLayerCompositor::logLayerInfo): Ditto.
* svg/SVGAngleValue.cpp:
(WebCore::SVGAngleValue::valueAsString const): Ditto.

* svg/SVGLengthList.h: Added now-needed include of StringBuilder.h.

* svg/SVGLengthValue.cpp:
(WebCore::SVGLengthValue::valueAsString const): Use shortest instead of fixed.
* svg/SVGNumberList.h: Ditto.

* svg/SVGPathStringBuilder.cpp:
(WebCore::appendFlag): Use multiple-argument append for better efficiency.
(WebCore::appendNumber): Added a comment about why we can't yet convert this to use
shortest instead of fixed: code that parses floats but then creates a CG path
that stores things as double and does math as double then converts back to float
results in float values that didn't round trip well and have fractions. This is
smoothed away (hidden) by using fixed precision to conver them to strings.
(WebCore::appendPoint): Call appendNumber to cut down on repeated code.

* svg/SVGPointList.h: Use shortest instead of fixed.

* svg/SVGTransformValue.h:
(WebCore::SVGTransformValue::prefixForTransfromType): Return a string literal
instead of a WTF::String to avoid creating and destroying an object each time.
(WebCore::SVGTransformValue::appendFixedPrecisionNumbers): Added a comment explaining
why we need to continue to use fixed precision here. Same issue with CGAffineTransform
using double as we have with CGPath above.

* svg/properties/SVGPropertyTraits.h:
(WebCore::SVGPropertyTraits<float>::toString): Use shortest instead of fixed.
(WebCore::SVGPropertyTraits<FloatPoint>::toString): Ditto.
(WebCore::SVGPropertyTraits<FloatRect>::toString): Ditto.

* testing/Internals.cpp:
(WebCore::Internals::dumpMarkerRects): Added a comment explaining why we have to use
fixed precision here. There are many test results that we would need to update.
(WebCore::Internals::configurationForViewport): Ditto.
(WebCore::Internals::getCurrentCursorInfo): Use shortest instead of fixed.
* xml/XPathValue.cpp:
(WebCore::XPath::Value::toString const): Ditto.

Source/WebKit:

* NetworkProcess/cache/NetworkCache.cpp:
(WebKit::NetworkCache::Cache::dumpContentsToFile): Use shortest instead of fixed.
Also use makeString instead of StringBuilder.

* NetworkProcess/cache/NetworkCacheEntry.cpp:
(WebKit::NetworkCache::Entry::asJSON const): Use shortest instead of fixed.
Also use multiple-argument append on StringBuilder to make the function shorter
and easier to read.
* Shared/Gamepad/GamepadData.cpp:
(WebKit::GamepadData::loggingString const): Ditto.

* UIProcess/ViewGestureController.cpp:
(WebKit::ViewGestureController::SnapshotRemovalTracker::startWatchdog):
Use shortest instead of fixed.

Source/WTF:

* wtf/Logger.h:
(WTF::LogArgument::toString): Log floating point numbers as shortest form instead of fixed precision.

* wtf/MediaTime.cpp:
(WTF::MediaTime::toString const): Convert time to string as shortest form instead of fixed precision.
Also use multiple-argument append for great simplicity and clarity.

LayoutTests:

* platform/mac/svg/dom/length-list-parser-expected.png: Removed. Not sure how many other pixel
results we have like this, but this included an ancient style Aqua scrollbar so hasn't matched
anything for years and would clearly not have any value for regression testing.

* svg/dom/length-list-parser-expected.txt: Updated to expect logging of a 7 digit length as an
integer rather than rounded as "d.ddddd+6".

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

42 files changed:
LayoutTests/ChangeLog
LayoutTests/platform/mac/svg/dom/length-list-parser-expected.png [deleted file]
LayoutTests/svg/dom/length-list-parser-expected.txt
Source/JavaScriptCore/API/tests/ExecutionTimeLimitTest.cpp
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/runtime/Options.cpp
Source/WTF/ChangeLog
Source/WTF/wtf/Logger.h
Source/WTF/wtf/MediaTime.cpp
Source/WebCore/ChangeLog
Source/WebCore/accessibility/AccessibilityNodeObject.cpp
Source/WebCore/css/CSSAspectRatioValue.cpp
Source/WebCore/css/CSSFontVariationValue.cpp
Source/WebCore/css/CSSGradientValue.cpp
Source/WebCore/css/CSSKeyframeRule.cpp
Source/WebCore/css/CSSTimingFunctionValue.cpp
Source/WebCore/css/MediaQueryEvaluator.cpp
Source/WebCore/css/TransformFunctions.h
Source/WebCore/css/parser/CSSParserToken.cpp
Source/WebCore/html/HTMLImageElement.cpp
Source/WebCore/html/track/VTTCue.cpp
Source/WebCore/inspector/InspectorOverlay.cpp
Source/WebCore/page/CaptionUserPreferencesMediaAF.cpp
Source/WebCore/page/scrolling/AxisScrollSnapOffsets.cpp
Source/WebCore/platform/graphics/Color.cpp
Source/WebCore/platform/graphics/ExtendedColor.cpp
Source/WebCore/rendering/RenderLayerCompositor.cpp
Source/WebCore/svg/SVGAngleValue.cpp
Source/WebCore/svg/SVGLengthList.h
Source/WebCore/svg/SVGLengthValue.cpp
Source/WebCore/svg/SVGNumberList.h
Source/WebCore/svg/SVGPathStringBuilder.cpp
Source/WebCore/svg/SVGPointList.h
Source/WebCore/svg/SVGTransformValue.h
Source/WebCore/svg/properties/SVGPropertyTraits.h
Source/WebCore/testing/Internals.cpp
Source/WebCore/xml/XPathValue.cpp
Source/WebKit/ChangeLog
Source/WebKit/NetworkProcess/cache/NetworkCache.cpp
Source/WebKit/NetworkProcess/cache/NetworkCacheEntry.cpp
Source/WebKit/Shared/Gamepad/GamepadData.cpp
Source/WebKit/UIProcess/ViewGestureController.cpp

index f3a5358..3c94573 100644 (file)
@@ -1,3 +1,17 @@
+2020-01-13  Darin Adler  <darin@apple.com>
+
+        Use even more "shortest form" formatting, and less "fixed precision" and "fixed width"
+        https://bugs.webkit.org/show_bug.cgi?id=198918
+
+        Reviewed by Sam Weinig.
+
+        * platform/mac/svg/dom/length-list-parser-expected.png: Removed. Not sure how many other pixel
+        results we have like this, but this included an ancient style Aqua scrollbar so hasn't matched
+        anything for years and would clearly not have any value for regression testing.
+
+        * svg/dom/length-list-parser-expected.txt: Updated to expect logging of a 7 digit length as an
+        integer rather than rounded as "d.ddddd+6".
+
 2020-01-14  Chris Lord  <clord@igalia.com>
 
         [GTK][WPE] Failures in imported/w3c/web-platform-tests/2dcontext/imagebitmap due to missing OffscreenCanvas.copiedImage
diff --git a/LayoutTests/platform/mac/svg/dom/length-list-parser-expected.png b/LayoutTests/platform/mac/svg/dom/length-list-parser-expected.png
deleted file mode 100644 (file)
index 987fd45..0000000
Binary files a/LayoutTests/platform/mac/svg/dom/length-list-parser-expected.png and /dev/null differ
index eef0630..dd5689c 100644 (file)
@@ -4,7 +4,7 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE
 
 
 Parsed as 1 length(s) [ 8.847 ]: 8.847
-Parsed as 1 length(s) [ 8.62569e+6 ]: 8625686 3.0-0.00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001, 90,e4997..,782-4326e.  -0.000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000013e,+e9     e 4829.,063721-0.00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001.-0.00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001e6 072
+Parsed as 1 length(s) [ 8625686 ]: 8625686 3.0-0.00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001, 90,e4997..,782-4326e.     -0.000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000013e,+e9     e 4829.,063721-0.00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001.-0.00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001e6 072
 Parsed as 1 length(s) [ 61822 ]: 61822,15-00.166.7 
 Parsed as 2 length(s) [ 2, 0.5 ]: 2    .5,658-7 e ,8477553
 Parsed as 3 length(s) [ 8, 2, 589 ]: 8 2 589   -45e58 -4       -0.00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001e58-++689.-98-,42 94564.,967-0.000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000011+ ,9+6-  .765+-87 e. 68  9452182-0.000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000016.. 1       +
index 68a3d41..a26595d 100644 (file)
@@ -198,12 +198,18 @@ int testExecutionTimeLimit()
         {
             Seconds timeAfterWatchdogShouldHaveFired = 300_ms + tierAdjustment;
 
-            StringBuilder scriptBuilder;
-            scriptBuilder.appendLiteral("function foo() { var startTime = currentCPUTime(); while (true) { for (var i = 0; i < 1000; i++); if (currentCPUTime() - startTime > ");
-            scriptBuilder.append(FormattedNumber::fixedPrecision(timeAfterWatchdogShouldHaveFired.seconds()));
-            scriptBuilder.appendLiteral(") break; } } foo();");
-
-            JSStringRef script = JSStringCreateWithUTF8CString(scriptBuilder.toString().utf8().data());
+            CString scriptText = makeString(
+                "function foo() {"
+                    "var startTime = currentCPUTime();"
+                    "while (true) {"
+                        "for (var i = 0; i < 1000; i++);"
+                        "if (currentCPUTime() - startTime > ", timeAfterWatchdogShouldHaveFired.seconds(), ") break;"
+                    "}"
+                "}"
+                "foo();"
+            ).utf8();
+
+            JSStringRef script = JSStringCreateWithUTF8CString(scriptText.data());
             exception = nullptr;
             shouldTerminateCallbackWasCalled = false;
             auto startTime = CPUTime::forCurrentThread();
@@ -235,19 +241,18 @@ int testExecutionTimeLimit()
         {
             Seconds timeAfterWatchdogShouldHaveFired = 300_ms + tierAdjustment;
 
-            StringBuilder scriptBuilder;
-            scriptBuilder.appendLiteral("var startTime = currentCPUTime();"
-                                 "function recurse(i) {"
-                                     "'use strict';"
-                                     "if (i % 1000 === 0) {"
-                                        "if (currentCPUTime() - startTime >");
-            scriptBuilder.append(FormattedNumber::fixedPrecision(timeAfterWatchdogShouldHaveFired.seconds()));
-            scriptBuilder.appendLiteral("       ) { return; }");
-            scriptBuilder.appendLiteral("    }");
-            scriptBuilder.appendLiteral("    return recurse(i + 1); }");
-            scriptBuilder.appendLiteral("recurse(0);");
-
-            JSStringRef script = JSStringCreateWithUTF8CString(scriptBuilder.toString().utf8().data());
+            CString scriptText = makeString(
+                "var startTime = currentCPUTime();"
+                "function recurse(i) {"
+                    "'use strict';"
+                    "if (i % 1000 === 0) {"
+                        "if (currentCPUTime() - startTime >", timeAfterWatchdogShouldHaveFired.seconds(), ") { return; }"
+                    "}"
+                "return recurse(i + 1); }"
+                "recurse(0);"
+            ).utf8();
+
+            JSStringRef script = JSStringCreateWithUTF8CString(scriptText.data());
             exception = nullptr;
             shouldTerminateCallbackWasCalled = false;
             auto startTime = CPUTime::forCurrentThread();
@@ -279,12 +284,20 @@ int testExecutionTimeLimit()
         {
             Seconds timeAfterWatchdogShouldHaveFired = 300_ms + tierAdjustment;
             
-            StringBuilder scriptBuilder;
-            scriptBuilder.appendLiteral("function foo() { var startTime = currentCPUTime(); try { while (true) { for (var i = 0; i < 1000; i++); if (currentCPUTime() - startTime > ");
-            scriptBuilder.append(FormattedNumber::fixedPrecision(timeAfterWatchdogShouldHaveFired.seconds()));
-            scriptBuilder.appendLiteral(") break; } } catch(e) { } } foo();");
-
-            JSStringRef script = JSStringCreateWithUTF8CString(scriptBuilder.toString().utf8().data());
+            CString scriptText = makeString(
+                "function foo() {"
+                    "var startTime = currentCPUTime();"
+                    "try {"
+                        "while (true) {"
+                            "for (var i = 0; i < 1000; i++);"
+                                "if (currentCPUTime() - startTime > ", timeAfterWatchdogShouldHaveFired.seconds(), ") break;"
+                        "}"
+                    "} catch(e) { }"
+                "}"
+                "foo();"
+            ).utf8();
+
+            JSStringRef script = JSStringCreateWithUTF8CString(scriptText.data());
             exception = nullptr;
             shouldTerminateCallbackWasCalled = false;
 
@@ -318,12 +331,18 @@ int testExecutionTimeLimit()
         {
             Seconds timeAfterWatchdogShouldHaveFired = 300_ms + tierAdjustment;
             
-            StringBuilder scriptBuilder;
-            scriptBuilder.appendLiteral("function foo() { var startTime = currentCPUTime(); while (true) { for (var i = 0; i < 1000; i++); if (currentCPUTime() - startTime > ");
-            scriptBuilder.append(FormattedNumber::fixedPrecision(timeAfterWatchdogShouldHaveFired.seconds()));
-            scriptBuilder.appendLiteral(") break; } } foo();");
+            CString scriptText = makeString(
+                "function foo() {"
+                    "var startTime = currentCPUTime();"
+                    "while (true) {"
+                        "for (var i = 0; i < 1000; i++);"
+                            "if (currentCPUTime() - startTime > ", timeAfterWatchdogShouldHaveFired.seconds(), ") break;"
+                    "}"
+                "}"
+                "foo();"
+            ).utf8();
             
-            JSStringRef script = JSStringCreateWithUTF8CString(scriptBuilder.toString().utf8().data());
+            JSStringRef script = JSStringCreateWithUTF8CString(scriptText.data());
             exception = nullptr;
             shouldTerminateCallbackWasCalled = false;
 
@@ -357,12 +376,18 @@ int testExecutionTimeLimit()
         {
             Seconds timeAfterWatchdogShouldHaveFired = 300_ms + tierAdjustment;
             
-            StringBuilder scriptBuilder;
-            scriptBuilder.appendLiteral("function foo() { var startTime = currentCPUTime(); while (true) { for (var i = 0; i < 1000; i++); if (currentCPUTime() - startTime > ");
-            scriptBuilder.append(FormattedNumber::fixedPrecision(timeAfterWatchdogShouldHaveFired.seconds()));
-            scriptBuilder.appendLiteral(") break; } } foo();");
-
-            JSStringRef script = JSStringCreateWithUTF8CString(scriptBuilder.toString().utf8().data());
+            CString scriptText = makeString(
+                "function foo() {"
+                    "var startTime = currentCPUTime();"
+                    "while (true) {"
+                        "for (var i = 0; i < 1000; i++);"
+                            "if (currentCPUTime() - startTime > ", timeAfterWatchdogShouldHaveFired.seconds(), ") break;"
+                    "}"
+                "}"
+                "foo();"
+            ).utf8();
+
+            JSStringRef script = JSStringCreateWithUTF8CString(scriptText.data());
             exception = nullptr;
             cancelTerminateCallbackWasCalled = false;
 
@@ -396,12 +421,18 @@ int testExecutionTimeLimit()
             Seconds timeAfterExtendedDeadline = 600_ms + tierAdjustment;
             Seconds maxBusyLoopTime = 750_ms + tierAdjustment;
 
-            StringBuilder scriptBuilder;
-            scriptBuilder.appendLiteral("function foo() { var startTime = currentCPUTime(); while (true) { for (var i = 0; i < 1000; i++); if (currentCPUTime() - startTime > ");
-            scriptBuilder.append(FormattedNumber::fixedPrecision(maxBusyLoopTime.seconds())); // in seconds.
-            scriptBuilder.appendLiteral(") break; } } foo();");
-
-            JSStringRef script = JSStringCreateWithUTF8CString(scriptBuilder.toString().utf8().data());
+            CString scriptText = makeString(
+                "function foo() {"
+                    "var startTime = currentCPUTime();"
+                    "while (true) {"
+                        "for (var i = 0; i < 1000; i++);"
+                            "if (currentCPUTime() - startTime > ", maxBusyLoopTime.seconds(), ") break;"
+                    "}"
+                "}"
+                "foo();"
+            ).utf8();
+
+            JSStringRef script = JSStringCreateWithUTF8CString(scriptText.data());
             exception = nullptr;
             extendTerminateCallbackCalled = 0;
 
@@ -439,12 +470,18 @@ int testExecutionTimeLimit()
         {
             Seconds timeAfterWatchdogShouldHaveFired = 300_ms + tierAdjustment;
 
-            StringBuilder scriptBuilder;
-            scriptBuilder.appendLiteral("function foo() { var startTime = currentCPUTime(); while (true) { for (var i = 0; i < 1000; i++); if (currentCPUTime() - startTime > ");
-            scriptBuilder.append(FormattedNumber::fixedPrecision(timeAfterWatchdogShouldHaveFired.seconds()));
-            scriptBuilder.appendLiteral(") break; } } foo();");
-
-            JSStringRef script = JSStringCreateWithUTF8CString(scriptBuilder.toString().utf8().data());
+            CString scriptText = makeString(
+                "function foo() {"
+                    "var startTime = currentCPUTime();"
+                    "while (true) {"
+                        "for (var i = 0; i < 1000; i++);"
+                            "if (currentCPUTime() - startTime > ", timeAfterWatchdogShouldHaveFired.seconds(), ") break;"
+                    "}"
+                "}"
+                "foo();"
+            ).utf8();
+
+            JSStringRef script = JSStringCreateWithUTF8CString(scriptText.data());
             exception = nullptr;
             dispatchTerminateCallbackCalled = false;
 
index cdbea88..796d7ee 100644 (file)
@@ -1,3 +1,18 @@
+2020-01-13  Darin Adler  <darin@apple.com>
+
+        Use even more "shortest form" formatting, and less "fixed precision" and "fixed width"
+        https://bugs.webkit.org/show_bug.cgi?id=198918
+
+        Reviewed by Sam Weinig.
+
+        * API/tests/ExecutionTimeLimitTest.cpp:
+        (testExecutionTimeLimit): Rewrote the string creation code to use makeString instead
+        of StringBuilder and no longer use any fixed precision.
+
+        * runtime/Options.cpp:
+        (JSC::OptionReader::Option::dump const): Dump doubles with shortest form instead of
+        fixed precision.
+
 2020-01-14  David Kilzer  <ddkilzer@apple.com>
 
         Enable -Wconditional-uninitialized in bmalloc, WTF, JavaScriptCore
index af72f97..d890e86 100644 (file)
@@ -1005,7 +1005,7 @@ void OptionReader::Option::dump(StringBuilder& builder) const
         builder.appendNumber(m_size);
         break;
     case Options::Type::Double:
-        builder.append(FormattedNumber::fixedPrecision(m_double));
+        builder.append(m_double);
         break;
     case Options::Type::Int32:
         builder.appendNumber(m_int32);
index 45808e6..fce9626 100644 (file)
@@ -1,3 +1,17 @@
+2020-01-13  Darin Adler  <darin@apple.com>
+
+        Use even more "shortest form" formatting, and less "fixed precision" and "fixed width"
+        https://bugs.webkit.org/show_bug.cgi?id=198918
+
+        Reviewed by Sam Weinig.
+
+        * wtf/Logger.h:
+        (WTF::LogArgument::toString): Log floating point numbers as shortest form instead of fixed precision.
+
+        * wtf/MediaTime.cpp:
+        (WTF::MediaTime::toString const): Convert time to string as shortest form instead of fixed precision.
+        Also use multiple-argument append for great simplicity and clarity.
+
 2020-01-14  Alicia Boya García  <aboya@igalia.com>
 
         [WTF] Make MediaTime constructor constexpr
index 02da704..94660e5 100644 (file)
@@ -41,8 +41,8 @@ struct LogArgument {
     template<typename U = T> static typename std::enable_if<std::is_same<U, unsigned long long>::value, String>::type toString(unsigned long long argument) { return String::number(argument); }
     template<typename U = T> static typename std::enable_if<std::is_same<U, long long>::value, String>::type toString(long long argument) { return String::number(argument); }
     template<typename U = T> static typename std::enable_if<std::is_enum<U>::value, String>::type toString(U argument) { return String::number(static_cast<typename std::underlying_type<U>::type>(argument)); }
-    template<typename U = T> static typename std::enable_if<std::is_same<U, float>::value, String>::type toString(float argument) { return String::numberToStringFixedPrecision(argument); }
-    template<typename U = T> static typename std::enable_if<std::is_same<U, double>::value, String>::type toString(double argument) { return String::numberToStringFixedPrecision(argument); }
+    template<typename U = T> static typename std::enable_if<std::is_same<U, float>::value, String>::type toString(float argument) { return String::number(argument); }
+    template<typename U = T> static typename std::enable_if<std::is_same<U, double>::value, String>::type toString(double argument) { return String::number(argument); }
     template<typename U = T> static typename std::enable_if<std::is_same<typename std::remove_reference<U>::type, AtomString>::value, String>::type toString(const AtomString& argument) { return argument.string(); }
     template<typename U = T> static typename std::enable_if<std::is_same<typename std::remove_reference<U>::type, String>::value, String>::type toString(String argument) { return argument; }
     template<typename U = T> static typename std::enable_if<std::is_same<typename std::remove_reference<U>::type, StringBuilder*>::value, String>::type toString(StringBuilder* argument) { return argument->toString(); }
index a3f6252..13e0899 100644 (file)
@@ -566,15 +566,10 @@ void MediaTime::dump(PrintStream& out) const
 String MediaTime::toString() const
 {
     StringBuilder builder;
-
     builder.append('{');
-    if (!hasDoubleValue()) {
-        builder.appendNumber(m_timeValue);
-        builder.append('/');
-        builder.appendNumber(m_timeScale);
-        builder.appendLiteral(" = ");
-    }
-    builder.append(FormattedNumber::fixedPrecision(toDouble()));
+    if (!hasDoubleValue())
+        builder.append(m_timeValue, '/', m_timeScale, " = ");
+    builder.append(toDouble());
     if (isInvalid())
         builder.appendLiteral(", invalid");
     builder.append('}');
index 26c8e58..839e2d5 100644 (file)
@@ -1,3 +1,107 @@
+2020-01-13  Darin Adler  <darin@apple.com>
+
+        Use even more "shortest form" formatting, and less "fixed precision" and "fixed width"
+        https://bugs.webkit.org/show_bug.cgi?id=198918
+
+        Reviewed by Sam Weinig.
+
+        The places left untouched are the ones where changing behavior has some kind of unwanted
+        observable effect for one of two reasons. Otherwise, switched almost all call sites.
+
+        1) Substantial number of test results that depend on the current behavior.
+
+        2) Poor rounding resulting in conversion from float to double and back (or similar) that
+           results in values with tiny fractional residue like "6.000001".
+
+        * accessibility/AccessibilityNodeObject.cpp:
+        (WebCore::AccessibilityNodeObject::changeValueByStep): Use shortest instead of fixed.
+        (WebCore::AccessibilityNodeObject::changeValueByPercent): Ditto.
+        * css/CSSAspectRatioValue.cpp:
+        (WebCore::CSSAspectRatioValue::customCSSText const): Ditto.
+        * css/CSSFontVariationValue.cpp:
+        (WebCore::CSSFontVariationValue::customCSSText const): Ditto. Also use makeString instead of
+        StringBuilder for better efficiency.
+        * css/CSSGradientValue.cpp:
+        (WebCore::appendGradientStops): Ditto.
+        * css/CSSKeyframeRule.cpp:
+        (WebCore::StyleRuleKeyframe::keyText const): Ditto.
+        * css/CSSTimingFunctionValue.cpp:
+        (WebCore::CSSCubicBezierTimingFunctionValue::customCSSText const): Ditto.
+        * css/MediaQueryEvaluator.cpp:
+        (WebCore::aspectRatioValueAsString): Ditto.
+
+        * css/TransformFunctions.h: Removed unnneeded forward declarations.
+
+        * css/parser/CSSParserToken.cpp:
+        (WebCore::CSSParserToken::serialize const): Use shortest instead of fixed.
+        * html/HTMLImageElement.cpp:
+        (WebCore::HTMLImageElement::completeURLsInAttributeValue const): Ditto.
+
+        * html/track/VTTCue.cpp:
+        (WebCore::VTTCueBox::applyCSSProperties): Use shortest instead of fixed.
+        Also wrote a FIXME abot this strange code that uses "calc()" to do math on two numbers,
+        math that could instead be done by the code converting the numbers to a style string.
+
+        * inspector/InspectorOverlay.cpp:
+        (WebCore::InspectorOverlay::drawRulers): Use shortest instead of fixed.
+        * page/CaptionUserPreferencesMediaAF.cpp:
+        (WebCore::CaptionUserPreferencesMediaAF::windowRoundedCornerRadiusCSS const): Ditto.
+        * page/scrolling/AxisScrollSnapOffsets.cpp:
+        (WebCore::snapOffsetsToString): Ditto.
+        (WebCore::snapOffsetRangesToString): Ditto.
+        (WebCore::snapPortOrAreaToString): Ditto.
+
+        * platform/graphics/Color.cpp:
+        (WebCore::decimalDigit): Added.
+        (WebCore::serializedFractionDigitsForFractionalAlphaValue): Added.
+        (WebCore::Color::cssText const): Rewrote to generate the same results using
+        makeString rather than StringBuilder, and integer math rather than converting from
+        integer to floating point and then doing floating point math.
+
+        * platform/graphics/ExtendedColor.cpp:
+        (WebCore::ExtendedColor::cssText const): Use shortest instead of fixed.
+        * rendering/RenderLayerCompositor.cpp:
+        (WebCore::RenderLayerCompositor::logLayerInfo): Ditto.
+        * svg/SVGAngleValue.cpp:
+        (WebCore::SVGAngleValue::valueAsString const): Ditto.
+
+        * svg/SVGLengthList.h: Added now-needed include of StringBuilder.h.
+
+        * svg/SVGLengthValue.cpp:
+        (WebCore::SVGLengthValue::valueAsString const): Use shortest instead of fixed.
+        * svg/SVGNumberList.h: Ditto.
+
+        * svg/SVGPathStringBuilder.cpp:
+        (WebCore::appendFlag): Use multiple-argument append for better efficiency.
+        (WebCore::appendNumber): Added a comment about why we can't yet convert this to use
+        shortest instead of fixed: code that parses floats but then creates a CG path
+        that stores things as double and does math as double then converts back to float
+        results in float values that didn't round trip well and have fractions. This is
+        smoothed away (hidden) by using fixed precision to conver them to strings.
+        (WebCore::appendPoint): Call appendNumber to cut down on repeated code.
+
+        * svg/SVGPointList.h: Use shortest instead of fixed.
+
+        * svg/SVGTransformValue.h:
+        (WebCore::SVGTransformValue::prefixForTransfromType): Return a string literal
+        instead of a WTF::String to avoid creating and destroying an object each time.
+        (WebCore::SVGTransformValue::appendFixedPrecisionNumbers): Added a comment explaining
+        why we need to continue to use fixed precision here. Same issue with CGAffineTransform
+        using double as we have with CGPath above.
+
+        * svg/properties/SVGPropertyTraits.h:
+        (WebCore::SVGPropertyTraits<float>::toString): Use shortest instead of fixed.
+        (WebCore::SVGPropertyTraits<FloatPoint>::toString): Ditto.
+        (WebCore::SVGPropertyTraits<FloatRect>::toString): Ditto.
+
+        * testing/Internals.cpp:
+        (WebCore::Internals::dumpMarkerRects): Added a comment explaining why we have to use
+        fixed precision here. There are many test results that we would need to update.
+        (WebCore::Internals::configurationForViewport): Ditto.
+        (WebCore::Internals::getCurrentCursorInfo): Use shortest instead of fixed.
+        * xml/XPathValue.cpp:
+        (WebCore::XPath::Value::toString const): Ditto.
+
 2020-01-14  Peng Liu  <peng.liu6@apple.com>
 
         A video element cannot enter fullscreen from PiP mode
index 16b6240..4dc73ec 100644 (file)
@@ -1104,7 +1104,7 @@ void AccessibilityNodeObject::changeValueByStep(bool increase)
 
     value += increase ? step : -step;
 
-    setValue(String::numberToStringFixedPrecision(value));
+    setValue(String::number(value));
 
     auto objectCache = axObjectCache();
     if (objectCache)
@@ -1122,7 +1122,7 @@ void AccessibilityNodeObject::changeValueByPercent(float percentChange)
         step = std::abs(percentChange) * (1 / percentChange);
 
     value += step;
-    setValue(String::numberToStringFixedPrecision(value));
+    setValue(String::number(value));
 
     auto objectCache = axObjectCache();
     if (objectCache)
index 7856097..e8251fc 100644 (file)
@@ -35,7 +35,7 @@ namespace WebCore {
 
 String CSSAspectRatioValue::customCSSText() const
 {
-    return makeString(FormattedNumber::fixedPrecision(m_numeratorValue), '/', FormattedNumber::fixedPrecision(m_denominatorValue));
+    return makeString(m_numeratorValue, '/', m_denominatorValue);
 }
 
 bool CSSAspectRatioValue::equals(const CSSAspectRatioValue& other) const
index 4372a73..6630e44 100644 (file)
@@ -29,8 +29,6 @@
 
 #include "CSSFontVariationValue.h"
 
-#include <wtf/text/StringBuilder.h>
-
 namespace WebCore {
 
 CSSFontVariationValue::CSSFontVariationValue(FontTag tag, float value)
@@ -42,13 +40,7 @@ CSSFontVariationValue::CSSFontVariationValue(FontTag tag, float value)
 
 String CSSFontVariationValue::customCSSText() const
 {
-    StringBuilder builder;
-    builder.append('"');
-    for (char c : m_tag)
-        builder.append(c);
-    builder.appendLiteral("\" ");
-    builder.append(FormattedNumber::fixedPrecision(m_value));
-    return builder.toString();
+    return makeString('"', m_tag[0], m_tag[1], m_tag[2], m_tag[3], "\" ", m_value);
 }
 
 bool CSSFontVariationValue::equals(const CSSFontVariationValue& other) const
index 454e3ea..75d8095 100644 (file)
@@ -700,7 +700,7 @@ static void appendGradientStops(StringBuilder& builder, const Vector<CSSGradient
         else if (position == 1)
             builder.append(", to(", stop.m_color->cssText(), ')');
         else
-            builder.append(", color-stop(", FormattedNumber::fixedPrecision(position), ", ", stop.m_color->cssText(), ')');
+            builder.append(", color-stop(", position, ", ", stop.m_color->cssText(), ')');
     }
 }
 
index 4d700aa..807192a 100644 (file)
@@ -59,14 +59,11 @@ MutableStyleProperties& StyleRuleKeyframe::mutableProperties()
 String StyleRuleKeyframe::keyText() const
 {
     StringBuilder keyText;
-
     for (size_t i = 0; i < m_keys.size(); ++i) {
         if (i)
             keyText.append(',');
-        keyText.append(FormattedNumber::fixedPrecision(m_keys.at(i) * 100));
-        keyText.append('%');
+        keyText.append(m_keys[i] * 100, '%');
     }
-
     return keyText.toString();
 }
     
index 0b54c91..ff4ba4e 100644 (file)
@@ -32,17 +32,7 @@ namespace WebCore {
 
 String CSSCubicBezierTimingFunctionValue::customCSSText() const
 {
-    StringBuilder builder;
-    builder.appendLiteral("cubic-bezier(");
-    builder.append(FormattedNumber::fixedPrecision(m_x1));
-    builder.appendLiteral(", ");
-    builder.append(FormattedNumber::fixedPrecision(m_y1));
-    builder.appendLiteral(", ");
-    builder.append(FormattedNumber::fixedPrecision(m_x2));
-    builder.appendLiteral(", ");
-    builder.append(FormattedNumber::fixedPrecision(m_y2));
-    builder.append(')');    
-    return builder.toString();
+    return makeString("cubic-bezier(", m_x1, ", ", m_y1, ", ", m_x2, ", ", m_y2, ')');
 }
 
 bool CSSCubicBezierTimingFunctionValue::equals(const CSSCubicBezierTimingFunctionValue& other) const
index 32870f2..2ac1630 100644 (file)
@@ -240,7 +240,7 @@ static String aspectRatioValueAsString(CSSValue* value)
         return emptyString();
 
     auto& aspectRatio = downcast<CSSAspectRatioValue>(*value);
-    return makeString(FormattedNumber::fixedWidth(aspectRatio.numeratorValue(), 6), '/', FormattedNumber::fixedWidth(aspectRatio.denominatorValue(), 6));
+    return makeString(aspectRatio.numeratorValue(), '/', aspectRatio.denominatorValue());
 }
 
 #endif
index 5a320c3..84c6b9d 100644 (file)
@@ -36,9 +36,6 @@ namespace WebCore {
 class CSSPrimitiveValue;
 class CSSToLengthConversionData;
 class CSSValue;
-class RenderStyle;
-
-struct Length;
 
 bool transformsForValue(const CSSValue&, const CSSToLengthConversionData&, TransformOperations&);
 Length convertToFloatLength(const CSSPrimitiveValue*, const CSSToLengthConversionData&);
index e820907..f5b2499 100644 (file)
@@ -421,15 +421,14 @@ void CSSParserToken::serialize(StringBuilder& builder) const
         // These won't properly preserve the NumericValueType flag
         if (m_numericSign == PlusSign)
             builder.append('+');
-        builder.append(FormattedNumber::fixedPrecision(numericValue()));
+        builder.append(numericValue());
         break;
     case PercentageToken:
-        builder.append(FormattedNumber::fixedPrecision(numericValue()));
-        builder.append('%');
+        builder.append(numericValue(), '%');
         break;
     case DimensionToken:
         // This will incorrectly serialize e.g. 4e3e2 as 4000e2
-        builder.append(FormattedNumber::fixedPrecision(numericValue()));
+        builder.append(numericValue());
         serializeIdentifier(value().toString(), builder);
         break;
     case UnicodeRangeToken:
index 3cb1eb6..b2702cf 100755 (executable)
@@ -584,16 +584,10 @@ String HTMLImageElement::completeURLsInAttributeValue(const URL& base, const Att
             if (&candidate != &imageCandidates[0])
                 result.appendLiteral(", ");
             result.append(URL(base, candidate.string.toString()).string());
-            if (candidate.density != UninitializedDescriptor) {
-                result.append(' ');
-                result.append(FormattedNumber::fixedPrecision(candidate.density));
-                result.append('x');
-            }
-            if (candidate.resourceWidth != UninitializedDescriptor) {
-                result.append(' ');
-                result.appendNumber(candidate.resourceWidth);
-                result.append('w');
-            }
+            if (candidate.density != UninitializedDescriptor)
+                result.append(' ', candidate.density, 'x');
+            if (candidate.resourceWidth != UninitializedDescriptor)
+                result.append(' ', candidate.resourceWidth, 'w');
         }
         return result.toString();
     }
index a06001b..ec4984e 100644 (file)
@@ -169,8 +169,10 @@ void VTTCueBox::applyCSSProperties(const IntSize& videoSize)
     // the 'left' property must be set to left
     if (cue->vertical() == horizontalKeyword())
         setInlineStyleProperty(CSSPropertyLeft, position.first, CSSUnitType::CSS_PERCENTAGE);
-    else if (cue->vertical() == verticalGrowingRightKeyword())
-        setInlineStyleProperty(CSSPropertyLeft, makeString("calc(-", FormattedNumber::fixedWidth(videoSize.width(), 2), "px - ", FormattedNumber::fixedWidth(cue->getCSSSize(), 2), "px)"));
+    else if (cue->vertical() == verticalGrowingRightKeyword()) {
+        // FIXME: Why use calc to do the math instead of doing the subtraction here?
+        setInlineStyleProperty(CSSPropertyLeft, makeString("calc(-", videoSize.width(), "px - ", cue->getCSSSize(), "px)"));
+    }
 
     double authorFontSize = std::min(videoSize.width(), videoSize.height()) * DEFAULTCAPTIONFONTSIZEPERCENTAGE / 100.0;
     double multiplier = 1.0;
@@ -220,8 +222,7 @@ void VTTCueBox::applyCSSProperties(const IntSize& videoSize)
         // of the way across the height of the video's rendering area, while
         // maintaining the relative positions of the boxes in boxes to each
         // other.
-        setInlineStyleProperty(CSSPropertyTransform,
-            makeString("translate(", FormattedNumber::fixedWidth(-position.first, 2), "%, ", FormattedNumber::fixedWidth(-position.second, 2), "%)"));
+        setInlineStyleProperty(CSSPropertyTransform, makeString("translate(", -position.first, "%, ", -position.second, "%)"));
 
         setInlineStyleProperty(CSSPropertyWhiteSpace, CSSValuePre);
     }
index 6ebdda3..b045526 100644 (file)
@@ -791,7 +791,7 @@ void InspectorOverlay::drawRulers(GraphicsContext& context, const InspectorOverl
 
                 GraphicsContextStateSaver verticalLabelStateSaver(context);
                 context.translate(zoom(x) + 0.5f, scrollY);
-                context.drawText(font, TextRun(String::numberToStringFixedPrecision(x)), { 2, drawTopEdge ? rulerLabelSize : rulerLabelSize - rulerSize + font.fontMetrics().height() - 1.0f });
+                context.drawText(font, TextRun(String::number(x)), { 2, drawTopEdge ? rulerLabelSize : rulerLabelSize - rulerSize + font.fontMetrics().height() - 1.0f });
             }
         }
 
@@ -829,7 +829,7 @@ void InspectorOverlay::drawRulers(GraphicsContext& context, const InspectorOverl
                 GraphicsContextStateSaver horizontalLabelStateSaver(context);
                 context.translate(scrollX, zoom(y) + 0.5f);
                 context.rotate(drawLeftEdge ? -piOverTwoFloat : piOverTwoFloat);
-                context.drawText(font, TextRun(String::numberToStringFixedPrecision(y)), { 2, drawLeftEdge ? rulerLabelSize : rulerLabelSize - rulerSize });
+                context.drawText(font, TextRun(String::number(y)), { 2, drawLeftEdge ? rulerLabelSize : rulerLabelSize - rulerSize });
             }
         }
     }
@@ -844,9 +844,7 @@ void InspectorOverlay::drawRulers(GraphicsContext& context, const InspectorOverl
         font.update(nullptr);
 
         auto viewportRect = pageView->visualViewportRect();
-        auto viewportWidthText = String::numberToStringFixedPrecision(viewportRect.width() / pageZoomFactor);
-        auto viewportHeightText = String::numberToStringFixedPrecision(viewportRect.height() / pageZoomFactor);
-        TextRun viewportTextRun(makeString(viewportWidthText, "px", ' ', multiplicationSign, ' ', viewportHeightText, "px"));
+        TextRun viewportTextRun(makeString(viewportRect.width() / pageZoomFactor, "px", ' ', multiplicationSign, ' ', viewportRect.height() / pageZoomFactor, "px"));
 
         const float margin = 4;
         const float padding = 2;
index a9ab5f9..4bd9b94 100644 (file)
@@ -351,7 +351,7 @@ String CaptionUserPreferencesMediaAF::windowRoundedCornerRadiusCSS() const
         return emptyString();
 
     StringBuilder builder;
-    appendCSS(builder, CSSPropertyBorderRadius, makeString(FormattedNumber::fixedWidth(radius, 2), "px"), behavior == kMACaptionAppearanceBehaviorUseValue);
+    appendCSS(builder, CSSPropertyBorderRadius, makeString(radius, "px"), behavior == kMACaptionAppearanceBehaviorUseValue);
     return builder.toString();
 }
 
index 76f0467..d26cefe 100644 (file)
@@ -78,10 +78,8 @@ static String snapOffsetsToString(const Vector<LayoutUnit>& snapOffsets)
 {
     StringBuilder builder;
     builder.appendLiteral("[ ");
-    for (auto& offset : snapOffsets) {
-        builder.append(FormattedNumber::fixedWidth(offset.toFloat(), 1));
-        builder.append(' ');
-    }
+    for (auto& offset : snapOffsets)
+        builder.append(offset.toFloat(), ' ');
     builder.append(']');
     return builder.toString();
 }
@@ -90,24 +88,15 @@ static String snapOffsetRangesToString(const Vector<ScrollOffsetRange<LayoutUnit
 {
     StringBuilder builder;
     builder.appendLiteral("[ ");
-    for (auto& range : ranges) {
-        builder.append('(');
-        builder.append(FormattedNumber::fixedWidth(range.start.toFloat(), 1));
-        builder.appendLiteral(", ");
-        builder.append(FormattedNumber::fixedWidth(range.end.toFloat(), 1));
-        builder.appendLiteral(") ");
-    }
+    for (auto& range : ranges)
+        builder.append('(', range.start.toFloat(), ", ", range.end.toFloat(), ") ");
     builder.append(']');
     return builder.toString();
 }
 
 static String snapPortOrAreaToString(const LayoutRect& rect)
 {
-    return makeString("{{",
-        FormattedNumber::fixedWidth(rect.x(), 1), ", ",
-        FormattedNumber::fixedWidth(rect.y(), 1), "} {",
-        FormattedNumber::fixedWidth(rect.width(), 1), ", ",
-        FormattedNumber::fixedWidth(rect.height(), 1), "}}");
+    return makeString("{{", rect.x().toFloat(), ", ", rect.y().toFloat(), "} {", rect.width().toFloat(), ", ", rect.height().toFloat(), "}}");
 }
 
 #endif
index 8805b6c..1660764 100644 (file)
@@ -320,38 +320,36 @@ String Color::serialized() const
     return cssText();
 }
 
+static char decimalDigit(unsigned number)
+{
+    ASSERT(number < 10);
+    return '0' + number;
+}
+
+static std::array<char, 4> serializedFractionDigitsForFractionalAlphaValue(uint8_t alpha)
+{
+    ASSERT(alpha > 0);
+    ASSERT(alpha < 0xFF);
+    if (((alpha * 100 + 0x7F) / 0xFF * 0xFF + 50) / 100 != alpha)
+        return { { decimalDigit(alpha * 10 / 0xFF % 10), decimalDigit(alpha * 100 / 0xFF % 10), decimalDigit((alpha * 1000 + 0x7F) / 0xFF % 10), '\0' } };
+    if (int thirdDigit = (alpha * 100 + 0x7F) / 0xFF % 10)
+        return { { decimalDigit(alpha * 10 / 0xFF), decimalDigit(thirdDigit), '\0', '\0' } };
+    return { { decimalDigit((alpha * 10 + 0x7F) / 0xFF), '\0', '\0', '\0' } };
+}
+
 String Color::cssText() const
 {
     if (isExtended())
         return asExtended().cssText();
-
-    StringBuilder builder;
-    builder.reserveCapacity(28);
-    bool colorHasAlpha = !isOpaque();
-    if (colorHasAlpha)
-        builder.appendLiteral("rgba(");
-    else
-        builder.appendLiteral("rgb(");
-
-    builder.appendNumber(static_cast<unsigned char>(red()));
-    builder.appendLiteral(", ");
-
-    builder.appendNumber(static_cast<unsigned char>(green()));
-    builder.appendLiteral(", ");
-
-    builder.appendNumber(static_cast<unsigned char>(blue()));
-    if (colorHasAlpha) {
-        // https://drafts.csswg.org/cssom/#serializing-css-values
-        builder.appendLiteral(", ");
-        int alpha = this->alpha();
-        float rounded = round(alpha * 100 / 255.0f) / 100;
-        if (round(rounded * 255) != alpha)
-            rounded = round(alpha * 1000 / 255.0f) / 1000;
-        builder.append(FormattedNumber::fixedPrecision(rounded));
+    uint8_t alpha = this->alpha();
+    switch (alpha) {
+    case 0:
+        return makeString("rgba(", red(), ", ", green(), ", ", blue(), ", 0)");
+    case 0xFF:
+        return makeString("rgb(", red(), ", ", green(), ", ", blue(), ')');
+    default:
+        return makeString("rgba(", red(), ", ", green(), ", ", blue(), ", 0.", serializedFractionDigitsForFractionalAlphaValue(alpha).data(), ')');
     }
-        
-    builder.append(')');
-    return builder.toString();
 }
 
 String Color::nameForRenderTreeAsText() const
index e09d287..a481cce 100644 (file)
 #include "config.h"
 #include "ExtendedColor.h"
 
-#include "ColorSpace.h"
 #include <wtf/MathExtras.h>
-#include <wtf/dtoa.h>
-#include <wtf/text/StringBuilder.h>
 
 namespace WebCore {
 
@@ -40,36 +37,22 @@ Ref<ExtendedColor> ExtendedColor::create(float red, float green, float blue, flo
 
 String ExtendedColor::cssText() const
 {
-    StringBuilder builder;
-    builder.reserveCapacity(40);
-    builder.appendLiteral("color(");
-
+    const char* colorSpace;
     switch (m_colorSpace) {
     case ColorSpace::SRGB:
-        builder.appendLiteral("srgb ");
+        colorSpace = "srgb";
         break;
     case ColorSpace::DisplayP3:
-        builder.appendLiteral("display-p3 ");
+        colorSpace = "display-p3";
         break;
     default:
         ASSERT_NOT_REACHED();
         return WTF::emptyString();
     }
 
-    builder.append(FormattedNumber::fixedPrecision(red()));
-    builder.append(' ');
-
-    builder.append(FormattedNumber::fixedPrecision(green()));
-    builder.append(' ');
-
-    builder.append(FormattedNumber::fixedPrecision(blue()));
-    if (!WTF::areEssentiallyEqual(alpha(), 1.0f)) {
-        builder.appendLiteral(" / ");
-        builder.append(FormattedNumber::fixedPrecision(alpha()));
-    }
-    builder.append(')');
-
-    return builder.toString();
+    if (WTF::areEssentiallyEqual(alpha(), 1.0f))
+        return makeString("color(", colorSpace, ' ', red(), ' ', green(), ' ', blue(), ')');
+    return makeString("color(", colorSpace, ' ', red(), ' ', green(), ' ', blue(), " / ", alpha(), ')');
 }
 
 }
index 61a039f..4e92e6e 100644 (file)
@@ -1391,7 +1391,7 @@ void RenderLayerCompositor::logLayerInfo(const RenderLayer& layer, const char* p
     absoluteBounds.move(layer.offsetFromAncestor(m_renderView.layer()));
     
     StringBuilder logString;
-    logString.append(pad(' ', 12 + depth * 2, hex(reinterpret_cast<uintptr_t>(&layer), Lowercase)), " id ", backing->graphicsLayer()->primaryLayerID(), " (", FormattedNumber::fixedWidth(absoluteBounds.x().toFloat(), 3), ',', FormattedNumber::fixedWidth(absoluteBounds.y().toFloat(), 3), '-', FormattedNumber::fixedWidth(absoluteBounds.maxX().toFloat(), 3), ',', FormattedNumber::fixedWidth(absoluteBounds.maxY().toFloat(), 3), ") ", FormattedNumber::fixedWidth(backing->backingStoreMemoryEstimate() / 1024, 2), "KB");
+    logString.append(pad(' ', 12 + depth * 2, hex(reinterpret_cast<uintptr_t>(&layer), Lowercase)), " id ", backing->graphicsLayer()->primaryLayerID(), " (", absoluteBounds.x().toFloat(), ',', absoluteBounds.y().toFloat(), '-', absoluteBounds.maxX().toFloat(), ',', absoluteBounds.maxY().toFloat(), ") ", FormattedNumber::fixedWidth(backing->backingStoreMemoryEstimate() / 1024, 2), "KB");
 
     if (!layer.renderer().style().hasAutoUsedZIndex())
         logString.append(" z-index: ", layer.renderer().style().usedZIndex());
index 1ea285f..85a7198 100644 (file)
@@ -66,14 +66,14 @@ String SVGAngleValue::valueAsString() const
 {
     switch (m_unitType) {
     case SVG_ANGLETYPE_DEG:
-        return makeString(FormattedNumber::fixedPrecision(m_valueInSpecifiedUnits), "deg");
+        return makeString(m_valueInSpecifiedUnits, "deg");
     case SVG_ANGLETYPE_RAD:
-        return makeString(FormattedNumber::fixedPrecision(m_valueInSpecifiedUnits), "rad");
+        return makeString(m_valueInSpecifiedUnits, "rad");
     case SVG_ANGLETYPE_GRAD:
-        return makeString(FormattedNumber::fixedPrecision(m_valueInSpecifiedUnits), "grad");
+        return makeString(m_valueInSpecifiedUnits, "grad");
     case SVG_ANGLETYPE_UNSPECIFIED:
     case SVG_ANGLETYPE_UNKNOWN:
-        return String::numberToStringFixedPrecision(m_valueInSpecifiedUnits);
+        return String::number(m_valueInSpecifiedUnits);
     }
 
     ASSERT_NOT_REACHED();
index 5eb5701..a82c76d 100644 (file)
@@ -27,6 +27,7 @@
 
 #include "SVGLength.h"
 #include "SVGValuePropertyList.h"
+#include <wtf/text/StringBuilder.h>
 
 namespace WebCore {
 
index 2314b87..c3126e3 100644 (file)
@@ -257,7 +257,7 @@ float SVGLengthValue::value(const SVGLengthContext& context) const
 
 String SVGLengthValue::valueAsString() const
 {
-    return makeString(FormattedNumber::fixedPrecision(m_valueInSpecifiedUnits), lengthTypeToString(m_lengthType));
+    return makeString(m_valueInSpecifiedUnits, lengthTypeToString(m_lengthType));
 }
 
 ExceptionOr<float> SVGLengthValue::valueForBindings(const SVGLengthContext& context) const
index 7946012..3cede35 100644 (file)
@@ -78,7 +78,7 @@ public:
             if (builder.length())
                 builder.append(' ');
 
-            builder.append(FormattedNumber::fixedPrecision(number->value()));
+            builder.append(number->value());
         }
 
         return builder.toString();
index 093467a..7a73045 100644 (file)
@@ -51,22 +51,19 @@ bool SVGPathStringBuilder::continueConsuming()
 
 static void appendFlag(StringBuilder& stringBuilder, bool flag)
 {
-    stringBuilder.append(flag ? '1' : '0');
-    stringBuilder.append(' ');
+    stringBuilder.append(flag ? '1' : '0', ' ');
 }
 
 static void appendNumber(StringBuilder& stringBuilder, float number)
 {
-    stringBuilder.append(FormattedNumber::fixedPrecision(number));
-    stringBuilder.append(' ');
+    // FIXME: Shortest form would be better, but fixed precision is required for now to smooth over precision errors caused by converting float to double and back in CGPath on Cocoa platforms.
+    stringBuilder.append(FormattedNumber::fixedPrecision(number), ' ');
 }
 
 static void appendPoint(StringBuilder& stringBuilder, const FloatPoint& point)
 {
-    stringBuilder.append(FormattedNumber::fixedPrecision(point.x()));
-    stringBuilder.append(' ');
-    stringBuilder.append(FormattedNumber::fixedPrecision(point.y()));
-    stringBuilder.append(' ');
+    appendNumber(stringBuilder, point.x());
+    appendNumber(stringBuilder, point.y());
 }
 
 void SVGPathStringBuilder::moveTo(const FloatPoint& targetPoint, bool, PathCoordinateMode mode)
index 01b3022..25d033a 100644 (file)
@@ -93,9 +93,7 @@ public:
             if (builder.length())
                 builder.append(' ');
 
-            builder.append(FormattedNumber::fixedPrecision(point->x()));
-            builder.append(' ');
-            builder.append(FormattedNumber::fixedPrecision(point->y()));
+            builder.append(point->x(), ' ', point->y());
         }
 
         return builder.toString();
index d390d09..60111aa 100644 (file)
@@ -1,7 +1,7 @@
 /*
  * Copyright (C) 2004, 2005, 2008 Nikolas Zimmermann <zimmermann@kde.org>
  * Copyright (C) 2004, 2005 Rob Buis <buis@kde.org>
- * Copyright (C) 2019 Apple Inc.  All rights reserved.
+ * Copyright (C) 2019 Apple Inc. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
@@ -206,26 +206,26 @@ public:
         return builder.toString();
     }
 
-    static String prefixForTransfromType(SVGTransformType type)
+    static const char* prefixForTransfromType(SVGTransformType type)
     {
         switch (type) {
         case SVG_TRANSFORM_UNKNOWN:
-            return emptyString();
+            return "";
         case SVG_TRANSFORM_MATRIX:
-            return "matrix("_s;
+            return "matrix(";
         case SVG_TRANSFORM_TRANSLATE:
-            return "translate("_s;
+            return "translate(";
         case SVG_TRANSFORM_SCALE:
-            return "scale("_s;
+            return "scale(";
         case SVG_TRANSFORM_ROTATE:
-            return "rotate("_s;
+            return "rotate(";
         case SVG_TRANSFORM_SKEWX:
-            return "skewX("_s;
+            return "skewX(";
         case SVG_TRANSFORM_SKEWY:
-            return "skewY("_s;
+            return "skewY(";
         }
         ASSERT_NOT_REACHED();
-        return emptyString();
+        return "";
     }
 
 private:
@@ -239,6 +239,7 @@ private:
     {
         if (builder.length() && builder[builder.length() - 1] != '(')
             builder.append(' ');
+        // FIXME: Shortest form would be better, but fixed precision is required for now to smooth over precision errors caused by converting float to double and back since we use AffineTransform to store transforms.
         builder.append(FormattedNumber::fixedPrecision(number));
         appendFixedPrecisionNumbers(builder, numbers...);
     }
index fafec73..bfcd211 100644 (file)
@@ -27,8 +27,6 @@
 #include "FloatRect.h"
 #include "QualifiedName.h"
 #include "SVGParserUtilities.h"
-#include <wtf/text/StringBuilder.h>
-#include <wtf/text/WTFString.h>
 
 namespace WebCore {
 
@@ -103,7 +101,7 @@ struct SVGPropertyTraits<float> {
             return WTF::nullopt;
         return number;
     }
-    static String toString(float type) { return String::numberToStringFixedPrecision(type); }
+    static String toString(float type) { return String::number(type); }
 };
 
 template<>
@@ -139,11 +137,7 @@ struct SVGPropertyTraits<FloatPoint> {
     }
     static String toString(const FloatPoint& type)
     {
-        StringBuilder builder;
-        builder.append(FormattedNumber::fixedPrecision(type.x()));
-        builder.append(' ');
-        builder.append(FormattedNumber::fixedPrecision(type.y()));
-        return builder.toString();
+        return makeString(type.x(), ' ', type.y());
     }
 };
 
@@ -166,15 +160,7 @@ struct SVGPropertyTraits<FloatRect> {
     }
     static String toString(const FloatRect& type)
     {
-        StringBuilder builder;
-        builder.append(FormattedNumber::fixedPrecision(type.x()));
-        builder.append(' ');
-        builder.append(FormattedNumber::fixedPrecision(type.y()));
-        builder.append(' ');
-        builder.append(FormattedNumber::fixedPrecision(type.width()));
-        builder.append(' ');
-        builder.append(FormattedNumber::fixedPrecision(type.height()));
-        return builder.toString();
+        return makeString(type.x(), ' ', type.y(), ' ', type.width(), ' ', type.height());
     }
 };
 
index c2b2044..1cea774 100644 (file)
@@ -1691,19 +1691,11 @@ ExceptionOr<String> Internals::dumpMarkerRects(const String& markerTypeString)
     contextDocument()->markers().updateRectsForInvalidatedMarkersOfType(markerType);
     auto rects = contextDocument()->markers().renderedRectsForMarkers(markerType);
 
+    // FIXME: Using fixed precision here for width because of test results that contain numbers with specific precision. Would be nice to update the test results and move to default formatting.
     StringBuilder rectString;
     rectString.appendLiteral("marker rects: ");
-    for (const auto& rect : rects) {
-        rectString.append('(');
-        rectString.append(FormattedNumber::fixedPrecision(rect.x()));
-        rectString.appendLiteral(", ");
-        rectString.append(FormattedNumber::fixedPrecision(rect.y()));
-        rectString.appendLiteral(", ");
-        rectString.append(FormattedNumber::fixedPrecision(rect.width()));
-        rectString.appendLiteral(", ");
-        rectString.append(FormattedNumber::fixedPrecision(rect.height()));
-        rectString.appendLiteral(") ");
-    }
+    for (const auto& rect : rects)
+        rectString.append('(', rect.x(), ", ", rect.y(), ", ", FormattedNumber::fixedPrecision(rect.width()), ", ", rect.height(), ") ");
     return rectString.toString();
 }
 
@@ -1884,6 +1876,7 @@ ExceptionOr<String> Internals::configurationForViewport(float devicePixelRatio,
     restrictMinimumScaleFactorToViewportSize(attributes, IntSize(availableWidth, availableHeight), devicePixelRatio);
     restrictScaleFactorToInitialScaleIfNotUserScalable(attributes);
 
+    // FIXME: Using fixed precision here because of test results that contain numbers with specific precision. Would be nice to update the test results and move to default formatting.
     return makeString("viewport size ", FormattedNumber::fixedPrecision(attributes.layoutSize.width()), 'x', FormattedNumber::fixedPrecision(attributes.layoutSize.height()), " scale ", FormattedNumber::fixedPrecision(attributes.initialScale), " with limits [", FormattedNumber::fixedPrecision(attributes.minimumScale), ", ", FormattedNumber::fixedPrecision(attributes.maximumScale), "] and userScalable ", (attributes.userScalable ? "true" : "false"));
 }
 
@@ -3446,24 +3439,14 @@ ExceptionOr<String> Internals::getCurrentCursorInfo()
     Cursor cursor = document->frame()->eventHandler().currentMouseCursor();
 
     StringBuilder result;
-    result.appendLiteral("type=");
-    result.append(cursorTypeToString(cursor.type()));
-    result.appendLiteral(" hotSpot=");
-    result.appendNumber(cursor.hotSpot().x());
-    result.append(',');
-    result.appendNumber(cursor.hotSpot().y());
+    result.append("type=", cursorTypeToString(cursor.type()), " hotSpot=", cursor.hotSpot().x(), ',', cursor.hotSpot().y());
     if (cursor.image()) {
         FloatSize size = cursor.image()->size();
-        result.appendLiteral(" image=");
-        result.append(FormattedNumber::fixedPrecision(size.width()));
-        result.append('x');
-        result.append(FormattedNumber::fixedPrecision(size.height()));
+        result.append(" image=", size.width(), 'x', size.height());
     }
 #if ENABLE(MOUSE_CURSOR_SCALE)
-    if (cursor.imageScaleFactor() != 1) {
-        result.appendLiteral(" scale=");
-        result.append(FormattedNumber::fixedPrecision(cursor.imageScaleFactor(), 8));
-    }
+    if (cursor.imageScaleFactor() != 1)
+        result.append(" scale=", cursor.imageScaleFactor());
 #endif
     return result.toString();
 #else
index a9c9bd3..0605121 100644 (file)
@@ -126,7 +126,7 @@ String Value::toString() const
                 return "0"_s;
             if (std::isinf(m_number))
                 return std::signbit(m_number) ? "-Infinity"_s : "Infinity"_s;
-            return String::numberToStringFixedPrecision(m_number);
+            return String::number(m_number);
         case BooleanValue:
             return m_bool ? "true"_s : "false"_s;
     }
index 685c04e..68d8936 100644 (file)
@@ -1,3 +1,25 @@
+2020-01-13  Darin Adler  <darin@apple.com>
+
+        Use even more "shortest form" formatting, and less "fixed precision" and "fixed width"
+        https://bugs.webkit.org/show_bug.cgi?id=198918
+
+        Reviewed by Sam Weinig.
+
+        * NetworkProcess/cache/NetworkCache.cpp:
+        (WebKit::NetworkCache::Cache::dumpContentsToFile): Use shortest instead of fixed.
+        Also use makeString instead of StringBuilder.
+
+        * NetworkProcess/cache/NetworkCacheEntry.cpp:
+        (WebKit::NetworkCache::Entry::asJSON const): Use shortest instead of fixed.
+        Also use multiple-argument append on StringBuilder to make the function shorter
+        and easier to read.
+        * Shared/Gamepad/GamepadData.cpp:
+        (WebKit::GamepadData::loggingString const): Ditto.
+
+        * UIProcess/ViewGestureController.cpp:
+        (WebKit::ViewGestureController::SnapshotRemovalTracker::startWatchdog):
+        Use shortest instead of fixed.
+
 2020-01-14  Víctor Manuel Jáquez Leal  <vjaquez@igalia.com>
 
         [GLib] Remove unused private variable
index bf64aca..73192e1 100644 (file)
@@ -575,23 +575,16 @@ void Cache::dumpContentsToFile()
     size_t capacity = m_storage->capacity();
     m_storage->traverse(resourceType(), flags, [fd, totals, capacity](const Storage::Record* record, const Storage::RecordInfo& info) mutable {
         if (!record) {
-            StringBuilder epilogue;
-            epilogue.appendLiteral("{}\n],\n");
-            epilogue.appendLiteral("\"totals\": {\n");
-            epilogue.appendLiteral("\"capacity\": ");
-            epilogue.appendNumber(capacity);
-            epilogue.appendLiteral(",\n");
-            epilogue.appendLiteral("\"count\": ");
-            epilogue.appendNumber(totals.count);
-            epilogue.appendLiteral(",\n");
-            epilogue.appendLiteral("\"bodySize\": ");
-            epilogue.appendNumber(totals.bodySize);
-            epilogue.appendLiteral(",\n");
-            epilogue.appendLiteral("\"averageWorth\": ");
-            epilogue.append(FormattedNumber::fixedPrecision(totals.count ? totals.worth / totals.count : 0));
-            epilogue.appendLiteral("\n");
-            epilogue.appendLiteral("}\n}\n");
-            auto writeData = epilogue.toString().utf8();
+            CString writeData = makeString(
+                "{}\n"
+                "],\n"
+                "\"totals\": {\n"
+                "\"capacity\": ", capacity, ",\n"
+                "\"count\": ", totals.count, ",\n"
+                "\"bodySize\": ", totals.bodySize, ",\n"
+                "\"averageWorth\": ", totals.count ? totals.worth / totals.count : 0, "\n"
+                "}\n}\n"
+            ).utf8();
             writeToFile(fd, writeData.data(), writeData.length());
             closeFile(fd);
             return;
index 26fa30d..2ded43a 100644 (file)
@@ -211,32 +211,24 @@ void Entry::setNeedsValidation(bool value)
 
 void Entry::asJSON(StringBuilder& json, const Storage::RecordInfo& info) const
 {
-    json.appendLiteral("{\n");
-    json.appendLiteral("\"hash\": ");
+    json.appendLiteral("{\n"
+        "\"hash\": ");
     json.appendQuotedJSONString(m_key.hashAsString());
-    json.appendLiteral(",\n");
-    json.appendLiteral("\"bodySize\": ");
-    json.appendNumber(info.bodySize);
-    json.appendLiteral(",\n");
-    json.appendLiteral("\"worth\": ");
-    json.append(FormattedNumber::fixedPrecision(info.worth));
-    json.appendLiteral(",\n");
-    json.appendLiteral("\"partition\": ");
+    json.append(",\n"
+        "\"bodySize\": ", info.bodySize, ",\n"
+        "\"worth\": ", info.worth, ",\n"
+        "\"partition\": ");
     json.appendQuotedJSONString(m_key.partition());
-    json.appendLiteral(",\n");
-    json.appendLiteral("\"timestamp\": ");
-    json.append(FormattedNumber::fixedPrecision(m_timeStamp.secondsSinceEpoch().milliseconds()));
-    json.appendLiteral(",\n");
-    json.appendLiteral("\"URL\": ");
+    json.append(",\n"
+        "\"timestamp\": ", m_timeStamp.secondsSinceEpoch().milliseconds(), ",\n"
+        "\"URL\": ");
     json.appendQuotedJSONString(m_response.url().string());
-    json.appendLiteral(",\n");
-    json.appendLiteral("\"bodyHash\": ");
+    json.appendLiteral(",\n"
+        "\"bodyHash\": ");
     json.appendQuotedJSONString(info.bodyHash);
-    json.appendLiteral(",\n");
-    json.appendLiteral("\"bodyShareCount\": ");
-    json.appendNumber(info.bodyShareCount);
-    json.appendLiteral(",\n");
-    json.appendLiteral("\"headers\": {\n");
+    json.append(",\n"
+        "\"bodyShareCount\": ", info.bodyShareCount, ",\n"
+        "\"headers\": {\n");
     bool firstHeader = true;
     for (auto& header : m_response.httpHeaderFields()) {
         if (!firstHeader)
@@ -247,8 +239,9 @@ void Entry::asJSON(StringBuilder& json, const Storage::RecordInfo& info) const
         json.appendLiteral(": ");
         json.appendQuotedJSONString(header.value);
     }
-    json.appendLiteral("\n}\n");
-    json.appendLiteral("}");
+    json.appendLiteral("\n"
+        "}\n"
+        "}");
 }
 
 }
index 7e43264..f9ba49d 100644 (file)
@@ -87,32 +87,24 @@ Optional<GamepadData> GamepadData::decode(IPC::Decoder& decoder)
 }
 
 #if !LOG_DISABLED
+
 String GamepadData::loggingString() const
 {
     StringBuilder builder;
 
-    builder.appendNumber(m_axisValues.size());
-    builder.appendLiteral(" axes, ");
-    builder.appendNumber(m_buttonValues.size());
-    builder.appendLiteral(" buttons\n");
+    builder.append(m_axisValues.size(), " axes, ", m_buttonValues.size(), " buttons\n");
 
-    for (size_t i = 0; i < m_axisValues.size(); ++i) {
-        builder.appendLiteral(" Axis ");
-        builder.appendNumber(i);
-        builder.appendLiteral(": ");
-        builder.append(FormattedNumber::fixedPrecision(m_axisValues[i]));
-    }
+    for (size_t i = 0; i < m_axisValues.size(); ++i)
+        builder.append(" Axis ", i, ": ", m_axisValues[i]);
 
     builder.append('\n');
-    for (size_t i = 0; i < m_buttonValues.size(); ++i) {
-        builder.appendLiteral(" Button ");
-        builder.appendNumber(i);
-        builder.appendLiteral(": ");
-        builder.append(FormattedNumber::fixedPrecision(m_buttonValues[i]));
-    }
+
+    for (size_t i = 0; i < m_buttonValues.size(); ++i)
+        builder.append(" Button ", i, ": ", FormattedNumber::fixedPrecision(m_buttonValues[i]));
 
     return builder.toString();
 }
+
 #endif
 
 } // namespace WebKit
index 4b61e3b..8a173bf 100644 (file)
@@ -387,7 +387,7 @@ void ViewGestureController::SnapshotRemovalTracker::watchdogTimerFired()
 
 void ViewGestureController::SnapshotRemovalTracker::startWatchdog(Seconds duration)
 {
-    log(makeString("(re)started watchdog timer for ", FormattedNumber::fixedWidth(duration.seconds(), 1), " seconds"));
+    log(makeString("(re)started watchdog timer for ", duration.seconds(), " seconds"));
     m_watchdogTimer.startOneShot(duration);
 }