Skip to content

GitLab

T
tangled
Commit 19f5db9f authored by Timothy Loh's avatar Timothy Loh Committed by Commit Bot
Browse files
Options

Revert "[css-typed-om] Implement CSSNumericValue.parse." 

This reverts commit 86e5b818.

Reason for revert: Added test is failing on Mac, e.g.
https://storage.googleapis.com/chromium-layout-test-archives/Mac10_11_Tests/20861/layout-test-results/results.html

FAIL Parsing a calc with mixed compatible units returns correct CSSMathValue assert_equals: expected "px" but got "number"

Original change's description:
> [css-typed-om] Implement CSSNumericValue.parse.
> 
> This patch implements CSSNumericValue.parse. We reuse the existing
> calc() parser, modifying it to keep bracket information since the result
> of CSSNumericValue.parse is sensitive to brackets. We also have to
> merge certain nodes in the parsed calc tree (e.g. calc(1 + 2 + 3) gets
> merged into one sum node).
> 
> Spec: https://drafts.css-houdini.org/css-typed-om-1/#dom-cssnumericvalue-parse
> 
> Bug: 776173, 788570
> Change-Id: Ia4bef7c3a2eb580d11a5e51d3921ed52e1f17bf3
> Reviewed-on: https://chromium-review.googlesource.com/792670
> Reviewed-by: nainar <nainar@chromium.org>
> Commit-Queue: Darren Shen <shend@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#520361}

TBR=nainar@chromium.org,shend@chromium.org

