[webkit-reviews] review granted: [Bug 80540] [Chromium] Single buffered canvas layers with the threaded compositor : [Attachment 138849] Patch

[bugzilla-daemon at webkit.org]

James Robinson <jamesr at chromium.org> has granted Justin Novosad
<junov at chromium.org>'s request for review:
Bug 80540: [Chromium] Single buffered canvas layers with the threaded
compositor
https://bugs.webkit.org/show_bug.cgi?id=80540

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

------- Additional Comments from James Robinson <jamesr at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=138849&action=review


The scheduler changes look good.

R=me but please address the issues I've pointed out in this comment and the
ones previous and fix the build before landing.

> Source/WebCore/platform/graphics/chromium/Canvas2DLayerChromium.cpp:74
>      if (m_context && layerTreeHost())

think you want to check for m_useRateLimiter too here, no?

> Source/WebKit/chromium/tests/Canvas2DLayerChromiumTest.cpp:100
> +    void fullLifecycleTest(bool threaded, bool deferred, bool
doubleBuffered)

we should test the configurations that we care about - some of the configs here
just don't make any sense (like single-threaded and double-buffered). let the
layer decide if it wants to double-buffer or not based on the other settings