Added disabling EGL extensions support with --disable-extensions.

Added disabling EGL extensions support with --disable-extensions.

R=sievers@chromium.org, zmo@chromium.org
BUG=482067
Committed: https://crrev.com/9deab29ad0b594c8a308f78913923d65ceb7b93f
Cr-Commit-Position: refs/heads/master@{#333602}

[David Yen, 2015-05-27 23:17:04]

On 2015/05/27 23:15:38, David Yen wrote:

I've implemented disabling EGL extensions almost exactly the same as the GL
extensions. The only thing I was a bit unsure of is whether I need to check if
the display matches when eglQueryString is called, do I need to verify that it's
the same display? or possibly cache it per display?

[no sievers, 2015-05-28 19:25:11]

https://codereview.chromium.org/1151093006/diff/20001/content/browser/gpu/gpu...
File content/browser/gpu/gpu_process_host.cc (right):

https://codereview.chromium.org/1151093006/diff/20001/content/browser/gpu/gpu...
content/browser/gpu/gpu_process_host.cc:80: switches::kDisableExtensions,
I wouldn't use this name since extensions means something else in the more
general context of Chrome.
Can we just leave the current name and live with it being a bit imprecise?

https://codereview.chromium.org/1151093006/diff/20001/ui/gl/BUILD.gn
File ui/gl/BUILD.gn (right):

https://codereview.chromium.org/1151093006/diff/20001/ui/gl/BUILD.gn#newcode293
ui/gl/BUILD.gn:293: include_dirs = [ "//third_party/khronos" ]
Can you just generally add this (outside the if-block) for gl_unittests? Same
for .gyp

https://codereview.chromium.org/1151093006/diff/20001/ui/gl/egl_api_unittest.cc
File ui/gl/egl_api_unittest.cc (right):

https://codereview.chromium.org/1151093006/diff/20001/ui/gl/egl_api_unittest....
ui/gl/egl_api_unittest.cc:88: }  // namespace gpu
nit: s/gpu/gfx

https://codereview.chromium.org/1151093006/diff/20001/ui/gl/gl_egl_api_implem...
File ui/gl/gl_egl_api_implementation.cc (right):

https://codereview.chromium.org/1151093006/diff/20001/ui/gl/gl_egl_api_implem...
ui/gl/gl_egl_api_implementation.cc:26: void
InitializeDynamicGLBindingsEGL(GLContext* context) {
I had actually previously removed the notion of initializing dynamic bindings
for EGL etc. :)

You don't need a context to call eglQueryString, can you do it from the end of
InitializeStaticGLBindingsEGL() instead?

https://codereview.chromium.org/1151093006/diff/20001/ui/gl/gl_egl_api_implem...
ui/gl/gl_egl_api_implementation.cc:98: const char* egl_extensions =
EGLApiBase::eglQueryStringFn(display,
You can call DriverEGL::GetPlatformExtensions() and
DriverEGL::GetClientExtensions() here (use the combined string). That takes care
of the display.

[David Yen, 2015-06-05 23:59:51]

https://codereview.chromium.org/1151093006/diff/20001/content/browser/gpu/gpu...
File content/browser/gpu/gpu_process_host.cc (right):

https://codereview.chromium.org/1151093006/diff/20001/content/browser/gpu/gpu...
content/browser/gpu/gpu_process_host.cc:80: switches::kDisableExtensions,
On 2015/05/28 19:25:10, sievers wrote:
> I wouldn't use this name since extensions means something else in the more
> general context of Chrome.
> Can we just leave the current name and live with it being a bit imprecise?

Done.

https://codereview.chromium.org/1151093006/diff/20001/ui/gl/BUILD.gn
File ui/gl/BUILD.gn (right):

https://codereview.chromium.org/1151093006/diff/20001/ui/gl/BUILD.gn#newcode293
ui/gl/BUILD.gn:293: include_dirs = [ "//third_party/khronos" ]
On 2015/05/28 19:25:10, sievers wrote:
> Can you just generally add this (outside the if-block) for gl_unittests? Same
> for .gyp

Done.

https://codereview.chromium.org/1151093006/diff/20001/ui/gl/egl_api_unittest.cc
File ui/gl/egl_api_unittest.cc (right):

https://codereview.chromium.org/1151093006/diff/20001/ui/gl/egl_api_unittest....
ui/gl/egl_api_unittest.cc:88: }  // namespace gpu
On 2015/05/28 19:25:10, sievers wrote:
> nit: s/gpu/gfx

Done.

https://codereview.chromium.org/1151093006/diff/20001/ui/gl/gl_egl_api_implem...
File ui/gl/gl_egl_api_implementation.cc (right):

