Bug 74974

Summary:

[chromium] m_triggerIdlePaints not reset after a compositeAndReadback

Product:

WebKit

Reporter:

Rachel Blum <groby>

Component:

New Bugs

Assignee:

Eric Penner <epenner>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

cc-bugs, epenner, jamesr, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	none

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	Details Formatted Diff Diff
Patch (1.46 KB, patch) 2011-12-20 18:32 PST, Eric Penner	no flags	Details Formatted Diff Diff
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.