[webkit-reviews] review denied: [Bug 49073] Move resumeAnimations/suspendAnimations from Frame to AnimationController. : [Attachment 73103] Patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has denied Chris Marrin
<cmarrin at apple.com>'s request for review:
Bug 49073: Move resumeAnimations/suspendAnimations from Frame to
AnimationController.
https://bugs.webkit.org/show_bug.cgi?id=49073

Attachment 73103: Patch
https://bugs.webkit.org/attachment.cgi?id=73103&action=review

------- Additional Comments from Simon Fraser (smfr) <simon.fraser at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=73103&action=review

> WebCore/page/animation/AnimationController.cpp:241
>      setBeginAnimationUpdateTime(cBeginAnimationUpdateTimeNotSet);
>      
>      RenderObjectAnimationMap::const_iterator animationsEnd =
m_compositeAnimations.end();
>      for (RenderObjectAnimationMap::const_iterator it =
m_compositeAnimations.begin(); it != animationsEnd; ++it) {
>	   RenderObject* renderer = it->first;
> +	   if (renderer->document() == m_frame->document()) {
> +	       CompositeAnimation* compAnim = it->second.get();
> +	       compAnim->suspendAnimations();
> +	   }
> +    }

Too much copied code here. This should just do
suspendAnimationsForDocument(m_frame->document());

> WebCore/page/animation/AnimationController.cpp:261
> +    setBeginAnimationUpdateTime(cBeginAnimationUpdateTimeNotSet);
> +    
> +    RenderObjectAnimationMap::const_iterator animationsEnd =
m_compositeAnimations.end();
> +    for (RenderObjectAnimationMap::const_iterator it =
m_compositeAnimations.begin(); it != animationsEnd; ++it) {
> +	   RenderObject* renderer = it->first;
> +	   if (renderer->document() == m_frame->document()) {
> +	       CompositeAnimation* compAnim = it->second.get();
> +	       compAnim->resumeAnimations();
> +	   }
> +    }

Same, just call resumeAnimationsForDocument(m_frame->document());

> WebCore/page/animation/AnimationController.cpp:311
> +    /*
> +    bool traverseSubframes = false;
> +    if (!document) {
> +	   document = m_frame->document();
> +	   traverseSubframes = true;
> +    }
> +    */
> +

Remove!