cc: cc pixel tests test both single/multi texture mask code paths.

cc: cc pixel tests test both single/multi texture mask code paths.

We want to test both code paths that tile or not tile any mask in cc
pixel tests.

BUG=708582
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2852083002
Cr-Commit-Position: refs/heads/master@{#468744}
Committed: https://chromium.googlesource.com/chromium/src/+/60e148f1da0c27818ec63406b731ca686393440c

[sunxd, 2017-05-01 15:02:27]

Description was changed from

==========
cc: Add single texture mask version of cc pixel tests.

We want to test both code paths that tile or not tile any mask in cc
pixel tests.

BUG=708582
==========

to

==========
cc: Add single texture mask version of cc pixel tests.

We want to test both code paths that tile or not tile any mask in cc
pixel tests.

BUG=708582
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

[sunxd, 2017-05-01 15:03:05]

Description was changed from

==========
cc: Add single texture mask version of cc pixel tests.

We want to test both code paths that tile or not tile any mask in cc
pixel tests.

BUG=708582
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

to

==========
cc: Add single texture mask version of cc pixel tests.

We want to test both code paths that tile or not tile any mask in cc
pixel tests.

BUG=708582
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

[sunxd, 2017-05-01 15:03:05]

sunxd@chromium.org changed reviewers:
+ enne@chromium.org, trchen@chromium.org

[sunxd, 2017-05-01 15:26:23]

PTAL.

I do not feel very comfortable with duplicating these tests. Do you have any
suggestion what I can do with these parameterized tests?

[trchen, 2017-05-01 21:09:21]

On 2017/05/01 15:26:23, sunxd wrote:
> PTAL.
> 
> I do not feel very comfortable with duplicating these tests. Do you have any
> suggestion what I can do with these parameterized tests?

Looks like only LayerTreeHostMasksPixelTest is using
ParameterizedPixelResourceTest and INSTANTIATE_PIXEL_RESOURCE_TEST_CASE_P.

How about just move those from layer_tree_pixel_resource_test.h to
layer_tree_host_pixeltest_masks.cc? Then we can extend the test fixture to take
two parameters.

[enne (OOO), 2017-05-01 21:49:13]

On 2017/05/01 at 21:09:21, trchen wrote:
> On 2017/05/01 15:26:23, sunxd wrote:
> > PTAL.
> > 
> > I do not feel very comfortable with duplicating these tests. Do you have any
> > suggestion what I can do with these parameterized tests?
> 
> Looks like only LayerTreeHostMasksPixelTest is using
ParameterizedPixelResourceTest and INSTANTIATE_PIXEL_RESOURCE_TEST_CASE_P.
> 
> How about just move those from layer_tree_pixel_resource_test.h to
layer_tree_host_pixeltest_masks.cc? Then we can extend the test fixture to take
two parameters.

Yeah, I think the intention was that other tests would use it, but none really
did.  I think as long as one set of tests is testing all the resource types,
that's probably sufficient.  So, it's fine to move it.

I was going to suggest something similar, to have the test take a tuple
parameter and use testing::Combine to get the combination of mask types and
resource types instead of duplicating the tests.

[sunxd, 2017-05-02 17:58:57]

Description was changed from

==========
cc: Add single texture mask version of cc pixel tests.

We want to test both code paths that tile or not tile any mask in cc
pixel tests.

BUG=708582
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

to

==========
cc: cc pixel tests test both single/multi texture mask code paths.

We want to test both code paths that tile or not tile any mask in cc
pixel tests.

BUG=708582
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

[sunxd, 2017-05-02 18:00:18]

PTAL.

I made the change in layer_tree_pixel_resource_test. I added mask type as an
extra parameters because the old parameters only make sense in certain cases.

[enne (OOO), 2017-05-02 18:12:12]

Thanks!! lgtm

[sunxd, 2017-05-02 18:29:10]

The CQ bit was checked by sunxd@chromium.org

[commit-bot: I haz the power, 2017-05-02 18:29:43]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-02 19:33:25]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1493749750549160,
"parent_rev": "2c0eac854c5dde7f389ac60adc2acca1245bef94", "commit_rev":
"60e148f1da0c27818ec63406b731ca686393440c"}

[commit-bot: I haz the power, 2017-05-02 19:33:38]

Description was changed from

==========
cc: cc pixel tests test both single/multi texture mask code paths.

We want to test both code paths that tile or not tile any mask in cc
pixel tests.

BUG=708582
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

to

==========
cc: cc pixel tests test both single/multi texture mask code paths.

We want to test both code paths that tile or not tile any mask in cc
pixel tests.

BUG=708582
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2852083002
Cr-Commit-Position: refs/heads/master@{#468744}
Committed:
https://chromium.googlesource.com/chromium/src/+/60e148f1da0c27818ec63406b731...
==========

[commit-bot: I haz the power, 2017-05-02 19:33:39]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/60e148f1da0c27818ec63406b731...