gpu: Use shader cache on ES3

gpu: Use shader cache on ES3

The ES3 core specs supports gl(Get)ProgramBinary().
BUG=510172
Committed: https://crrev.com/809d9b979c0edd3976132eaaf77fc4740fcee019
Cr-Commit-Position: refs/heads/master@{#339369}

[no sievers, 2015-07-17 23:25:27]

sievers@chromium.org changed reviewers:
+ dyen@chromium.org, zmo@chromium.org

[no sievers, 2015-07-17 23:25:35]

ptal

[Zhenyao Mo, 2015-07-17 23:34:04]

LGTM

https://codereview.chromium.org/1223393003/diff/20001/content/common/gpu/gpu_...
File content/common/gpu/gpu_channel_manager.cc (right):

https://codereview.chromium.org/1223393003/diff/20001/content/common/gpu/gpu_...
content/common/gpu/gpu_channel_manager.cc:73: gfx::GetGLVersionInfo()->is_es3)
&&
I feel somehow this decision should be made in command buffer and propagate
here, but let's land this for now.

[no sievers, 2015-07-17 23:39:10]

https://codereview.chromium.org/1223393003/diff/20001/content/common/gpu/gpu_...
File content/common/gpu/gpu_channel_manager.cc (right):

https://codereview.chromium.org/1223393003/diff/20001/content/common/gpu/gpu_...
content/common/gpu/gpu_channel_manager.cc:73: gfx::GetGLVersionInfo()->is_es3)
&&
On 2015/07/17 23:34:04, Zhenyao Mo wrote:
> I feel somehow this decision should be made in command buffer and propagate
> here, but let's land this for now.

Yes. I think it just exists this way using the static checks because we don't
have a context, or necessarily even a decoder (and which one should we look
at?). And we want to init the cache early so that it can receive the shaders
from the browser from disk.

[no sievers, 2015-07-17 23:42:25]

On 2015/07/17 23:39:10, sievers wrote:
>
https://codereview.chromium.org/1223393003/diff/20001/content/common/gpu/gpu_...
> File content/common/gpu/gpu_channel_manager.cc (right):
> 
>
https://codereview.chromium.org/1223393003/diff/20001/content/common/gpu/gpu_...
> content/common/gpu/gpu_channel_manager.cc:73: gfx::GetGLVersionInfo()->is_es3)
> &&
> On 2015/07/17 23:34:04, Zhenyao Mo wrote:
> > I feel somehow this decision should be made in command buffer and propagate
> > here, but let's land this for now.
> 
> Yes. I think it just exists this way using the static checks because we don't
> have a context, or necessarily even a decoder (and which one should we look
> at?). And we want to init the cache early so that it can receive the shaders
> from the browser from disk.

Maybe we could slightly refactor it so that the cache always gets created (if
needed), but the decision to use it is driven from the context (and extensions
etc.), similar to the workaround flag I added (disable_shader_cache).

I'm not sure how important the cmdline to disable the cache entirely is
otherwise. But we would then also at least avoid storing shaders if the cache is
disabled, but we'd still be populating the cache from disk.

[Zhenyao Mo, 2015-07-17 23:57:15]

On 2015/07/17 23:42:25, sievers wrote:
> On 2015/07/17 23:39:10, sievers wrote:
> >
>
https://codereview.chromium.org/1223393003/diff/20001/content/common/gpu/gpu_...
> > File content/common/gpu/gpu_channel_manager.cc (right):
> > 
> >
>
https://codereview.chromium.org/1223393003/diff/20001/content/common/gpu/gpu_...
> > content/common/gpu/gpu_channel_manager.cc:73:
gfx::GetGLVersionInfo()->is_es3)
> > &&
> > On 2015/07/17 23:34:04, Zhenyao Mo wrote:
> > > I feel somehow this decision should be made in command buffer and
propagate
> > > here, but let's land this for now.
> > 
> > Yes. I think it just exists this way using the static checks because we
don't
> > have a context, or necessarily even a decoder (and which one should we look
> > at?). And we want to init the cache early so that it can receive the shaders
> > from the browser from disk.
> 
> Maybe we could slightly refactor it so that the cache always gets created (if
> needed), but the decision to use it is driven from the context (and extensions
> etc.), similar to the workaround flag I added (disable_shader_cache).
> 
> I'm not sure how important the cmdline to disable the cache entirely is
> otherwise. But we would then also at least avoid storing shaders if the cache
is
> disabled, but we'd still be populating the cache from disk.

I think as far as we don't use the cache, it still obeys disable_shader_cache.

The CL is good as it.

[Zhenyao Mo, 2015-07-17 23:57:25]

On 2015/07/17 23:57:15, Zhenyao Mo wrote:
> On 2015/07/17 23:42:25, sievers wrote:
> > On 2015/07/17 23:39:10, sievers wrote:
> > >
> >
>
https://codereview.chromium.org/1223393003/diff/20001/content/common/gpu/gpu_...
> > > File content/common/gpu/gpu_channel_manager.cc (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/1223393003/diff/20001/content/common/gpu/gpu_...
> > > content/common/gpu/gpu_channel_manager.cc:73:
> gfx::GetGLVersionInfo()->is_es3)
> > > &&
> > > On 2015/07/17 23:34:04, Zhenyao Mo wrote:
> > > > I feel somehow this decision should be made in command buffer and
> propagate
> > > > here, but let's land this for now.
> > > 
> > > Yes. I think it just exists this way using the static checks because we
> don't
> > > have a context, or necessarily even a decoder (and which one should we
look
> > > at?). And we want to init the cache early so that it can receive the
shaders
> > > from the browser from disk.
> > 
> > Maybe we could slightly refactor it so that the cache always gets created
(if
> > needed), but the decision to use it is driven from the context (and
extensions
> > etc.), similar to the workaround flag I added (disable_shader_cache).
> > 
> > I'm not sure how important the cmdline to disable the cache entirely is
> > otherwise. But we would then also at least avoid storing shaders if the
cache
> is
> > disabled, but we'd still be populating the cache from disk.
> 
> I think as far as we don't use the cache, it still obeys disable_shader_cache.
> 
> The CL is good as it.

I mean as is.

[no sievers, 2015-07-18 00:15:43]

The CQ bit was checked by sievers@chromium.org

[commit-bot: I haz the power, 2015-07-18 00:16:30]

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

[commit-bot: I haz the power, 2015-07-18 01:14:34]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-07-18 01:15:24]

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

[no sievers, 2015-07-22 00:46:03]

A revert of this CL (patchset #2 id:20001) has been created in
https://codereview.chromium.org/1247233002/ by sievers@chromium.org.

The reason for reverting is: This codepath is actually flaky.

OnLoadedShader() races with dynamic GL binding initialization. Previously it
meant that we just might not end up loading this shader (as long as
b_GL_OES_get_program_binary etc. default to false), but the GetGLVersionInfo()
might actually crash..