Bug 49073 - Move resumeAnimations/suspendAnimations from Frame to AnimationController.
Summary: Move resumeAnimations/suspendAnimations from Frame to AnimationController.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Chris Marrin
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-11-05 09:55 PDT by Chris Marrin
Modified: 2010-11-05 14:45 PDT (History)
1 user (show)

See Also:


Attachments
Patch (11.82 KB, patch)
2010-11-05 11:22 PDT, Chris Marrin
no flags Details | Formatted Diff | Diff
Patch (19.60 KB, patch)
2010-11-05 13:02 PDT, Chris Marrin
no flags Details | Formatted Diff | Diff
Patch (17.12 KB, patch)
2010-11-05 13:37 PDT, Chris Marrin
simon.fraser: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Marrin 2010-11-05 09:55:15 PDT
Darin doesn't want to increase the API of Frame, so these should be moved to AnimationController.
Comment 1 Chris Marrin 2010-11-05 11:22:45 PDT
Created attachment 73088 [details]
Patch
Comment 2 Darin Adler 2010-11-05 12:36:49 PDT
Comment on attachment 73088 [details]
Patch

I think it would be cleaner to rename the existing function resumeAnimationsForThisFrameOnly or something like that, and have a new function with the loop in it, instead of using a magic value of 0 to mean "loop through subframes".
Comment 3 Simon Fraser (smfr) 2010-11-05 12:44:33 PDT
Comment on attachment 73088 [details]
Patch

Right, Chris is going to add new methods to AnimationController.
Comment 4 Chris Marrin 2010-11-05 13:02:37 PDT
Created attachment 73103 [details]
Patch
Comment 5 Simon Fraser (smfr) 2010-11-05 13:13:10 PDT
Comment on attachment 73103 [details]
Patch

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!
Comment 6 Chris Marrin 2010-11-05 13:37:23 PDT
Created attachment 73110 [details]
Patch
Comment 7 Simon Fraser (smfr) 2010-11-05 13:57:27 PDT
Comment on attachment 73110 [details]
Patch

So much nicer!
Comment 8 Chris Marrin 2010-11-05 14:45:59 PDT
Committed r71451: <http://trac.webkit.org/changeset/71451>