Add two pixel-based compositor unittests.

Adds a couple of pixel-based compositor unittests. One to test that opacity is handled correctly, and one to check that MoveToFront and MoveAbove really do change the compositor output as expected.

Noteworthy changes are in layer_unittest.cc. Most of the changes below are due to moving compositor testing code into ui/gfx/compositor/test.

Depends on http://codereview.chromium.org/8463024/ and http://codereview.chromium.org/8590019/

BUG=None
TEST=compositor_unittests

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=110657

[jonathan.backer, 2011-11-14 20:22:40]

Just nits.

http://codereview.chromium.org/8533019/diff/1/ui/gfx/compositor/layer_unittes...
File ui/gfx/compositor/layer_unittest.cc (right):

http://codereview.chromium.org/8533019/diff/1/ui/gfx/compositor/layer_unittes...
ui/gfx/compositor/layer_unittest.cc:138: // Compares with a ping file on disk,
and returns true if it is the same as
s/ping/PNG

http://codereview.chromium.org/8533019/diff/1/ui/gfx/compositor/layer_unittes...
ui/gfx/compositor/layer_unittest.cc:139: // the given image.
Your call. Seems like something that belongs in the anon namespace at the top.
Just pass the file path instead of the file.

http://codereview.chromium.org/8533019/diff/1/ui/gfx/compositor/layer_unittes...
ui/gfx/compositor/layer_unittest.cc:178: // Write the generated and diff images
if the comparison failed.
Stale comment?

http://codereview.chromium.org/8533019/diff/1/ui/gfx/compositor/layer_unittes...
ui/gfx/compositor/layer_unittest.cc:926: //            
FilePath("ui/gfx/test/data/compositor/ModifyHierarchy1.png"));
This is probably OK. But I'm guessing that you need a space after //

[sky, 2011-11-14 21:44:26]

http://codereview.chromium.org/8533019/diff/1/ui/gfx/compositor/layer_unittes...
File ui/gfx/compositor/layer_unittest.cc (right):

http://codereview.chromium.org/8533019/diff/1/ui/gfx/compositor/layer_unittes...
ui/gfx/compositor/layer_unittest.cc:32: int bytes_written =
file_util::WriteFile(
nit: make this one statement,
return file_util::WriteFile(..) == static_cast<int>(png_data.size());

http://codereview.chromium.org/8533019/diff/1/ui/gfx/compositor/layer_unittes...
ui/gfx/compositor/layer_unittest.cc:140: bool IsSameAsPNGFile(const SkBitmap&
gen_bmp, const char* file) {
Document what file is relative to.

http://codereview.chromium.org/8533019/diff/1/ui/gfx/compositor/layer_unittes...
ui/gfx/compositor/layer_unittest.cc:146: should_compare = false;
Is there any point on continuing on if you can't read the file? Maybe all these
should be ASSERTs?

http://codereview.chromium.org/8533019/diff/1/ui/gfx/compositor/layer_unittes...
ui/gfx/compositor/layer_unittest.cc:157: should_compare = false;
Return here too?

[Ian Vollick, 2011-11-15 02:08:29]

On 2011/11/14 20:22:40, jonathan.backer wrote:
> Just nits.
> 
>
http://codereview.chromium.org/8533019/diff/1/ui/gfx/compositor/layer_unittes...
> File ui/gfx/compositor/layer_unittest.cc (right):
> 
>
http://codereview.chromium.org/8533019/diff/1/ui/gfx/compositor/layer_unittes...
> ui/gfx/compositor/layer_unittest.cc:138: // Compares with a ping file on disk,
> and returns true if it is the same as
> s/ping/PNG
Done.
> 
>
http://codereview.chromium.org/8533019/diff/1/ui/gfx/compositor/layer_unittes...
> ui/gfx/compositor/layer_unittest.cc:139: // the given image.
> Your call. Seems like something that belongs in the anon namespace at the top.
> Just pass the file path instead of the file.
I've put it in the anon namespace.
> 
>
http://codereview.chromium.org/8533019/diff/1/ui/gfx/compositor/layer_unittes...
> ui/gfx/compositor/layer_unittest.cc:178: // Write the generated and diff
images
> if the comparison failed.
> Stale comment?
Yep. Removed.
> 
>
http://codereview.chromium.org/8533019/diff/1/ui/gfx/compositor/layer_unittes...
> ui/gfx/compositor/layer_unittest.cc:926: //            
> FilePath("ui/gfx/test/data/compositor/ModifyHierarchy1.png"));
> This is probably OK. But I'm guessing that you need a space after //
Done.

[Ian Vollick, 2011-11-15 02:09:49]

On 2011/11/14 21:44:26, sky wrote:
>
http://codereview.chromium.org/8533019/diff/1/ui/gfx/compositor/layer_unittes...
> File ui/gfx/compositor/layer_unittest.cc (right):
> 
>
http://codereview.chromium.org/8533019/diff/1/ui/gfx/compositor/layer_unittes...
> ui/gfx/compositor/layer_unittest.cc:32: int bytes_written =
> file_util::WriteFile(
> nit: make this one statement,
> return file_util::WriteFile(..) == static_cast<int>(png_data.size());
Done.
> 
>
http://codereview.chromium.org/8533019/diff/1/ui/gfx/compositor/layer_unittes...
> ui/gfx/compositor/layer_unittest.cc:140: bool IsSameAsPNGFile(const SkBitmap&
> gen_bmp, const char* file) {
> Document what file is relative to.
Now takes an absolute path.
> 
>
http://codereview.chromium.org/8533019/diff/1/ui/gfx/compositor/layer_unittes...
> ui/gfx/compositor/layer_unittest.cc:146: should_compare = false;
> Is there any point on continuing on if you can't read the file? Maybe all
these
> should be ASSERTs?
The function now bails early if there's no point continuing.
> 
>
http://codereview.chromium.org/8533019/diff/1/ui/gfx/compositor/layer_unittes...
> ui/gfx/compositor/layer_unittest.cc:157: should_compare = false;
> Return here too?
Done.

[sky, 2011-11-15 05:09:48]

LGTM

[jonathan.backer, 2011-11-15 12:47:47]

LGTM