Title: [276262] trunk
Revision
276262
Author
da...@apple.com
Date
2021-04-19 10:51:46 -0700 (Mon, 19 Apr 2021)

Log Message

Nullptr crash in CSSCalcValue::category() via HTMLConverterCaches::floatPropertyValueForNode
https://bugs.webkit.org/show_bug.cgi?id=221392

Reviewed by Simon Fraser.

LayoutTests/imported/w3c:

* web-platform-tests/css/css-values/minmax-length-percent-serialize-expected.txt:
Updated to reflect 8 tests passing that were failing before.

Source/WebCore:

* css/CSSCalculationValue.cpp:
(WebCore::CSSCalcOperationNode::createCalcExpression const): Pass in a destination category
when creating a CalcExpressionOperation.
(WebCore::createCSS): Pass the destination category from the CalcExpressionOperation when
creating a CSSCalcOperationNode.

* css/CSSCalculationValue.h: Moved the CalculationCategory enumeration from here to
CalculationValue.h.

* platform/CalculationValue.cpp:
(WebCore::operator==): Include destination category when comparing.

* platform/CalculationValue.h: Moved CalculationCategory here. Added a destination
category constructor argument, data members, and getter function to the
CalcExpressionOperation class.

LayoutTests:

* fast/css/calc-parsing-expected.txt: Updated for change below.
* fast/css/calc-parsing.html: Added more test cases so this covers the affected cases of computed
style for calc expressions that mix percentages and numbers. Alternatively, we could remove this
entire test case because the web platform tests also cover this pretty well.

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (276261 => 276262)


--- trunk/LayoutTests/ChangeLog	2021-04-19 17:17:54 UTC (rev 276261)
+++ trunk/LayoutTests/ChangeLog	2021-04-19 17:51:46 UTC (rev 276262)
@@ -1,3 +1,15 @@
+2021-04-19  Darin Adler  <da...@apple.com>
+
+        Nullptr crash in CSSCalcValue::category() via HTMLConverterCaches::floatPropertyValueForNode
+        https://bugs.webkit.org/show_bug.cgi?id=221392
+
+        Reviewed by Simon Fraser.
+
+        * fast/css/calc-parsing-expected.txt: Updated for change below.
+        * fast/css/calc-parsing.html: Added more test cases so this covers the affected cases of computed
+        style for calc expressions that mix percentages and numbers. Alternatively, we could remove this
+        entire test case because the web platform tests also cover this pretty well.
+
 2021-04-19  Chris Gambrell  <cgambr...@apple.com>
 
         [LayoutTests] Convert http/tests/blink convert PHP to Python

Modified: trunk/LayoutTests/fast/css/calc-parsing-expected.txt (276261 => 276262)


--- trunk/LayoutTests/fast/css/calc-parsing-expected.txt	2021-04-19 17:17:54 UTC (rev 276261)
+++ trunk/LayoutTests/fast/css/calc-parsing-expected.txt	2021-04-19 17:51:46 UTC (rev 276262)
@@ -1,96 +1,208 @@
-Tests parsing of various valid and invalid calc expressions.
+Tests parsing and re-serializing of various valid and invalid calc expressions.
 
 On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
 
 
 
-testDiv.style["width"] = "calc(100px)"
-PASS testDiv.style['width'] is "calc(100px)"
-PASS window.getComputedStyle(testDiv).getPropertyValue('width') is "100px"
+element.style["width"] = "calc(100px)"
+PASS element.style['width'] is "calc(100px)"
+PASS getComputedStyle(element).getPropertyValue('width') is "100px"
 
-testDiv.style["width"] = "max(100px + 200px)"
-PASS testDiv.style['width'] is "max(300px)"
-PASS window.getComputedStyle(testDiv).getPropertyValue('width') is "300px"
+element.style["width"] = "max(100px + 200px)"
+PASS element.style['width'] is "max(300px)"
+PASS getComputedStyle(element).getPropertyValue('width') is "300px"
 
-testDiv.style["width"] = "max(100px , 200px)"
-PASS testDiv.style['width'] is "max(200px)"
-PASS window.getComputedStyle(testDiv).getPropertyValue('width') is "200px"
+element.style["width"] = "max(100px , 200px)"
+PASS element.style['width'] is "max(200px)"
+PASS getComputedStyle(element).getPropertyValue('width') is "200px"
 
-testDiv.style["width"] = "max(100px,200px)"
-PASS testDiv.style['width'] is "max(200px)"
-PASS window.getComputedStyle(testDiv).getPropertyValue('width') is "200px"
+element.style["width"] = "max(100px,200px)"
+PASS element.style['width'] is "max(200px)"
+PASS getComputedStyle(element).getPropertyValue('width') is "200px"
 
