[webkit-reviews] review granted: [Bug 58907] [chromium] Don't unnecessarily resize skia/cg canvases when painting in compositor : [Attachment 91343] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Apr 28 15:38:17 PDT 2011


James Robinson <jamesr at chromium.org> has granted Adrienne Walker
<enne at google.com>'s request for review:
Bug 58907: [chromium] Don't unnecessarily resize skia/cg canvases when painting
in compositor
https://bugs.webkit.org/show_bug.cgi?id=58907

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

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

R=me, few things to address before landing.

> LayoutTests/compositing/repaint/same-size-invalidation.html:22
> +	   setTimeout("changeBackground1();", 0);

nit: setTimeout(changeBackground1, 0) is a bit better. move doTest below the
declarations of the functions.

> LayoutTests/platform/chromium/test_expectations.txt:3973
> +BUGENNE : compositing/repaint/same-size-invalidation.html = FAIL

= FAIL isn't sufficient - these will fail with MISSING on windows and mac since
there is no baseline for those platforms.

This test should render the same on all platforms so I'd just check
same-size-invalidation-expected.png/checksum into platform/chromium-gpu/ and
not modify test_expectations at all.

> Source/WebCore/platform/graphics/chromium/PlatformCanvas.h:84
> +	   // Destructor restores canvas to pre-construction state.

does the Painter manipulate any state on the PlatformCanvas? 
Painter::Painter() and Painter::~Painter() only seem to mutate their m_context,
which doesn't do anything to the PlatformCanvas other than render into it.


More information about the webkit-reviews mailing list