In the damage tracker, the original logic did an early-exit if we knew the entire surface was damaged. This is wrong, however, since an early exit means that the tracking state of all other layers will not be updated. Jonathan and I already verified a simple change that fixes the problem, but we should still create a nice unit test to reproduce this.
Created attachment 123778 [details] Patch
Comment on attachment 123778 [details] Patch Unit test?
(In reply to comment #2) > (From update of attachment 123778 [details]) > Unit test? I hit the --no-review flag. I wasn't sure that Shawn had the cycles to work on this, so I just wanted to document the patch. Turns out that he's got a different version (with a unit-test). I'm sure he'll upload shortly.
(In reply to comment #3) > (In reply to comment #2) > > (From update of attachment 123778 [details] [details]) > > Unit test? > > I hit the --no-review flag. I wasn't sure that Shawn had the cycles to work on this, so I just wanted to document the patch. > > Turns out that he's got a different version (with a unit-test). I'm sure he'll upload shortly. I have a fix, and it does work. But it actually ends up breaking other CCDamageTracker unit tests. As far as I have debugged so far, I believe it is the result of a unrelated bug in CCLayerTreeHostCommon... setting an incorrect contentRect when there is a replicaLayer. I would prefer to fully understand and fix that bug first, and then submit the patch for this. I will be able to look into that issue starting early next week.
Created attachment 123997 [details] not for review, just for reference Jonathan requested me to upload this, just for reference and temporary testing
Created attachment 125561 [details] Patch
(In reply to comment #6) > Created an attachment (id=125561) [details] > Patch The patch just uploaded does three things: (1) adds unit test that demonstrates that early exiting in CCDamageTracker is wrong, (2) removes the early exit and cleans up the surrounding code, (3) re-names several functions in CCDamageTracker to reflect that they do more than just "compute" something, they also update state held by the damage tracker. Before renaming, this state tracking was more of a "side-effect"... renaming them makes it more clear that this is intended, and not an un-desired side effect. The contentRect problem I found is actually un-related, and I placed a FIXME in this patch to reflect it.
Jamesr - when you have a chance, could you please review this? Thanks in advance.
Comment on attachment 125561 [details] Patch OK, R=me
Comment on attachment 125561 [details] Patch Clearing flags on attachment: 125561 Committed r107130: <http://trac.webkit.org/changeset/107130>
All reviewed patches have been landed. Closing bug.