-testDiv.style["width"] = "clamp(100px,123px,200px)"
-PASS testDiv.style['width'] is "clamp(100px, 123px, 200px)"
-PASS window.getComputedStyle(testDiv).getPropertyValue('width') is "123px"
+element.style["width"] = "clamp(100px,123px,200px)"
+PASS element.style['width'] is "clamp(100px, 123px, 200px)"
+PASS getComputedStyle(element).getPropertyValue('width') is "123px"
 
-testDiv.style["width"] = "clamp(100px,300px,200px)"
-PASS testDiv.style['width'] is "clamp(100px, 300px, 200px)"
-PASS window.getComputedStyle(testDiv).getPropertyValue('width') is "200px"
+element.style["width"] = "clamp(100px,300px,200px)"
+PASS element.style['width'] is "clamp(100px, 300px, 200px)"
+PASS getComputedStyle(element).getPropertyValue('width') is "200px"
 
-testDiv.style["width"] = "clamp(200px,300px,100px)"
-PASS testDiv.style['width'] is "clamp(200px, 300px, 100px)"
-PASS window.getComputedStyle(testDiv).getPropertyValue('width') is "200px"
+element.style["width"] = "clamp(200px,300px,100px)"
+PASS element.style['width'] is "clamp(200px, 300px, 100px)"
+PASS getComputedStyle(element).getPropertyValue('width') is "200px"
 
-testDiv.style["width"] = "clamp((50px + 50px),40px,200px)"
-PASS testDiv.style['width'] is "clamp(100px, 40px, 200px)"
-PASS window.getComputedStyle(testDiv).getPropertyValue('width') is "100px"
+element.style["width"] = "clamp((50px + 50px),40px,200px)"
+PASS element.style['width'] is "clamp(100px, 40px, 200px)"
+PASS getComputedStyle(element).getPropertyValue('width') is "100px"
 
-testDiv.style["width"] = "clamp(50px + 50px,40px,200px)"
-PASS testDiv.style['width'] is "clamp(100px, 40px, 200px)"
-PASS window.getComputedStyle(testDiv).getPropertyValue('width') is "100px"
+element.style["width"] = "clamp(50px + 50px,40px,200px)"
+PASS element.style['width'] is "clamp(100px, 40px, 200px)"
+PASS getComputedStyle(element).getPropertyValue('width') is "100px"
 
-testDiv.style["width"] = "calc(100px, 200px)"
-PASS testDiv.style['width'] is "999px"
-PASS window.getComputedStyle(testDiv).getPropertyValue('width') is "999px"
+element.style["width"] = "min(100px,0%)"
+PASS element.style['width'] is "min(100px, 0%)"
+PASS getComputedStyle(element).getPropertyValue('width') is "0px"
 
-testDiv.style["width"] = "calc(100px  200px)"
-PASS testDiv.style['width'] is "999px"
-PASS window.getComputedStyle(testDiv).getPropertyValue('width') is "999px"
+element.style["width"] = "max(100px,0%)"
+PASS element.style['width'] is "max(100px, 0%)"
+PASS getComputedStyle(element).getPropertyValue('width') is "100px"
 
-testDiv.style["width"] = "calc(100px ( 200px)"
-PASS testDiv.style['width'] is "999px"
-PASS window.getComputedStyle(testDiv).getPropertyValue('width') is "999px"
+element.style["width"] = "clamp(100px,0%,1%)"
+PASS element.style['width'] is "clamp(100px, 0%, 1%)"
+PASS getComputedStyle(element).getPropertyValue('width') is "100px"
 
-testDiv.style["width"] = "min(100px 200px)"
-PASS testDiv.style['width'] is "999px"
-PASS window.getComputedStyle(testDiv).getPropertyValue('width') is "999px"
+element.style["width"] = "calc(100px, 200px)"
+PASS element.style['width'] is "999px"
+PASS getComputedStyle(element).getPropertyValue('width') is "999px"
 
-testDiv.style["width"] = "max(100px 200px)"
-PASS testDiv.style['width'] is "999px"
-PASS window.getComputedStyle(testDiv).getPropertyValue('width') is "999px"
+element.style["width"] = "calc(100px  200px)"
+PASS element.style['width'] is "999px"
+PASS getComputedStyle(element).getPropertyValue('width') is "999px"
 
-testDiv.style["width"] = "max(100px,, 200px)"
-PASS testDiv.style['width'] is "999px"
-PASS window.getComputedStyle(testDiv).getPropertyValue('width') is "999px"
+element.style["width"] = "calc(100px ( 200px)"
+PASS element.style['width'] is "999px"
+PASS getComputedStyle(element).getPropertyValue('width') is "999px"
 
