ASSERTION FAILED: !length.isUndefined() in WebCore::GridLength::GridLength
authorsvillar@igalia.com <svillar@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 26 Feb 2015 10:58:15 +0000 (10:58 +0000)
committersvillar@igalia.com <svillar@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 26 Feb 2015 10:58:15 +0000 (10:58 +0000)
https://bugs.webkit.org/show_bug.cgi?id=141645

Reviewed by Chris Dumez.

Source/WebCore:

This bug has been here since r110484 but was uncovered by
r180140. The problem r110484 was trying to fix was that
CSSPrimitiveValue::convertToLength<Length> ended up calling
CSSPrimitiveValue::computeLengthDouble() which was apparently
dereferencing conversionData.style() and
conversionData.rootStyle() pointers without checking them. That's
why that fix added this condition to convertToLength():

isFontRelativeLength() && (!conversionData.style() || !conversionData.rootStyle())

which is not correct, because for the 4 possible font relative
length types, 3 of them just use the style() pointer and the other
one just uses rootStyle() which BTW could be NULL. This erroneous
condition makes that function to return Length(Undefined) more
often than it should.

From now on it only returns Length(Undefined) if the style()
pointer is NULL and the font relative length type is one in the
set (CSS_EMS, CSS_EXS, CSS_CHS);

Test: fast/css-grid-layout/grid-with-relative-font-length-crash.html

* css/CSSPrimitiveValue.h:
* css/CSSPrimitiveValueMappings.h:
(WebCore::CSSPrimitiveValue::convertingToLengthRequiresNonNullStyle):
(WebCore::CSSPrimitiveValue::convertToLength):

LayoutTests:

* fast/css-grid-layout/grid-with-relative-font-length-crash-expected.txt: Added.
* fast/css-grid-layout/grid-with-relative-font-length-crash.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/css-grid-layout/grid-with-relative-font-length-crash-expected.txt [new file with mode: 0644]
LayoutTests/fast/css-grid-layout/grid-with-relative-font-length-crash.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/css/CSSPrimitiveValue.h
Source/WebCore/css/CSSPrimitiveValueMappings.h

index b61d873..b785825 100644 (file)
@@ -1,3 +1,13 @@
+2015-02-26  Sergio Villar Senin  <svillar@igalia.com>
+
+        ASSERTION FAILED: !length.isUndefined() in WebCore::GridLength::GridLength
+        https://bugs.webkit.org/show_bug.cgi?id=141645
+
+        Reviewed by Chris Dumez.
+
+        * fast/css-grid-layout/grid-with-relative-font-length-crash-expected.txt: Added.
+        * fast/css-grid-layout/grid-with-relative-font-length-crash.html: Added.
+
 2015-02-25  Zalan Bujtas  <zalan@apple.com>
 
         Black line across screen on Adobe Illustrator detail page (non-retina only)
diff --git a/LayoutTests/fast/css-grid-layout/grid-with-relative-font-length-crash-expected.txt b/LayoutTests/fast/css-grid-layout/grid-with-relative-font-length-crash-expected.txt
new file mode 100644 (file)
index 0000000..72f6218
--- /dev/null
@@ -0,0 +1 @@
+The test PASSES if it does not crash on Debug builds.
diff --git a/LayoutTests/fast/css-grid-layout/grid-with-relative-font-length-crash.html b/LayoutTests/fast/css-grid-layout/grid-with-relative-font-length-crash.html
new file mode 100644 (file)
index 0000000..3d762a7
--- /dev/null
@@ -0,0 +1,13 @@
+<!DOCTYPE html>
+<style>
+title {
+    -webkit-grid: dense row 1ex;
+}
+</style>
+<title>
+</title>
+<script type="text/javascript">
+if (window.testRunner)
+    testRunner.dumpAsText();
+</script>
+<p>The test PASSES if it does not crash on Debug builds.</p>
index 462d053..df557f5 100644 (file)
@@ -1,3 +1,38 @@
+2015-02-26  Sergio Villar Senin  <svillar@igalia.com>
+
+        ASSERTION FAILED: !length.isUndefined() in WebCore::GridLength::GridLength
+        https://bugs.webkit.org/show_bug.cgi?id=141645
+
+        Reviewed by Chris Dumez.
+
+        This bug has been here since r110484 but was uncovered by
+        r180140. The problem r110484 was trying to fix was that
+        CSSPrimitiveValue::convertToLength<Length> ended up calling
+        CSSPrimitiveValue::computeLengthDouble() which was apparently
+        dereferencing conversionData.style() and
+        conversionData.rootStyle() pointers without checking them. That's
+        why that fix added this condition to convertToLength():
+
+        isFontRelativeLength() && (!conversionData.style() || !conversionData.rootStyle())
+
+        which is not correct, because for the 4 possible font relative
+        length types, 3 of them just use the style() pointer and the other
+        one just uses rootStyle() which BTW could be NULL. This erroneous
+        condition makes that function to return Length(Undefined) more
+        often than it should.
+
+        From now on it only returns Length(Undefined) if the style()
+        pointer is NULL and the font relative length type is one in the
+        set (CSS_EMS, CSS_EXS, CSS_CHS);
+
+        Test: fast/css-grid-layout/grid-with-relative-font-length-crash.html
+
+        * css/CSSPrimitiveValue.h:
+        * css/CSSPrimitiveValueMappings.h:
+        (WebCore::CSSPrimitiveValue::convertingToLengthRequiresNonNullStyle):
+        (WebCore::CSSPrimitiveValue::convertToLength):
+
+
 2015-02-26  Andreas Kling  <akling@apple.com>
 
         [Cocoa] Prod libcache to drop caches in memory pressure relief handler.
index 50874ee..e8a7d8f 100644 (file)
@@ -291,6 +291,8 @@ public:
     // Converts to a Length, mapping various unit types appropriately.
     template<int> Length convertToLength(const CSSToLengthConversionData&) const;
 
+    bool convertingToLengthRequiresNonNullStyle(int lengthConversion) const;
+
     // use with care!!!
     void setPrimitiveType(unsigned short type) { m_primitiveUnitType = type; }
 
index d723377..ee791af 100644 (file)
@@ -4585,9 +4585,23 @@ enum LengthConversion {
     CalculatedConversion = 1 << 4
 };
 
+inline bool CSSPrimitiveValue::convertingToLengthRequiresNonNullStyle(int lengthConversion) const
+{
+    ASSERT(isFontRelativeLength());
+    // This matches the implementation in CSSPrimitiveValue::computeLengthDouble().
+    switch (m_primitiveUnitType) {
+    case CSS_EMS:
+    case CSS_EXS:
+    case CSS_CHS:
+        return lengthConversion & (FixedIntegerConversion | FixedFloatConversion);
+    default:
+        return false;
+    }
+}
+
 template<int supported> Length CSSPrimitiveValue::convertToLength(const CSSToLengthConversionData& conversionData) const
 {
-    if ((supported & (FixedIntegerConversion | FixedFloatConversion)) && isFontRelativeLength() && (!conversionData.style() || !conversionData.rootStyle()))
+    if (isFontRelativeLength() && convertingToLengthRequiresNonNullStyle(supported) && !conversionData.style())
         return Length(Undefined);
     if ((supported & FixedIntegerConversion) && isLength())
         return computeLength<Length>(conversionData);