575672 - Interpolate out-of-range assertion for tabcandy style animation

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Ed Lee :Mardak]

Running without --enable-debug seems to work. I'm not sure what transition it was failing on, but Ian or Aza should know what transitions tabcandy uses for what.

###!!! ABORT: out of range: '0.0 <= aPortion && aPortion <= 1.0', file /Users/Ed/tabcandy-central/layout/style/nsStyleAnimation.h, line 132
nsStyleAnimation::Interpolate(nsCSSProperty, nsStyleAnimation::Value const&, nsStyleAnimation::Value const&, double, nsStyleAnimation::Value&)+0x00000073 [/Users/Ed/tabcandy-central/objdir/dist/MinefieldDebug.app/Contents/MacOS/./XUL +0x004E5E17]
ElementTransitionsStyleRule::MapRuleInfoInto(nsRuleData*)+0x0000020F [/Users/Ed/tabcandy-central/objdir/dist/MinefieldDebug.app/Contents/MacOS/./XUL +0x004E309F]
nsRuleNode::WalkRuleTree(nsStyleStructID, nsStyleContext*, nsRuleData*, nsCSSStruct*)+0x0000016E [/Users/Ed/tabcandy-central/objdir/dist/MinefieldDebug.app/Contents/MacOS/./XUL +0x004A6422]
nsRuleNode::GetPositionData(nsStyleContext*)+0x0000006F [/Users/Ed/tabcandy-central/objdir/dist/MinefieldDebug.app/Contents/MacOS/./XUL +0x004A8C31]
nsRuleNode::GetStylePosition(nsStyleContext*, int)+0x000000D5 [/Users/Ed/tabcandy-central/objdir/dist/MinefieldDebug.app/Contents/MacOS/./XUL +0x004A8D21]
nsStyleContext::DoGetStylePosition(int)+0x00000055 [/Users/Ed/tabcandy-central/objdir/dist/MinefieldDebug.app/Contents/MacOS/./XUL +0x004C114D]
nsStyleContext::GetStylePosition()+0x00000019 [/Users/Ed/tabcandy-central/objdir/dist/MinefieldDebug.app/Contents/MacOS/./XUL +0x002E7D73]
nsStyleContext::CalcStyleDifference(nsStyleContext*)+0x000019A9 [/Users/Ed/tabcandy-central/objdir/dist/MinefieldDebug.app/Contents/MacOS/./XUL +0x004CBD6B]
CaptureChange(nsStyleContext*, nsStyleContext*, nsIFrame*, nsIContent*, nsStyleChangeList*, nsChangeHint, nsChangeHint)+0x0000001F [/Users/Ed/tabcandy-central/objdir/dist/MinefieldDebug.app/Contents/MacOS/./XUL +0x002D1E9E]
nsFrameManager::ReResolveStyleContext(nsPresContext*, nsIFrame*, nsIContent*, nsStyleChangeList*, nsChangeHint, nsRestyleHint, int, mozilla::css::RestyleTracker&)+0x0000081A [/Users/Ed/tabcandy-central/objdir/dist/MinefieldDebug.app/Contents/MacOS/./XUL +0x002D2876]
nsFrameManager::ComputeStyleChangeFor(nsIFrame*, nsStyleChangeList*, nsChangeHint, mozilla::css::RestyleTracker&, int)+0x0000011B [/Users/Ed/tabcandy-central/objdir/dist/MinefieldDebug.app/Contents/MacOS/./XUL +0x002D352F]
nsCSSFrameConstructor::RestyleElement(mozilla::dom::Element*, nsIFrame*, nsChangeHint, mozilla::css::RestyleTracker&, int)+0x00000177 [/Users/Ed/tabcandy-central/objdir/dist/MinefieldDebug.app/Contents/MacOS/./XUL +0x002908FB]
mozilla::css::RestyleTracker::ProcessOneRestyle(mozilla::dom::Element*, nsRestyleHint, nsChangeHint)+0x0000013A [/Users/Ed/tabcandy-central/objdir/dist/MinefieldDebug.app/Contents/MacOS/./XUL +0x00276262]
mozilla::css::RestyleTracker::ProcessRestyles()+0x00000411 [/Users/Ed/tabcandy-central/objdir/dist/MinefieldDebug.app/Contents/MacOS/./XUL +0x00274E9B]
nsCSSFrameConstructor::ProcessPendingRestyles()+0x00000135 [/Users/Ed/tabcandy-central/objdir/dist/MinefieldDebug.app/Contents/MacOS/./XUL +0x002906EB]
PresShell::FlushPendingNotifications(mozFlushType)+0x0000023A [/Users/Ed/tabcandy-central/objdir/dist/MinefieldDebug.app/Contents/MacOS/./XUL +0x0030CDBE]
nsRefreshDriver::Notify(nsITimer*)+0x00000210 [/Users/Ed/tabcandy-central/objdir/dist/MinefieldDebug.app/Contents/MacOS/./XUL +0x003162AA]
nsTimerImpl::Fire()+0x0000037B [/Users/Ed/tabcandy-central/objdir/dist/MinefieldDebug.app/Contents/MacOS/./XUL +0x014CF48F]
nsTimerEvent::Run()+0x000000C0 [/Users/Ed/tabcandy-central/objdir/dist/MinefieldDebug.app/Contents/MacOS/./XUL +0x014CF6AC]
nsThread::ProcessNextEvent(int, int*)+0x000002A4 [/Users/Ed/tabcandy-central/objdir/dist/MinefieldDebug.app/Contents/MacOS/./XUL +0x014C85CC]
NS_ProcessPendingEvents_P(nsIThread*, unsigned int)+0x00000092 [/Users/Ed/tabcandy-central/objdir/dist/MinefieldDebug.app/Contents/MacOS/./XUL +0x01454DFF]
nsBaseAppShell::NativeEventCallback()+0x000000B5 [/Users/Ed/tabcandy-central/objdir/dist/MinefieldDebug.app/Contents/MacOS/./XUL +0x012CE19D]
nsAppShell::ProcessGeckoEvents(void*)+0x000001F1 [/Users/Ed/tabcandy-central/objdir/dist/MinefieldDebug.app/Contents/MacOS/./XUL +0x0127FEF9]
__CFRunLoopDoSources0+0x0000061B [/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation +0x0003F0FB]
__CFRunLoopRun+0x0000042F [/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation +0x0003CBBF]
CFRunLoopRunSpecific+0x000001C4 [/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation +0x0003C094]
CFRunLoopRunInMode+0x00000061 [/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation +0x0003BEC1]
RunCurrentEventLoopInMode+0x00000188 [/System/Library/Frameworks/Carbon.framework/Versions/A/Frameworks/HIToolbox.framework/Versions/A/HIToolbox +0x00034F9C]
ReceiveNextEventCommon+0x00000162 [/System/Library/Frameworks/Carbon.framework/Versions/A/Frameworks/HIToolbox.framework/Versions/A/HIToolbox +0x00034D51]
BlockUntilNextEventMatchingListInMode+0x00000051 [/System/Library/Frameworks/Carbon.framework/Versions/A/Frameworks/HIToolbox.framework/Versions/A/HIToolbox +0x00034BD6]
_DPSNextEvent+0x0000034F [/System/Library/Frameworks/AppKit.framework/Versions/C/AppKit +0x00048A89]
-[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:]+0x0000009C [/System/Library/Frameworks/AppKit.framework/Versions/C/AppKit +0x000482CA]
-[NSApplication run]+0x00000335 [/System/Library/Frameworks/AppKit.framework/Versions/C/AppKit +0x0000A55B]
nsAppShell::Run()+0x00000123 [/Users/Ed/tabcandy-central/objdir/dist/MinefieldDebug.app/Contents/MacOS/./XUL +0x0127EC4B]
nsAppStartup::Run()+0x00000094 [/Users/Ed/tabcandy-central/objdir/dist/MinefieldDebug.app/Contents/MacOS/./XUL +0x00FE2360]
XRE_main+0x00002C5E [/Users/Ed/tabcandy-central/objdir/dist/MinefieldDebug.app/Contents/MacOS/./XUL +0x000203D8]
main+0x000002CA [/Users/Ed/tabcandy-central/objdir/dist/MinefieldDebug.app/Contents/MacOS/./firefox-bin +0x000015F9]
start+0x00000036 [/Users/Ed/tabcandy-central/objdir/dist/MinefieldDebug.app/Contents/MacOS/./firefox-bin +0x0000127A]
###!!! ABORT: out of range: '0.0 <= aPortion && aPortion <= 1.0', file /Users/Ed/tabcandy-central/layout/style/nsStyleAnimation.h, line 132

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Yeah, have a patch-in-progress in my tree for this, but need to finish it up.

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I think this should block because we need to (1) do the range check on the X values and (2) audit the code to make sure it correctly handles Y values with wider range.

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