-testDiv.style["width"] = "max(100px, , 200px)"
-PASS testDiv.style['width'] is "999px"
-PASS window.getComputedStyle(testDiv).getPropertyValue('width') is "999px"
+element.style["width"] = "min(100px 200px)"
+PASS element.style['width'] is "999px"
+PASS getComputedStyle(element).getPropertyValue('width') is "999px"
 
-testDiv.style["width"] = "max(100px, 200px,)"
-PASS testDiv.style['width'] is "999px"
-PASS window.getComputedStyle(testDiv).getPropertyValue('width') is "999px"
+element.style["width"] = "max(100px 200px)"
+PASS element.style['width'] is "999px"
+PASS getComputedStyle(element).getPropertyValue('width') is "999px"
 
-testDiv.style["width"] = "clamp(200px,300px)"
-PASS testDiv.style['width'] is "999px"
-PASS window.getComputedStyle(testDiv).getPropertyValue('width') is "999px"
+element.style["width"] = "max(100px,, 200px)"
+PASS element.style['width'] is "999px"
+PASS getComputedStyle(element).getPropertyValue('width') is "999px"
 
-testDiv.style["width"] = "clamp(200px,300px,)"
-PASS testDiv.style['width'] is "999px"
-PASS window.getComputedStyle(testDiv).getPropertyValue('width') is "999px"
+element.style["width"] = "max(100px, , 200px)"
+PASS element.style['width'] is "999px"
+PASS getComputedStyle(element).getPropertyValue('width') is "999px"
 
-testDiv.style["width"] = "clamp(200px,,300px)"
-PASS testDiv.style['width'] is "999px"
-PASS window.getComputedStyle(testDiv).getPropertyValue('width') is "999px"
+element.style["width"] = "max(100px, 200px,)"
+PASS element.style['width'] is "999px"
+PASS getComputedStyle(element).getPropertyValue('width') is "999px"
 
-testDiv.style["width"] = "clamp((),,300px)"
-PASS testDiv.style['width'] is "999px"
-PASS window.getComputedStyle(testDiv).getPropertyValue('width') is "999px"
+element.style["width"] = "clamp(200px,300px)"
+PASS element.style['width'] is "999px"
+PASS getComputedStyle(element).getPropertyValue('width') is "999px"
 
