Changeset 178156 in webkit


Ignore:
Timestamp:
Jan 8, 2015 9:29:09 PM (9 years ago)
Author:
Chris Dumez
Message:

ASSERTION FAILED: !valueWithCalculation.calculation() in WebCore::CSSParser::validateCalculationUnit
​https://bugs.webkit.org/show_bug.cgi?id=140251

Reviewed by Darin Adler.

Source/WebCore:

Using a calculated value for text-shadow's blur-radius was hitting an
assertion in CSSParser::validateCalculationUnit() because validUnit()
is called twice, first with 'FLength' unit, then more stricly with
'FLength|FNonNeg' if parsing the blur-radius as it cannot be negative
as per the specification:

On the second call, the ValueWithCalculation's m_calculation member
was already initialized and the code did not handle this. This patch
updates validateCalculationUnit() to teach it to reuse the previously
parsed calculation in this case. All it needs to do is to update the
existing CSSCalcValue's range to allow negative values or not.

When writing the layout test for this, I also noticed that the CSS
parser was not rejecting negative calculated values for blur-radius
(only negative non-calculated ones). This is because
validateCalculationUnit() was ignoring FNonNeg if the calculated
value is a Length. This patch also addresses the issue.

Test: fast/css/text-shadow-calc-value.html

  • css/CSSCalculationValue.h:

(WebCore::CSSCalcValue::setPermittedValueRange):
Add a setter to update the CSSCalculationValue's permitted value range
so that the CSS parser does not need to fully reparse the calculation
only to update the permitted value range.

  • css/CSSParser.cpp:

(WebCore::CSSParser::validateCalculationUnit):

  • Teach the code to reuse the previously parsed calculation value.
  • Do the FNonNeg check for Length calculations as well.

LayoutTests:

Add a layout test to check that using calculated values for
'text-shadow' CSS doesn't crash and works as intended. Also check
that the CSS parser is correctly validating the blur-radius, which
is supposed to be non-negative, as per the specification:

  • fast/css/text-shadow-calc-value-expected.txt: Added.
  • fast/css/text-shadow-calc-value.html: Added.
