Bug ID: JDK-8163582 JavaFX browser can get stuck in an infinite loop when calling path.getTotalLength()

Type: Bug
Component: javafx
Sub-Component: web
Affected Version: 8u112,9

Priority: P3
Status: Resolved
Resolution: Fixed

Submitted: 2016-08-10
Updated: 2020-01-31
Resolved: 2016-08-12

JDK 8	JDK 9
8u112Fixed	9Fixed

It seems that the WebView JavaFx Browser can get stuck in an infinite loop when calling getTotalLength() on a particular svg <path> object.

Here are two Jsfiddles, the first one works, the second one gets in an infinite loop. The only difference is changing the last decimal of the path data from a 6 to a 7.

http://jsfiddle.net/UBsu2/
http://jsfiddle.net/UBsu2/1/

This problem is also exhibited in Chrome and Safari, but not in Firefox and IE9.

UR SQE OK to take the fix of regression introduced in the PSU16_04
15-08-2016
Changeset: 1356c2c9355a Author: ghb Date: 2016-08-12 22:23 +0530 URL: http://hg.openjdk.java.net/openjfx/9-dev/rt/rev/1356c2c9355a
12-08-2016
The new version of the test is much cleaner. Thank you Arun for your suggestion and Guru for implementing it. +1 Please push the fix today if possible.
12-08-2016
With nit : http://cr.openjdk.java.net/~ghb/8163582/webrev.04/
12-08-2016
lgtm with nit. + svgPaths.forEach((pathData, expected)->{ space should be given before and after -> e.g. svgPaths.forEach((pathData, expected) -> { + assertEquals(msg, + expected[0], (Double) totalLength, expected[1]); typecasting to Double is not required. + svgPaths.put("'M 778.4191616766467 375.19086364081954 C 781.239563 " + + "375.1908569 786.8525244750526 346.60170830052556 786.8802395209582 346.87991373394766'", + new Double[]{29.86020, 0.0001}); leave a space between [] and { while initializing array
12-08-2016
Thanks Arun, updated webrev : http://cr.openjdk.java.net/~ghb/8163582/webrev.03/ Casting to Double and used HashMap.
12-08-2016
Then instead of Double, you can typecast it to Number. That should work for both Integer and Double double totalLength = ((Number)executeScript("document.getElementById('pathId').getTotalLength();")).doubleValue()
12-08-2016
I should have given this exception before posting my webrev, never mind. "Why do we need Integer path? Typecast it to double and have a single path. " java.lang.ClassCastException: java.lang.Integer cannot be cast to java.lang.Double Let me implement your suggested Map version and send an updated webrev.
12-08-2016
+ // Get svg path's total length + // Computed value will be either Integer or Double + Object totalLength = executeScript("document.getElementById('pathId').getTotalLength();"); + if (totalLength instanceof Integer) { + final String msg = String.format( + "svg.path.getTotalLength() for %s , expected %d, actual : %d", + pathData.getKey(), pathData.getValue()[0].intValue(), + totalLength); + + assertEquals(msg, + pathData.getValue()[0].intValue(), totalLength); + } else if (totalLength instanceof Double){ + final String msg = String.format( + "svg.path.getTotalLength() for %s , expected %f, actual : %f", + pathData.getKey(), pathData.getValue()[0], + totalLength); + + assertEquals(msg, + pathData.getValue()[0], (Double) totalLength, pathData.getValue()[1]); + } else { + fail("Expected Integer or Double"); + } Why do we need Integer path? Typecast it to double and have a single path. double totalLength = (double)executeScript("document.getElementById('pathId').getTotalLength();"); I feel using Map may look good. Feel free to ignore this comment if you are not find it good. :) + // <Path, [Expected, Error Tolerance]> + final List<Pair<String, Double[]>> svgPaths = new ArrayList<>(6); final HashMap<String, Double[]> svgPaths = new HashMap<>(); ... svgPaths.forEach((path, expected) - > { .. });
12-08-2016
version .02 looks good. +1
12-08-2016
lgtm.
12-08-2016
Thanks Kevin , using assertEquals at 1st was unnecessary, used 4-arg variant in 2nd place. Update webrev http://cr.openjdk.java.net/~ghb/8163582/webrev.01/ Thanks Arun, Update webrev : http://cr.openjdk.java.net/~ghb/8163582/webrev.02/ (in addition to rev .01) Removed PLATFORM(JAVA) macro and kept single // NOTE : 8163582 which was causing the infinite loop. Tested on Windows & Linux.
11-08-2016
Looks good. I also verified that the new unit tests fails without the fix and passes with the fix. Two minor suggestions (not required) on the test assertions: 1. In the integer case, testing against epsilon seems unnecessary: assertTrue(msg, Math.abs(pathData.getValue()[0].intValue() - (Integer) totalLength) < pathData.getValue()[1]); they will either be exactly equal or off by 1 or more. You might want to consider using assertEquals with an absolute comparison. 2. In the double case, it might be clearer to use the 4-arg variant of assertEquals that is designed to handle floating point comparison with tolerance. Up to you whether you want to make this change.
11-08-2016
I suggest to remove the PLATFORM(JAVA) define. Anyhow we will get merge conflicts during new revision migration.
11-08-2016
webrev : http://cr.openjdk.java.net/~ghb/8163582/webrev.00/ RC : Initial fix taken from Chromium, same problem still exist in Safari browser (loading http://jsfiddle.net/UBsu2/1/ cause un-responsive state) and in nightly build of gtkwebkit. During our webkit upgrade, the files were replaced and didn't had Note / Info / FIXME as its taken from separate branch. By running DRT test neither path-getTotalLength.svg failed / passed / or in Skipped list. Solution : Merged back the initial fix with "PLATFORM(JAVA) // 8163582" (so done as part of JDK-8090035
11-08-2016
Merged the Initial fix done in JDK-8090035. Written unit test. Will post webrev shortly.
11-08-2016
I know we have a DRT test that covers this, but would it be possible to add a unit test as well?
10-08-2016
Partial merge of JDK-8090035 done while JDK-8156698 (New webkit merge) .
10-08-2016

Cloners :	JDK-8090035 - JavaFX browser can get stuck in an infinite loop when calling path.getTotalLength()
Relates :	JDK-8167100 - Minor source diffs introduced in backports of JDK-8160837 and JDK-8163582