-testDiv.style["width"] = "clamp(1px,2px,2px,4px)"
-PASS testDiv.style['width'] is "999px"
-PASS window.getComputedStyle(testDiv).getPropertyValue('width') is "999px"
+element.style["width"] = "clamp(200px,300px,)"
+PASS element.style['width'] is "999px"
+PASS getComputedStyle(element).getPropertyValue('width') is "999px"
+
+element.style["width"] = "clamp(200px,,300px)"
+PASS element.style['width'] is "999px"
+PASS getComputedStyle(element).getPropertyValue('width') is "999px"
+
+element.style["width"] = "clamp((),,300px)"
+PASS element.style['width'] is "999px"
+PASS getComputedStyle(element).getPropertyValue('width') is "999px"
+
+element.style["width"] = "clamp(1px,2px,2px,4px)"
+PASS element.style['width'] is "999px"
+PASS getComputedStyle(element).getPropertyValue('width') is "999px"
+
+element.style["min-width"] = "calc(100px)"
+PASS element.style['min-width'] is "calc(100px)"
+PASS getComputedStyle(element).getPropertyValue('min-width') is "100px"
+
+element.style["min-width"] = "max(100px + 200px)"
+PASS element.style['min-width'] is "max(300px)"
+PASS getComputedStyle(element).getPropertyValue('min-width') is "300px"
+
+element.style["min-width"] = "max(100px , 200px)"
+PASS element.style['min-width'] is "max(200px)"
+PASS getComputedStyle(element).getPropertyValue('min-width') is "200px"
+
+element.style["min-width"] = "max(100px,200px)"
+PASS element.style['min-width'] is "max(200px)"
+PASS getComputedStyle(element).getPropertyValue('min-width') is "200px"
+
+element.style["min-width"] = "clamp(100px,123px,200px)"
+PASS element.style['min-width'] is "clamp(100px, 123px, 200px)"
+PASS getComputedStyle(element).getPropertyValue('min-width') is "123px"
+
+element.style["min-width"] = "clamp(100px,300px,200px)"
+PASS element.style['min-width'] is "clamp(100px, 300px, 200px)"
+PASS getComputedStyle(element).getPropertyValue('min-width') is "200px"
+
+element.style["min-width"] = "clamp(200px,300px,100px)"
+PASS element.style['min-width'] is "clamp(200px, 300px, 100px)"
+PASS getComputedStyle(element).getPropertyValue('min-width') is "200px"
+
+element.style["min-width"] = "clamp((50px + 50px),40px,200px)"
+PASS element.style['min-width'] is "clamp(100px, 40px, 200px)"
+PASS getComputedStyle(element).getPropertyValue('min-width') is "100px"
+
+element.style["min-width"] = "clamp(50px + 50px,40px,200px)"
+PASS element.style['min-width'] is "clamp(100px, 40px, 200px)"
+PASS getComputedStyle(element).getPropertyValue('min-width') is "100px"
+
+element.style["min-width"] = "min(100px,0%)"
+PASS element.style['min-width'] is "min(100px, 0%)"
+PASS getComputedStyle(element).getPropertyValue('min-width') is "min(100px, 0%)"
+
+element.style["min-width"] = "max(100px,0%)"
+PASS element.style['min-width'] is "max(100px, 0%)"
+PASS getComputedStyle(element).getPropertyValue('min-width') is "max(100px, 0%)"
+
+element.style["min-width"] = "clamp(100px,0%,1%)"
+PASS element.style['min-width'] is "clamp(100px, 0%, 1%)"
+PASS getComputedStyle(element).getPropertyValue('min-width') is "clamp(100px, 0%, 1%)"
+
+element.style["min-width"] = "calc(100px, 200px)"
+PASS element.style['min-width'] is "999px"
+PASS getComputedStyle(element).getPropertyValue('min-width') is "999px"
+
+element.style["min-width"] = "calc(100px  200px)"
+PASS element.style['min-width'] is "999px"
+PASS getComputedStyle(element).getPropertyValue('min-width') is "999px"
+
+element.style["min-width"] = "calc(100px ( 200px)"
+PASS element.style['min-width'] is "999px"
+PASS getComputedStyle(element).getPropertyValue('min-width') is "999px"
+
+element.style["min-width"] = "min(100px 200px)"
+PASS element.style['min-width'] is "999px"
+PASS getComputedStyle(element).getPropertyValue('min-width') is "999px"
+
+element.style["min-width"] = "max(100px 200px)"
+PASS element.style['min-width'] is "999px"
+PASS getComputedStyle(element).getPropertyValue('min-width') is "999px"
+
+element.style["min-width"] = "max(100px,, 200px)"
+PASS element.style['min-width'] is "999px"
+PASS getComputedStyle(element).getPropertyValue('min-width') is "999px"
+
+element.style["min-width"] = "max(100px, , 200px)"
+PASS element.style['min-width'] is "999px"
+PASS getComputedStyle(element).getPropertyValue('min-width') is "999px"
+
+element.style["min-width"] = "max(100px, 200px,)"
+PASS element.style['min-width'] is "999px"
+PASS getComputedStyle(element).getPropertyValue('min-width') is "999px"
+
+element.style["min-width"] = "clamp(200px,300px)"
+PASS element.style['min-width'] is "999px"
+PASS getComputedStyle(element).getPropertyValue('min-width') is "999px"
+
+element.style["min-width"] = "clamp(200px,300px,)"
+PASS element.style['min-width'] is "999px"
+PASS getComputedStyle(element).getPropertyValue('min-width') is "999px"
+
+element.style["min-width"] = "clamp(200px,,300px)"
+PASS element.style['min-width'] is "999px"
+PASS getComputedStyle(element).getPropertyValue('min-width') is "999px"
+
+element.style["min-width"] = "clamp((),,300px)"
+PASS element.style['min-width'] is "999px"
+PASS getComputedStyle(element).getPropertyValue('min-width') is "999px"
+
+element.style["min-width"] = "clamp(1px,2px,2px,4px)"
+PASS element.style['min-width'] is "999px"
+PASS getComputedStyle(element).getPropertyValue('min-width') is "999px"
 PASS successfullyParsed is true
 
 TEST COMPLETE

Modified: trunk/LayoutTests/fast/css/calc-parsing.html (276261 => 276262)


--- trunk/LayoutTests/fast/css/calc-parsing.html	2021-04-19 17:17:54 UTC (rev 276261)
+++ trunk/LayoutTests/fast/css/calc-parsing.html	2021-04-19 17:51:46 UTC (rev 276262)
@@ -4,48 +4,54 @@
         <script src=""
         <div id="testDiv"></div>
         <script>
-            description("Tests parsing of various valid and invalid calc expressions.");
+            description("Tests parsing and re-serializing of various valid and invalid calc expressions.");
 
