[webkit-reviews] review granted: [Bug 95526] [chromium] Consolidate geometry unit testing functions for cc : [Attachment 161776] fix includes

[bugzilla-daemon at webkit.org]

Julien Chaffraix <jchaffraix at webkit.org> has granted James Robinson
<jamesr at chromium.org>'s request for review:
Bug 95526: [chromium] Consolidate geometry unit testing functions for cc
https://bugs.webkit.org/show_bug.cgi?id=95526

Attachment 161776: fix includes
https://bugs.webkit.org/attachment.cgi?id=161776&action=review

------- Additional Comments from Julien Chaffraix <jchaffraix at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=161776&action=review


Provided it compiles on the EWS, r=me.

>>> Source/WebKit/chromium/tests/CCGeometryTestUtils.h:41
>>> +#define EXPECT_RECT_EQ(expected, actual)				 \
>> 
>> For me personally, the reason I liked EXPECT_INT_RECT_EQ instead of
EXPECT_RECT_EQ was because its a little easier to make us aware when we
accidentally use floatRects/intRects inside of the wrong EXPECT macro.
>> 
>> Again, the patch unofficially LGTM other than this.
> 
> Well, without this patch we were also using EXPECT_EQ_RECT all over the place
too.  I kind of like EXPECT_RECT_EQ() for where you don't really care and
EXPECT_FLOAT_RECT_EQ where you explicitly want fuzzy matching.	I think this is
all temporary anyway since I anticipate we'll switch to different types and
just write operator== so these will all turn into EXPECT_EQ() which does the
right thing on the the types provided.
> 
> The motivation for this (beyond cleanup) is moving the matrix equality
operator out of CCLayerTreeHostCommonTest.cpp because currently I can't get
cc_unittests and webkit_compositor_unittests linking at all downstream, so I'd
like to get that resolved ASAPly and follow up on any other cleanups we want
later.

Macros don't give much guarantees that the arguments are IntRects so it's
probably better to use EXPECT_RECT_EQ from that perspective. This is just my 2
cents from someone not involved as strictly in the compositor code.