cc: Add finch feature flag for checker-imaging.

cc: Add finch feature flag for checker-imaging.

Add a base::Feature entry for checker-imaging finch trial and report
the status on chrome://gpu

BUG=725351
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2902773004
Cr-Commit-Position: refs/heads/master@{#474880}
Committed: https://chromium.googlesource.com/chromium/src/+/ab4ae0f9d8efe37e7357c8ada6c84b86b56cd1bd

[Khushal, 2017-05-24 01:02:00]

Description was changed from

==========
cc: Add finch feature flag for checker-imaging.

Add a base::Feature entry for checker-imaging finch trial and report
the status on chrome://gpu

BUG=725351
==========

to

==========
cc: Add finch feature flag for checker-imaging.

Add a base::Feature entry for checker-imaging finch trial and report
the status on chrome://gpu

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

[Khushal, 2017-05-24 02:10:48]

khushalsagar@chromium.org changed reviewers:
+ enne@chromium.org, piman@chromium.org

[piman, 2017-05-24 18:59:11]

https://codereview.chromium.org/2902773004/diff/1/cc/base/features.h
File cc/base/features.h (right):

https://codereview.chromium.org/2902773004/diff/1/cc/base/features.h#newcode14
cc/base/features.h:14: CC_BASE_EXPORT extern const base::Feature
kEnableCheckerImaging;
Is this something that belongs in CC - that is, is it something that we would
turn on/off at the same time in every single compositor in existence (in
particular, UI vs renderer compositor). Typically these kind of features are
not, and so they are activated via the LayerTreeSettings.

In particular for this one, I don't think it ever makes sense to checker images
in the UI, so I think we should make it a LayerTreeSetting and the feature would
belong in content, and apply to the renderer only.

[Khushal, 2017-05-24 20:45:07]

https://codereview.chromium.org/2902773004/diff/1/cc/base/features.h
File cc/base/features.h (right):

https://codereview.chromium.org/2902773004/diff/1/cc/base/features.h#newcode14
cc/base/features.h:14: CC_BASE_EXPORT extern const base::Feature
kEnableCheckerImaging;
On 2017/05/24 18:59:11, piman wrote:
> Is this something that belongs in CC - that is, is it something that we would
> turn on/off at the same time in every single compositor in existence (in
> particular, UI vs renderer compositor). Typically these kind of features are
> not, and so they are activated via the LayerTreeSettings.

Yeah, its only for the renderer compositor. And the activation would happen
using layer tree settings in RenderWidgetCompositor. That's how its being done
already. I just changed it so we plumb it to the renderer based on the finch
config using compositor command line flags appended in RenderProcessHostImpl.

I wasn't sure where I should put the finch feature. Moved it to content_features
since that makes more sense.

> 
> In particular for this one, I don't think it ever makes sense to checker
images
> in the UI, so I think we should make it a LayerTreeSetting and the feature
would
> belong in content, and apply to the renderer only.

[enne (OOO), 2017-05-24 21:11:52]

I'm not an owner of any of this anymore, but non-owner lgtm.  Thanks for moving
the feature out of cc; definitely agreed that it makes more sense to have it in
content.

[piman, 2017-05-24 21:12:24]

LGTM, thanks!

[Khushal, 2017-05-24 21:14:34]

The CQ bit was checked by khushalsagar@chromium.org

[commit-bot: I haz the power, 2017-05-24 21:15:05]

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-24 22:54:54]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-24 22:54:56]

Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Khushal, 2017-05-24 23:19:03]

The CQ bit was checked by khushalsagar@chromium.org

[commit-bot: I haz the power, 2017-05-24 23:19:52]

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-25 00:38:57]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-25 00:38:58]

Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Khushal, 2017-05-25 03:29:54]

Description was changed from

==========
cc: Add finch feature flag for checker-imaging.

Add a base::Feature entry for checker-imaging finch trial and report
the status on chrome://gpu

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

to

==========
cc: Add finch feature flag for checker-imaging.