-            var testDiv = document.getElementById("testDiv");
+            var element = document.getElementById("testDiv");
             
-            function testExpression(_expression_, specifiedValue, computedValue)
+            function testProperty(propertyName)
             {
-                debug('');
-                testDiv.style["width"] = '999px';
+                function testExpression(_expression_, specifiedValue, computedValue)
+                {
+                    debug('');
+                    element.style[propertyName] = '999px';
 
-                evalAndLog(`testDiv.style["width"] = "${_expression_}"`);
-                shouldBeEqualToString("testDiv.style['width']", `${specifiedValue}`);
-                shouldBeEqualToString("window.getComputedStyle(testDiv).getPropertyValue('width')", `${computedValue}`);
-            }
+                    evalAndLog(`element.style["${propertyName}"] = "${_expression_}"`);
+                    shouldBeEqualToString(`element.style['${propertyName}']`, `${specifiedValue}`);
+                    shouldBeEqualToString(`getComputedStyle(element).getPropertyValue('${propertyName}')`, `${computedValue}`);
+                }
 
-            // Valid expressions.
-            testExpression('calc(100px)', 'calc(100px)', '100px');
-            testExpression('max(100px + 200px)', 'max(300px)', '300px');
-            testExpression('max(100px , 200px)', 'max(200px)', '200px');
-            testExpression('max(100px,200px)', 'max(200px)', '200px');
-            testExpression('clamp(100px,123px,200px)', 'clamp(100px, 123px, 200px)', '123px');
-            testExpression('clamp(100px,300px,200px)', 'clamp(100px, 300px, 200px)', '200px');
-            testExpression('clamp(200px,300px,100px)', 'clamp(200px, 300px, 100px)', '200px');
-            testExpression('clamp((50px + 50px),40px,200px)', 'clamp(100px, 40px, 200px)', '100px');
-            testExpression('clamp(50px + 50px,40px,200px)', 'clamp(100px, 40px, 200px)', '100px');
+                // Valid expressions.
+                testExpression('calc(100px)', 'calc(100px)', '100px');
+                testExpression('max(100px + 200px)', 'max(300px)', '300px');
+                testExpression('max(100px , 200px)', 'max(200px)', '200px');
+                testExpression('max(100px,200px)', 'max(200px)', '200px');
+                testExpression('clamp(100px,123px,200px)', 'clamp(100px, 123px, 200px)', '123px');
+                testExpression('clamp(100px,300px,200px)', 'clamp(100px, 300px, 200px)', '200px');
+                testExpression('clamp(200px,300px,100px)', 'clamp(200px, 300px, 100px)', '200px');
+                testExpression('clamp((50px + 50px),40px,200px)', 'clamp(100px, 40px, 200px)', '100px');
+                testExpression('clamp(50px + 50px,40px,200px)', 'clamp(100px, 40px, 200px)', '100px');
+                testExpression('min(100px,0%)', 'min(100px, 0%)', propertyName == 'width' ? '0px' : "min(100px, 0%)");
+                testExpression('max(100px,0%)', 'max(100px, 0%)', propertyName == 'width' ? '100px' : "max(100px, 0%)");
+                testExpression('clamp(100px,0%,1%)', 'clamp(100px, 0%, 1%)', propertyName == 'width' ? '100px' : "clamp(100px, 0%, 1%)");
 
-            // Non-parsing _expression_.
-            testExpression('calc(100px, 200px)', '999px', '999px');
-            testExpression('calc(100px  200px)', '999px', '999px');
+                // Non-parsing expressions.
+                testExpression('calc(100px, 200px)', '999px', '999px');
+                testExpression('calc(100px  200px)', '999px', '999px');
+                testExpression('calc(100px ( 200px)', '999px', '999px');
+                testExpression('min(100px 200px)', '999px', '999px');
+                testExpression('max(100px 200px)', '999px', '999px');
+                testExpression('max(100px,, 200px)', '999px', '999px');
+                testExpression('max(100px, , 200px)', '999px', '999px');
+                testExpression('max(100px, 200px,)', '999px', '999px');
+                testExpression('clamp(200px,300px)', '999px', '999px');
+                testExpression('clamp(200px,300px,)', '999px', '999px');
+                testExpression('clamp(200px,,300px)', '999px', '999px');
+                testExpression('clamp((),,300px)', '999px', '999px');
+                testExpression('clamp(1px,2px,2px,4px)', '999px', '999px');
+            }
 