https://codereview.chromium.org/1151093006/diff/20001/ui/gl/gl_egl_api_implem...
ui/gl/gl_egl_api_implementation.cc:26: void
InitializeDynamicGLBindingsEGL(GLContext* context) {
On 2015/05/28 19:25:10, sievers wrote:
> I had actually previously removed the notion of initializing dynamic bindings
> for EGL etc. :)
> 
> You don't need a context to call eglQueryString, can you do it from the end of
> InitializeStaticGLBindingsEGL() instead?

Done.

https://codereview.chromium.org/1151093006/diff/20001/ui/gl/gl_egl_api_implem...
ui/gl/gl_egl_api_implementation.cc:98: const char* egl_extensions =
EGLApiBase::eglQueryStringFn(display,
On 2015/05/28 19:25:10, sievers wrote:
> You can call DriverEGL::GetPlatformExtensions() and
> DriverEGL::GetClientExtensions() here (use the combined string). That takes
care
> of the display.

Done, although I had to make RealEGLApi a friend of DriverEGL since those
functions are private. I combined them like you said and set it up to return no
matter what display is queried, is that correct? It seems like the
ClientExtensions() should be filtered and returned when the display is
EGL_NO_DISPLAY.

[no sievers, 2015-06-08 22:26:35]

lgtm w/nits

https://codereview.chromium.org/1151093006/diff/80001/ui/gl/egl_api_unittest.cc
File ui/gl/egl_api_unittest.cc (right):

https://codereview.chromium.org/1151093006/diff/80001/ui/gl/egl_api_unittest....
ui/gl/egl_api_unittest.cc:18: g_driver_egl.ClearBindings();
Can you add a TODO (and maybe file a bug) so that we add a proper common mock GL
setup to the gl_unittests suite?
I think this is getting a bit hacky now.

https://codereview.chromium.org/1151093006/diff/80001/ui/gl/gl_bindings.h
File ui/gl/gl_bindings.h (right):

https://codereview.chromium.org/1151093006/diff/80001/ui/gl/gl_bindings.h#new...
ui/gl/gl_bindings.h:390: friend class RealEGLApi;
I think you should just make the functions public instead.

https://codereview.chromium.org/1151093006/diff/80001/ui/gl/gl_egl_api_implem...
File ui/gl/gl_egl_api_implementation.cc (right):

https://codereview.chromium.org/1151093006/diff/80001/ui/gl/gl_egl_api_implem...
ui/gl/gl_egl_api_implementation.cc:97: client_extensions_vec.end(),
nit: indent

[David Yen, 2015-06-09 20:43:15]

https://codereview.chromium.org/1151093006/diff/80001/ui/gl/egl_api_unittest.cc
File ui/gl/egl_api_unittest.cc (right):

https://codereview.chromium.org/1151093006/diff/80001/ui/gl/egl_api_unittest....
ui/gl/egl_api_unittest.cc:18: g_driver_egl.ClearBindings();
On 2015/06/08 22:26:35, sievers wrote:
> Can you add a TODO (and maybe file a bug) so that we add a proper common mock
GL
> setup to the gl_unittests suite?
> I think this is getting a bit hacky now.

Done.

https://codereview.chromium.org/1151093006/diff/80001/ui/gl/gl_bindings.h
File ui/gl/gl_bindings.h (right):

https://codereview.chromium.org/1151093006/diff/80001/ui/gl/gl_bindings.h#new...
ui/gl/gl_bindings.h:390: friend class RealEGLApi;
On 2015/06/08 22:26:35, sievers wrote:
> I think you should just make the functions public instead.

Done.

https://codereview.chromium.org/1151093006/diff/80001/ui/gl/gl_egl_api_implem...
File ui/gl/gl_egl_api_implementation.cc (right):

https://codereview.chromium.org/1151093006/diff/80001/ui/gl/gl_egl_api_implem...
ui/gl/gl_egl_api_implementation.cc:97: client_extensions_vec.end(),
On 2015/06/08 22:26:35, sievers wrote:
> nit: indent

Done.

[David Yen, 2015-06-09 20:43:20]

The CQ bit was checked by dyen@chromium.org

[David Yen, 2015-06-09 20:43:21]

The patchset sent to the CQ was uploaded after l-g-t-m from sievers@chromium.org
Link to the patchset: https://codereview.chromium.org/1151093006/#ps100001
(title: "Applied nits")

[commit-bot: I haz the power, 2015-06-09 20:44:44]

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

[commit-bot: I haz the power, 2015-06-09 23:11:38]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2015-06-09 23:12:33]

Patchset 6 (id:??) landed as
https://crrev.com/9deab29ad0b594c8a308f78913923d65ceb7b93f
Cr-Commit-Position: refs/heads/master@{#333602}