Make using virtual contexts a workaround flag rather than cmdline

Make using virtual contexts a workaround flag rather than cmdline

This removes the commandline used on Android and adds references
to each particular bug in the json workaround list.

BUG=280609
R=piman@chromium.org, zmo@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=221832

[no sievers, 2013-09-06 21:16:14]

ptal

[piman, 2013-09-06 21:21:50]

I'm supportive of this change :)
One thing though...

https://codereview.chromium.org/23503038/diff/1/content/browser/gpu/gpu_data_...
File content/browser/gpu/gpu_data_manager_impl_private.cc (left):

https://codereview.chromium.org/23503038/diff/1/content/browser/gpu/gpu_data_...
content/browser/gpu/gpu_data_manager_impl_private.cc:288: ((is_nvidia ||
is_qualcomm) && sdk_int < 18) || is_broadcom) {
You're losing the sdk_int < 18 selector in the json. Something that should be
added? Otherwise this will broaden the use of virtual contexts.

[no sievers, 2013-09-06 21:22:57]

On 2013/09/06 21:21:50, piman wrote:
> I'm supportive of this change :)
> One thing though...
> 
>
https://codereview.chromium.org/23503038/diff/1/content/browser/gpu/gpu_data_...
> File content/browser/gpu/gpu_data_manager_impl_private.cc (left):
> 
>
https://codereview.chromium.org/23503038/diff/1/content/browser/gpu/gpu_data_...
> content/browser/gpu/gpu_data_manager_impl_private.cc:288: ((is_nvidia ||
> is_qualcomm) && sdk_int < 18) || is_broadcom) {
> You're losing the sdk_int < 18 selector in the json. Something that should be
> added? Otherwise this will broaden the use of virtual contexts.

Argh, thanks. I knew I forgot something :)

[no sievers, 2013-09-06 21:26:45]

On 2013/09/06 21:22:57, sievers wrote:
> On 2013/09/06 21:21:50, piman wrote:
> > I'm supportive of this change :)
> > One thing though...
> > 
> >
>
https://codereview.chromium.org/23503038/diff/1/content/browser/gpu/gpu_data_...
> > File content/browser/gpu/gpu_data_manager_impl_private.cc (left):
> > 
> >
>
https://codereview.chromium.org/23503038/diff/1/content/browser/gpu/gpu_data_...
> > content/browser/gpu/gpu_data_manager_impl_private.cc:288: ((is_nvidia ||
> > is_qualcomm) && sdk_int < 18) || is_broadcom) {
> > You're losing the sdk_int < 18 selector in the json. Something that should
be
> > added? Otherwise this will broaden the use of virtual contexts.
> 
> Argh, thanks. I knew I forgot something :)

Done.

[piman, 2013-09-06 21:30:48]

lgtm

[Zhenyao Mo, 2013-09-06 21:49:09]

LGTM

https://codereview.chromium.org/23503038/diff/8001/content/common/gpu/gpu_com...
File content/common/gpu/gpu_command_buffer_stub.cc (right):

https://codereview.chromium.org/23503038/diff/8001/content/common/gpu/gpu_com...
content/common/gpu/gpu_command_buffer_stub.cc:155: use_virtualized_gl_context_ =
context_group_->feature_info()
nit: could just be "|="

[no sievers, 2013-09-06 22:07:34]

On 2013/09/06 21:49:09, Zhenyao Mo wrote:
> LGTM
> 
>
https://codereview.chromium.org/23503038/diff/8001/content/common/gpu/gpu_com...
> File content/common/gpu/gpu_command_buffer_stub.cc (right):
> 
>
https://codereview.chromium.org/23503038/diff/8001/content/common/gpu/gpu_com...
> content/common/gpu/gpu_command_buffer_stub.cc:155: use_virtualized_gl_context_
=
> context_group_->feature_info()
> nit: could just be "|="

Done.

The json list is pretty touchy when it comes to missing commas and such. It will
silently fail with all workarounds disabled.
Can we DCHECK() somewhere that it parsed it correctly?

[commit-bot: I haz the power, 2013-09-06 22:07:58]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sievers@chromium.org/23503038/11001

[Zhenyao Mo, 2013-09-06 22:19:25]

On 2013/09/06 22:07:34, sievers wrote:
> On 2013/09/06 21:49:09, Zhenyao Mo wrote:
> > LGTM
> > 
> >
>
https://codereview.chromium.org/23503038/diff/8001/content/common/gpu/gpu_com...
> > File content/common/gpu/gpu_command_buffer_stub.cc (right):
> > 
> >
>
https://codereview.chromium.org/23503038/diff/8001/content/common/gpu/gpu_com...
> > content/common/gpu/gpu_command_buffer_stub.cc:155:
use_virtualized_gl_context_
> =
> > context_group_->feature_info()
> > nit: could just be "|="
> 
> Done.
> 
> The json list is pretty touchy when it comes to missing commas and such. It
will
> silently fail with all workarounds disabled.
> Can we DCHECK() somewhere that it parsed it correctly?

We have a unit test in gpu_unittests to catch this issue.

But I am thinking to allow missing commas in json processing - it really
shouldn't matter.

[no sievers, 2013-09-06 22:22:32]

On 2013/09/06 22:19:25, Zhenyao Mo wrote:
> On 2013/09/06 22:07:34, sievers wrote:
> > On 2013/09/06 21:49:09, Zhenyao Mo wrote:
> > > LGTM
> > > 
> > >
> >
>
https://codereview.chromium.org/23503038/diff/8001/content/common/gpu/gpu_com...
> > > File content/common/gpu/gpu_command_buffer_stub.cc (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/23503038/diff/8001/content/common/gpu/gpu_com...
> > > content/common/gpu/gpu_command_buffer_stub.cc:155:
> use_virtualized_gl_context_
> > =
> > > context_group_->feature_info()
> > > nit: could just be "|="
> > 
> > Done.
> > 
> > The json list is pretty touchy when it comes to missing commas and such. It
> will
> > silently fail with all workarounds disabled.
> > Can we DCHECK() somewhere that it parsed it correctly?
> 
> We have a unit test in gpu_unittests to catch this issue.

Yes, my bad. I had run it, but right before I introduced the missing comma.
I think the unit test does actually catch it.

> 
> But I am thinking to allow missing commas in json processing - it really
> shouldn't matter.

[no sievers, 2013-09-07 00:24:39]

Committed patchset #5 manually as r221832 (presubmit successful).