Location:
trunk
Files:
2 added
4 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r178150 r178156  
     12015-01-08  Chris Dumez  <cdumez@apple.com>
     2
     3        ASSERTION FAILED: !valueWithCalculation.calculation() in WebCore::CSSParser::validateCalculationUnit
     4        https://bugs.webkit.org/show_bug.cgi?id=140251
     5
     6        Reviewed by Darin Adler.
     7
     8        Add a layout test to check that using calculated values for
     9        'text-shadow' CSS doesn't crash and works as intended. Also check
     10        that the CSS parser is correctly validating the blur-radius, which
     11        is supposed to be non-negative, as per the specification:
     12        - http://dev.w3.org/csswg/css-text-decor-3/#text-shadow-property
     13        - http://dev.w3.org/csswg/css-backgrounds-3/#shadow
     14
     15        * fast/css/text-shadow-calc-value-expected.txt: Added.
     16        * fast/css/text-shadow-calc-value.html: Added.
     17
    1182015-01-08  Benjamin Poulain  <bpoulain@apple.com>
    219

  • trunk/Source/WebCore/ChangeLog

    r178154 r178156  
     12015-01-08  Chris Dumez  <cdumez@apple.com>
     2
     3        ASSERTION FAILED: !valueWithCalculation.calculation() in WebCore::CSSParser::validateCalculationUnit
     4        https://bugs.webkit.org/show_bug.cgi?id=140251
     5
     6        Reviewed by Darin Adler.
     7
     8        Using a calculated value for text-shadow's blur-radius was hitting an
     9        assertion in CSSParser::validateCalculationUnit() because validUnit()
     10        is called twice, first with 'FLength' unit, then more stricly with
     11        'FLength|FNonNeg' if parsing the blur-radius as it cannot be negative
     12        as per the specification:
     13        - http://dev.w3.org/csswg/css-text-decor-3/#text-shadow-property
     14        - http://dev.w3.org/csswg/css-backgrounds-3/#shadow
     15
     16        On the second call, the ValueWithCalculation's m_calculation member
     17        was already initialized and the code did not handle this. This patch
     18        updates validateCalculationUnit() to teach it to reuse the previously
     19        parsed calculation in this case. All it needs to do is to update the
     20        existing CSSCalcValue's range to allow negative values or not.
     21
     22        When writing the layout test for this, I also noticed that the CSS
     23        parser was not rejecting negative calculated values for blur-radius
     24        (only negative non-calculated ones). This is because
     25        validateCalculationUnit() was ignoring FNonNeg if the calculated
     26        value is a Length. This patch also addresses the issue.
     27
     28        Test: fast/css/text-shadow-calc-value.html
     29
     30        * css/CSSCalculationValue.h:
     31        (WebCore::CSSCalcValue::setPermittedValueRange):
     32        Add a setter to update the CSSCalculationValue's permitted value range
     33        so that the CSS parser does not need to fully reparse the calculation
     34        only to update the permitted value range.
     35
     36        * css/CSSParser.cpp:
     37        (WebCore::CSSParser::validateCalculationUnit):
     38        - Teach the code to reuse the previously parsed calculation value.
     39        - Do the FNonNeg check for Length calculations as well.
     40
    1412015-01-08  Darin Adler  <darin@apple.com>
    242

  • trunk/Source/WebCore/css/CSSCalculationValue.h

    r177947 r178156  
    9898
    9999    Ref<CalculationValue> createCalculationValue(const CSSToLengthConversionData&) const;
     100    void setPermittedValueRange(CalculationPermittedValueRange);
    100101
    101102    String customCSSText() const;
    … …  
    108109
    109110    const Ref<CSSCalcExpressionNode> m_expression;
    110     const bool m_shouldClampToNonNegative;
     111    bool m_shouldClampToNonNegative;
    111112};
    112113
    … …  
    124125}
    125126
     127inline void CSSCalcValue::setPermittedValueRange(CalculationPermittedValueRange range)
     128{
     129    m_shouldClampToNonNegative = range != CalculationRangeAll;
     130}
     131
    126132} // namespace WebCore
    127133

  • trunk/Source/WebCore/css/CSSParser.cpp

    r178010 r178156  
    15871587    bool mustBeNonNegative = unitFlags & FNonNeg;
    15881588
    1589     ASSERT(!valueWithCalculation.calculation());
    1590     RefPtr<CSSCalcValue> calculation = parseCalculation(valueWithCalculation, mustBeNonNegative ? CalculationRangeNonNegative : CalculationRangeAll);
    1591     if (!calculation)
    1592         return false;
     1589    RefPtr<CSSCalcValue> calculation;
     1590    if (valueWithCalculation.calculation()) {
     1591        // The calculation value was already parsed so we reuse it. However, we may need to update its range.
     1592        calculation = valueWithCalculation.calculation();
     1593        calculation->setPermittedValueRange(mustBeNonNegative ? CalculationRangeNonNegative : CalculationRangeAll);
     1594    } else {
     1595        valueWithCalculation.setCalculation(parseCalculation(valueWithCalculation, mustBeNonNegative ? CalculationRangeNonNegative : CalculationRangeAll));
     1596        calculation = valueWithCalculation.calculation();
     1597        if (!calculation)
     1598            return false;
     1599    }
    15931600
    15941601    bool isValid = false;
    … …  
    16051612    case CalcLength:
    16061613        isValid = (unitFlags & FLength);
     1614        if (isValid && mustBeNonNegative && calculation->isNegative())
     1615            isValid = false;
    16071616        break;
    16081617    case CalcPercent:
    … …  
    16291638        break;
    16301639    }
    1631     if (isValid)
    1632         valueWithCalculation.setCalculation(WTF::move(calculation));
     1640
    16331641    return isValid;
    16341642}
Note: See TracChangeset for help on using the changeset viewer.