And see http://lists.w3.org/Archives/Public/www-style/2010Jun/0269.html for the spec change agreement.

Comment 5

[Aza Raskin [:aza]]

Agreed on blocking. We use this animation for most things in Tab Candy :)

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

->beta4+, since it blocks tabcandy landing

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

OK, I just did the code audit, and the only open question that I need to investigate is whether nsSMILKeySpline is ok with these splines.  In other words, does anything it does assume that the timing function is nondecreasing?

I'll try looking into this next, but also cc:ing birtles for his thoughts.

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Actually, it's pretty trivial to demonstrate that nsSMILKeySpline is fine, since all the fancy stuff we're doing is on the X values, and the X values are still restricted to being between 0 and 1.  The only thing nsSMILKeySpline does with the Y values is plug them in to the final bezier calculation once it approximates the correct t from the x value.

So everything is fine.  I'll attach the patch in a sec.

Comment 9

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch

Here's the main patch, which:
 * checks the X values properly
 * removes the assertion that asserts about the range of the Y values

Comment 10

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: parsing fixes

Mats's patch was trying to work around some deeper parenthesis-skipping bugs in the parser; this fixes them (I audited all the _Function checks in ParseVariant).

Things are a little inconsistent here about whether the SkipUntil is done by ParseVariant or the thing that it calls, but I think that's straightforward enough to figure out by reading the code.

Comment 11

[Boris Zbarsky [:bzbarsky]]

Is that second patch in the right bug?

Comment 12

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Yes; Mats's patch was on the duplicate, bug 583305.  His code there called my attention to the fact that the paren matching for errors inside cubic-bezier() was wrong, which in turn called my attention to the fact that it was wrong for a bunch of other things too.

Comment 13

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 464217 [details] [diff] [review]
parsing fixes

r=bzbarsky

Comment 14

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

http://hg.mozilla.org/mozilla-central/rev/638dab2836b5
http://hg.mozilla.org/mozilla-central/rev/4c518456865a