-            testExpression('calc(100px ( 200px)', '999px', '999px');
-
-            testExpression('min(100px 200px)', '999px', '999px');
-            testExpression('max(100px 200px)', '999px', '999px');
-            testExpression('max(100px,, 200px)', '999px', '999px');
-            testExpression('max(100px, , 200px)', '999px', '999px');
-            testExpression('max(100px, 200px,)', '999px', '999px');
-
-            testExpression('clamp(200px,300px)', '999px', '999px');
-            testExpression('clamp(200px,300px,)', '999px', '999px');
-            testExpression('clamp(200px,,300px)', '999px', '999px');
-            testExpression('clamp((),,300px)', '999px', '999px');
-            testExpression('clamp(1px,2px,2px,4px)', '999px', '999px');
+            testProperty("width");
+            testProperty("min-width");
         </script>
         <script src=""
     </body>

Modified: trunk/LayoutTests/imported/w3c/ChangeLog (276261 => 276262)


--- trunk/LayoutTests/imported/w3c/ChangeLog	2021-04-19 17:17:54 UTC (rev 276261)
+++ trunk/LayoutTests/imported/w3c/ChangeLog	2021-04-19 17:51:46 UTC (rev 276262)
@@ -1,3 +1,13 @@
+2021-04-19  Darin Adler  <da...@apple.com>
+
+        Nullptr crash in CSSCalcValue::category() via HTMLConverterCaches::floatPropertyValueForNode
+        https://bugs.webkit.org/show_bug.cgi?id=221392
+
+        Reviewed by Simon Fraser.
+
+        * web-platform-tests/css/css-values/minmax-length-percent-serialize-expected.txt:
+        Updated to reflect 8 tests passing that were failing before.
+
 2021-04-19  Youenn Fablet  <you...@apple.com>
 
         Migrate some WebRTC encoded transform tests to WPT

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-values/minmax-length-percent-serialize-expected.txt (276261 => 276262)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-values/minmax-length-percent-serialize-expected.txt	2021-04-19 17:17:54 UTC (rev 276261)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-values/minmax-length-percent-serialize-expected.txt	2021-04-19 17:51:46 UTC (rev 276262)
@@ -15,28 +15,28 @@
 PASS 'max(1px + 1%)' as a computed value should serialize as 'max(1% + 1px)'.
 PASS 'max(1px + 1%)' as a used value should serialize as '2px'.
 PASS 'min(20px, 10%)' as a specified value should serialize as 'min(20px, 10%)'.
-FAIL 'min(20px, 10%)' as a computed value should serialize as 'min(20px, 10%)'. assert_not_equals: 'min(20px, 10%)' should be valid in text-indent. got disallowed value ""
+PASS 'min(20px, 10%)' as a computed value should serialize as 'min(20px, 10%)'.
 PASS 'min(20px, 10%)' as a used value should serialize as '10px'.
 PASS 'min(1em, 10%)' as a specified value should serialize as 'min(1em, 10%)'.
-FAIL 'min(1em, 10%)' as a computed value should serialize as 'min(16px, 10%)'. assert_not_equals: 'min(1em, 10%)' should be valid in text-indent. got disallowed value ""
+PASS 'min(1em, 10%)' as a computed value should serialize as 'min(16px, 10%)'.
 PASS 'min(1em, 10%)' as a used value should serialize as '10px'.
 PASS 'min(10%, 20px)' as a specified value should serialize as 'min(10%, 20px)'.
-FAIL 'min(10%, 20px)' as a computed value should serialize as 'min(10%, 20px)'. assert_not_equals: 'min(10%, 20px)' should be valid in text-indent. got disallowed value ""
+PASS 'min(10%, 20px)' as a computed value should serialize as 'min(10%, 20px)'.
 PASS 'min(10%, 20px)' as a used value should serialize as '10px'.
 PASS 'min(10%, 1em)' as a specified value should serialize as 'min(10%, 1em)'.
-FAIL 'min(10%, 1em)' as a computed value should serialize as 'min(10%, 16px)'. assert_not_equals: 'min(10%, 1em)' should be valid in text-indent. got disallowed value ""
+PASS 'min(10%, 1em)' as a computed value should serialize as 'min(10%, 16px)'.
 PASS 'min(10%, 1em)' as a used value should serialize as '10px'.
 PASS 'max(20px, 10%)' as a specified value should serialize as 'max(20px, 10%)'.
-FAIL 'max(20px, 10%)' as a computed value should serialize as 'max(20px, 10%)'. assert_not_equals: 'max(20px, 10%)' should be valid in text-indent. got disallowed value ""
+PASS 'max(20px, 10%)' as a computed value should serialize as 'max(20px, 10%)'.
 PASS 'max(20px, 10%)' as a used value should serialize as '20px'.
 PASS 'max(1em, 10%)' as a specified value should serialize as 'max(1em, 10%)'.