Add a base::Feature entry for checker-imaging finch trial and report
the status on chrome://gpu

BUG=725351
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[Khushal, 2017-05-25 03:30:23]

The CQ bit was checked by khushalsagar@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-25 03:31:03]

Dry run: 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-25 05:08:28]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-25 05:08:28]

Dry run: This issue passed the CQ dry run.

[Khushal, 2017-05-25 21:15:44]

https://codereview.chromium.org/2902773004/diff/40001/content/test/gpu/gpu_te...
File content/test/gpu/gpu_tests/gpu_process_integration_test.py (right):

https://codereview.chromium.org/2902773004/diff/40001/content/test/gpu/gpu_te...
content/test/gpu/gpu_tests/gpu_process_integration_test.py:357: elif name ==
'checker_imaging':
Had to update this test. I wasn't sure if I should be hard-coding a status here.
Does this look okay?

[piman, 2017-05-25 23:24:32]

lgtm+nit

https://codereview.chromium.org/2902773004/diff/40001/content/test/gpu/gpu_te...
File content/test/gpu/gpu_tests/gpu_process_integration_test.py (right):

https://codereview.chromium.org/2902773004/diff/40001/content/test/gpu/gpu_te...
content/test/gpu/gpu_tests/gpu_process_integration_test.py:357: elif name ==
'checker_imaging':
On 2017/05/25 21:15:44, Khushal wrote:
> Had to update this test. I wasn't sure if I should be hard-coding a status
here.
> Does this look okay?

I'd just do the python statement 'pass' rather than the redundant 'result =
result'

Other than that, I think this is fine in that the test shouldn't depend on the
status of this.

[Khushal, 2017-05-25 23:36:45]

https://codereview.chromium.org/2902773004/diff/40001/content/test/gpu/gpu_te...
File content/test/gpu/gpu_tests/gpu_process_integration_test.py (right):

https://codereview.chromium.org/2902773004/diff/40001/content/test/gpu/gpu_te...
content/test/gpu/gpu_tests/gpu_process_integration_test.py:357: elif name ==
'checker_imaging':
On 2017/05/25 23:24:32, piman wrote:
> On 2017/05/25 21:15:44, Khushal wrote:
> > Had to update this test. I wasn't sure if I should be hard-coding a status
> here.
> > Does this look okay?
> 
> I'd just do the python statement 'pass' rather than the redundant 'result =
> result'
> 
> Other than that, I think this is fine in that the test shouldn't depend on the
> status of this.

Done. Thanks!

[Khushal, 2017-05-25 23:36:49]

The CQ bit was checked by khushalsagar@chromium.org

[Khushal, 2017-05-25 23:36:50]

The patchset sent to the CQ was uploaded after l-g-t-m from enne@chromium.org,
piman@chromium.org
Link to the patchset: https://codereview.chromium.org/2902773004/#ps60001
(title: "pass")

[commit-bot: I haz the power, 2017-05-25 23:37:31]

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-26 02:13:08]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1495755409878560,
"parent_rev": "cfcf2ce3d89129e3b262036d58e91a53cb3cf649", "commit_rev":
"ab4ae0f9d8efe37e7357c8ada6c84b86b56cd1bd"}

[commit-bot: I haz the power, 2017-05-26 02:13:23]

Description was changed from

==========
cc: Add finch feature flag for checker-imaging.

Add a base::Feature entry for checker-imaging finch trial and report
the status on chrome://gpu

BUG=725351
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
cc: Add finch feature flag for checker-imaging.

Add a base::Feature entry for checker-imaging finch trial and report
the status on chrome://gpu

BUG=725351
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2902773004
Cr-Commit-Position: refs/heads/master@{#474880}
Committed:
https://chromium.googlesource.com/chromium/src/+/ab4ae0f9d8efe37e7357c8ada6c8...
==========

[commit-bot: I haz the power, 2017-05-26 02:13:25]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/ab4ae0f9d8efe37e7357c8ada6c8...