Bug 48785

Summary: Setting attr repeatDur=0 on SVG element causes hang
Product: WebKit Reporter: Matthew Delaney <mdelaney7>
Component: SVGAssignee: Matthew Delaney <mdelaney7>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, bdakin, eric, mdelaney7, webkit.review.bot, zimmermann
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: OS X 10.6   
URL: http://www.w3.org/1999/07/30/WD-SVG-19990730/animate.html
Attachments:
Description Flags
Patch
none
Patch none

Description Matthew Delaney 2010-11-01 14:36:55 PDT
There was a hang being caused because the bound checking was just "<0" and not "<=0". As per the spec, a repeatDur of 0 should have no effect:

""
repeatDur = "<clock-value>"
This causes the element to play repeatedly (loop) for the specified duration. Each repeat iteration lasts for the simple duration. This attribute has no affect if the duration is 0. A repeat duration less than the simple duration specified in dur will cut short the duration. A repeat duration equal to the simple duration is a no-op. Legal values are clock values greater than 0. In addition, "indefinite" may be specified to indicate that the element should repeat indefinitely (subject to the time container semantics). At most one of repeat or repeatDur should be specified (if both are specified, the repeat duration is defined as the minimum of the specified repeatDur, and the simple duration multiplied by the repeat (count).
""
Comment 1 Matthew Delaney 2010-11-01 14:44:20 PDT
Created attachment 72563 [details]
Patch
Comment 2 Simon Fraser (smfr) 2010-11-01 14:59:36 PDT
Comment on attachment 72563 [details]
Patch

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

> WebCore/svg/animation/SVGSMILElement.cpp:531
> -    return m_cachedRepeatDur = clockValue < 0 ? SMILTime::unresolved() : clockValue;
> +    return m_cachedRepeatDur = clockValue <= 0 ? SMILTime::unresolved() : clockValue;

The assignment in the return statement is pretty grody here. Please change to:

m_cachedRepeatDur = clockValue <= 0 ? SMILTime::unresolved() : clockValue;
return m_cachedRepeatDur;

> LayoutTests/svg/animations/repeatDur-zero.xhtml:10
> +</svg>
>  \ No newline at end of file

Please add the newline.
Comment 3 Matthew Delaney 2010-11-01 15:08:48 PDT
Created attachment 72570 [details]
Patch
Comment 4 Matthew Delaney 2010-11-01 15:41:03 PDT
Committed r71066: <http://trac.webkit.org/changeset/71066>
Comment 5 WebKit Review Bot 2010-11-01 16:05:05 PDT
http://trac.webkit.org/changeset/71066 might have broken GTK Linux 64-bit Debug
The following tests are not passing:
accessibility/aria-activedescendant-crash.html
accessibility/aria-checkbox-text.html
accessibility/aria-hidden-update.html
accessibility/contenteditable-hidden-div.html
accessibility/crash-with-noelement-selectbox.html
accessibility/crashing-a-tag-in-map.html
accessibility/document-attributes.html
accessibility/first-letter-text-transform-causes-crash.html
accessibility/hang-in-isignored.html
accessibility/nested-layout-crash.html
accessibility/nochildren-elements.html
accessibility/non-data-table-cell-title-ui-element.html
accessibility/non-native-image-crash.html
accessibility/radio-button-checkbox-size.html
accessibility/removed-anonymous-block-child-causes-crash.html
accessibility/removed-continuation-element-causes-crash.html
accessibility/table-modification-crash.html
accessibility/table-nofirstbody.html
accessibility/table-notbody.html
accessibility/table-with-empty-thead-causes-crash.html
Comment 6 Nikolas Zimmermann 2010-11-03 08:38:58 PDT
Matthew, can you change the testcase to include
if (window.layoutTestController)
    layoutTestController.dumpAsText()

there's no need to dump render tree information there.
At the moment the expected.png/checksum files are missing, but it's useless to add them anyway.

When doing that, you might also want to remove the existing expected.txt files from platform/mac/svg/animations, and from the gtk/qt/win/chromium* platform directories (in case they have already landed their own results), and move the expected.txt file right into svg/animations/

Thanks in advance!