Skip to content

Commit

Permalink
css/css-values/hypot-pow-sqrt-computed.html WPT crashes
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=254392

Reviewed by Tim Nguyen.

When simplifying hypot CSSCalcOperationNodes we may have only one child and
it may not be a CSSCalcPrimitiveValueNode, in that case do not simplify.

Add a similar check and tests for round, mod and rem.

* LayoutTests/imported/w3c/web-platform-tests/css/css-values/hypot-pow-sqrt-computed-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-values/hypot-pow-sqrt-computed.html:
* LayoutTests/imported/w3c/web-platform-tests/css/css-values/round-mod-rem-computed-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-values/round-mod-rem-computed.html:
* Source/WebCore/css/calc/CSSCalcOperationNode.cpp:
(WebCore::CSSCalcOperationNode::combineChildren):
* Source/WebCore/css/calc/CSSCalcOperationNode.h:

Canonical link: https://commits.webkit.org/263345@main
  • Loading branch information
rwlbuis authored and nt1m committed Apr 24, 2023
1 parent 2eb43a9 commit 1643a89
Show file tree
Hide file tree
Showing 6 changed files with 32 additions and 33 deletions.
Expand Up @@ -10,12 +10,18 @@ PASS calc(1px * pow(2, 3)) should be used-value-equivalent to 8px
PASS calc(100px * sqrt(100)) should be used-value-equivalent to 1000px
PASS calc(1px * pow(2, sqrt(100)) should be used-value-equivalent to 1024px
PASS hypot(3px, 4px) should be used-value-equivalent to 5px
PASS hypot(3e+9px, 4e+9px) should be used-value-equivalent to 5e+9px
PASS calc(100px * hypot(3, 4)) should be used-value-equivalent to 500px
PASS hypot(-5px) should be used-value-equivalent to 5px
PASS calc(1px * hypot(-5)) should be used-value-equivalent to 5px
PASS calc(1px * hypot(10000)) should be used-value-equivalent to 10000px
PASS calc(2px * sqrt(100000000)) should be used-value-equivalent to 20000px
PASS calc(3px * pow(200, 4)) should be used-value-equivalent to 33554428px
PASS calc(3px * pow(20, 4)) should be used-value-equivalent to 480000px
PASS calc(-2 * hypot(3px, 4px)) should be used-value-equivalent to -10px
FAIL hypot(0% + 3px, 0% + 4px) should be used-value-equivalent to 5px assert_equals: hypot(0% + 3px, 0% + 4px) and 5px serialize to the same thing in used values. expected "5px" but got "25px"
PASS hypot(0% + 772.333px) should be used-value-equivalent to calc(0% + 772.333px)
PASS hypot(0% + 772.35px) should be used-value-equivalent to calc(0% + 772.35px)
FAIL hypot(0% + 600px, 0% + 800px) should be used-value-equivalent to 1000px assert_equals: hypot(0% + 600px, 0% + 800px) and 1000px serialize to the same thing in used values. expected "1000px" but got "1000000px"
PASS hypot(1px) should be used-value-equivalent to 1px
PASS hypot(1cm) should be used-value-equivalent to 1cm
PASS hypot(1mm) should be used-value-equivalent to 1mm
Expand Down
Expand Up @@ -10,27 +10,33 @@
<script>

// Identity tests
test_math_used('pow(1,1)', '1', {type:'number'});
test_math_used('sqrt(1)', '1', {type:'number'});
test_math_used('hypot(1)', '1', {type:'number'});
test_math_used('pow(1,1)', '1', {type:'integer'});
test_math_used('sqrt(1)', '1', {type:'integer'});
test_math_used('hypot(1)', '1', {type:'integer'});

// Nestings
test_math_used('sqrt(pow(1,1))', '1');
test_math_used('hypot(pow(1, sqrt(1)))', '1');
test_math_used('calc(hypot(pow((1 + sqrt(1)) / 2, sqrt(1))))', '1');
test_math_used('sqrt(pow(1,1))', '1', {type:'integer'});
test_math_used('hypot(pow(1, sqrt(1)))', '1', {type:'integer'});
test_math_used('calc(hypot(pow((1 + sqrt(1)) / 2, sqrt(1))))', '1', {type:'integer'});

// General calculations
test_math_used('calc(100px * pow(2, pow(2, 2)))','1600px');
test_math_used('calc(1px * pow(2, 3))', '8px')
test_math_used('calc(100px * sqrt(100))', '1000px');
test_math_used('calc(1px * pow(2, sqrt(100))', '1024px');
test_math_used('hypot(3px, 4px)', '5px');
test_math_used('hypot(3e+9px, 4e+9px)', '5e+9px');
test_math_used('calc(100px * hypot(3, 4))', '500px');
test_math_used('hypot(-5px)', '5px');
test_math_used('calc(1px * hypot(-5))', '5px');
test_math_used('calc(1px * hypot(10000))','10000px');
test_math_used('calc(2px * sqrt(100000000))','20000px');
test_math_used('calc(3px * pow(200, 4))', '33554428px');
test_math_used('calc(3px * pow(20, 4))', '480000px');
test_math_used('calc(-2 * hypot(3px, 4px))', '-10px');
test_math_used('hypot(0% + 3px, 0% + 4px)', '5px');
test_math_used('hypot(0% + 772.333px)', 'calc(0% + 772.333px)');
test_math_used('hypot(0% + 772.35px)', 'calc(0% + 772.35px)');
test_math_used('hypot(0% + 600px, 0% + 800px)', '1000px');

//Type checking hypot
test_math_used('hypot(1px)', '1px');
Expand Down
Expand Up @@ -27,6 +27,9 @@ PASS calc(rem(mod(18,5),5)) should be used-value-equivalent to 3
PASS calc(rem(mod(18,5),mod(17,5))) should be used-value-equivalent to 1
PASS calc(mod(-140,-90)) should be used-value-equivalent to -50
PASS calc(mod(rem(1,18)* -1,5)) should be used-value-equivalent to -1
PASS calc(round(1px + 0%, 1px + 0%)) should be used-value-equivalent to 1px
PASS calc(mod(1px + 0%, 1px + 0%)) should be used-value-equivalent to 0px
PASS calc(rem(1px + 0%, 1px + 0%)) should be used-value-equivalent to 0px
PASS round(10px,6px) should be used-value-equivalent to 12px
PASS round(10cm,6cm) should be used-value-equivalent to 12cm
PASS round(10mm,6mm) should be used-value-equivalent to 12mm
Expand Down
Expand Up @@ -43,6 +43,9 @@
test_math_used('calc(rem(mod(18,5),mod(17,5)))', '1', {type:'number'});
test_math_used('calc(mod(-140,-90))', '-50', {type:'number'});
test_math_used('calc(mod(rem(1,18)* -1,5))', '-1', {type:'number'});
test_math_used('calc(round(1px + 0%, 1px + 0%))', '1px');
test_math_used('calc(mod(1px + 0%, 1px + 0%))', '0px');
test_math_used('calc(rem(1px + 0%, 1px + 0%))', '0px');

// Type check
test_math_used('round(10px,6px)', '12px');
Expand Down
28 changes: 4 additions & 24 deletions Source/WebCore/css/calc/CSSCalcOperationNode.cpp
Expand Up @@ -650,26 +650,19 @@ void CSSCalcOperationNode::combineChildren()
m_isRoot = IsRoot::No;

if (m_children.size() < 2) {
if (m_children.size() == 1 && isTrigNode()) {
if (isTrigNode() || isExpNode() || isSqrtNode()) {
double resolvedValue = doubleValue(m_children[0]->primitiveType());
auto newChild = CSSCalcPrimitiveValueNode::create(CSSPrimitiveValue::create(resolvedValue));
m_children.clear();
m_children.append(WTFMove(newChild));
}

if (isExpNode()) {
double resolvedValue = doubleValue(m_children[0]->primitiveType());
auto newChild = CSSCalcPrimitiveValueNode::create(CSSPrimitiveValue::create(resolvedValue));
m_children.clear();
m_children.append(WTFMove(newChild));
}
if (m_children.size() == 1 && isInverseTrigNode()) {
if (isInverseTrigNode()) {
double resolvedValue = doubleValue(m_children[0]->primitiveType());
auto newChild = CSSCalcPrimitiveValueNode::create(CSSPrimitiveValue::create(resolvedValue, CSSUnitType::CSS_DEG));
m_children.clear();
m_children.append(WTFMove(newChild));
}
if (isSignNode() || isHypotNode()) {
if (isSignNode() || (isHypotNode() && canCombineAllChildren())) {
auto combinedUnitType = m_children[0]->primitiveType();
if (calcOperator() == CalcOperator::Sign)
combinedUnitType = CSSUnitType::CSS_NUMBER;
Expand All @@ -678,12 +671,6 @@ void CSSCalcOperationNode::combineChildren()
m_children.clear();
m_children.append(WTFMove(newChild));
}
if (calcOperator() == CalcOperator::Sqrt) {
double resolvedValue = doubleValue(m_children[0]->primitiveType());
auto newChild = CSSCalcPrimitiveValueNode::create(CSSPrimitiveValue::create(resolvedValue));
m_children.clear();
m_children.append(WTFMove(newChild));
}
return;
}

Expand Down Expand Up @@ -823,14 +810,7 @@ void CSSCalcOperationNode::combineChildren()
m_children.clear();
m_children.append(WTFMove(newChild));
}
if (isSteppedNode()) {
auto combinedUnitType = m_children[0]->primitiveType();
double resolvedValue = doubleValue(combinedUnitType);
auto newChild = CSSCalcPrimitiveValueNode::create(CSSPrimitiveValue::create(resolvedValue, combinedUnitType));
m_children.clear();
m_children.append(WTFMove(newChild));
}
if (isRoundOperation()) {
if ((isSteppedNode() || isRoundOperation()) && canCombineAllChildren()) {
auto combinedUnitType = m_children[0]->primitiveType();
double resolvedValue = doubleValue(combinedUnitType);
auto newChild = CSSCalcPrimitiveValueNode::create(CSSPrimitiveValue::create(resolvedValue, combinedUnitType));
Expand Down
3 changes: 2 additions & 1 deletion Source/WebCore/css/calc/CSSCalcOperationNode.h
Expand Up @@ -69,7 +69,8 @@ class CSSCalcOperationNode final : public CSSCalcExpressionNode {
bool isRoundOperation() const { return m_operator == CalcOperator::Down || m_operator == CalcOperator::Up || m_operator == CalcOperator::ToZero || m_operator == CalcOperator::Nearest; }
bool isRoundConstant() const { return (isRoundOperation()) && !m_children.size(); }
bool isHypotNode() const { return m_operator == CalcOperator::Hypot; }
bool isPowOrSqrtNode() const { return m_operator == CalcOperator::Pow || m_operator == CalcOperator::Sqrt; }
bool isSqrtNode() const { return m_operator == CalcOperator::Sqrt; }
bool isPowOrSqrtNode() const { return m_operator == CalcOperator::Pow || isSqrtNode(); }
bool shouldPreserveFunction() const { return isTrigNode() || isExpNode() || isInverseTrigNode() || isAtan2Node() || isSignNode() || isSignNode() || isSteppedNode() || isRoundOperation() || isPowOrSqrtNode() || isClampNode(); }
bool isClampNode() const { return m_operator == CalcOperator::Clamp; }

Expand Down

0 comments on commit 1643a89

Please sign in to comment.