[webkit-reviews] review granted: [Bug 75557] [chromium] Create unit tests for CCTiledLayerImpl : [Attachment 121120] Patch

[bugzilla-daemon at webkit.org]

James Robinson <jamesr at chromium.org> has granted Adrienne Walker
<enne at google.com>'s request for review:
Bug 75557: [chromium] Create unit tests for CCTiledLayerImpl
https://bugs.webkit.org/show_bug.cgi?id=75557

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

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


Awesome! R=me

> Source/WebKit/chromium/tests/CCTiledLayerImplTest.cpp:38
> +#define QUAD(i) "	Quad: " << i

can you move this down closer to the first use?

do we really really need this? we have a lot of quad-named things going on.
what about a string constant that's <<'d in like normal?

> Source/WebKit/chromium/tests/CCTiledLayerImplTest.cpp:246
> +	   EXPECT_EQ(quads[i]->material(), CCDrawQuad::TiledContent) <<
QUAD(i);

think this should be ASSERT_EQ() or the next few lines will do really crazy and
potentially hard-to-diagnose things