As a side effect, all the APIs that takes float or double will automatically throw when a non-finite value is passed in. I compared all the IDL files with Firefox. There were only 2 files that use unrestricted: WebGL and Canvas 2D. For IDL files that were internal to WebKit, I change the IDL to unrestricted.
Created attachment 230648 [details] Not for review
Created attachment 230650 [details] not for review
Created attachment 230651 [details] not for review
Comment on attachment 230651 [details] not for review Attachment 230651 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/5337908114358272 New failing tests: svg/custom/elementTimeControl-nan-crash.html canvas/philip/tests/2d.missingargs.html fast/dom/HTMLMeterElement/set-meter-properties.html fast/dom/Window/window-resize-and-move-arguments.html fast/forms/range/input-valueasnumber-range.html fast/forms/number/number-valueasnumber.html svg/animations/animateTransform-translate-invalid-attributetype.html media/track/track-cue-negative-timestamp.html svg/dom/SVGRect.html svg/dom/SVGNumber.html svg/dom/SVGAnimatedNumber.html transforms/cssmatrix-2d-interface.xhtml fast/dom/HTMLProgressElement/set-progress-properties.html media/track/track-add-remove-cue.html media/W3C/video/networkState/networkState_during_loadstart.html svg/dom/SVGPoint.html svg/dom/SVGAngle.html svg/animations/animate-setcurrenttime.html svg/dom/SVGTransform.html svg/dom/SVGMatrix.html fast/dom/non-numeric-values-numeric-parameters.html svg/dom/SVGLength.html fast/canvas/canvas-path-addPath.html
Created attachment 230656 [details] Archive of layout-test-results from webkit-ews-11 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-11 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5
Comment on attachment 230651 [details] not for review Attachment 230651 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/4829341036838912 New failing tests: svg/custom/elementTimeControl-nan-crash.html canvas/philip/tests/2d.missingargs.html svg/dom/SVGPolygonElement-baseVal-list-removal-crash.html fast/dom/HTMLMeterElement/set-meter-properties.html fast/dom/Window/window-resize-and-move-arguments.html fast/forms/range/input-valueasnumber-range.html fast/forms/number/number-valueasnumber.html svg/animations/animateTransform-translate-invalid-attributetype.html media/track/track-cue-negative-timestamp.html svg/dom/SVGRect.html svg/dom/SVGNumber.html svg/dom/SVGAnimatedNumber.html transforms/cssmatrix-2d-interface.xhtml fast/dom/HTMLProgressElement/set-progress-properties.html media/track/track-add-remove-cue.html svg/dom/SVGPoint.html svg/dom/SVGAngle.html svg/animations/animate-setcurrenttime.html svg/dom/SVGTransform.html svg/dom/SVGMatrix.html fast/dom/non-numeric-values-numeric-parameters.html svg/dom/SVGLength.html fast/canvas/canvas-path-addPath.html
Created attachment 230657 [details] Archive of layout-test-results from webkit-ews-08 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-08 Port: mac-mountainlion Platform: Mac OS X 10.8.5
Comment on attachment 230651 [details] not for review Attachment 230651 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/6322138524942336 New failing tests: svg/custom/elementTimeControl-nan-crash.html canvas/philip/tests/2d.missingargs.html svg/animations/animateTransform-translate-invalid-attributetype.html fast/dom/HTMLMeterElement/set-meter-properties.html fast/dom/Window/window-resize-and-move-arguments.html fast/forms/range/input-valueasnumber-range.html fast/forms/number/number-valueasnumber.html svg/dom/SVGPolygonElement-baseVal-list-removal-crash.html media/track/track-cue-negative-timestamp.html svg/dom/SVGRect.html svg/dom/SVGNumber.html svg/dom/SVGAnimatedNumber.html transforms/cssmatrix-2d-interface.xhtml fast/dom/HTMLProgressElement/set-progress-properties.html media/track/track-add-remove-cue.html svg/dom/SVGPoint.html svg/dom/SVGAngle.html svg/animations/animate-setcurrenttime.html svg/dom/SVGTransform.html svg/dom/SVGMatrix.html fast/dom/non-numeric-values-numeric-parameters.html svg/dom/SVGLength.html fast/canvas/canvas-path-addPath.html
Created attachment 230659 [details] Archive of layout-test-results from webkit-ews-06 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-06 Port: mac-mountainlion Platform: Mac OS X 10.8.5
(In reply to comment #8) > (From update of attachment 230651 [details]) > Attachment 230651 [details] did not pass mac-ews (mac): > Output: http://webkit-queues.appspot.com/results/6322138524942336 > > New failing tests: > svg/custom/elementTimeControl-nan-crash.html > canvas/philip/tests/2d.missingargs.html > svg/animations/animateTransform-translate-invalid-attributetype.html > fast/dom/HTMLMeterElement/set-meter-properties.html > fast/dom/Window/window-resize-and-move-arguments.html > fast/forms/range/input-valueasnumber-range.html > fast/forms/number/number-valueasnumber.html > svg/dom/SVGPolygonElement-baseVal-list-removal-crash.html > media/track/track-cue-negative-timestamp.html > svg/dom/SVGRect.html > svg/dom/SVGNumber.html > svg/dom/SVGAnimatedNumber.html > transforms/cssmatrix-2d-interface.xhtml > fast/dom/HTMLProgressElement/set-progress-properties.html > media/track/track-add-remove-cue.html > svg/dom/SVGPoint.html > svg/dom/SVGAngle.html > svg/animations/animate-setcurrenttime.html > svg/dom/SVGTransform.html > svg/dom/SVGMatrix.html > fast/dom/non-numeric-values-numeric-parameters.html > svg/dom/SVGLength.html > fast/canvas/canvas-path-addPath.html Patch looks great and the failing tests not that bad. You indeed have to add some more "unrestricted"s. SVGPoint, SVGRect and SVGMatrix should be unrestricted, SVGAngle, SVGTransform, SVGNumber and SVGLength should be brought up to SVG WG call. For now, make them unrestricted as well (probably the outcome of a resolution anyway).
Created attachment 230671 [details] Patch
Comment on attachment 230671 [details] Patch Attachment 230671 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/6034294917038080 New failing tests: fast/forms/range/input-valueasnumber-range.html fast/forms/number/number-valueasnumber.html fast/dom/non-numeric-values-numeric-parameters.html canvas/philip/tests/2d.missingargs.html fast/dom/HTMLProgressElement/set-progress-properties.html media/track/track-cue-negative-timestamp.html fast/dom/HTMLMeterElement/set-meter-properties.html transforms/cssmatrix-2d-interface.xhtml fast/canvas/canvas-path-addPath.html media/track/track-add-remove-cue.html fast/dom/Window/window-resize-and-move-arguments.html svg/dom/SVGMatrix.html
Created attachment 230678 [details] Archive of layout-test-results from webkit-ews-11 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-11 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5
Comment on attachment 230671 [details] Patch Attachment 230671 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/5515877332025344 New failing tests: fast/forms/range/input-valueasnumber-range.html fast/forms/number/number-valueasnumber.html fast/dom/non-numeric-values-numeric-parameters.html canvas/philip/tests/2d.missingargs.html fast/dom/HTMLProgressElement/set-progress-properties.html media/track/track-cue-negative-timestamp.html fast/dom/HTMLMeterElement/set-meter-properties.html transforms/cssmatrix-2d-interface.xhtml fast/canvas/canvas-path-addPath.html media/track/track-add-remove-cue.html fast/dom/Window/window-resize-and-move-arguments.html svg/dom/SVGMatrix.html
Created attachment 230680 [details] Archive of layout-test-results from webkit-ews-06 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-06 Port: mac-mountainlion Platform: Mac OS X 10.8.5
Created attachment 230704 [details] Patch
Created attachment 230709 [details] Patch
Created attachment 230732 [details] Patch
Created attachment 230734 [details] Patch
Created attachment 230740 [details] Patch
The latest patch is ready for review and is passing all tests. Cleaning up the WK code that checks for finite values will be done in subsequent patches.
Comment on attachment 230740 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=230740&action=review Wasn't that bad actually. Thought the patch would be bigger. Still some snippets. > Source/WebCore/ChangeLog:9 > + Alse updated the IDL files so they use unrestricted float and Also > Source/WebCore/bindings/scripts/CodeGenerator.pm:52 > "float" => 1, "double" => 1, "byte" => 1, > - "octet" => 1); > + "octet" => 1, "unrestricted float" => 1, "unrestricted double" => 1); move "unrestricted float" => 1, "unrestricted double" => 1 right after the line with float and double, shift "byte" to the line with octet for readability. > Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:1722 > + $implIncludes{"<wtf/MathExtras.h>"} = 1; # for isfinite I wonder if we still use MathExtras.h or some std lib logic instead. > Source/WebCore/bindings/scripts/CodeGeneratorObjC.pm:1191 > + push(@implContent, "#import <wtf/MathExtras.h>\n\n"); # for isfinite Ditto. Need to check. > Source/WebCore/bindings/scripts/test/TestTypedefs.idl:45 > + void setShadow(DOUBLE width, DOUBLE height, unrestricted float blur, [StrictTypeChecking] optional STRING color, optional DOUBLE alpha); What is DOUBLE? > Source/WebCore/bindings/scripts/test/TestTypedefs.idl:68 > +typedef unrestricted float DOUBLE; I see. > Source/WebCore/html/canvas/CanvasRenderingContext2D.idl:65 > + void fillRect(unrestricted float x, unrestricted float y, unrestricted float width, unrestricted float height); Interestingly, the Canvas spec uses double everywhere. > Source/WebCore/html/track/TextTrackCue.idl:29 > + Constructor(unrestricted double startTime, unrestricted double endTime, DOMString text), I would make them restricted, see next comment. > Source/WebCore/html/track/TextTrackCue.idl:41 > + [SetterRaisesException] attribute unrestricted double startTime; > + [SetterRaisesException] attribute unrestricted double endTime; according to the spec, these should be restricted, not unrestricted. > Source/WebCore/page/DOMWindow.idl:111 > + void moveBy([Default=Undefined] optional unrestricted float x, [Default=Undefined] optional unrestricted float y); // FIXME: this should take longs not floats. > + void moveTo([Default=Undefined] optional unrestricted float x, [Default=Undefined] optional unrestricted float y); // FIXME: this should take longs not floats. > + void resizeBy([Default=Undefined] optional unrestricted float x, [Default=Undefined] optional unrestricted float y); // FIXME: this should take longs not floats. > + void resizeTo([Default=Undefined] optional unrestricted float width, [Default=Undefined] optional unrestricted float height); // FIXME: this should take longs not floats. If it should take longs and not floats, the first step might be to use restricted floats, no? > Source/WebCore/svg/SVGAnimationElement.idl:31 > + unrestricted float getStartTime(); > + unrestricted float getCurrentTime(); These are tricky. If we restrict for media elements, we might want to restrict for SVGAnimationElement as well. On the other hand, these are getters so doesn't matter anyway. > Source/WebCore/svg/SVGPathElement.idl:30 > + unrestricted float getTotalLength(); We should clarify in the SVG spec where we want to restrict floats and where we don't. I am fine for doing unrestricted for all SVG element for now. This is the current behavior anyway. > LayoutTests/canvas/philip/tests/2d.missingargs-expected.txt:1 > +Failed assertion: expected exception of type TypeError, got: Error: SyntaxError: DOM Exception 12 Hm. This should really not be a TypeError, no? > LayoutTests/platform/mac/canvas/philip/tests/2d.missingargs-expected.txt:1 > Failed assertion: expected exception of type TypeError, got: Error: SyntaxError: DOM Exception 12 Why didn't that go away? Did you add an unrestricted somewhere where it should be restricted? Is the test incorrect?
Created attachment 230765 [details] Patch
Comment on attachment 230740 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=230740&action=review >> Source/WebCore/ChangeLog:9 >> + Alse updated the IDL files so they use unrestricted float and > > Also Fixed >> Source/WebCore/bindings/scripts/CodeGenerator.pm:52 >> + "octet" => 1, "unrestricted float" => 1, "unrestricted double" => 1); > > move "unrestricted float" => 1, "unrestricted double" => 1 right after the line with float and double, shift "byte" to the line with octet for readability. Fixed >> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:1722 >> + $implIncludes{"<wtf/MathExtras.h>"} = 1; # for isfinite > > I wonder if we still use MathExtras.h or some std lib logic instead. I removed this header. It wasn't needed. >> Source/WebCore/bindings/scripts/CodeGeneratorObjC.pm:1191 >> + push(@implContent, "#import <wtf/MathExtras.h>\n\n"); # for isfinite > > Ditto. Need to check. removed. >> Source/WebCore/bindings/scripts/test/TestTypedefs.idl:68 >> +typedef unrestricted float DOUBLE; > > I see. :-) >> Source/WebCore/html/track/TextTrackCue.idl:41 >> + [SetterRaisesException] attribute unrestricted double endTime; > > according to the spec, these should be restricted, not unrestricted. The test talks about testing old behavior. Maybe they were unrestricted in the past and we don't want to break existing code. >> Source/WebCore/page/DOMWindow.idl:111 >> + void resizeTo([Default=Undefined] optional unrestricted float width, [Default=Undefined] optional unrestricted float height); // FIXME: this should take longs not floats. > > If it should take longs and not floats, the first step might be to use restricted floats, no? The code is testing with NaN. I'm worried about changing behavior of such an important object. >> LayoutTests/canvas/philip/tests/2d.missingargs-expected.txt:1 >> +Failed assertion: expected exception of type TypeError, got: Error: SyntaxError: DOM Exception 12 > > Hm. This should really not be a TypeError, no? No, according to the spec, it should be a syntax error: http://www.whatwg.org/specs/web-apps/current-work/multipage/the-canvas-element.html#dom-canvasgradient-addcolorstop I updated the test. >> LayoutTests/platform/mac/canvas/philip/tests/2d.missingargs-expected.txt:1 >> Failed assertion: expected exception of type TypeError, got: Error: SyntaxError: DOM Exception 12 > > Why didn't that go away? Did you add an unrestricted somewhere where it should be restricted? Is the test incorrect? Test was incorrect. I updated it and now everything passes.
Comment on attachment 230765 [details] Patch Attachment 230765 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/5108628264583168 New failing tests: media/track/track-add-remove-cue.html media/track/track-cue-negative-timestamp.html
Created attachment 230770 [details] Archive of layout-test-results from webkit-ews-02 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-02 Port: mac-mountainlion Platform: Mac OS X 10.8.5
Comment on attachment 230765 [details] Patch Attachment 230765 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/4768088897617920 New failing tests: media/track/track-add-remove-cue.html media/track/track-cue-negative-timestamp.html
Created attachment 230772 [details] Archive of layout-test-results from webkit-ews-10 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-10 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5
Comment on attachment 230765 [details] Patch Attachment 230765 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/5258618991542272 New failing tests: media/track/track-add-remove-cue.html media/track/track-cue-negative-timestamp.html
Created attachment 230775 [details] Archive of layout-test-results from webkit-ews-07 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-07 Port: mac-mountainlion Platform: Mac OS X 10.8.5
Comment on attachment 230765 [details] Patch Attachment 230765 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/4906775270653952 New failing tests: media/track/track-add-remove-cue.html media/track/track-cue-negative-timestamp.html
Created attachment 230778 [details] Archive of layout-test-results from webkit-ews-09 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-09 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5
Created attachment 230780 [details] Patch
I'm worried about changing the default behavior of all floats and doubles like this, and potentially throwing in many cases where weren't before. Can we add tests for all the now restricted types?
(In reply to comment #35) > I'm worried about changing the default behavior of all floats and doubles like this, and potentially throwing in many cases where weren't before. Can we add tests for all the now restricted types? I worry too. Maybe I should make everything in the idl files unrestricted except for canvas 2D?
Created attachment 230782 [details] Patch
(In reply to comment #36) > (In reply to comment #35) > > I'm worried about changing the default behavior of all floats and doubles like this, and potentially throwing in many cases where weren't before. Can we add tests for all the now restricted types? > > I worry too. Maybe I should make everything in the idl files unrestricted except for canvas 2D? I would certainly feel better about the change if it moved in that direction. One option would be to first make the change to mark everything unrestricted. Then, once that is landed, selectively restrict.
Created attachment 230795 [details] Patch
Comment on attachment 230795 [details] Patch not for review. To check for failing tests and GTK build status
Comment on attachment 230795 [details] Patch Attachment 230795 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/6383338050813952 New failing tests: fast/forms/range/input-valueasnumber-range.html fast/forms/number/number-valueasnumber.html fast/dom/HTMLProgressElement/set-progress-properties.html fast/dom/HTMLMeterElement/set-meter-properties.html
Created attachment 230796 [details] Archive of layout-test-results from webkit-ews-09 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-09 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5
Comment on attachment 230795 [details] Patch Attachment 230795 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/5337272996069376 New failing tests: fast/forms/range/input-valueasnumber-range.html fast/forms/number/number-valueasnumber.html fast/dom/HTMLProgressElement/set-progress-properties.html fast/dom/HTMLMeterElement/set-meter-properties.html
Created attachment 230798 [details] Archive of layout-test-results from webkit-ews-06 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-06 Port: mac-mountainlion Platform: Mac OS X 10.8.5
Comment on attachment 230795 [details] Patch Attachment 230795 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/4764856934727680 New failing tests: fast/forms/range/input-valueasnumber-range.html fast/forms/number/number-valueasnumber.html fast/dom/HTMLProgressElement/set-progress-properties.html fast/dom/HTMLMeterElement/set-meter-properties.html
Created attachment 230801 [details] Archive of layout-test-results from webkit-ews-11 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-11 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5
Comment on attachment 230795 [details] Patch Attachment 230795 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/5082858997678080 New failing tests: fast/forms/range/input-valueasnumber-range.html fast/forms/number/number-valueasnumber.html fast/dom/HTMLProgressElement/set-progress-properties.html fast/dom/HTMLMeterElement/set-meter-properties.html
Created attachment 230803 [details] Archive of layout-test-results from webkit-ews-03 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-03 Port: mac-mountainlion Platform: Mac OS X 10.8.5
Created attachment 230806 [details] Patch
Comment on attachment 230806 [details] Patch LGTM. Thanks for taking the time. r=me
Comment on attachment 230806 [details] Patch Clearing flags on attachment: 230806 Committed r168302: <http://trac.webkit.org/changeset/168302>
All reviewed patches have been landed. Closing bug.
*** Bug 132423 has been marked as a duplicate of this bug. ***