Change-Id: Ib2acc1f4fc07390c75ecdda9991f984a55009a81
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 776173, 788570
Reviewed-on: https://chromium-review.googlesource.com/798991Reviewed-by: default avatarTimothy Loh <timloh@chromium.org>
Commit-Queue: Timothy Loh <timloh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520421}
parent 19ff5882
Show whitespace changes
Inline Side-by-side
Showing with 11 additions and 277 deletions
third_party/WebKit/LayoutTests/typedcssom/stylevalue-subclasses/numeric-objects/parse.html deleted 100644 → 0
View file @ 19ff5882
<meta charset="utf-8">
<title>CSSNumericValue.parse tests</title>
<link rel="help" href="https://drafts.css-houdini.org/css-typed-om-1/#dom-cssnumericvalue-parse">
<script src="../../../resources/testharness.js"></script>
<script src="../../../resources/testharnessreport.js"></script>
<script src="../../resources/testhelper.js"></script>
<script>
'use strict';
test(() => {
assert_throws(new SyntaxError(), () => CSSNumericValue.parse('%#('));
assert_throws(new SyntaxError(), () => CSSNumericValue.parse('auto'));
assert_throws(new SyntaxError(), () => CSSNumericValue.parse('1 2'));
}, 'Parsing an invalid string throws SyntaxError');
test(() => {
assert_style_value_equals(
CSSNumericValue.parse('-3.14'),
CSS.number(-3.14));
}, 'Parsing a <number-token> returns a number CSSUnitValue');
test(() => {
assert_style_value_equals(
CSSNumericValue.parse('-3.14%'),
CSS.percent(-3.14));
}, 'Parsing a <percentage-token> returns a percent CSSUnitValue');
test(() => {
assert_style_value_equals(
CSSNumericValue.parse('-3.14px'),
CSS.px(-3.14));
}, 'Parsing a <dimension-token> returns a CSSUnitValue with correct unit');
test(() => {
assert_style_value_equals(
CSSNumericValue.parse('0'),
CSS.number(0));
}, 'Parsing a unitless zero returns zero');
test(() => {
assert_style_value_equals(
CSSNumericValue.parse('calc(1)'),
new CSSMathSum(1));
}, 'Parsing a calc with only one argument returns a CSSMathSum with that argument');
test(() => {
assert_style_value_equals(
CSSNumericValue.parse('calc(1 + 1)'),
new CSSMathSum(1, 1));
}, 'Parsing a calc with addition returns a CSSMathSum');
test(() => {
assert_style_value_equals(
CSSNumericValue.parse('calc(1 - 1)'),
new CSSMathSum(1, new CSSMathNegate(1)));
}, 'Parsing a calc with subtraction returns a CSSMathSum with negated argument');
test(() => {
assert_style_value_equals(
CSSNumericValue.parse('calc(1 * 1)'),
new CSSMathProduct(1, 1));
}, 'Parsing a calc with multiplication returns a CSSMathProduct');
test(() => {
assert_style_value_equals(
CSSNumericValue.parse('calc(1 / 1)'),
new CSSMathProduct(1, new CSSMathInvert(1)));
}, 'Parsing a calc with division returns a CSSMathProduct with inverted argument');
test(() => {
assert_style_value_equals(
CSSNumericValue.parse('calc(1 + 2 - 3 + 4)'),
new CSSMathSum(1, 2, new CSSMathNegate(3), 4));
}, 'Parsing a calc with n-ary addition and subtraction returns a flat CSSMathSum');
test(() => {
assert_style_value_equals(
CSSNumericValue.parse('calc(1 * 2 / 3 * 4)'),
new CSSMathProduct(1, 2, new CSSMathInvert(3), 4));
}, 'Parsing a calc with n-ary multiplication and division returns a flat CSSMathProduct');
test(() => {
assert_style_value_equals(
CSSNumericValue.parse('calc(calc(1 + 2) + 3)'),
new CSSMathSum(new CSSMathSum(1, 2), 3));
assert_style_value_equals(
CSSNumericValue.parse('calc(1 + calc(2 + 3))'),
new CSSMathSum(1, new CSSMathSum(2, 3)));
}, 'Parsing a calc with nested sum returns nested CSSMathSum');
test(() => {
assert_style_value_equals(
CSSNumericValue.parse('calc(calc(1 * 2) * 3)'),
new CSSMathProduct(new CSSMathProduct(1, 2), 3));
assert_style_value_equals(
CSSNumericValue.parse('calc(1 * calc(2 * 3))'),
new CSSMathProduct(1, new CSSMathProduct(2, 3)));
}, 'Parsing a calc with nested product returns nested CSSMathProduct');
test(() => {
assert_style_value_equals(
CSSNumericValue.parse('calc(calc(1px * 2) + 3%)'),
new CSSMathSum(new CSSMathProduct(CSS.px(1), 2), CSS.percent(3)));
}, 'Parsing a calc with mixed compatible units returns correct CSSMathValue');
test(() => {
assert_throws(new SyntaxError(), () => CSSNumericValue.parse('calc(calc(1px * 2s) + 3%)'));
}, 'Parsing a calc with incompatible units throws a SyntaxError');
</script>
third_party/WebKit/Source/core/css/CSSCalculationValue.cpp
View file @ 19f5db9f
...@@ -681,25 +681,10 @@ static ParseState CheckDepthAndIndex(int* depth, CSSParserTokenRange tokens) { ...@@ -681,25 +681,10 @@ static ParseState CheckDepthAndIndex(int* depth, CSSParserTokenRange tokens) {
return OK; return OK;
} }
namespace {
CSSCalcExpressionNode* CreateCalcBinaryOperation(CSSCalcExpressionNode* lhs,
CSSCalcExpressionNode* rhs,
CalcOperator op,
bool simplify) {
if (simplify)
return CSSCalcBinaryOperation::CreateSimplified(lhs, rhs, op);
return CSSCalcBinaryOperation::Create(lhs, rhs, op);
}
} // namespace
class CSSCalcExpressionNodeParser { class CSSCalcExpressionNodeParser {
STACK_ALLOCATED(); STACK_ALLOCATED();
public: public:
CSSCalcExpressionNodeParser(bool simplify) : simplify_(simplify) {}
CSSCalcExpressionNode* ParseCalc(CSSParserTokenRange tokens) { CSSCalcExpressionNode* ParseCalc(CSSParserTokenRange tokens) {
Value result; Value result;
tokens.ConsumeWhitespace(); tokens.ConsumeWhitespace();
...@@ -750,10 +735,7 @@ class CSSCalcExpressionNodeParser { ...@@ -750,10 +735,7 @@ class CSSCalcExpressionNodeParser {
CSSParserTokenRange inner_range = tokens.ConsumeBlock(); CSSParserTokenRange inner_range = tokens.ConsumeBlock();
tokens.ConsumeWhitespace(); tokens.ConsumeWhitespace();
inner_range.ConsumeWhitespace(); inner_range.ConsumeWhitespace();
if (!ParseValueExpression(inner_range, depth, result)) return ParseValueExpression(inner_range, depth, result);
return false;
result->value->SetIsNestedCalc();
return true;
} }
return ParseValue(tokens, result); return ParseValue(tokens, result);
...@@ -779,10 +761,9 @@ class CSSCalcExpressionNodeParser { ...@@ -779,10 +761,9 @@ class CSSCalcExpressionNodeParser {
if (!ParseValueTerm(tokens, depth, &rhs)) if (!ParseValueTerm(tokens, depth, &rhs))
return false; return false;
result->value = CreateCalcBinaryOperation( result->value = CSSCalcBinaryOperation::CreateSimplified(
result->value, rhs.value, result->value, rhs.value,
static_cast<CalcOperator>(operator_character), simplify_); static_cast<CalcOperator>(operator_character));
if (!result->value) if (!result->value)
return false; return false;
} }
...@@ -814,10 +795,9 @@ class CSSCalcExpressionNodeParser { ...@@ -814,10 +795,9 @@ class CSSCalcExpressionNodeParser {
if (!ParseValueMultiplicativeExpression(tokens, depth, &rhs)) if (!ParseValueMultiplicativeExpression(tokens, depth, &rhs))
return false; return false;
result->value = CreateCalcBinaryOperation( result->value = CSSCalcBinaryOperation::CreateSimplified(
result->value, rhs.value, result->value, rhs.value,
static_cast<CalcOperator>(operator_character), simplify_); static_cast<CalcOperator>(operator_character));
if (!result->value) if (!result->value)
return false; return false;
} }
...@@ -830,9 +810,6 @@ class CSSCalcExpressionNodeParser { ...@@ -830,9 +810,6 @@ class CSSCalcExpressionNodeParser {
Value* result) { Value* result) {
return ParseAdditiveValueExpression(tokens, depth, result); return ParseAdditiveValueExpression(tokens, depth, result);
} }
private:
const bool simplify_;
}; };
CSSCalcExpressionNode* CSSCalcValue::CreateExpressionNode( CSSCalcExpressionNode* CSSCalcValue::CreateExpressionNode(
...@@ -863,15 +840,7 @@ CSSCalcExpressionNode* CSSCalcValue::CreateExpressionNode(double pixels, ...@@ -863,15 +840,7 @@ CSSCalcExpressionNode* CSSCalcValue::CreateExpressionNode(double pixels,
CSSCalcValue* CSSCalcValue::Create(const CSSParserTokenRange& tokens, CSSCalcValue* CSSCalcValue::Create(const CSSParserTokenRange& tokens,
ValueRange range) { ValueRange range) {
CSSCalcExpressionNodeParser parser(false /* simplify */); CSSCalcExpressionNodeParser parser;
CSSCalcExpressionNode* expression = parser.ParseCalc(tokens);
return expression ? new CSSCalcValue(expression, range) : nullptr;
}
CSSCalcValue* CSSCalcValue::CreateSimplified(const CSSParserTokenRange& tokens,
ValueRange range) {
CSSCalcExpressionNodeParser parser(true /* simplify */);
CSSCalcExpressionNode* expression = parser.ParseCalc(tokens); CSSCalcExpressionNode* expression = parser.ParseCalc(tokens);
return expression ? new CSSCalcValue(expression, range) : nullptr; return expression ? new CSSCalcValue(expression, range) : nullptr;
......
third_party/WebKit/Source/core/css/CSSCalculationValue.h
View file @ 19f5db9f
...@@ -90,9 +90,6 @@ class CSSCalcExpressionNode : public GarbageCollected<CSSCalcExpressionNode> { ...@@ -90,9 +90,6 @@ class CSSCalcExpressionNode : public GarbageCollected<CSSCalcExpressionNode> {
virtual CSSPrimitiveValue::UnitType TypeWithCalcResolved() const = 0; virtual CSSPrimitiveValue::UnitType TypeWithCalcResolved() const = 0;
bool IsInteger() const { return is_integer_; } bool IsInteger() const { return is_integer_; }
bool IsNestedCalc() const { return is_nested_calc_; }
void SetIsNestedCalc() { is_nested_calc_ = true; }
virtual void Trace(blink::Visitor* visitor) {} virtual void Trace(blink::Visitor* visitor) {}
protected: protected:
...@@ -103,13 +100,11 @@ class CSSCalcExpressionNode : public GarbageCollected<CSSCalcExpressionNode> { ...@@ -103,13 +100,11 @@ class CSSCalcExpressionNode : public GarbageCollected<CSSCalcExpressionNode> {
CalculationCategory category_; CalculationCategory category_;
bool is_integer_; bool is_integer_;
bool is_nested_calc_ = false;
}; };
class CORE_EXPORT CSSCalcValue : public GarbageCollected<CSSCalcValue> { class CORE_EXPORT CSSCalcValue : public GarbageCollected<CSSCalcValue> {
public: public:
static CSSCalcValue* Create(const CSSParserTokenRange&, ValueRange); static CSSCalcValue* Create(const CSSParserTokenRange&, ValueRange);
static CSSCalcValue* CreateSimplified(const CSSParserTokenRange&, ValueRange);
static CSSCalcValue* Create(CSSCalcExpressionNode*, static CSSCalcValue* Create(CSSCalcExpressionNode*,
ValueRange = kValueRangeAll); ValueRange = kValueRangeAll);
......
third_party/WebKit/Source/core/css/cssom/CSSNumericValue.cpp
View file @ 19f5db9f
...@@ -5,7 +5,6 @@ ...@@ -5,7 +5,6 @@
#include "core/css/cssom/CSSNumericValue.h" #include "core/css/cssom/CSSNumericValue.h"
#include "bindings/core/v8/ExceptionState.h" #include "bindings/core/v8/ExceptionState.h"
#include "core/css/CSSCalculationValue.h"
#include "core/css/CSSPrimitiveValue.h" #include "core/css/CSSPrimitiveValue.h"
#include "core/css/cssom/CSSMathInvert.h" #include "core/css/cssom/CSSMathInvert.h"
#include "core/css/cssom/CSSMathMax.h" #include "core/css/cssom/CSSMathMax.h"
...@@ -14,8 +13,6 @@ ...@@ -14,8 +13,6 @@
#include "core/css/cssom/CSSMathProduct.h" #include "core/css/cssom/CSSMathProduct.h"
#include "core/css/cssom/CSSMathSum.h" #include "core/css/cssom/CSSMathSum.h"
#include "core/css/cssom/CSSUnitValue.h" #include "core/css/cssom/CSSUnitValue.h"
#include "core/css/parser/CSSParserTokenStream.h"
#include "core/css/parser/CSSTokenizer.h"
namespace blink { namespace blink {
...@@ -53,90 +50,6 @@ CSSUnitValue* MaybeSimplifyAsUnitValue(const CSSNumericValueVector& values, ...@@ -53,90 +50,6 @@ CSSUnitValue* MaybeSimplifyAsUnitValue(const CSSNumericValueVector& values,
return CSSUnitValue::Create(final_value, first_unit_value->GetInternalUnit()); return CSSUnitValue::Create(final_value, first_unit_value->GetInternalUnit());
} }
CalcOperator CanonicalOperator(CalcOperator op) {
if (op == kCalcAdd || op == kCalcSubtract)
return kCalcAdd;
return kCalcMultiply;
}
bool CanCombineNodes(const CSSCalcExpressionNode& root,
const CSSCalcExpressionNode& node) {
DCHECK_EQ(root.GetType(), CSSCalcExpressionNode::kCssCalcBinaryOperation);
if (node.GetType() == CSSCalcExpressionNode::kCssCalcPrimitiveValue)
return false;
return !node.IsNestedCalc() && CanonicalOperator(root.OperatorType()) ==
CanonicalOperator(node.OperatorType());
}
CSSNumericValue* NegateOrInvertIfRequired(CalcOperator parent_op,
CSSNumericValue* value) {
DCHECK(value);
if (parent_op == kCalcSubtract)
return CSSMathNegate::Create(value);
if (parent_op == kCalcDivide)
return CSSMathInvert::Create(value);
return value;
}
CSSNumericValue* CalcToNumericValue(const CSSCalcExpressionNode& root) {
if (root.GetType() == CSSCalcExpressionNode::kCssCalcPrimitiveValue) {
auto* value = CSSUnitValue::Create(root.DoubleValue());
DCHECK(value);
// For cases like calc(1), we need to wrap the value in a CSSMathSum
if (!root.IsNestedCalc())
return value;
CSSNumericValueVector values;
values.push_back(value);
return CSSMathSum::Create(std::move(values));
}
// When the node is a binary operator, we return either a CSSMathSum or a
// CSSMathProduct.
DCHECK_EQ(root.GetType(), CSSCalcExpressionNode::kCssCalcBinaryOperation);
CSSNumericValueVector values;
// For cases like calc(1 + 2 + 3), the calc expression tree looks like:
// +
// / \
// + 3
// / \
// 1 2
//
// But we want to produce a CSSMathValue tree that looks like:
// +
// /|\
// 1 2 3
//
// So when the left child has the same operator as its parent, we can combine
// the two nodes. We keep moving down the left side of the tree as long as the
// current node and the root can be combined, collecting the right child of
// the nodes that we encounter.
const CSSCalcExpressionNode* cur_node = &root;
do {
DCHECK(cur_node->LeftExpressionNode());
DCHECK(cur_node->RightExpressionNode());
const auto& value = CalcToNumericValue(*cur_node->RightExpressionNode());
// If the current node is a '-' or '/', it's really just a '+' or '*' with
// the right child negated or inverted, respectively.
values.push_back(NegateOrInvertIfRequired(cur_node->OperatorType(), value));
cur_node = cur_node->LeftExpressionNode();
} while (CanCombineNodes(root, *cur_node));
DCHECK(cur_node);
values.push_back(CalcToNumericValue(*cur_node));
// Our algorithm collects the children in reverse order, so we have to reverse
// the values.
std::reverse(values.begin(), values.end());
if (root.OperatorType() == kCalcAdd || root.OperatorType() == kCalcSubtract)
return CSSMathSum::Create(std::move(values));
return CSSMathProduct::Create(std::move(values));
}
} // namespace } // namespace
bool CSSNumericValue::IsValidUnit(CSSPrimitiveValue::UnitType unit) { bool CSSNumericValue::IsValidUnit(CSSPrimitiveValue::UnitType unit) {
...@@ -161,39 +74,8 @@ CSSPrimitiveValue::UnitType CSSNumericValue::UnitFromName(const String& name) { ...@@ -161,39 +74,8 @@ CSSPrimitiveValue::UnitType CSSNumericValue::UnitFromName(const String& name) {
} }
CSSNumericValue* CSSNumericValue::parse(const String& css_text, CSSNumericValue* CSSNumericValue::parse(const String& css_text,
ExceptionState& exception_state) { ExceptionState&) {
CSSTokenizer tokenizer(css_text); // TODO(meade): Implement
CSSParserTokenStream stream(tokenizer);
auto range = stream.ConsumeUntilPeekedTypeIs<>();
if (!stream.AtEnd()) {
exception_state.ThrowDOMException(kSyntaxError, "Invalid math expression");
return nullptr;
}
switch (range.Peek().GetType()) {
case kNumberToken:
case kPercentageToken:
case kDimensionToken: {
const auto token = range.Consume();
if (!range.AtEnd())
break;
return CSSUnitValue::Create(token.NumericValue(), token.GetUnitType());
}
case kFunctionToken:
if (range.Peek().FunctionId() == CSSValueCalc ||
range.Peek().FunctionId() == CSSValueWebkitCalc) {
CSSCalcValue* calc_value = CSSCalcValue::Create(range, kValueRangeAll);
if (!calc_value)
break;
DCHECK(calc_value->ExpressionNode());
return CalcToNumericValue(*calc_value->ExpressionNode());
}
default:
break;
}
exception_state.ThrowDOMException(kSyntaxError, "Invalid math expression");
return nullptr; return nullptr;
} }
......
third_party/WebKit/Source/core/css/cssom/CSSNumericValue.h
View file @ 19f5db9f
...@@ -28,8 +28,8 @@ class CORE_EXPORT CSSNumericValue : public CSSStyleValue { ...@@ -28,8 +28,8 @@ class CORE_EXPORT CSSNumericValue : public CSSStyleValue {
DEFINE_WRAPPERTYPEINFO(); DEFINE_WRAPPERTYPEINFO();
public: public:
static CSSNumericValue* parse(const String& css_text, ExceptionState&);
// Blink-internal ways of creating CSSNumericValues. // Blink-internal ways of creating CSSNumericValues.
static CSSNumericValue* parse(const String& css_text, ExceptionState&);
static CSSNumericValue* FromCSSValue(const CSSPrimitiveValue&); static CSSNumericValue* FromCSSValue(const CSSPrimitiveValue&);
// https://drafts.css-houdini.org/css-typed-om-1/#rectify-a-numberish-value // https://drafts.css-houdini.org/css-typed-om-1/#rectify-a-numberish-value
static CSSNumericValue* FromNumberish(const CSSNumberish& value); static CSSNumericValue* FromNumberish(const CSSNumberish& value);
......
third_party/WebKit/Source/core/css/parser/CSSPropertyParserHelpers.cpp
View file @ 19f5db9f
...@@ -190,10 +190,8 @@ class CalcParser { ...@@ -190,10 +190,8 @@ class CalcParser {
: source_range_(range), range_(range) { : source_range_(range), range_(range) {
const CSSParserToken& token = range.Peek(); const CSSParserToken& token = range.Peek();
if (token.FunctionId() == CSSValueCalc || if (token.FunctionId() == CSSValueCalc ||
token.FunctionId() == CSSValueWebkitCalc) { token.FunctionId() == CSSValueWebkitCalc)
calc_value_ = calc_value_ = CSSCalcValue::Create(ConsumeFunction(range_), value_range);
CSSCalcValue::CreateSimplified(ConsumeFunction(range_), value_range);
}
} }
const CSSCalcValue* Value() const { return calc_value_.Get(); } const CSSCalcValue* Value() const { return calc_value_.Get(); }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment