flexbox assert fails with auto-sized item with padding
authortony@chromium.org <tony@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 28 Sep 2012 17:14:43 +0000 (17:14 +0000)
committertony@chromium.org <tony@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 28 Sep 2012 17:14:43 +0000 (17:14 +0000)
https://bugs.webkit.org/show_bug.cgi?id=97606

Reviewed by Ojan Vafai.

Source/WebCore:

Depending on the denominator of FractionalLayoutUnit, we can lose precision when
converting to a float.  This would cause a rounding error in flex-shrink to trigger an ASSERT.
To avoid this problem in the future, switch to using doubles for flex-shrink and flex-grow
at layout time.  The CSS values themselves are still floats.

Test: css3/flexbox/negative-flex-rounding-assert.html

* rendering/RenderFlexibleBox.cpp:
(WebCore::RenderFlexibleBox::layoutFlexItems): Use doubles for local variables.
(WebCore::RenderFlexibleBox::computeNextFlexLine): Pass in doubles.
(WebCore::RenderFlexibleBox::freezeViolations): Pass in doubles.
(WebCore::RenderFlexibleBox::resolveFlexibleLengths): Pass in doubles.
* rendering/RenderFlexibleBox.h:

LayoutTests:

Add a test that hits an ASSERT when FractionalLayoutUnit denominator is 60.

* css3/flexbox/negative-flex-rounding-assert-expected.txt: Added.
* css3/flexbox/negative-flex-rounding-assert.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/css3/flexbox/negative-flex-rounding-assert-expected.txt [new file with mode: 0644]
LayoutTests/css3/flexbox/negative-flex-rounding-assert.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderFlexibleBox.cpp
Source/WebCore/rendering/RenderFlexibleBox.h

index 93fe32a..58c04b4 100644 (file)
@@ -1,3 +1,15 @@
+2012-09-27  Tony Chang  <tony@chromium.org>
+
+        flexbox assert fails with auto-sized item with padding
+        https://bugs.webkit.org/show_bug.cgi?id=97606
+
+        Reviewed by Ojan Vafai.
+
+        Add a test that hits an ASSERT when FractionalLayoutUnit denominator is 60.
+
+        * css3/flexbox/negative-flex-rounding-assert-expected.txt: Added.
+        * css3/flexbox/negative-flex-rounding-assert.html: Added.
+
 2012-09-28  Harald Tveit Alvestrand  <harald@alvestrand.no>
         
         Implement the GetStats interface on PeerConnection
diff --git a/LayoutTests/css3/flexbox/negative-flex-rounding-assert-expected.txt b/LayoutTests/css3/flexbox/negative-flex-rounding-assert-expected.txt
new file mode 100644 (file)
index 0000000..21a3df5
--- /dev/null
@@ -0,0 +1,8 @@
+This test passes if it does not crash.
+
+x
+y
+x
+y
+z
+
diff --git a/LayoutTests/css3/flexbox/negative-flex-rounding-assert.html b/LayoutTests/css3/flexbox/negative-flex-rounding-assert.html
new file mode 100644 (file)
index 0000000..1391dc0
--- /dev/null
@@ -0,0 +1,47 @@
+<!DOCTYPE html>
+<html>
+<style>
+.math {
+    display: -webkit-inline-flex;
+}
+.math > * {
+    padding-bottom: 0.35em;
+}
+.mfrac {
+    display: -webkit-inline-flex;
+    -webkit-flex-direction: column;
+}
+.mfrac > :first-child {
+    -webkit-margin-after: 0.2em;
+}
+.mfrac > :last-child {
+    -webkit-margin-before: 0.2em;
+}
+.x {
+    line-height: 9px;
+}
+.y {
+    line-height: 12px;
+}
+</style>
+<script>
+if (window.testRunner)
+    testRunner.dumpAsText();
+</script>
+<p>This test passes if it does not crash.</p>
+<div class=math>
+    <div class=mfrac>
+        <div class=mfrac>
+            <div class=x>x</div>
+            <div class=y>y</div>
+        </div>
+        <div class=mfrac>
+            <div class=mfrac>
+                <div class=x>x</div>
+                <div class=y>y</div>
+            </div>
+            <div class=x>z</div>
+        </div>
+    </div>
+</div>
+</html>
index 50061fa..ecbb56b 100644 (file)
@@ -1,3 +1,24 @@
+2012-09-27  Tony Chang  <tony@chromium.org>
+
+        flexbox assert fails with auto-sized item with padding
+        https://bugs.webkit.org/show_bug.cgi?id=97606
+
+        Reviewed by Ojan Vafai.
+
+        Depending on the denominator of FractionalLayoutUnit, we can lose precision when
+        converting to a float.  This would cause a rounding error in flex-shrink to trigger an ASSERT.
+        To avoid this problem in the future, switch to using doubles for flex-shrink and flex-grow
+        at layout time.  The CSS values themselves are still floats.
+
+        Test: css3/flexbox/negative-flex-rounding-assert.html
+
+        * rendering/RenderFlexibleBox.cpp:
+        (WebCore::RenderFlexibleBox::layoutFlexItems): Use doubles for local variables.
+        (WebCore::RenderFlexibleBox::computeNextFlexLine): Pass in doubles.
+        (WebCore::RenderFlexibleBox::freezeViolations): Pass in doubles.
+        (WebCore::RenderFlexibleBox::resolveFlexibleLengths): Pass in doubles.
+        * rendering/RenderFlexibleBox.h:
+
 2012-09-28  Sheriff Bot  <webkit.review.bot@gmail.com>
 
         Unreviewed, rolling out r129751.
