As mentioned in Bug 1004871 comment 43, in layers code we use numIterations = -1 to represent infinity whereas elsewhere we use infinity (i.e. NS_IEEEPositiveInfinity()). We should make this consistent. While we're at it we should probably rename numIterations to iterationCount. See: http://dxr.mozilla.org/mozilla-central/source/gfx/layers/ipc/LayersMessages.ipdlh#182 And: http://dxr.mozilla.org/mozilla-central/source/layout/base/nsDisplayList.cpp#332

In the IPC Animation struct used in layers code we have a member called 'numIterations' where 'iterate forever' is represented by -1. In layout/style however we have an AnimationTiming struct with an mIterationCount member where 'iterate forever' is represented by NS_IEEEPositiveInfinity(). This patch renames 'numIterations' to 'iterationCount' and uses infinity to represent 'iterate forever'.

Sorry about all the review requests but this one should be easy. I didn't change the type to double (despite the suggestion in Bug 1004871 comment 44) since AnimationTiming::mIterationCount is actually not a double but a float. I think a float is sufficient for representing the number of iterations desired. (One place where we might have range considerations is representing the current iteration however since that may be reporting on an animation that repeats indefinitely. For an animation that repeats every millisecond, a 32-bit integer would be enough for 50 days. That's probably enough since I can't think of any practical uses of an animation that repeats every millisecond, but we're using 64-bit integers there anyway.) I wondered if the use of -1 to represent "forever" in layers code was because our IPC code can't serialize the value "infinity" but I made up a test case and ran through it with a debugger and didn't see any issues there.

Comment on attachment 8429019 [details] [diff] [review] Align Layers' Animation.numIterations with AnimationTiming.mIterationCount >+++ b/gfx/layers/ipc/LayersMessages.ipdlh >- // How many times to repeat the animation. -1 means "forever". >- float numIterations; >+ // Number of times to repeat the animation, including positive infinity. >+ float iterationCount; While you're here, it's probably worth also documenting: ...what 0 represents (don't play at all? or play but don't repeat?) ...that negative values are invalid (if that's the case) (we probably should have some assertions to that effect, if we don't already. That doesn't have to happen here though.) r=me with that.

(In reply to Brian Birtles (:birtles) from comment #2) > I didn't change the type to double (despite the suggestion in Bug 1004871 > comment 44) since AnimationTiming::mIterationCount is actually not a double > but a float. (Sorry, not sure why I thought it was a double there. I must've been looking at another variable.)

(In reply to Daniel Holbert [:dholbert] from comment #3) > Comment on attachment 8429019 [details] [diff] [review] > Align Layers' Animation.numIterations with AnimationTiming.mIterationCount > > >+++ b/gfx/layers/ipc/LayersMessages.ipdlh > >- // How many times to repeat the animation. -1 means "forever". > >- float numIterations; > >+ // Number of times to repeat the animation, including positive infinity. > >+ float iterationCount; > > While you're here, it's probably worth also documenting: > ...what 0 represents (don't play at all? or play but don't repeat?) > ...that negative values are invalid (if that's the case) (we probably > should have some assertions to that effect, if we don't already. That > doesn't have to happen here though.) > > r=me with that. I added the following: // Values <= 0 mean the animation will not play (although events are still // dispatched on the main thread).