Remove content switches dependency in content/common/gpu.

Remove content switches dependency in content/common/gpu.

BUG=590825
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel

Committed: https://crrev.com/ea5b740f5c57b1f5218f062106cb68a8660c9e5f
Cr-Commit-Position: refs/heads/master@{#381874}

[Peng, 2016-03-15 19:50:46]

Description was changed from

==========
Remove content switches dependency in content/common/gpu.

BUG=590825
==========

to

==========
Remove content switches dependency in content/common/gpu.

BUG=590825
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel
==========

[Peng, 2016-03-15 19:54:08]

penghuang@chromium.org changed reviewers:
+ piman@chromium.org

[Peng, 2016-03-15 19:54:09]

Hi piman, PTAL. Thanks.

[Peng, 2016-03-16 20:27:04]

On 2016/03/15 19:54:09, Peng wrote:
> Hi piman, PTAL. Thanks.

kindly ping.

[piman, 2016-03-17 00:17:25]

https://codereview.chromium.org/1805133002/diff/1/content/common/gpu/media/vi...
File content/common/gpu/media/video_decode_accelerator_unittest.cc (left):

https://codereview.chromium.org/1805133002/diff/1/content/common/gpu/media/vi...
content/common/gpu/media/video_decode_accelerator_unittest.cc:526:
cmd_line->HasSwitch(switches::kEnableAcceleratedVpxDecode);
Aren't we removing functionality here?

[Peng, 2016-03-17 14:22:31]

https://codereview.chromium.org/1805133002/diff/1/content/common/gpu/media/vi...
File content/common/gpu/media/video_decode_accelerator_unittest.cc (left):

https://codereview.chromium.org/1805133002/diff/1/content/common/gpu/media/vi...
content/common/gpu/media/video_decode_accelerator_unittest.cc:526:
cmd_line->HasSwitch(switches::kEnableAcceleratedVpxDecode);
On 2016/03/17 00:17:25, piman wrote:
> Aren't we removing functionality here?

I didn't find any code sets this flag in code search. Probably it is fine.

[Peng, 2016-03-17 15:00:44]

https://codereview.chromium.org/1805133002/diff/1/content/common/gpu/media/vi...
File content/common/gpu/media/video_decode_accelerator_unittest.cc (left):

https://codereview.chromium.org/1805133002/diff/1/content/common/gpu/media/vi...
content/common/gpu/media/video_decode_accelerator_unittest.cc:526:
cmd_line->HasSwitch(switches::kEnableAcceleratedVpxDecode);
On 2016/03/17 14:22:31, Peng wrote:
> On 2016/03/17 00:17:25, piman wrote:
> > Aren't we removing functionality here?
> 
> I didn't find any code sets this flag in code search. Probably it is fine.

Or we could define a duplicated const string variable in this file, what do you
think?

[piman, 2016-03-17 17:40:08]

piman@chromium.org changed reviewers:
+ dalecurtis@chromium.org

[piman, 2016-03-17 17:40:09]

+dalecurtis who added this flag. Is this still used?

[DaleCurtis, 2016-03-17 17:43:21]

dalecurtis@chromium.org changed reviewers:
+ ananta@chromium.org

[DaleCurtis, 2016-03-17 17:43:22]

https://codereview.chromium.org/1805133002/diff/1/content/common/gpu/media/vi...
File content/common/gpu/media/video_decode_accelerator_unittest.cc (left):

https://codereview.chromium.org/1805133002/diff/1/content/common/gpu/media/vi...
content/common/gpu/media/video_decode_accelerator_unittest.cc:526:
cmd_line->HasSwitch(switches::kEnableAcceleratedVpxDecode);
On 2016/03/17 at 15:00:43, Peng wrote:
> On 2016/03/17 14:22:31, Peng wrote:
> > On 2016/03/17 00:17:25, piman wrote:
> > > Aren't we removing functionality here?
> > 
> > I didn't find any code sets this flag in code search. Probably it is fine.
> 
> Or we could define a duplicated const string variable in this file, what do
you think?

This was kept here to allow Intel and other manufacturers who are experimenting
with hardware VP8/VP9 decode to test their implementations. If we remove this,
it should be part of a larger cleanup to delete all of this code in favor of a
DXVA2 based decoder (since manufacturers seem to be leaning towards that instead
of MFT variants). +ananta

[DaleCurtis, 2016-03-17 18:42:18]

lgtm for deletion from the DXVA unittest unless ananta@ objects.

https://codereview.chromium.org/1805133002/diff/1/content/common/gpu/media/vi...
File content/common/gpu/media/video_decode_accelerator_unittest.cc (left):

https://codereview.chromium.org/1805133002/diff/1/content/common/gpu/media/vi...
content/common/gpu/media/video_decode_accelerator_unittest.cc:526:
cmd_line->HasSwitch(switches::kEnableAcceleratedVpxDecode);
On 2016/03/17 at 17:43:22, DaleCurtis wrote:
> On 2016/03/17 at 15:00:43, Peng wrote:
> > On 2016/03/17 14:22:31, Peng wrote:
> > > On 2016/03/17 00:17:25, piman wrote:
> > > > Aren't we removing functionality here?
> > > 
> > > I didn't find any code sets this flag in code search. Probably it is fine.
> > 
> > Or we could define a duplicated const string variable in this file, what do
you think?
> 
> This was kept here to allow Intel and other manufacturers who are
experimenting with hardware VP8/VP9 decode to test their implementations. If we
remove this, it should be part of a larger cleanup to delete all of this code in
favor of a DXVA2 based decoder (since manufacturers seem to be leaning towards
that instead of MFT variants). +ananta

Whoops, just saw this is in the unittest. It's probably fine to delete this
here.

[piman, 2016-03-17 18:47:52]

LGTM if dalecurtis/ananta are happy

[ananta, 2016-03-17 22:47:09]

lgtm

[Peng, 2016-03-17 23:35:47]

The CQ bit was checked by penghuang@chromium.org

[Peng, 2016-03-17 23:35:47]

The patchset sent to the CQ was uploaded after l-g-t-m from
dalecurtis@chromium.org
Link to the patchset: https://codereview.chromium.org/1805133002/#ps20001
(title: "Update")

[commit-bot: I haz the power, 2016-03-17 23:36:25]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1805133002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1805133002/20001

[commit-bot: I haz the power, 2016-03-18 01:30:48]

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

[commit-bot: I haz the power, 2016-03-18 01:30:49]

Try jobs failed on following builders:
  win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[Peng, 2016-03-18 01:31:55]

The CQ bit was checked by penghuang@chromium.org

[commit-bot: I haz the power, 2016-03-18 01:32:18]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1805133002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1805133002/20001

[Pawel Osciak, 2016-03-18 01:56:56]

posciak@chromium.org changed reviewers:
+ posciak@chromium.org

[Pawel Osciak, 2016-03-18 01:56:57]

https://codereview.chromium.org/1805133002/diff/20001/content/common/gpu/medi...
File content/common/gpu/media/video_decode_accelerator_unittest.cc (right):

https://codereview.chromium.org/1805133002/diff/20001/content/common/gpu/medi...
content/common/gpu/media/video_decode_accelerator_unittest.cc:523: const bool
enable_accelerated_vpx_decode = false;
Removing this may prevent us from testing this feature I believe. I'm not the
owner of DXVA, but this is perhaps needed?

[commit-bot: I haz the power, 2016-03-18 02:25:29]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-03-18 02:26:37]

