Compute the keyTimes index correctly for discrete (values) animations

Compute the keyTimes index correctly for discrete (values) animations

The last value is allowed to be non-one for discrete mode animations (but
not for linear and spline mode animations).

TEST=svg/animations/animate-elem-{14,18}-t-drt.html
BUG=350318
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=169211

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=169351

[pdr., 2014-03-13 06:15:38]

Can we assert here that calcMode() isn't CalcModePaced?

LGTM

[fs, 2014-03-13 15:21:28]

On 2014/03/13 06:15:38, pdr wrote:
> Can we assert here that calcMode() isn't CalcModePaced?

Sure. Added.

While looking through the callers (to see if any would trivially trigger that
assert - one might BTW), I noticed some indexing errors which I fixed up. That
might warrant a new LGTM (although it's a reasonably trivial move of 3 lines).

[pdr., 2014-03-13 19:07:06]

On 2014/03/13 15:21:28, fs wrote:
> On 2014/03/13 06:15:38, pdr wrote:
> > Can we assert here that calcMode() isn't CalcModePaced?
> 
> Sure. Added.
> 
> While looking through the callers (to see if any would trivially trigger that
> assert - one might BTW), I noticed some indexing errors which I fixed up. That
> might warrant a new LGTM (although it's a reasonably trivial move of 3 lines).

Enthusiastic LGTM. Thanks for the cleanup

[pdr., 2014-03-13 19:07:09]

The CQ bit was checked by pdr@chromium.org

[commit-bot: I haz the power, 2014-03-13 19:07:16]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/fs@opera.com/197283002/20001

[commit-bot: I haz the power, 2014-03-14 03:34:07]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/fs@opera.com/197283002/20001

[commit-bot: I haz the power, 2014-03-14 05:32:52]

Change committed as 169211

[f(malita), 2014-03-14 20:03:42]

On 2014/03/14 05:32:52, I haz the power (commit-bot) wrote:
> Change committed as 169211

This appears to be causing crashes for the following:

+svg/W3C-SVG-1.1/animate-elem-82-t.svg
+svg/W3C-SVG-1.1/animate-elem-11-t.svg
+svg/W3C-SVG-1.1/animate-elem-15-t.svg
+svg/animations/animateMotion-multiple.html
+svg/animations/mozilla/animateMotion-to-overridden-1.svg
+svg/animations/animate-keytimes-crash.html
+svg/animations/animate-elem-15-t-drt.html

I see the gardened already reverted, but how in the world did the CQ not catch
these?!

[fs, 2014-03-17 08:32:51]

On 2014/03/14 20:03:42, Florin Malita wrote:
> On 2014/03/14 05:32:52, I haz the power (commit-bot) wrote:
> > Change committed as 169211
> 
> This appears to be causing crashes for the following:
> 
> +svg/W3C-SVG-1.1/animate-elem-82-t.svg
> +svg/W3C-SVG-1.1/animate-elem-11-t.svg
> +svg/W3C-SVG-1.1/animate-elem-15-t.svg
> +svg/animations/animateMotion-multiple.html
> +svg/animations/mozilla/animateMotion-to-overridden-1.svg
> +svg/animations/animate-keytimes-crash.html
> +svg/animations/animate-elem-15-t-drt.html
> 
> I see the gardened already reverted, but how in the world did the CQ not catch
> these?!

Indeed strange... Maybe something with the new tryserver.blink master.

[fs, 2014-03-17 12:12:26]

On 2014/03/17 08:32:51, fs wrote:
> On 2014/03/14 20:03:42, Florin Malita wrote:
> > On 2014/03/14 05:32:52, I haz the power (commit-bot) wrote:
> > > Change committed as 169211
> > 
> > This appears to be causing crashes for the following:
> > 
> > +svg/W3C-SVG-1.1/animate-elem-82-t.svg
> > +svg/W3C-SVG-1.1/animate-elem-11-t.svg
> > +svg/W3C-SVG-1.1/animate-elem-15-t.svg
> > +svg/animations/animateMotion-multiple.html
> > +svg/animations/mozilla/animateMotion-to-overridden-1.svg
> > +svg/animations/animate-keytimes-crash.html
> > +svg/animations/animate-elem-15-t-drt.html
> > 
> > I see the gardened already reverted, but how in the world did the CQ not
catch
> > these?!
> 
> Indeed strange... Maybe something with the new tryserver.blink master.

Those are all caused by the new ASSERT(calcMode() != CalcModePaced) triggering.
I filed crbug.com/353108 about that and will reland without said ASSERT (and
make sure some Debug bot is run...)

[fs, 2014-03-17 13:55:35]

The CQ bit was checked by fs@opera.com

[commit-bot: I haz the power, 2014-03-17 13:55:41]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/fs@opera.com/197283002/40001

[commit-bot: I haz the power, 2014-03-17 13:56:00]

Change committed as 169351