Bug 46945 - Make suspendAnimations/resumeAnimations and setCSSAnimations traverse through subframes and remember state
: Make suspendAnimations/resumeAnimations and setCSSAnimations traverse through...
Status: RESOLVED FIXED
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering
: 528+ (Nightly build)
: PC Mac OS X 10.5
: P2 Normal
Assigned To: Chris Marrin
: InRadar
Depends on:
Blocks: 46525 46540
  Show dependency treegraph
 
Reported: 2010-09-30 17:26 PDT by Chris Marrin
Modified: 2010-11-05 09:56 PDT (History)
2 users (show)

See Also:


Attachments
Patch (18.69 KB, patch)
2010-11-04 15:56 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-09-30 17:26:47 PDT
Currently suspendAnimations/resumeAnimations in WebCore take a document, but the pass through from WebKit, which allows access from LayoutTestController simply passes in the top-level document. The WebKit versions should traverse all subframes. Also, we don't remember the suspend state in AnimationController, so starting a new animation while suspended doesn't suspend.

I'm fixing these together because the test case I am writing needs the functionality in the former to test the latter.
Comment 1 Simon Fraser (smfr) 2010-09-30 17:32:55 PDT
Dup of bug 46429?
Comment 2 Chris Marrin 2010-10-01 08:33:11 PDT
*** Bug 46944 has been marked as a duplicate of this bug. ***
Comment 3 Chris Marrin 2010-10-01 08:34:59 PDT
*** Bug 46429 has been marked as a duplicate of this bug. ***
Comment 4 Chris Marrin 2010-10-01 08:35:45 PDT
<rdar://problem/8472265>
Comment 5 Chris Marrin 2010-11-04 15:56:22 PDT
Created attachment 72995 [details]
Patch
Comment 6 Simon Fraser (smfr) 2010-11-04 16:17:14 PDT
Comment on attachment 72995 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=72995&action=review

> LayoutTests/animations/animation-test-helpers.js:82
> +        }
> +    }

inIframe is redundant, because you could just test if (iframeId)...

> LayoutTests/animations/stop-animation-on-suspend.html:56
> +        setTimeout(suspend, 250);
> +        setTimeout(resume, 500);

Can we make this test faster?
Comment 7 Darin Adler 2010-11-05 09:51:03 PDT
Comment on attachment 72995 [details]
Patch

We should not be adding new functions to Frame. We’ve been working hard slowly moving functions off of it and adding new ones is not a good idea. These function should go on the animation controller.
Comment 8 Chris Marrin 2010-11-05 09:55:35 PDT
(In reply to comment #7)
> (From update of attachment 72995 [details])
> We should not be adding new functions to Frame. We’ve been working hard slowly moving functions off of it and adding new ones is not a good idea. These function should go on the animation controller.

Sorry, but I already checked in before seeing this. I have opened a new bug (https://bugs.webkit.org/show_bug.cgi?id=49073) to make this change.
Comment 9 Chris Marrin 2010-11-05 09:56:16 PDT
Landed in http://trac.webkit.org/changeset/71424