Bug 83179

Summary: [chromium] Race condition in CCLayerTreeHostTestSynchronizeAnimationStartTimes
Product: WebKit Reporter: vollick
Component: WebKit Misc.Assignee: vollick
Status: RESOLVED FIXED    
Severity: Normal CC: enne, jamesr, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

vollick
Reported 2012-04-04 10:23:52 PDT
This test checks if animateLayers has been called before notifyAnimationStarted, but with the threaded compositior, this may not be true.
Attachments
Patch (1.81 KB, patch)
2012-04-04 10:29 PDT, vollick 		no flags
DetailsFormatted DiffDiff
Patch (3.52 KB, patch)
2012-04-04 12:18 PDT, vollick 		no flags
DetailsFormatted DiffDiff
Patch (3.75 KB, patch)
2012-04-04 12:20 PDT, vollick 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
vollick
Comment 1 2012-04-04 10:29:29 PDT
Created attachment 135621 [details] Patch Removes the race condition in CCLayerTreeHostTestSynchronizeAnimationStartTimes. Also re-enables CCLayerTreeHostTestAddAnimationWithTimingFunction which I think may have been mistakenly disabled rather than CCLayerTreeHostTestSynchronizeAnimationStartTimes.
vollick
Comment 2 2012-04-04 12:18:57 PDT
Created attachment 135648 [details] Patch Before the call to notifyAnimationStarted, we must have a valid layerTreeHostImpl or else we won't be able to compare the start times of the animations. Previously, we'd grabbed this value in TestHooks::animateLayers. This function is called on the impl thread _after_ the call to CCLayerTreeHostImpl::animateLayers. The problem is that we check this value in notifyAnimationStarted which executes on the main thread, possibly before TestHooks::animateLayers. To get around this problem, we grab the pointer to the layerTreeHostImpl in willAnimateLayers instead. This is guaranteed to be called before notifyAnimationStarted (notifyAnimationStarted is called due to a task posted by CCLayerTreeHostImpl::animateLayers, which is called after willAnimateLayers).
vollick
Comment 3 2012-04-04 12:20:44 PDT
Created attachment 135651 [details] Patch Re-enables CCLayerTreeHostTestAddAnimationWithTimingFunction.
WebKit Review Bot
Comment 4 2012-04-05 11:58:12 PDT
Comment on attachment 135651 [details] Patch Clearing flags on attachment: 135651 Committed r113346: <http://trac.webkit.org/changeset/113346>
WebKit Review Bot
Comment 5 2012-04-05 11:58:19 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.