[webkit-reviews] review requested: [Bug 87766] [chromium] Ensure that skipping frames during an accelerated animation doesn't cause starvation : [Attachment 144860] Patch

[bugzilla-daemon at webkit.org]

vollick at chromium.org has asked	for review:
Bug 87766: [chromium] Ensure that skipping frames during an accelerated
animation doesn't cause starvation
https://bugs.webkit.org/show_bug.cgi?id=87766

Attachment 144860: Patch
https://bugs.webkit.org/attachment.cgi?id=144860&action=review

------- Additional Comments from vollick at chromium.org
(In reply to comment #6)
> (From update of attachment 144831 [details])
> View in context:
https://bugs.webkit.org/attachment.cgi?id=144831&action=review
>
> Can you piggy back or unify this code with the
m_lastFrameNumberWhereDrawWasCalled? I'm confused about what the hell that code
does anyway.
That won't work here, unfortunately. m_lastFrameNumberWhereDrawWasCalled is
reset to -1 every commit, but it's certainly possible to continue to
checkerboard before and after a commit, so I think we'll need another counter.
>
> > Source/WebCore/platform/graphics/chromium/cc/CCSchedulerStateMachine.cpp:30
> > +static const int maximumNumberOfFailedDrawsBeforeForce = 8;
>
> thats a hitch of 8*16ms = 133ms. You sure you want to wait that long?
Yeah, that is a bit too long. Switched to 3 frames.
>
> > Source/WebKit/chromium/tests/CCLayerTreeHostTest.cpp:79
> > +	 virtual void prepareToDrawOnCCThread(CCLayerTreeHostImpl*, bool&
shouldForceFailure) { }
>
> This hook is pretty confusing to me... why not make this return a bool
(default to true) and make the real function down around line 120 return the
value here.
Done.