Description was changed from

==========
Remove content switches dependency in content/common/gpu.

BUG=590825
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel
==========

to

==========
Remove content switches dependency in content/common/gpu.

BUG=590825
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel

Committed: https://crrev.com/ea5b740f5c57b1f5218f062106cb68a8660c9e5f
Cr-Commit-Position: refs/heads/master@{#381874}
==========

[commit-bot: I haz the power, 2016-03-18 02:26:38]

Patchset 2 (id:??) landed as
https://crrev.com/ea5b740f5c57b1f5218f062106cb68a8660c9e5f
Cr-Commit-Position: refs/heads/master@{#381874}

[Peng, 2016-03-18 15:16:44]

https://codereview.chromium.org/1805133002/diff/20001/content/common/gpu/medi...
File content/common/gpu/media/video_decode_accelerator_unittest.cc (right):

https://codereview.chromium.org/1805133002/diff/20001/content/common/gpu/medi...
content/common/gpu/media/video_decode_accelerator_unittest.cc:523: const bool
enable_accelerated_vpx_decode = false;
On 2016/03/18 01:56:57, Pawel Osciak wrote:
> Removing this may prevent us from testing this feature I believe. I'm not the
> owner of DXVA, but this is perhaps needed?

Do we need pass the --enable-accelerated-vpx-decode to this unit test manually?
If not, I think the change is OK. I think it is not hard to test DXVAVDA decoder
with both vpx enabled and disabled. Just need make the
enable_accelerated_vpx_decode as an argument, and call this function twice with
different bool values. But seems we are not testing the DXVAV decoder with vpx
enabled right now, so I didn't do it. But we could do it when it is needed.
Thought?