[Webkit-unassigned] [Bug 71920] [chromium] CCThreadProxy::finishAllRendering hangs if !visible

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=71920


James Robinson <jamesr at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #114481|review?                     |review-
               Flag|                            |




--- Comment #12 from James Robinson <jamesr at chromium.org>  2011-11-10 11:34:14 PST ---
(From update of attachment 114481)
View in context: https://bugs.webkit.org/attachment.cgi?id=114481&action=review

I don't see any tests that verify that if visible = false and setNeedsForcedRedraw = true then we actually draw, which seems like the most valuable thing to test since that's the new functionality for this patch.  Other than the tests, though, this looks great.

> Source/WebKit/chromium/tests/CCSchedulerStateMachineTest.cpp:129
> +        // We should get exactly the same results in three scenarios:
> +        //  0. visible=true, setNeedsRedraw=true
> +        //  1. visible=true, setNeedsForcedRedraw=true
> +        //  2. visible=false, setNeedsForcedRedraw=true
> +        // The final case is covered by TestNoCommitStatesRedrawWhenInvisible.
> +        for (size_t j = 0; j < 3; ++j) {
> +            state.setNeedsRedraw(true);
> +            state.setNeedsForcedRedraw(j >= 1);
> +            state.setVisible(j <= 1);

I like trying to hit all the states but this is pretty hard to read.  A better way might be to have an array of structs each of which has the desired value for visible/setNeedsRedraw/setNeedsForcedRedraw and then iterate through that array.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.