-FAIL 'max(1em, 10%)' as a computed value should serialize as 'max(16px, 10%)'. assert_not_equals: 'max(1em, 10%)' should be valid in text-indent. got disallowed value ""
+PASS 'max(1em, 10%)' as a computed value should serialize as 'max(16px, 10%)'.
 PASS 'max(1em, 10%)' as a used value should serialize as '16px'.
 PASS 'max(10%, 20px)' as a specified value should serialize as 'max(10%, 20px)'.
-FAIL 'max(10%, 20px)' as a computed value should serialize as 'max(10%, 20px)'. assert_not_equals: 'max(10%, 20px)' should be valid in text-indent. got disallowed value ""
+PASS 'max(10%, 20px)' as a computed value should serialize as 'max(10%, 20px)'.
 PASS 'max(10%, 20px)' as a used value should serialize as '20px'.
 PASS 'max(10%, 1em)' as a specified value should serialize as 'max(10%, 1em)'.
-FAIL 'max(10%, 1em)' as a computed value should serialize as 'max(10%, 16px)'. assert_not_equals: 'max(10%, 1em)' should be valid in text-indent. got disallowed value ""
+PASS 'max(10%, 1em)' as a computed value should serialize as 'max(10%, 16px)'.
 PASS 'max(10%, 1em)' as a used value should serialize as '16px'.
 PASS 'min(10% + 30px, 5em + 5%)' as a specified value should serialize as 'min(10% + 30px, 5% + 5em)'.
 PASS 'min(10% + 30px, 5em + 5%)' as a computed value should serialize as 'min(10% + 30px, 5% + 80px)'.

Modified: trunk/Source/WebCore/ChangeLog (276261 => 276262)


--- trunk/Source/WebCore/ChangeLog	2021-04-19 17:17:54 UTC (rev 276261)
+++ trunk/Source/WebCore/ChangeLog	2021-04-19 17:51:46 UTC (rev 276262)
@@ -1,3 +1,26 @@
+2021-04-19  Darin Adler  <da...@apple.com>
+
+        Nullptr crash in CSSCalcValue::category() via HTMLConverterCaches::floatPropertyValueForNode
+        https://bugs.webkit.org/show_bug.cgi?id=221392
+
+        Reviewed by Simon Fraser.
+
+        * css/CSSCalculationValue.cpp:
+        (WebCore::CSSCalcOperationNode::createCalcExpression const): Pass in a destination category
+        when creating a CalcExpressionOperation.
+        (WebCore::createCSS): Pass the destination category from the CalcExpressionOperation when
+        creating a CSSCalcOperationNode.
+
+        * css/CSSCalculationValue.h: Moved the CalculationCategory enumeration from here to
+        CalculationValue.h.
+
+        * platform/CalculationValue.cpp:
+        (WebCore::operator==): Include destination category when comparing.
+
+        * platform/CalculationValue.h: Moved CalculationCategory here. Added a destination
+        category constructor argument, data members, and getter function to the
+        CalcExpressionOperation class.
+
 2021-04-19  Antti Koivisto  <an...@apple.com>
 
         CSSValuePool should be non-copyable

Modified: trunk/Source/WebCore/css/CSSCalculationValue.cpp (276261 => 276262)


--- trunk/Source/WebCore/css/CSSCalculationValue.cpp	2021-04-19 17:17:54 UTC (rev 276261)
+++ trunk/Source/WebCore/css/CSSCalculationValue.cpp	2021-04-19 17:51:46 UTC (rev 276262)
@@ -1367,7 +1367,16 @@
             return nullptr;
         nodes.uncheckedAppend(WTFMove(node));
     }
-    return makeUnique<CalcExpressionOperation>(WTFMove(nodes), m_operator);
+
+    // Reverse the operation we did when creating this node, recovering a suitable destination category for otherwise-ambiguous min/max/clamp nodes.
+    // Note that this category is really only good enough for that purpose and is not accurate for other node types; we could use a boolean instead.
+    auto destinationCategory = CalculationCategory::Other;
+    if (category() == CalculationCategory::PercentLength)
+        destinationCategory = CalculationCategory::Length;
+    else if (category() == CalculationCategory::PercentNumber)
+        destinationCategory = CalculationCategory::Number;
+
+    return makeUnique<CalcExpressionOperation>(WTFMove(nodes), m_operator, destinationCategory);
 }
 
 double CSSCalcOperationNode::doubleValue(CSSUnitType unitType) const
@@ -2036,7 +2045,7 @@
             auto children = createCSS(operationChildren, style);
             if (children.isEmpty())
                 return nullptr;