index abc73dd..b530b3a 100644 (file)
@@ -615,8 +615,8 @@ void RenderFlexibleBox::layoutFlexItems(OrderIterator& iterator, WTF::Vector<Lin
 {
     OrderedFlexItemList orderedChildren;
     LayoutUnit preferredMainAxisExtent;
-    float totalFlexGrow;
-    float totalWeightedFlexShrink;
+    double totalFlexGrow;
+    double totalWeightedFlexShrink;
     LayoutUnit minMaxAppliedMainAxisExtent;
 
     LayoutUnit crossAxisOffset = flowAwareBorderBefore() + flowAwarePaddingBefore();
@@ -797,7 +797,7 @@ LayoutUnit RenderFlexibleBox::adjustChildSizeForMinAndMax(RenderBox* child, Layo
     return std::max(childSize, minExtent);
 }
 
-bool RenderFlexibleBox::computeNextFlexLine(OrderIterator& iterator, OrderedFlexItemList& orderedChildren, LayoutUnit& preferredMainAxisExtent, float& totalFlexGrow, float& totalWeightedFlexShrink, LayoutUnit& minMaxAppliedMainAxisExtent)
+bool RenderFlexibleBox::computeNextFlexLine(OrderIterator& iterator, OrderedFlexItemList& orderedChildren, LayoutUnit& preferredMainAxisExtent, double& totalFlexGrow, double& totalWeightedFlexShrink, LayoutUnit& minMaxAppliedMainAxisExtent)
 {
     orderedChildren.clear();
     preferredMainAxisExtent = 0;
@@ -832,7 +832,7 @@ bool RenderFlexibleBox::computeNextFlexLine(OrderIterator& iterator, OrderedFlex
     return true;
 }
 
-void RenderFlexibleBox::freezeViolations(const WTF::Vector<Violation>& violations, LayoutUnit& availableFreeSpace, float& totalFlexGrow, float& totalWeightedFlexShrink, InflexibleFlexItemSize& inflexibleItems)
+void RenderFlexibleBox::freezeViolations(const WTF::Vector<Violation>& violations, LayoutUnit& availableFreeSpace, double& totalFlexGrow, double& totalWeightedFlexShrink, InflexibleFlexItemSize& inflexibleItems)
 {
     for (size_t i = 0; i < violations.size(); ++i) {
         RenderBox* child = violations[i].child;
@@ -846,7 +846,7 @@ void RenderFlexibleBox::freezeViolations(const WTF::Vector<Violation>& violation
 }
 
 // Returns true if we successfully ran the algorithm and sized the flex items.
-bool RenderFlexibleBox::resolveFlexibleLengths(FlexSign flexSign, const OrderedFlexItemList& children, LayoutUnit& availableFreeSpace, float& totalFlexGrow, float& totalWeightedFlexShrink, InflexibleFlexItemSize& inflexibleItems, WTF::Vector<LayoutUnit>& childSizes)
+bool RenderFlexibleBox::resolveFlexibleLengths(FlexSign flexSign, const OrderedFlexItemList& children, LayoutUnit& availableFreeSpace, double& totalFlexGrow, double& totalWeightedFlexShrink, InflexibleFlexItemSize& inflexibleItems, WTF::Vector<LayoutUnit>& childSizes)
 {
     childSizes.clear();
     LayoutUnit totalViolation = 0;
index fafa8be..2ace3c4 100644 (file)
@@ -123,10 +123,10 @@ private:
     LayoutUnit computeChildMarginValue(Length margin, RenderView*);
     void computeMainAxisPreferredSizes(bool relayoutChildren, OrderHashSet&);
     LayoutUnit adjustChildSizeForMinAndMax(RenderBox*, LayoutUnit childSize);
-    bool computeNextFlexLine(OrderIterator&, OrderedFlexItemList& orderedChildren, LayoutUnit& preferredMainAxisExtent, float& totalFlexGrow, float& totalWeightedFlexShrink, LayoutUnit& minMaxAppliedMainAxisExtent);
+    bool computeNextFlexLine(OrderIterator&, OrderedFlexItemList& orderedChildren, LayoutUnit& preferredMainAxisExtent, double& totalFlexGrow, double& totalWeightedFlexShrink, LayoutUnit& minMaxAppliedMainAxisExtent);
 
-    bool resolveFlexibleLengths(FlexSign, const OrderedFlexItemList&, LayoutUnit& availableFreeSpace, float& totalFlexGrow, float& totalWeightedFlexShrink, InflexibleFlexItemSize&, WTF::Vector<LayoutUnit>& childSizes);
-    void freezeViolations(const WTF::Vector<Violation>&, LayoutUnit& availableFreeSpace, float& totalFlexGrow, float& totalWeightedFlexShrink, InflexibleFlexItemSize&);
+    bool resolveFlexibleLengths(FlexSign, const OrderedFlexItemList&, LayoutUnit& availableFreeSpace, double& totalFlexGrow, double& totalWeightedFlexShrink, InflexibleFlexItemSize&, WTF::Vector<LayoutUnit>& childSizes);
+    void freezeViolations(const WTF::Vector<Violation>&, LayoutUnit& availableFreeSpace, double& totalFlexGrow, double& totalWeightedFlexShrink, InflexibleFlexItemSize&);
 
     void setLogicalOverrideSize(RenderBox* child, LayoutUnit childPreferredSize);
     void prepareChildForPositionedLayout(RenderBox* child, LayoutUnit mainAxisOffset, LayoutUnit crossAxisOffset, PositionedLayoutMode);