svgPath.getTotalLength() freezes webkit
authorsimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 21 Jun 2017 21:13:58 +0000 (21:13 +0000)
committersimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 21 Jun 2017 21:13:58 +0000 (21:13 +0000)
https://bugs.webkit.org/show_bug.cgi?id=173566
<rdar://problem/32866731>

Reviewed by Dean Jackson.

Source/WebCore:

Ensure that curveLength() progresses by making split() return a bool indicating
whether either of the resulting curves are the same as the original. This can happen
when midPoint() on two close points returns a point that is the same as one of the
arguments because of floating-point precision limitations.

Test: svg/custom/path-getTotalLength-hang.html

* platform/graphics/PathTraversalState.cpp:
(WebCore::QuadraticBezier::operator ==):
(WebCore::QuadraticBezier::split):
(WebCore::CubicBezier::operator ==):
(WebCore::CubicBezier::split):
(WebCore::curveLength):

LayoutTests:

* svg/custom/path-getTotalLength-hang.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/svg/custom/path-getTotalLength-hang-expected.txt [new file with mode: 0644]
LayoutTests/svg/custom/path-getTotalLength-hang.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/PathTraversalState.cpp

index 90b176c..1abd497 100644 (file)
@@ -1,3 +1,13 @@
+2017-06-21  Simon Fraser  <simon.fraser@apple.com>
+
+        svgPath.getTotalLength() freezes webkit
+        https://bugs.webkit.org/show_bug.cgi?id=173566
+        <rdar://problem/32866731>
+
+        Reviewed by Dean Jackson.
+
+        * svg/custom/path-getTotalLength-hang.html: Added.
+
 2017-06-21  Claudio Saavedra  <csaavedra@igalia.com>
 
         [WPE] Unreviewed gardening
diff --git a/LayoutTests/svg/custom/path-getTotalLength-hang-expected.txt b/LayoutTests/svg/custom/path-getTotalLength-hang-expected.txt
new file mode 100644 (file)
index 0000000..86c88ab
--- /dev/null
@@ -0,0 +1,5 @@
+This test should not hang.
+
+Path length is 19.154
+
+. 
diff --git a/LayoutTests/svg/custom/path-getTotalLength-hang.html b/LayoutTests/svg/custom/path-getTotalLength-hang.html
new file mode 100644 (file)
index 0000000..2122d19
--- /dev/null
@@ -0,0 +1,25 @@
+<!DOCTYPE html>
+
+<html>
+<style>
+    svg {
+        border: 1px solid black;
+        width: 400px;
+        height: 400px;
+    }
+</style>
+<body>
+    <p>This test should not hang.</p>
+    <p>Path length is <span id="result">uncomputed</span></p>.
+    <svg version="1.1" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" x="0px" y="0px" viewBox="0 0 5000 5000" xml:space="preserve">
+      <path id="word" stroke="#000000" fill="none" stroke-width="20" d="M3045.44,2522.588c-0.73,0.432-1.927,0.575-2.438,0.568 c-1.12-0.01-4.15-0.989-5.847-0.917c-0.543,0.021,0.176-0.286,0.355-0.343c1.537-0.473,5.494-1.193,7.539-0.701 C3046.408,2521.523,3046.107,2522.196,3045.44,2522.588z"/>
+    </svg>
+    <script>
+    if (window.testRunner)
+        testRunner.dumpAsText();
+    var myPath = document.getElementById("word");
+    var length = myPath.getTotalLength();
+    document.getElementById('result').textContent = length.toFixed(3);
+    </script>
+</body>
+</html>
index 6c76bd2..7d4e62b 100644 (file)
@@ -1,3 +1,25 @@
+2017-06-20  Simon Fraser  <simon.fraser@apple.com>
+
+        svgPath.getTotalLength() freezes webkit
+        https://bugs.webkit.org/show_bug.cgi?id=173566
+        <rdar://problem/32866731>
+
+        Reviewed by Dean Jackson.
+
+        Ensure that curveLength() progresses by making split() return a bool indicating
+        whether either of the resulting curves are the same as the original. This can happen
+        when midPoint() on two close points returns a point that is the same as one of the
+        arguments because of floating-point precision limitations.
+
+        Test: svg/custom/path-getTotalLength-hang.html
+
+        * platform/graphics/PathTraversalState.cpp:
+        (WebCore::QuadraticBezier::operator ==):
+        (WebCore::QuadraticBezier::split):
+        (WebCore::CubicBezier::operator ==):
+        (WebCore::CubicBezier::split):
+        (WebCore::curveLength):
+
 2017-06-21  Youenn Fablet  <youenn@apple.com>
 
         Fix AVVideoCaptureSource frameRate setter and getter
index c6bf7a6..9c5339a 100644 (file)
@@ -48,13 +48,20 @@ struct QuadraticBezier {
         , end(e)
     {
     }
+
+    bool operator==(const QuadraticBezier& rhs) const
+    {
+        return start == rhs.start
+            && control == rhs.control
+            && end == rhs.end;
+    }
     
     float approximateDistance() const
     {
         return distanceLine(start, control) + distanceLine(control, end);
     }
     
-    void split(QuadraticBezier& left, QuadraticBezier& right) const
+    bool split(QuadraticBezier& left, QuadraticBezier& right) const
     {
         left.control = midPoint(start, control);
         right.control = midPoint(control, end);
@@ -65,6 +72,8 @@ struct QuadraticBezier {
 
         left.start = start;
         right.end = end;
+
+        return !(left == *this) && !(right == *this);
     }
     
     FloatPoint start;
@@ -81,13 +90,21 @@ struct CubicBezier {
         , end(e)
     {
     }
-    
+
+    bool operator==(const CubicBezier& rhs) const
+    {
+        return start == rhs.start
+            && control1 == rhs.control1
+            && control2 == rhs.control2
+            && end == rhs.end;
+    }
+
     float approximateDistance() const
     {
         return distanceLine(start, control1) + distanceLine(control1, control2) + distanceLine(control2, end);
     }
         
-    void split(CubicBezier& left, CubicBezier& right) const
+    bool split(CubicBezier& left, CubicBezier& right) const
     {    
         FloatPoint startToControl1 = midPoint(control1, control2);
         
@@ -102,6 +119,8 @@ struct CubicBezier {
         FloatPoint leftControl2ToRightControl1 = midPoint(left.control2, right.control1);
         left.end = leftControl2ToRightControl1;
         right.start = leftControl2ToRightControl1;
+
+        return !(left == *this) && !(right == *this);
     }
     
     FloatPoint start;
@@ -127,10 +146,10 @@ static float curveLength(const PathTraversalState& traversalState, const CurveTy
     while (true) {
         float length = curve.approximateDistance();
 
-        if ((length - distanceLine(curve.start, curve.end)) > kPathSegmentLengthTolerance && curveStack.size() < curveStackDepthLimit) {
-            CurveType leftCurve;
-            CurveType rightCurve;
-            curve.split(leftCurve, rightCurve);
+        CurveType leftCurve;
+        CurveType rightCurve;
+
+        if ((length - distanceLine(curve.start, curve.end)) > kPathSegmentLengthTolerance && curveStack.size() < curveStackDepthLimit && curve.split(leftCurve, rightCurve)) {
             curve = leftCurve;
             curveStack.append(rightCurve);
             continue;