Make sure begin and end events are dispatched for back-to-back intervals

Make sure begin and end events are dispatched for back-to-back intervals

When the end time of the previous interval is the same as the
begin time of the new/current interval, the active state won't 
visibly transition Inactive -> Active (for 'begin' - the other
way around for 'end') if the interval can be resolved up
front (i.e. isn't event-base).

Fix by having SVGSMILElement::checkRestart return a bool indicating
whether an interval ended up being restarted, and then use this bool to
extend the predicates looking for Inactive<->Active transitions.

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

[fs, 2014-01-31 14:16:31]

Try-bots seems to be in agony, but PTAL...

[pdr., 2014-02-03 23:22:59]

Sorry about the slow response times (most of the chrome team was on a trip last
week). Please make noise, ping people, complain, cry, etc, if you don't get
reviews in 24hours :)

LGTM for this codechange but I have a nit about the name "checkRestart". I'd
also prefer to use an enum instead of a bool, but I could be convinced
otherwise.

https://codereview.chromium.org/141013017/diff/1/Source/core/svg/animation/SV...
File Source/core/svg/animation/SVGSMILElement.cpp (right):

https://codereview.chromium.org/141013017/diff/1/Source/core/svg/animation/SV...
Source/core/svg/animation/SVGSMILElement.cpp:1003: bool
SVGSMILElement::checkRestart(SMILTime elapsed)
Can we clean this up a bit?

I'll offer a suggestion:
RestartedInterval SVGSMILElement::maybeTriggerRestart(SMILTime elapsed)
where RestartedInterval is an enum with two values: DidRestartInterval and
DidNotRestartInterval.

[fs, 2014-02-04 08:55:10]

On 2014/02/03 23:22:59, pdr wrote:
> Sorry about the slow response times (most of the chrome team was on a trip
last
> week). Please make noise, ping people, complain, cry, etc, if you don't get
> reviews in 24hours :)
> 
> LGTM for this codechange but I have a nit about the name "checkRestart". I'd
> also prefer to use an enum instead of a bool, but I could be convinced
> otherwise.
> 
>
https://codereview.chromium.org/141013017/diff/1/Source/core/svg/animation/SV...
> File Source/core/svg/animation/SVGSMILElement.cpp (right):
> 
>
https://codereview.chromium.org/141013017/diff/1/Source/core/svg/animation/SV...
> Source/core/svg/animation/SVGSMILElement.cpp:1003: bool
> SVGSMILElement::checkRestart(SMILTime elapsed)
> Can we clean this up a bit?
> 
> I'll offer a suggestion:
> RestartedInterval SVGSMILElement::maybeTriggerRestart(SMILTime elapsed)
> where RestartedInterval is an enum with two values: DidRestartInterval and
> DidNotRestartInterval.

I went with maybeRestartInterval instead. Otherwise as above.

[fs, 2014-02-04 09:56:29]

The CQ bit was checked by fs@opera.com

[commit-bot: I haz the power, 2014-02-04 09:56:37]

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

[commit-bot: I haz the power, 2014-02-04 10:08:12]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-02-04 10:08:12]

Retried try job too often on mac_blink_rel for step(s) blink_heap_unittests,
blink_platform_unittests, webkit_lint, webkit_python_tests, webkit_tests,
webkit_unit_tests, wtf_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_blink_...

[fs, 2014-02-04 11:17:27]

The CQ bit was checked by fs@opera.com

[commit-bot: I haz the power, 2014-02-04 11:17:41]

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

[commit-bot: I haz the power, 2014-02-04 13:56:39]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-02-04 13:56:39]

Retried try job too often on linux_blink for step(s) webkit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_blin...

[skia-commit-bot, 2014-02-04 13:56:50]

CQ bit was unchecked on CL. Ignoring.

[fs, 2014-02-04 14:21:03]

The CQ bit was checked by fs@opera.com

[commit-bot: I haz the power, 2014-02-04 14:21:16]

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

[commit-bot: I haz the power, 2014-02-04 17:22:58]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-02-04 17:22:59]

Retried try job too often on linux_blink for step(s) webkit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_blin...

[pdr., 2014-02-04 18:27:57]

The CQ bit was checked by pdr@chromium.org

[commit-bot: I haz the power, 2014-02-04 18:28:16]

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

[commit-bot: I haz the power, 2014-02-05 00:38:02]

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

[fs, 2014-02-05 14:19:37]

The CQ bit was unchecked by fs@opera.com

[fs, 2014-02-05 14:19:58]

The CQ bit was checked by fs@opera.com

[commit-bot: I haz the power, 2014-02-05 14:20:15]

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

[commit-bot: I haz the power, 2014-02-05 17:34:14]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-02-05 17:34:15]

Retried try job too often on linux_blink for step(s) webkit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_blin...

[fs, 2014-02-06 10:33:39]

The CQ bit was checked by fs@opera.com

[commit-bot: I haz the power, 2014-02-06 10:33:54]

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

[fs, 2014-02-06 14:55:15]

The CQ bit was checked by fs@opera.com

[commit-bot: I haz the power, 2014-02-06 14:55:27]

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

[commit-bot: I haz the power, 2014-02-06 17:39:08]

Change committed as 166625