RESOLVED FIXED 74974
[chromium] m_triggerIdlePaints not reset after a compositeAndReadback 
https://bugs.webkit.org/show_bug.cgi?id=74974
Summary [chromium] m_triggerIdlePaints not reset after a compositeAndReadback
Rachel Blum
Reported 2011-12-20 16:41:07 PST
[Coverity] removed dead code
Attachments
Patch (1.34 KB, patch)
2011-12-20 16:50 PST, Rachel Blum 		no flags
DetailsFormatted DiffDiff
Patch (1.46 KB, patch)
2011-12-20 18:32 PST, Eric Penner 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Rachel Blum
Comment 1 2011-12-20 16:50:19 PST
Created attachment 120115 [details] Patch
James Robinson
Comment 2 2011-12-20 17:04:26 PST
Comment on attachment 120115 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=120115&action=review I am significantly skeptical of the correctness of this patch. What's the reasoning behind it? > Source/WebCore/ChangeLog:8 > + No new tests. (OOPS!) this will fail an SVN presubmit check
Rachel Blum
Comment 3 2011-12-20 17:13:39 PST
I'm significantly certain that code after a return is never executed :) This is a simple removal of dead code - the deleted line can NEVER be hit. (fix for ChangeLog coming)
James Robinson
Comment 4 2011-12-20 17:15:57 PST
Oh! I misread the code. In that case the code is buggy and the fix is wrong - we really intend to set that bool member back to false before returning from the function. Eric, you wrote this - can you fix it?
James Robinson
Comment 5 2011-12-20 17:17:41 PST
What's really going on here is compositeAndReadback() is effectively disabling idle paints. Since we do compositeAndReadback() on every navigation (for thumbnailing) prepainting is effectively off for all pages currently. Definitely not what we intend. Hurray for coverity for finding this.
Eric Penner
Comment 6 2011-12-20 17:24:48 PST
(In reply to comment #5) > What's really going on here is compositeAndReadback() is effectively disabling idle paints. Since we do compositeAndReadback() on every navigation (for thumbnailing) prepainting is effectively off for all pages currently. Definitely not what we intend. Hurray for coverity for finding this. Arg. This was a last minute change too. It was previously in another function and I 'cleaned it up'. The reason it's not noticeable is that scrolling will trigger sufficient commits anyway. I will fix it.
Rachel Blum
Comment 7 2011-12-20 17:28:59 PST
Works for me :) I'll leave this one to Eric, then.
Eric Penner
Comment 8 2011-12-20 17:35:57 PST
(In reply to comment #7) > Works for me :) I'll leave this one to Eric, then. Thanks for finding it! :)
Eric Penner
Comment 9 2011-12-20 18:32:34 PST
Created attachment 120132 [details] Patch
James Robinson
Comment 10 2011-12-20 18:58:27 PST
Comment on attachment 120132 [details] Patch Test coverage would be even better, but this looks correct.
WebKit Review Bot
Comment 11 2011-12-20 20:36:31 PST
Comment on attachment 120132 [details] Patch Clearing flags on attachment: 120132 Committed r103388: <http://trac.webkit.org/changeset/103388>
WebKit Review Bot
Comment 12 2011-12-20 20:36:35 PST
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.