Bug 173566 - svgPath.getTotalLength() freezes webkit
Summary: svgPath.getTotalLength() freezes webkit
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Blocker
Assignee: Simon Fraser (smfr)
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-06-19 14:54 PDT by xerxesnoble
Modified: 2017-06-21 14:14 PDT (History)
7 users (show)

See Also:


Attachments
Testcase (this will cause a hang) (664 bytes, text/html)
2017-06-20 10:47 PDT, Simon Fraser (smfr)
no flags Details
Possible patch, needs test case. (3.74 KB, patch)
2017-06-20 19:16 PDT, Simon Fraser (smfr)
no flags Details | Formatted Diff | Diff
Patch (5.99 KB, patch)
2017-06-21 13:51 PDT, Simon Fraser (smfr)
dino: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description xerxesnoble 2017-06-19 14:54:54 PDT
Browser simply just chokes and crashes while evaluating the total length of a specific path. I've tried to remove certain points from the path to try and break down the issue. It works on every other browser I've tested so far. Refactored my code to avoid using getTotalLength(). 

My guess is an infinite loop. 

Stack overflow example here: https://stackoverflow.com/questions/44055554/svgpath-gettotallength-crashes-safari-on-particular-path


Example code:


var myPath = document.getElementById("word");
var length = myPath.getTotalLength();
alert(length);

<svg version="1.1" id="Layer_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>
Comment 1 Radar WebKit Bug Importer 2017-06-19 23:09:28 PDT
<rdar://problem/32866731>
Comment 2 Alexey Proskuryakov 2017-06-20 03:17:47 PDT
I cannot reproduce a crash, looks like a permanent freeze.
Comment 3 Simon Fraser (smfr) 2017-06-20 10:47:59 PDT
Created attachment 313413 [details]
Testcase (this will cause a hang)
Comment 4 Simon Fraser (smfr) 2017-06-20 10:48:26 PDT
Stuck under:

   1699 WebCore::SVGPathParser::parseCurveToCubicSegment()  (in WebCore) + 222  [0x109c2c80e]
     1699 WebCore::SVGPathTraversalStateBuilder::curveToCubic(WebCore::FloatPoint const&, WebCore::FloatPoint const&, WebCore::FloatPoint const&, WebCore::PathCoordinateMode)  (in WebCore) + 62  [0x10ac65ede]
       1699 WebCore::PathTraversalState::processPathElement(WebCore::PathElementType, WebCore::FloatPoint const*)  (in WebCore) + 101  [0x10a8cd2c5]
         1699 WebCore::PathTraversalState::appendPathElement(WebCore::PathElementType, WebCore::FloatPoint const*)  (in WebCore) + 151  [0x10a8cd1f7]
           1699 WebCore::PathTraversalState::cubicBezierTo(WebCore::FloatPoint const&, WebCore::FloatPoint const&, WebCore::FloatPoint const&)  (in WebCore) + 390,383,...  [0x10a8cce16,0x10a8cce0f,...]
Comment 5 xerxesnoble 2017-06-20 16:20:02 PDT
(In reply to Alexey Proskuryakov from comment #2)
> I cannot reproduce a crash, looks like a permanent freeze.

Yes, sorry poor choice of working there. There is no crash- it's just completely stalled.
Comment 6 Simon Fraser (smfr) 2017-06-20 18:52:36 PDT
In curveLength(), length = curve.approximateDistance(); is 0, so we never make progress.
Comment 7 Simon Fraser (smfr) 2017-06-20 18:59:36 PDT
Actually length is just small value, but after the split, leftCurve is empty, and rightCurve is the same as the original curve, so we make no progress.
Comment 8 Simon Fraser (smfr) 2017-06-20 19:07:09 PDT
(lldb) p curve
(WebCore::CubicBezier) $6 = {
  start = (m_x = 3036.95605, m_y = 2522.19727)
  control1 = (m_x = 3036.95605, m_y = 2522.19727)
  control2 = (m_x = 3036.95581, m_y = 2522.19727)
  end = (m_x = 3036.95581, m_y = 2522.19702)
}
(lldb) p leftCurve
(WebCore::CubicBezier) $7 = {
  start = (m_x = 3036.95605, m_y = 2522.19727)
  control1 = (m_x = 3036.95605, m_y = 2522.19727)
  control2 = (m_x = 3036.95605, m_y = 2522.19727)
  end = (m_x = 3036.95605, m_y = 2522.19727)
}
(lldb) p rightCurve
(WebCore::CubicBezier) $8 = {
  start = (m_x = 3036.95605, m_y = 2522.19727)
  control1 = (m_x = 3036.95605, m_y = 2522.19727)
  control2 = (m_x = 3036.95581, m_y = 2522.19727)
  end = (m_x = 3036.95581, m_y = 2522.19702)
}

In floats, (3036.95605 + 3036.95581) / 2.0f == 3036.95605 so split() returns a curve that matches the original.
Comment 9 Simon Fraser (smfr) 2017-06-20 19:16:06 PDT
Created attachment 313467 [details]
Possible patch, needs test case.
Comment 10 Build Bot 2017-06-20 19:17:35 PDT
Attachment 313467 [details] did not pass style-queue:


ERROR: Source/WebCore/ChangeLog:8:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
Total errors found: 1 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 11 Simon Fraser (smfr) 2017-06-21 13:51:23 PDT
Created attachment 313545 [details]
Patch
Comment 12 Dean Jackson 2017-06-21 13:52:54 PDT
Comment on attachment 313545 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=313545&action=review

> Source/WebCore/platform/graphics/PathTraversalState.cpp:52
> +    bool operator ==(const QuadraticBezier& rhs)

Nit: operator==
Comment 13 Simon Fraser (smfr) 2017-06-21 14:14:21 PDT
https://trac.webkit.org/r218648