Bug 82109 - [chromium] Fix race bug that clobbers CCLayerImpl updateRect
Summary: [chromium] Fix race bug that clobbers CCLayerImpl updateRect
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Shawn Singh
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-03-23 17:52 PDT by Shawn Singh
Modified: 2012-03-23 19:37 PDT (History)
8 users (show)

See Also:

Attachments
Patch (5.01 KB, patch)
2012-03-23 17:56 PDT, Shawn Singh 		dpranke: review+
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Shawn Singh 2012-03-23 17:52:50 PDT 
If the main thread commits twice before the impl thread actually draws, then the updateRect of the first frame gets lost forever, and not propagated to the damage tracker.

The solution is to accumulate the updateRect.  The updateRect is already being correctly cleared at the appropriate time after drawing.

Patch coming in a moment.
Comment 1 Shawn Singh 2012-03-23 17:56:49 PDT 
Created attachment 133600 [details]
Patch
Comment 2 Vangelis Kokkevis 2012-03-23 18:08:25 PDT 
This looks good to me. Alas, you'll have to get a WK reviewer to sign off on it.
Comment 3 Shawn Singh 2012-03-23 18:28:33 PDT 
Committed r111963: <http://trac.webkit.org/changeset/111963>
Comment 4 Adrienne Walker 2012-03-23 18:42:06 PDT 
Comment on attachment 133600 [details]
Patch

How do we commit twice? I though the main thread was supposed to wait for BFAC before committing again?
Comment 5 Shawn Singh 2012-03-23 19:00:54 PDT 
I don't know.  Perhaps Nat can suggest why?

I can say however, that fprintfs clearly show that TreeSynchronizer::synchronizeTrees() does commonly occur twice in a row before CCLayerTreeHostImpl::drawLayers() completes once.

I agree this is a question we should find the answer. =)
Comment 6 Dana Jansens 2012-03-23 19:37:27 PDT 
Are there accelerated animations on your test?