Chromium Code Reviews| Index: Source/core/animation/CompositorAnimations.cpp | 
| diff --git a/Source/core/animation/CompositorAnimations.cpp b/Source/core/animation/CompositorAnimations.cpp | 
| index 1eb3d71076417b832f77022c2bb37dae92d4e602..390b1d39c19967457b8aff18dab8815d610587be 100644 | 
| --- a/Source/core/animation/CompositorAnimations.cpp | 
| +++ b/Source/core/animation/CompositorAnimations.cpp | 
| @@ -136,8 +136,9 @@ bool CompositorAnimations::canStartCompositorAnimation(const Element& element) | 
| return element.renderer() && element.renderer()->compositingState() == PaintsIntoOwnBacking; | 
| } | 
| -void CompositorAnimations::startCompositorAnimation(const Element& element, const Timing& timing, const AnimationEffect& effect, Vector<int>& startedAnimationIds) | 
| +bool CompositorAnimations::startCompositorAnimation(const Element& element, const Timing& timing, const AnimationEffect& effect, Vector<int>& startedAnimationIds) | 
| { | 
| + ASSERT(startedAnimationIds.isEmpty()); | 
| ASSERT(isCandidateForCompositorAnimation(timing, effect)); | 
| ASSERT(canStartCompositorAnimation(element)); | 
| @@ -147,19 +148,27 @@ void CompositorAnimations::startCompositorAnimation(const Element& element, cons | 
| ASSERT(layer); | 
| ASSERT(layer->renderBox()); | 
| - // FIXME: Should we check in some way if there is an animation already created? | 
| Vector<OwnPtr<blink::WebAnimation> > animations; | 
| CompositorAnimationsImpl::getCompositorAnimations(timing, keyframeEffect, animations, layer->renderBox()->pixelSnappedBorderBoxRect().size()); | 
| for (size_t i = 0; i < animations.size(); ++i) { | 
| - startedAnimationIds.append(animations[i]->id()); | 
| - layer->compositedLayerMapping()->mainGraphicsLayer()->addAnimation(animations[i].release()); | 
| + int id = animations[i]->id(); | 
| + if (!layer->compositedLayerMapping()->mainGraphicsLayer()->addAnimation(animations[i].release())) { | 
| + // FIXME: We should know ahead of time whether these animations can be started. | 
| + for (size_t j = 0; j < startedAnimationIds.size(); ++j) | 
| + cancelCompositorAnimation(element, startedAnimationIds[j]); | 
| + startedAnimationIds.clear(); | 
| + return false; | 
| + } | 
| + startedAnimationIds.append(id); | 
| } | 
| + return true; | 
| } | 
| void CompositorAnimations::cancelCompositorAnimation(const Element& element, int id) | 
| { | 
| - // FIXME: Implement. | 
| - ASSERT_NOT_REACHED(); | 
| + if (!element.renderer() || element.renderer()->compositingState() != PaintsIntoOwnBacking) | 
| + return; | 
| + toRenderBoxModelObject(element.renderer())->layer()->compositedLayerMapping()->mainGraphicsLayer()->removeAnimation(id); | 
| } | 
| // ----------------------------------------------------------------------- | 
| @@ -246,7 +255,6 @@ bool CompositorAnimationsImpl::isCandidateForCompositor(const KeyframeAnimationE | 
| bool CompositorAnimationsImpl::isCandidateForCompositor(const Timing& timing, const KeyframeAnimationEffect::KeyframeVector& frames) | 
| { | 
| - | 
| CompositorTiming out; | 
| if (!convertTimingForCompositor(timing, out)) | 
| return false; | 
| @@ -317,9 +325,8 @@ bool CompositorAnimationsImpl::convertTimingForCompositor(const Timing& timing, | 
| if (timing.iterationStart) | 
| return false; | 
| - // FIXME: Compositor only supports finite, non-zero, integer iteration | 
| - // counts. | 
| - if (!std::isfinite(timing.iterationCount) || (std::floor(timing.iterationCount) != timing.iterationCount) || !timing.iterationCount) | 
| + // FIXME: Compositor only non-zero, integer iteration counts. | 
| 
 
shans
2013/11/18 01:02:06
I liked your old FIXME better than your new FIXME
 
dstockwell
2013/11/18 05:30:34
This is being fixed in a separate patch.
 
 | 
| + if ((std::floor(timing.iterationCount) != timing.iterationCount) || !timing.iterationCount) | 
| return false; | 
| if (!timing.iterationDuration) | 
| @@ -351,7 +358,9 @@ bool CompositorAnimationsImpl::convertTimingForCompositor(const Timing& timing, | 
| out.reverse = !out.reverse; | 
| out.adjustedIterationCount = std::floor(timing.iterationCount) - skippedIterations; | 
| - ASSERT(out.adjustedIterationCount > 0); | 
| + if (!std::isfinite(timing.iterationCount)) | 
| + out.adjustedIterationCount = -1; | 
| + // ASSERT(out.adjustedIterationCount > 0); | 
| out.scaledTimeOffset = scaledStartDelay + skippedIterations * out.scaledDuration; | 
| ASSERT(out.scaledTimeOffset <= 0); | 
| @@ -472,7 +481,8 @@ void CompositorAnimationsImpl::addKeyframesToCurve(PlatformAnimationCurveType& c | 
| // ChainedTimingFunction criteria was checked in isCandidate, | 
| // assert it is valid. | 
| ASSERT(values.size() == chained->m_segments.size() + 1); | 
| - ASSERT(values[i].first == chained->m_segments[i].m_min); | 
| + // FIXME: first has been scaled, this assertion is not valid | 
| + // ASSERT(values[i].first == chained->m_segments[i].m_min); | 
| 
 
shans
2013/11/18 01:02:06
Just remove then?
 
dstockwell
2013/11/18 05:30:34
This is being fixed in a separate patch.
 
 | 
| keyframeTimingFunction = chained->m_segments[i].m_timingFunction.get(); | 
| break; |