Bug 49182 - animations/stop-animation-on-suspend.html sometimes fails on all platforms
Summary: animations/stop-animation-on-suspend.html sometimes fails on all platforms
Status: REOPENED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Windows XP
: P2 Normal
Assignee: Chris Marrin
URL: http://build.webkit.org/results/Windo...
Keywords: InRadar, LayoutTestFailure
: 84592 (view as bug list)
Depends on: 102905
Blocks: 49179 43905
  Show dependency treegraph
 
Reported: 2010-11-08 08:56 PST by Adam Roben (:aroben)
Modified: 2020-02-09 10:19 PST (History)
11 users (show)

See Also:


Attachments
Patch (10.96 KB, patch)
2012-11-01 06:57 PDT, Jussi Kukkonen (jku)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Adam Roben (:aroben) 2010-11-08 08:56:12 PST
animations/stop-animation-on-suspend.html sometimes fails on Windows. See the URL for one example. Here's another:

http://build.webkit.org/results/Windows%20Debug%20(Tests)/r71518%20(22136)/animations/stop-animation-on-suspend-pretty-diff.html

I've added this test to the Skipped file.
Comment 1 Adam Roben (:aroben) 2010-11-08 08:57:48 PST
I think this has been happening since the test was added in r71424. It looks like r71454 tried to address some of the failures, but clearly there are still problems.
Comment 2 Adam Roben (:aroben) 2010-11-08 08:59:31 PST
<rdar://problem/8641506>
Comment 3 Eric Seidel (no email) 2010-12-02 10:47:15 PST
animations/stop-animation-on-suspend.html is flaky on all platforms.  We should skip it.
Comment 4 Eric Seidel (no email) 2010-12-02 11:54:10 PST
Skipped for now:

Committed r73175: <http://trac.webkit.org/changeset/73175>
Comment 5 WebKit Review Bot 2010-12-02 12:34:40 PST
http://trac.webkit.org/changeset/73175 might have broken SnowLeopard Intel Release (Tests)
The following tests are not passing:
fast/profiler/throw-exception-from-eval.html
Comment 6 Eric Seidel (no email) 2010-12-02 12:38:46 PST
Um... The sherrifbot is right.  But it makes no sense.
http://trac.webkit.org/browser/trunk/LayoutTests/fast/profiler/throw-exception-from-eval.html
is now failing.  Perhaps it was dependent on this earlier test?
Comment 7 Eric Seidel (no email) 2010-12-02 12:39:42 PST
I wonder if this could have been caused by the rollout of bug 50401.
Comment 8 Adam Klein 2012-02-29 12:30:03 PST
This test has been skipped for >1 year on most platforms (on Chromium, we run it but expect it to fail flakily). Perhaps it's time to delete the test?
Comment 9 Simon Fraser (smfr) 2012-02-29 13:00:46 PST
..or fix it.
Comment 10 Dominik Röttsches (drott) 2012-10-30 05:53:12 PDT
*** Bug 84592 has been marked as a duplicate of this bug. ***
Comment 11 Dominik Röttsches (drott) 2012-10-30 06:01:16 PDT
Jussi writes in bug 84592:
"I believe the "webkitAnimationStart" event on document and the one on iframe happen at different times, but the test starts a single timer (on documents event) and of course the the iframe animation isn't synced"
Comment 12 Jussi Kukkonen (jku) 2012-11-01 02:35:09 PDT
(In reply to comment #11)
> Jussi writes in bug 84592:
> "I believe the "webkitAnimationStart" event on document and the one on iframe happen at different times, but the test starts a single timer (on documents event) and of course the the iframe animation isn't synced"

I could try doing something on this, but I'm not sure what the correct assumptions are:
The test assumes that 'box' and  'subframe-box' (inside the iframe) start animating at the same time. This isn't currently true, it seems 'subframe-box' is not even in the dom tree when 'box' animation starts.

If we don't care about animations starting (roughly) at the same time, I could probably just modify the test to ensure that both animations do suspend and resume.
Comment 13 Jussi Kukkonen (jku) 2012-11-01 05:43:51 PDT
(In reply to comment #12)
> If we don't care about animations starting (roughly) at the same time, I could probably just modify the test to ensure that both animations do suspend and resume.

I did just that and realised suspendAnimations() does not actually work. I'm pretty sure it did when I originally looked at this (in the duplicate bug). It's posisble we've missed a regression because this was skipped. I will take a closer look but comments on the assumption in comment 11 are still welcome.
Comment 14 Jussi Kukkonen (jku) 2012-11-01 06:01:56 PDT
(In reply to comment #13)
> I did just that and realised suspendAnimations() does not actually work. I'm pretty sure it did when I originally looked at this (in the duplicate bug). It's posisble we've missed a regression because this was skipped. I will take a closer look but comments on the assumption in comment 11 are still welcome.

Oh that was was silly of me -- I didn't notice it was a Internals call. Apologies for the spam.

I'll upload a patch that modifies this test to not expect animations to start at the same time and improves the documentation a bit -- there's a test for stopping on suspend already, this was added to make sure animations in subframes stop as well.
Comment 15 Jussi Kukkonen (jku) 2012-11-01 06:57:09 PDT
Created attachment 171837 [details]
Patch
Comment 16 Jussi Kukkonen (jku) 2012-11-01 07:02:53 PDT
(In reply to comment #15)
> Created an attachment (id=171837) [details]

Timing things like this in js is inherently flaky, but I think this should now be as good as the other tests (like suspend-resume-animation) although I'll take suggestions on a better way to do this than the iframe onload handler.

I've removed the failure expectations for this on all ports (except chromium who also had a crash expectation?) please let me know if you'd rather I didn't do that.
Comment 17 Antti Koivisto 2012-11-20 10:51:03 PST
Comment on attachment 171837 [details]
Patch

r=me
Comment 18 WebKit Review Bot 2012-11-20 13:00:49 PST
Comment on attachment 171837 [details]
Patch

Clearing flags on attachment: 171837

Committed r135307: <http://trac.webkit.org/changeset/135307>
Comment 19 WebKit Review Bot 2012-11-20 13:00:56 PST
All reviewed patches have been landed.  Closing bug.
Comment 20 WebKit Review Bot 2012-11-21 01:50:26 PST
Re-opened since this is blocked by bug 102905