Bug 26770 - Transitions fail to run sometimes
: Transitions fail to run sometimes
Status: ASSIGNED
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering
: 528+ (Nightly build)
: Macintosh Mac OS X 10.5
: P2 Normal
Assigned To: Simon Fraser (smfr)
:
Depends on:
Blocks: 93136
  Show dependency treegraph
 
Reported: 2009-06-26 22:44 PDT by Simon Fraser (smfr)
Modified: 2012-12-12 09:23 PST (History)
3 users (show)

See Also:


Attachments
Possible testcase (1.35 KB, text/html)
2009-10-01 21:23 PDT, Simon Fraser (smfr)
no flags Details
Patch (10.98 KB, patch)
2009-10-14 14:46 PDT, Simon Fraser (smfr)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Fraser (smfr) 2009-06-26 22:44:57 PDT
I'm playing with trying to make opacity transitions only run in hardware when the page is already in hardware, and am running into a bug.

AnimationControllerPrivate has a m_waitingForAResponse member variable, which gets set to 'true' when an animation is going to run in hardware, and give us a callback when it starts.

However, nothing sets this to false, so if you later try and run a software anim in the same frame (since the AnimationController has the same lifetime as the frame), then it never starts.

This may explain some of the transition weirdness.
Comment 1 Simon Fraser (smfr) 2009-06-26 22:47:14 PDT
Is this the right place to clear it?

diff --git a/WebCore/page/animation/AnimationController.cpp b/WebCore/page/animation/AnimationController.cpp
index 58a1f5b..c5bd82a 100644
--- a/WebCore/page/animation/AnimationController.cpp
+++ b/WebCore/page/animation/AnimationController.cpp
@@ -432,6 +432,7 @@ void AnimationControllerPrivate::startTimeResponse(double t)
     
     m_responseWaiters = 0;
     m_lastResponseWaiter = 0;
+    m_waitingForAResponse = false;
 }
 
 AnimationController::AnimationController(Frame* frame)
Comment 2 Simon Fraser (smfr) 2009-06-26 22:56:02 PDT
Actually this works fine as long as receivedStartTimeResponse() gets called. However, if it doesn't for some reason (layer gets blown away?), then that flag gets left set to true.
Comment 3 Chris Marrin 2009-06-27 06:32:23 PDT
You analysis looks right. The problem is what to do if the thing that will trigger the callback never happens. I think if we save a ref to the AnimationBase object that will trigger the callback, and then clear the flag if that object gets destroyed, it will ensure the flag is always in the right state. But in that case, if the flag gets cleared, we will have to make the callback happen right away to get the other animations started. This will destroy sync, but that won't be a big issue in this special case. It will be very rare.
Comment 4 Simon Fraser (smfr) 2009-10-01 21:23:49 PDT
Created attachment 40494 [details]
Possible testcase

Here's a test that I think is showing the same issue. After you run this one test, navigating to another page with animations asserts:
ASSERTION FAILED: !childNeedsStyleRecalc()
(/Volumes/WebKit/WebKit.git/WebCore/dom/Document.cpp:1193 void WebCore::Document::unscheduleStyleRecalc())
Comment 5 Simon Fraser (smfr) 2009-10-01 21:38:43 PDT
The change in comment 1 makes this problem caused by the testcase go away.
Comment 6 Simon Fraser (smfr) 2009-10-06 21:58:27 PDT
There's a reproducible instance of this issue in bug 29984.
Comment 7 Simon Fraser (smfr) 2009-10-14 14:46:48 PDT
Created attachment 41190 [details]
Patch
Comment 8 Simon Fraser (smfr) 2009-10-15 09:53:11 PDT
http://trac.webkit.org/changeset/49633
Comment 9 Simon Fraser (smfr) 2009-10-16 13:15:05 PDT
This broke accelerated transitions in some cases: bug 30453.
Comment 10 Simon Fraser (smfr) 2009-10-16 13:17:11 PDT
Reopening to cover the issues with broken transitions related to visibility:hidden (see also bug 29984).
Comment 11 Eric Seidel 2009-10-19 15:12:11 PDT
Comment on attachment 41190 [details]
Patch

Clearing mitz's r+ on this obsolete patch.
Comment 12 Simon Fraser (smfr) 2010-08-19 17:28:01 PDT
<rdar://problem/8281319>