Respect the disabled extension list during binding initialization.

Respect the disabled extension list during EGL/GL binding initialization.

BUG=502353, 504758
Committed: https://crrev.com/bbda4652027d46bae2212f8d8e0cc5f2426dae48
Cr-Commit-Position: refs/heads/master@{#336470}

[David Yen, 2015-06-23 16:28:04]

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

[David Yen, 2015-06-23 16:28:04]

+sievers for review

[David Yen, 2015-06-23 16:38:50]

https://codereview.chromium.org/1203513004/diff/20001/ui/gl/gl_bindings_autog...
File ui/gl/gl_bindings_autogen_egl.cc (right):

https://codereview.chromium.org/1203513004/diff/20001/ui/gl/gl_bindings_autog...
ui/gl/gl_bindings_autogen_egl.cc:110: const std::vector<std::string>&
disabled_extensions) {
Should this function simply take a filtered version of the extension instead? we
can just pass client_extensions.

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

https://codereview.chromium.org/1203513004/diff/20001/ui/gl/gl_egl_api_implem...
ui/gl/gl_egl_api_implementation.h:54: return disabled_exts_;
By returning filtered_exts_ instead, we could probably simplify the generated
code a bit.

[no sievers, 2015-06-23 20:48:25]

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

https://codereview.chromium.org/1203513004/diff/40001/ui/gl/gl_egl_api_implem...
ui/gl/gl_egl_api_implementation.cc:117: std::set<std::string>
RealEGLApi::GetEnabledExtensions() const {
This is basically duplicated code, see InitializeFilteredExtensions() above. Do
you just need to use filtered_exts_ and change the init order above?

[Tobias Sargeant, 2015-06-24 17:00:04]

On 2015/06/23 20:48:25, sievers wrote:
>
https://codereview.chromium.org/1203513004/diff/40001/ui/gl/gl_egl_api_implem...
> ui/gl/gl_egl_api_implementation.cc:117: std::set<std::string>
> RealEGLApi::GetEnabledExtensions() const {
> This is basically duplicated code, see InitializeFilteredExtensions() above.
Do
> you just need to use filtered_exts_ and change the init order above?

Using filtered_exts_ seems wrong, because it splats together client and platform
extensions, regardless of what the caller was requesting. I've refactored things
so that InitializeFilteredExtensions goes away.

What do you think?

[no sievers, 2015-06-24 23:03:00]

On 2015/06/24 17:00:04, Tobias Sargeant wrote:
> On 2015/06/23 20:48:25, sievers wrote:
> >
>
https://codereview.chromium.org/1203513004/diff/40001/ui/gl/gl_egl_api_implem...
> > ui/gl/gl_egl_api_implementation.cc:117: std::set<std::string>
> > RealEGLApi::GetEnabledExtensions() const {
> > This is basically duplicated code, see InitializeFilteredExtensions() above.
> Do
> > you just need to use filtered_exts_ and change the init order above?
> 
> Using filtered_exts_ seems wrong, because it splats together client and
platform
> extensions, regardless of what the caller was requesting. I've refactored
things
> so that InitializeFilteredExtensions goes away.
> 
> What do you think?

I find adding the notion of 'enabled extensions' a bit confusing on top of
everything we already have.

If you are splitting up the bindings init into two parts again, can we just do
this:
- init static bindings
- set current 'egl/wgl/glx' context and filter extensions
- call eglGetString() from the code that inits the static booleans, which then
automatically gives the correct (filtered) answer

The only reason for the GetClient/PlatformExtensions() helpers is that we cannot
call through the current API yet (it's not set), as things are now.
You can then inline the code that calls into the binding directly into
InitializeFilteredExtensions() which is the only place that should need to do
this.

wdyt? And I think we can leave GL unchanged. It's a bit different, since the
current binding does change, while it is clear to the caller that you can only
call into it (like glGetString) while something is current.

[Tobias Sargeant, 2015-06-24 23:17:13]

Ok, that seems like a reasonable approach. I thought passing the enabled
extension set was a suggestion made in previous comments, but maybe I
misunderstood.

Does the filtering of extensions in eglQueryString need to change as per
the previous comment? The doc for EGL_EXT_client_extensions does suggest
that there are disjoint sets of global and per-display extensions, and we
are currently returning the union of those two, regardless of whether the
caller passes a valid EGLDisplay or not.

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[no sievers, 2015-06-24 23:25:14]

On 2015/06/24 23:17:13, Tobias Sargeant wrote:
> Ok, that seems like a reasonable approach. I thought passing the enabled
> extension set was a suggestion made in previous comments, but maybe I
> misunderstood.
> 
> Does the filtering of extensions in eglQueryString need to change as per
> the previous comment? The doc for EGL_EXT_client_extensions does suggest
> that there are disjoint sets of global and per-display extensions, and we
> are currently returning the union of those two, regardless of whether the
> caller passes a valid EGLDisplay or not.

I think you are right, and eglQueryStringFn() needs a bit of fixing.

We can probably just have two sets of filtered extensions (client and display)
and filter both through the disabled string.
eglQueryString() can then return one or the other depending on whether |dpy| is
NO_DISPLAY or not.


> 
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

[Tobias Sargeant, 2015-06-25 17:00:18]

tobiasjs@chromium.org changed reviewers:
+ boliu@chromium.org, kbr@chromium.org

[Tobias Sargeant, 2015-06-25 17:00:43]

instrumentation tests fail on linux_android_rel_ng because now we are properly
disabling EGL_KHR_fence_sync on K devices, as per the blacklist, but webview
depends on that extension to run. It may be necessary to find a way to avoid
disabling that extension in the instrumentation shell.

I can't work out what is going on with the windows trybot. The logs aren't
terribly informative.

Bo said you might have some insight, Ken? I'd be grateful if you could take a
look.

[Ken Russell (switch to Gerrit), 2015-06-25 18:01:45]

On 2015/06/25 17:00:43, Tobias Sargeant wrote:
> instrumentation tests fail on linux_android_rel_ng because now we are properly
> disabling EGL_KHR_fence_sync on K devices, as per the blacklist, but webview
> depends on that extension to run. It may be necessary to find a way to avoid
> disabling that extension in the instrumentation shell.
> 
> I can't work out what is going on with the windows trybot. The logs aren't
> terribly informative.
> 
> Bo said you might have some insight, Ken? I'd be grateful if you could take a
> look.

The browser's crashing during startup. I apologize for the fact that the stack
trace is empty in e.g.
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...
-- we are supposed to be including the .pdb files in the isolates that are run
on the test machines so that the stack traces can be symbolized.

The best thing to do would be to find a Windows development machine on which to
catch the crash. Do you have one available or nearby?

[boliu, 2015-06-25 18:07:29]

On 2015/06/25 18:01:45, Ken Russell wrote:
> On 2015/06/25 17:00:43, Tobias Sargeant wrote:
> > instrumentation tests fail on linux_android_rel_ng because now we are
properly
> > disabling EGL_KHR_fence_sync on K devices, as per the blacklist, but webview
> > depends on that extension to run. It may be necessary to find a way to avoid
> > disabling that extension in the instrumentation shell.
> > 
> > I can't work out what is going on with the windows trybot. The logs aren't
> > terribly informative.
> > 
> > Bo said you might have some insight, Ken? I'd be grateful if you could take
a
> > look.
> 
> The browser's crashing during startup. I apologize for the fact that the stack
> trace is empty in e.g.
>
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...
> -- we are supposed to be including the .pdb files in the isolates that are run
> on the test machines so that the stack traces can be symbolized.
> 
> The best thing to do would be to find a Windows development machine on which
to
> catch the crash. Do you have one available or nearby?

Toby said on chat that the shared windows machine london was unavailable today.
:( Really just need a stack here probably.

I suspect the problem is similar to the webview one. This patch no longer
initialize bindings for the extensions that are unavailable or blacklisted, and
test is probably trying to use some of those extensions.

[no sievers, 2015-06-25 19:11:14]

On 2015/06/25 17:00:43, Tobias Sargeant wrote:
> instrumentation tests fail on linux_android_rel_ng because now we are properly
> disabling EGL_KHR_fence_sync on K devices, as per the blacklist, but webview
> depends on that extension to run. It may be necessary to find a way to avoid
> disabling that extension in the instrumentation shell.
> 

Here is how you can force the bindings to be resolved regardless of the
extension presence (the extension will still be filtered out but the respective
android webview code already has to ignore the presence of the extension
anyways):

In generate_bindings.py:

Add something like GL_CHROMIUM_egl_khr_fence_sync_bind_always_somebugid to
UNCONDITIONALLY_BOUND_EXTENSIONS.

Then in all the needed functions (eglCreateSync etc.) in GL_FUNCTIONS add
'GL_CHROMIUM_egl_khr_fence_sync_bind_always_somebugid' to the |extensions| list.

[Ken Russell (switch to Gerrit), 2015-06-26 00:09:43]

On 2015/06/25 18:07:29, boliu wrote:
> On 2015/06/25 18:01:45, Ken Russell wrote:
> > On 2015/06/25 17:00:43, Tobias Sargeant wrote:
> > > instrumentation tests fail on linux_android_rel_ng because now we are
> properly
> > > disabling EGL_KHR_fence_sync on K devices, as per the blacklist, but
webview
> > > depends on that extension to run. It may be necessary to find a way to
avoid
> > > disabling that extension in the instrumentation shell.
> > > 
> > > I can't work out what is going on with the windows trybot. The logs aren't
> > > terribly informative.
> > > 
> > > Bo said you might have some insight, Ken? I'd be grateful if you could
take
> a
> > > look.
> > 
> > The browser's crashing during startup. I apologize for the fact that the
stack
> > trace is empty in e.g.
> >
>
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...
> > -- we are supposed to be including the .pdb files in the isolates that are
run
> > on the test machines so that the stack traces can be symbolized.
> > 
> > The best thing to do would be to find a Windows development machine on which
> to
> > catch the crash. Do you have one available or nearby?
> 
> Toby said on chat that the shared windows machine london was unavailable
today.
> :( Really just need a stack here probably.
> 
> I suspect the problem is similar to the webview one. This patch no longer
> initialize bindings for the extensions that are unavailable or blacklisted,
and
> test is probably trying to use some of those extensions.

I tried patching this CL into my workspace but was unable to reproduce the
crash, even running the test via the isolate. I've just CQ'd
https://codereview.chromium.org/1213693003/ which I hope will cause the stack
traces on the bots to be symbolized, but am not sure whether it will. Please let
me know if you don't have any luck reproducing the crash.

[boliu, 2015-06-26 01:37:37]

On 2015/06/26 00:09:43, Ken Russell wrote:
> On 2015/06/25 18:07:29, boliu wrote:
> > On 2015/06/25 18:01:45, Ken Russell wrote:
> > > On 2015/06/25 17:00:43, Tobias Sargeant wrote:
> > > > instrumentation tests fail on linux_android_rel_ng because now we are
> > properly
> > > > disabling EGL_KHR_fence_sync on K devices, as per the blacklist, but
> webview
> > > > depends on that extension to run. It may be necessary to find a way to
> avoid
> > > > disabling that extension in the instrumentation shell.
> > > > 
> > > > I can't work out what is going on with the windows trybot. The logs
aren't
> > > > terribly informative.
> > > > 
> > > > Bo said you might have some insight, Ken? I'd be grateful if you could
> take
> > a
> > > > look.
> > > 
> > > The browser's crashing during startup. I apologize for the fact that the
> stack
> > > trace is empty in e.g.
> > >
> >
>
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...
> > > -- we are supposed to be including the .pdb files in the isolates that are
> run
> > > on the test machines so that the stack traces can be symbolized.
> > > 
> > > The best thing to do would be to find a Windows development machine on
which
> > to
> > > catch the crash. Do you have one available or nearby?
> > 
> > Toby said on chat that the shared windows machine london was unavailable
> today.
> > :( Really just need a stack here probably.
> > 
> > I suspect the problem is similar to the webview one. This patch no longer
> > initialize bindings for the extensions that are unavailable or blacklisted,
> and
> > test is probably trying to use some of those extensions.
> 
> I tried patching this CL into my workspace but was unable to reproduce the
> crash, even running the test via the isolate. I've just CQ'd
> https://codereview.chromium.org/1213693003/ which I hope will cause the stack
> traces on the bots to be symbolized, but am not sure whether it will. Please
let
> me know if you don't have any luck reproducing the crash.

Thanks Ken.

This run has the pdf change:
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...

Fingers crossed it produces a stack.

[boliu, 2015-06-26 02:04:12]

stack!

Backtrace:
	(No symbol) [0x00000000]
	gfx::`anonymous namespace'::GetPlatformANGLEDisplay [0x00B794F4+308]
	gfx::`anonymous namespace'::GetDisplayFromType [0x00B78EA0+112]
	gfx::GLSurfaceEGL::InitializeDisplay [0x00B79C34+196]
	gfx::DriverEGL::GetPlatformExtensions [0x00B7F660+32]
	gfx::DriverEGL::InitializeExtensionBindings [0x00B7CFD5+37]
	gfx::InitializeStaticGLBindings [0x00B5420B+1019]
	gfx::GLSurface::InitializeOneOffImplementation [0x00B51CA4+20]
	gfx::GLSurface::InitializeOneOffForTests [0x00B51C33+339]
	content::ContentTestSuite::Initialize [0x00B21BD0+96]
	base::TestSuite::Run [0x00B2DD39+25]
	`anonymous namespace'::RunHelper [0x00B12185+69]
	base::`anonymous namespace'::LaunchUnitTestsInternal [0x00B2B6FF+671]
	base::LaunchUnitTestsSerially [0x00B2B782+98]
	main [0x00B1B747+119]
	__tmainCRTStartup [0x01EB1565+254]
(f:\dd\vctools\crt\crtw32\startup\crt0.c:255)
	BaseThreadInitThunk [0x7767336A+18]
	RtlInitializeExceptionChain [0x77E492B2+99]
	RtlInitializeExceptionChain [0x77E49285+54]

I'm guessing eglGetPlatformDisplayEXT is not bound?

[Tobias Sargeant, 2015-06-26 11:19:47]

> I'm guessing eglGetPlatformDisplayEXT is not bound?

Yes. It seems that GetPlatformExtensions using ANGLE relies on having resolved
client extensions, so moving the position of the GetPlatformExtensions call was
not a no-op. It's a pity, because keeping the position makes the python
generator script more messy.

I think there's scope to clean up the python code by quite a bit (in a follow-up
CL, maybe).

I also noticed that a side-effect of UNCONDITIONALLY_BOUND_EXTENSIONS is that we
create extra b_ flags for each of them, and test whether they're enabled. It
might be good to clean that up too. By convention we could avoid generating a
flag and extension test for extensions ending with the string "hack".

[Ken Russell (switch to Gerrit), 2015-06-26 15:39:40]

+geofflang and jmadill in case they have any suggestions.

[Ken Russell (switch to Gerrit), 2015-06-26 15:40:25]

and +cwallez since this might be a general problem in the EGL implementation.

[Geoff Lang, 2015-06-26 15:48:52]

On 2015/06/26 11:19:47, Tobias Sargeant wrote:
> > I'm guessing eglGetPlatformDisplayEXT is not bound?
> 
> Yes. It seems that GetPlatformExtensions using ANGLE relies on having resolved
> client extensions, so moving the position of the GetPlatformExtensions call
was
> not a no-op. It's a pity, because keeping the position makes the python
> generator script more messy.

Yep, ANGLE needs eglGetPlatformDisplayEXT resolved so that we can initialize the
correct target platform (D3D9/D3D11/GL).

[Jamie Madill, 2015-06-26 15:51:24]

On 2015/06/26 15:48:52, Geoff Lang wrote:
> On 2015/06/26 11:19:47, Tobias Sargeant wrote:
> > > I'm guessing eglGetPlatformDisplayEXT is not bound?
> > 
> > Yes. It seems that GetPlatformExtensions using ANGLE relies on having
resolved
> > client extensions, so moving the position of the GetPlatformExtensions call
> was
> > not a no-op. It's a pity, because keeping the position makes the python
> > generator script more messy.
> 
> Yep, ANGLE needs eglGetPlatformDisplayEXT resolved so that we can initialize
the
> correct target platform (D3D9/D3D11/GL).

Yeah, I was the author of the CL that specified the order. Just confirming the
above.

[Geoff Lang, 2015-06-26 15:51:30]

I'll also add that client extensions usually expose features that need to be
known before initialization of the eglDisplay so it will be useful to have them
available as early as possible.

[Tobias Sargeant, 2015-06-26 15:57:25]

On 2015/06/26 15:51:30, Geoff Lang wrote:
> I'll also add that client extensions usually expose features that need to be
> known before initialization of the eglDisplay so it will be useful to have
them
> available as early as possible.

So this is fixed in PS9. The call to GetPlatformExtensions just needed to be
moved back to after the client extension function pointers were resolved.

Everything appears to be passing now.

[Ken Russell (switch to Gerrit), 2015-06-26 17:06:50]

Nice work. LGTM overall. One question.

P.S. If it would be appropriate or at all possible to add tests for this to
gl_unittests (see src/ui/gl/gl_tests.gyp) let's please do so. Otherwise, I
assume this will eventually be tested on the bots via the
android_webview_webgl_conformance_tests step, and the fact that some extensions
need to be disabled on some bots in that configuration.

https://codereview.chromium.org/1203513004/diff/160001/ui/gl/generate_binding...
File ui/gl/generate_bindings.py (right):

https://codereview.chromium.org/1203513004/diff/160001/ui/gl/generate_binding...
ui/gl/generate_bindings.py:29:
'GL_CHROMIUM_EGL_KHR_fence_sync_bind_always_504758_hack', # crbug.com/504758
That's a mouthful. Is it really worth embedding the bug ID in the name of the
hack?

https://codereview.chromium.org/1203513004/diff/160001/ui/gl/gl_egl_api_imple...
File ui/gl/gl_egl_api_implementation.cc (right):

https://codereview.chromium.org/1203513004/diff/160001/ui/gl/gl_egl_api_imple...
ui/gl/gl_egl_api_implementation.cc:99: auto it = filtered_exts_.find(dpy);
These are being cached per-EGLDisplay. Is this cache cleared out if eglTerminate
is called?

[no sievers, 2015-06-26 17:51:29]

lgtm stamp with kbr's comments addressed.

We should also handle GLX and WGL extension filtering as it's straightforward to
do in the same way as for EGL after this patch (TODO+follow-up sounds fine
though).

https://codereview.chromium.org/1203513004/diff/160001/ui/gl/generate_binding...
File ui/gl/generate_bindings.py (right):

https://codereview.chromium.org/1203513004/diff/160001/ui/gl/generate_binding...
ui/gl/generate_bindings.py:29:
'GL_CHROMIUM_EGL_KHR_fence_sync_bind_always_504758_hack', # crbug.com/504758
On 2015/06/26 17:06:49, Ken Russell wrote:
> That's a mouthful. Is it really worth embedding the bug ID in the name of the
> hack?

Yea sorry that was my overly explicit suggestion, something more compact sounds
good :)

https://codereview.chromium.org/1203513004/diff/160001/ui/gl/gl_egl_api_imple...
File ui/gl/gl_egl_api_implementation.cc (right):

https://codereview.chromium.org/1203513004/diff/160001/ui/gl/gl_egl_api_imple...
ui/gl/gl_egl_api_implementation.cc:20: base::SplitString(extensions ? extensions
: "", ' ', &extension_vec);
nit: |extensions| cannot be null here

[Tobias Sargeant, 2015-06-26 20:30:35]

> nit: |extensions| cannot be null here

removed.

[Tobias Sargeant, 2015-06-26 20:39:31]

> 'GL_CHROMIUM_EGL_KHR_fence_sync_bind_always_504758_hack', # crbug.com/504758
> That's a mouthful. Is it really worth embedding the bug ID in the name of the
> hack?

Shortened, and brought in line with the preceeding hacks (only GL_CHROMIUM
capitalized)

> These are being cached per-EGLDisplay. Is this cache cleared out if
eglTerminate
> is called?

The cache isn't cleared out when eglTerminate is called. You're right that that
could be a problem, in so far as the extension list could be stale if EGL
managed somehow to reuse the same display handle with a different extension
list.

Prior to this CL though, the same thing was true, because the filtered_exts_
string was immutable once set. Worse, if this was ever a problem, even if we
cleaned out the cache so that the extension list was right, it would not match
up with the extension flags and bindings stored in DriverEGL. For this reason,
I've cleared the filtered extension map in
RealEGLApi::InitializeWithCommandLine, on the assumption that the only way that
this can ever be correct is if any call to eglTerminate is followed up by
reinitialization of the extension list, before anything further happens.

[no sievers, 2015-06-26 20:43:33]

On 2015/06/26 20:39:31, Tobias Sargeant wrote:
> > 'GL_CHROMIUM_EGL_KHR_fence_sync_bind_always_504758_hack', # crbug.com/504758
> > That's a mouthful. Is it really worth embedding the bug ID in the name of
the
> > hack?
> 
> Shortened, and brought in line with the preceeding hacks (only GL_CHROMIUM
> capitalized)
> 
> > These are being cached per-EGLDisplay. Is this cache cleared out if
> eglTerminate
> > is called?
> 
> The cache isn't cleared out when eglTerminate is called. You're right that
that
> could be a problem, in so far as the extension list could be stale if EGL
> managed somehow to reuse the same display handle with a different extension
> list.
> 
> Prior to this CL though, the same thing was true, because the filtered_exts_
> string was immutable once set. Worse, if this was ever a problem, even if we
> cleaned out the cache so that the extension list was right, it would not match
> up with the extension flags and bindings stored in DriverEGL. For this reason,
> I've cleared the filtered extension map in
> RealEGLApi::InitializeWithCommandLine, on the assumption that the only way
that
> this can ever be correct is if any call to eglTerminate is followed up by
> reinitialization of the extension list, before anything further happens.

You could also consider just supporting two entries in the map for the time
being:
one for EGL_NO_DISPLAY, one for everything else.

The fact that this patch deals with the global/static extension bools shows that
we really assume only one consistent extension string. So you only have to
handle EGL_EXT_client_extensions ('client' vs 'platform').

[Tobias Sargeant, 2015-06-26 20:55:28]

> You could also consider just supporting two entries in the map for the time
> being:
> one for EGL_NO_DISPLAY, one for everything else.
> 
> The fact that this patch deals with the global/static extension bools shows
that
> we really assume only one consistent extension string. So you only have to
> handle EGL_EXT_client_extensions ('client' vs 'platform').

It's definitely true that we make that assumption for ourselves. I can't imagine
how for EGL there could be different extension lists available for different
displays (but maybe I'm not imagining hard enough). Do we handle the analogous
possibility for GL (different extension lists per context)? I guess that could
arise from two GPUs driving different displays (or even from one being used only
for video de/encode acceleration).

I doubt very much that eglQueryString will ever be called from other places, but
feel like we should get in the way of that minimally (by only removing
extensions we know are bad from whatever the real eglQueryString tells us). I'd
like to leave it as is, if you're ok with that, but I'm also happy to follow
your suggestion.

[no sievers, 2015-06-26 21:01:10]

On 2015/06/26 20:55:28, Tobias Sargeant wrote:
> > You could also consider just supporting two entries in the map for the time
> > being:
> > one for EGL_NO_DISPLAY, one for everything else.
> > 
> > The fact that this patch deals with the global/static extension bools shows
> that
> > we really assume only one consistent extension string. So you only have to
> > handle EGL_EXT_client_extensions ('client' vs 'platform').
> 
> It's definitely true that we make that assumption for ourselves. I can't
imagine
> how for EGL there could be different extension lists available for different
> displays (but maybe I'm not imagining hard enough). Do we handle the analogous
> possibility for GL (different extension lists per context)? 

Not really, we mostly assume every context we create has the same extensions.

> 
> I doubt very much that eglQueryString will ever be called from other places,
but
> feel like we should get in the way of that minimally (by only removing
> extensions we know are bad from whatever the real eglQueryString tells us).
I'd
> like to leave it as is, if you're ok with that, but I'm also happy to follow
> your suggestion.

It's just that there is a risk that the map grows on some platform and doesn't
get cleaned up because of changing display handles. I haven't looked at the code
in detail to see if that can happen in practice though.

[Tobias Sargeant, 2015-06-26 23:03:33]

> It's just that there is a risk that the map grows on some platform and doesn't
> get cleaned up because of changing display handles. I haven't looked at the
code
> in detail to see if that can happen in practice though.

It looks pretty safe. the only extension querying calls look to be with
g_display set up in GLSurfaceEGL::InitializeOneOff() or inside other code that
is called once.

[Tobias Sargeant, 2015-06-26 23:03:39]

The CQ bit was checked by tobiasjs@chromium.org

[Tobias Sargeant, 2015-06-26 23:03:40]

The patchset sent to the CQ was uploaded after l-g-t-m from kbr@chromium.org,
sievers@chromium.org
Link to the patchset: https://codereview.chromium.org/1203513004/#ps180001
(title: "address comments")

[commit-bot: I haz the power, 2015-06-26 23:04:28]

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

[commit-bot: I haz the power, 2015-06-26 23:09:01]

Committed patchset #10 (id:180001)

[commit-bot: I haz the power, 2015-06-26 23:09:55]

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

[no sievers, 2015-07-21 23:16:27]

Did we somehow miss fixing this for the GL core extension bools?

in gl_gl_api_implementation.cc it looks like we would init the bools before we
call InitializeFilteredExtensions().

[Tobias Sargeant, 2015-07-22 14:46:18]

On 2015/07/21 23:16:27, sievers wrote:
> Did we somehow miss fixing this for the GL core extension bools?
> 
> in gl_gl_api_implementation.cc it looks like we would init the bools before we
> call InitializeFilteredExtensions().

You're right. Sorry. https://codereview.chromium.org/1253433002/ for the fix.