ASSERTION FAILED: !rect.isEmpty() : void WebCore::GraphicsContext::drawRect(const...
authorjchaffraix@webkit.org <jchaffraix@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 30 Jul 2012 17:24:14 +0000 (17:24 +0000)
committerjchaffraix@webkit.org <jchaffraix@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 30 Jul 2012 17:24:14 +0000 (17:24 +0000)
https://bugs.webkit.org/show_bug.cgi?id=92187

Reviewed by Simon Fraser.

Source/WebCore:

The border painting logic (RenderBoxModelObject::paintOneBorderSide) would pass a rect with a 0px length
to RenderObject::drawLineForBoxSide. We do check the width (thickness) but not the length so we would pass
the rect to GraphicsContext and hit the ASSERT. This change adds a check for the length too as it is the safest
way, it means that we may still do unneeded operations before bailing out but that's an existing problem in the code.

Tests: fast/borders/0px-borders-no-line-height.html
       fast/borders/double-1px-border-assert.html

* rendering/RenderObject.cpp:
(WebCore::RenderObject::drawLineForBoxSide):
Added a 0px length check. While renaming confusing variables and re-using others,
I also found a potential empty border that I fixed (tested by the 2nd case above).

LayoutTests:

* fast/borders/0px-borders-no-line-height-expected.html: Added.
* fast/borders/0px-borders-no-line-height.html: Added.
* fast/borders/double-1px-border-assert-expected.html: Added.
* fast/borders/double-1px-border-assert.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/borders/0px-borders-no-line-height-expected.html [new file with mode: 0755]
LayoutTests/fast/borders/0px-borders-no-line-height.html [new file with mode: 0755]
LayoutTests/fast/borders/double-1px-border-assert-expected.html [new file with mode: 0644]
LayoutTests/fast/borders/double-1px-border-assert.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderObject.cpp

index 30c0463..9cb1f01 100644 (file)
@@ -1,3 +1,15 @@
+2012-07-30  Julien Chaffraix  <jchaffraix@webkit.org>
+
+        ASSERTION FAILED: !rect.isEmpty()  : void WebCore::GraphicsContext::drawRect(const WebCore::IntRect &)
+        https://bugs.webkit.org/show_bug.cgi?id=92187
+
+        Reviewed by Simon Fraser.
+
+        * fast/borders/0px-borders-no-line-height-expected.html: Added.
+        * fast/borders/0px-borders-no-line-height.html: Added.
+        * fast/borders/double-1px-border-assert-expected.html: Added.
+        * fast/borders/double-1px-border-assert.html: Added.
+
 2012-07-26  Stephen White  <senorblanco@chromium.org>
 
         [chromium] Refactor the computation of resampled bitmap size in
diff --git a/LayoutTests/fast/borders/0px-borders-no-line-height-expected.html b/LayoutTests/fast/borders/0px-borders-no-line-height-expected.html
new file mode 100755 (executable)
index 0000000..6cbd864
--- /dev/null
@@ -0,0 +1,17 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+.border {
+    border-right: 1px solid black;
+    border-left: 1px solid black;
+    line-height: 0;
+}
+</style>
+</head>
+<body>
+<p>Test for <a href="https://bugs.webkit.org/show_bug.cgi?id=92187">92187</a>: ASSERTION FAILED: !rect.isEmpty()  : void WebCore::GraphicsContext::drawRect(const WebCore::IntRect &)</p>
+<p>This test passes if it doesn't ASSERT.</p>
+<div class="border">&nsbp;</div>
+</body>
+</html>
diff --git a/LayoutTests/fast/borders/0px-borders-no-line-height.html b/LayoutTests/fast/borders/0px-borders-no-line-height.html
new file mode 100755 (executable)
index 0000000..f6dad41
--- /dev/null
@@ -0,0 +1,17 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+.border {
+    border-style: solid;
+    border-width: 0 1px 0 1px;
+    line-height: 0;
+}
+</style>
+</head>
+<body>
+<p>Test for <a href="https://bugs.webkit.org/show_bug.cgi?id=92187">92187</a>: ASSERTION FAILED: !rect.isEmpty()  : void WebCore::GraphicsContext::drawRect(const WebCore::IntRect &)</p>
+<p>This test passes if it doesn't ASSERT.</p>
+<div class="border">&nsbp;</div>
+</body>
+</html>
diff --git a/LayoutTests/fast/borders/double-1px-border-assert-expected.html b/LayoutTests/fast/borders/double-1px-border-assert-expected.html
new file mode 100644 (file)
index 0000000..92f5a7a
--- /dev/null
@@ -0,0 +1,16 @@
+<!DOCTYPE html>
+<html>
+<body>
+<style>
+.test {
+    width: 1px;
+    height: 1px;
+    border-style: double;
+    border-width: 0px 3px 0px 3px;
+}
+</style>
+<p>Test for <a href="https://bugs.webkit.org/show_bug.cgi?id=92187">92187</a>: ASSERTION FAILED: !rect.isEmpty()  : void WebCore::GraphicsContext::drawRect(const WebCore::IntRect &)</p>
+<p>This test passes if it doesn't ASSERT.</p>
+<div class="test">&nbsp;</div>
+</html>
+</body>
diff --git a/LayoutTests/fast/borders/double-1px-border-assert.html b/LayoutTests/fast/borders/double-1px-border-assert.html
new file mode 100644 (file)
index 0000000..92f5a7a
--- /dev/null
@@ -0,0 +1,16 @@
+<!DOCTYPE html>
+<html>
+<body>
+<style>
+.test {
+    width: 1px;
+    height: 1px;
+    border-style: double;
+    border-width: 0px 3px 0px 3px;
+}
+</style>
+<p>Test for <a href="https://bugs.webkit.org/show_bug.cgi?id=92187">92187</a>: ASSERTION FAILED: !rect.isEmpty()  : void WebCore::GraphicsContext::drawRect(const WebCore::IntRect &)</p>
+<p>This test passes if it doesn't ASSERT.</p>
+<div class="test">&nbsp;</div>
+</html>
+</body>
index 7e2f338..29e87b9 100644 (file)
@@ -1,3 +1,23 @@
+2012-07-30  Julien Chaffraix  <jchaffraix@webkit.org>
+
+        ASSERTION FAILED: !rect.isEmpty()  : void WebCore::GraphicsContext::drawRect(const WebCore::IntRect &)
+        https://bugs.webkit.org/show_bug.cgi?id=92187
+
+        Reviewed by Simon Fraser.
+
+        The border painting logic (RenderBoxModelObject::paintOneBorderSide) would pass a rect with a 0px length
+        to RenderObject::drawLineForBoxSide. We do check the width (thickness) but not the length so we would pass
+        the rect to GraphicsContext and hit the ASSERT. This change adds a check for the length too as it is the safest
+        way, it means that we may still do unneeded operations before bailing out but that's an existing problem in the code.
+
+        Tests: fast/borders/0px-borders-no-line-height.html
+               fast/borders/double-1px-border-assert.html
+
+        * rendering/RenderObject.cpp:
+        (WebCore::RenderObject::drawLineForBoxSide):
+        Added a 0px length check. While renaming confusing variables and re-using others,
+        I also found a potential empty border that I fixed (tested by the 2nd case above).
+
 2012-07-26  Stephen White  <senorblanco@chromium.org>
 
         [chromium] Refactor the computation of resampled bitmap size in
index 6391334..42cadb3 100755 (executable)
@@ -834,14 +834,22 @@ void RenderObject::drawLineForBoxSide(GraphicsContext* graphicsContext, int x1,
                                       BoxSide side, Color color, EBorderStyle style,
                                       int adjacentWidth1, int adjacentWidth2, bool antialias)
 {
-    int width = (side == BSTop || side == BSBottom ? y2 - y1 : x2 - x1);
+    int thickness;
+    int length;
+    if (side == BSTop || side == BSBottom) {
+        thickness = y2 - y1;
+        length = x2 - x1;
+    } else {
+        thickness = x2 - x1;
+        length = y2 - y1;
+    }
 
-    // FIXME: We really would like this check to be an ASSERT as we don't want to draw 0px borders. However
-    // nothing guarantees that the following recursive calls to drawLineForBoxSide will have non-null width.
-    if (!width)
+    // FIXME: We really would like this check to be an ASSERT as we don't want to draw empty borders. However
+    // nothing guarantees that the following recursive calls to drawLineForBoxSide will have non-null dimensions.
+    if (!thickness || !length)
         return;
 
-    if (style == DOUBLE && width < 3)
+    if (style == DOUBLE && thickness < 3)
         style = SOLID;
 
     switch (style) {
@@ -851,11 +859,11 @@ void RenderObject::drawLineForBoxSide(GraphicsContext* graphicsContext, int x1,
         case DOTTED:
         case DASHED: {
             graphicsContext->setStrokeColor(color, m_style->colorSpace());
-            graphicsContext->setStrokeThickness(width);
+            graphicsContext->setStrokeThickness(thickness);
             StrokeStyle oldStrokeStyle = graphicsContext->strokeStyle();
             graphicsContext->setStrokeStyle(style == DASHED ? DashedStroke : DottedStroke);
 
-            if (width > 0) {
+            if (thickness > 0) {
                 bool wasAntialiased = graphicsContext->shouldAntialias();
                 graphicsContext->setShouldAntialias(antialias);
 
@@ -875,7 +883,8 @@ void RenderObject::drawLineForBoxSide(GraphicsContext* graphicsContext, int x1,
             break;
         }
         case DOUBLE: {
-            int third = (width + 1) / 3;
+            int thirdOfThickness = (thickness + 1) / 3;
+            ASSERT(thirdOfThickness);
 
             if (adjacentWidth1 == 0 && adjacentWidth2 == 0) {
                 StrokeStyle oldStrokeStyle = graphicsContext->strokeStyle();
@@ -888,16 +897,16 @@ void RenderObject::drawLineForBoxSide(GraphicsContext* graphicsContext, int x1,
                 switch (side) {
                     case BSTop:
                     case BSBottom:
-                        graphicsContext->drawRect(IntRect(x1, y1, x2 - x1, third));
-                        graphicsContext->drawRect(IntRect(x1, y2 - third, x2 - x1, third));
+                        graphicsContext->drawRect(IntRect(x1, y1, length, thirdOfThickness));
+                        graphicsContext->drawRect(IntRect(x1, y2 - thirdOfThickness, length, thirdOfThickness));
                         break;
                     case BSLeft:
-                        graphicsContext->drawRect(IntRect(x1, y1 + 1, third, y2 - y1 - 1));
-                        graphicsContext->drawRect(IntRect(x2 - third, y1 + 1, third, y2 - y1 - 1));
-                        break;
                     case BSRight:
-                        graphicsContext->drawRect(IntRect(x1, y1 + 1, third, y2 - y1 - 1));
-                        graphicsContext->drawRect(IntRect(x2 - third, y1 + 1, third, y2 - y1 - 1));
+                        // FIXME: Why do we offset the border by 1 in this case but not the other one?
+                        if (length > 1) {
+                            graphicsContext->drawRect(IntRect(x1, y1 + 1, thirdOfThickness, length - 1));
+                            graphicsContext->drawRect(IntRect(x2 - thirdOfThickness, y1 + 1, thirdOfThickness, length - 1));
+                        }
                         break;
                 }
 
@@ -910,33 +919,33 @@ void RenderObject::drawLineForBoxSide(GraphicsContext* graphicsContext, int x1,
                 switch (side) {
                     case BSTop:
                         drawLineForBoxSide(graphicsContext, x1 + max((-adjacentWidth1 * 2 + 1) / 3, 0),
-                                   y1, x2 - max((-adjacentWidth2 * 2 + 1) / 3, 0), y1 + third,
+                                   y1, x2 - max((-adjacentWidth2 * 2 + 1) / 3, 0), y1 + thirdOfThickness,
                                    side, color, SOLID, adjacent1BigThird, adjacent2BigThird, antialias);
                         drawLineForBoxSide(graphicsContext, x1 + max((adjacentWidth1 * 2 + 1) / 3, 0),
-                                   y2 - third, x2 - max((adjacentWidth2 * 2 + 1) / 3, 0), y2,
+                                   y2 - thirdOfThickness, x2 - max((adjacentWidth2 * 2 + 1) / 3, 0), y2,
                                    side, color, SOLID, adjacent1BigThird, adjacent2BigThird, antialias);
                         break;
                     case BSLeft:
                         drawLineForBoxSide(graphicsContext, x1, y1 + max((-adjacentWidth1 * 2 + 1) / 3, 0),
-                                   x1 + third, y2 - max((-adjacentWidth2 * 2 + 1) / 3, 0),
+                                   x1 + thirdOfThickness, y2 - max((-adjacentWidth2 * 2 + 1) / 3, 0),
                                    side, color, SOLID, adjacent1BigThird, adjacent2BigThird, antialias);
-                        drawLineForBoxSide(graphicsContext, x2 - third, y1 + max((adjacentWidth1 * 2 + 1) / 3, 0),
+                        drawLineForBoxSide(graphicsContext, x2 - thirdOfThickness, y1 + max((adjacentWidth1 * 2 + 1) / 3, 0),
                                    x2, y2 - max((adjacentWidth2 * 2 + 1) / 3, 0),
                                    side, color, SOLID, adjacent1BigThird, adjacent2BigThird, antialias);
                         break;
                     case BSBottom:
                         drawLineForBoxSide(graphicsContext, x1 + max((adjacentWidth1 * 2 + 1) / 3, 0),
-                                   y1, x2 - max((adjacentWidth2 * 2 + 1) / 3, 0), y1 + third,
+                                   y1, x2 - max((adjacentWidth2 * 2 + 1) / 3, 0), y1 + thirdOfThickness,
                                    side, color, SOLID, adjacent1BigThird, adjacent2BigThird, antialias);
                         drawLineForBoxSide(graphicsContext, x1 + max((-adjacentWidth1 * 2 + 1) / 3, 0),
-                                   y2 - third, x2 - max((-adjacentWidth2 * 2 + 1) / 3, 0), y2,
+                                   y2 - thirdOfThickness, x2 - max((-adjacentWidth2 * 2 + 1) / 3, 0), y2,
                                    side, color, SOLID, adjacent1BigThird, adjacent2BigThird, antialias);
                         break;
                     case BSRight:
                         drawLineForBoxSide(graphicsContext, x1, y1 + max((adjacentWidth1 * 2 + 1) / 3, 0),
-                                   x1 + third, y2 - max((adjacentWidth2 * 2 + 1) / 3, 0),
+                                   x1 + thirdOfThickness, y2 - max((adjacentWidth2 * 2 + 1) / 3, 0),
                                    side, color, SOLID, adjacent1BigThird, adjacent2BigThird, antialias);
-                        drawLineForBoxSide(graphicsContext, x2 - third, y1 + max((-adjacentWidth1 * 2 + 1) / 3, 0),
+                        drawLineForBoxSide(graphicsContext, x2 - thirdOfThickness, y1 + max((-adjacentWidth1 * 2 + 1) / 3, 0),
                                    x2, y2 - max((-adjacentWidth2 * 2 + 1) / 3, 0),
                                    side, color, SOLID, adjacent1BigThird, adjacent2BigThird, antialias);
                         break;