-            return CSSCalcOperationNode::createMinOrMaxOrClamp(op, WTFMove(children), CalculationCategory::Other);
+            return CSSCalcOperationNode::createMinOrMaxOrClamp(op, WTFMove(children), operationNode.destinationCategory());
         }
         }
         return nullptr;

Modified: trunk/Source/WebCore/css/CSSCalculationValue.h (276261 => 276262)


--- trunk/Source/WebCore/css/CSSCalculationValue.h	2021-04-19 17:17:54 UTC (rev 276261)
+++ trunk/Source/WebCore/css/CSSCalculationValue.h	2021-04-19 17:51:46 UTC (rev 276262)
@@ -44,22 +44,6 @@
 class CSSToLengthConversionData;
 class RenderStyle;
 
-// FIXME: Unify with CSSPrimitiveValue::UnitCategory.
-enum class CalculationCategory : uint8_t {
-    Number = 0,
-    Length,
-    Percent,
-    PercentNumber,
-    PercentLength,
-    Angle,
-    Time,
-    Frequency,
-    // TODO:
-    // Flex,
-    // Resolution
-    Other
-};
-
 class CSSCalcExpressionNode : public RefCounted<CSSCalcExpressionNode> {
 public:
     enum Type {

Modified: trunk/Source/WebCore/platform/CalculationValue.cpp (276261 => 276262)


--- trunk/Source/WebCore/platform/CalculationValue.cpp	2021-04-19 17:17:54 UTC (rev 276261)
+++ trunk/Source/WebCore/platform/CalculationValue.cpp	2021-04-19 17:51:46 UTC (rev 276262)
@@ -180,9 +180,9 @@
 
 bool operator==(const CalcExpressionOperation& a, const CalcExpressionOperation& b)
 {
-    if (a.getOperator() != b.getOperator())
+    if (a.getOperator() != b.getOperator() || a.destinationCategory() != b.destinationCategory())
         return false;
-    // Maybe Vectors of unique_ptrs should always do deep compare?
+    // FIXME: Would be nice to have a helper function for doing a deep compare on a vector of pointers.
     if (a.children().size() != b.children().size())
         return false;
     for (unsigned i = 0; i < a.children().size(); ++i) {

Modified: trunk/Source/WebCore/platform/CalculationValue.h (276261 => 276262)


--- trunk/Source/WebCore/platform/CalculationValue.h	2021-04-19 17:17:54 UTC (rev 276261)
+++ trunk/Source/WebCore/platform/CalculationValue.h	2021-04-19 17:51:46 UTC (rev 276262)
@@ -42,6 +42,19 @@
 
 namespace WebCore {
 
+// FIXME: Find a way to unify this with CSSPrimitiveValue::UnitCategory?
+enum class CalculationCategory : uint8_t {
+    Number,
+    Length,
+    Percent,
+    PercentNumber,
+    PercentLength,
+    Angle,
+    Time,
+    Frequency,
+    Other
+};
+
 // Don't change these values; parsing uses them.
 enum class CalcOperator : uint8_t {
     Add = '+',
@@ -151,9 +164,10 @@
 
 class CalcExpressionOperation final : public CalcExpressionNode {
 public:
-    CalcExpressionOperation(Vector<std::unique_ptr<CalcExpressionNode>>&& children, CalcOperator);
+    CalcExpressionOperation(Vector<std::unique_ptr<CalcExpressionNode>>&& children, CalcOperator, CalculationCategory destinationCategory = CalculationCategory::Other);
 
     CalcOperator getOperator() const { return m_operator; }
+    CalculationCategory destinationCategory() const { return m_destinationCategory; }
 
     const Vector<std::unique_ptr<CalcExpressionNode>>& children() const { return m_children; }
 
@@ -164,6 +178,7 @@
 
     Vector<std::unique_ptr<CalcExpressionNode>> m_children;
     CalcOperator m_operator;
+    CalculationCategory m_destinationCategory { CalculationCategory::Other };
 };
 
 class CalcExpressionBlendLength final : public CalcExpressionNode {
@@ -237,10 +252,11 @@
     return a.length() == b.length();
 }
 
-inline CalcExpressionOperation::CalcExpressionOperation(Vector<std::unique_ptr<CalcExpressionNode>>&& children, CalcOperator op)
+inline CalcExpressionOperation::CalcExpressionOperation(Vector<std::unique_ptr<CalcExpressionNode>>&& children, CalcOperator op, CalculationCategory destinationCategory)
     : CalcExpressionNode(CalcExpressionNodeType::Operation)
     , m_children(WTFMove(children))
     , m_operator(op)
+    , m_destinationCategory(destinationCategory)
 {
 }
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to