During image destroy, delete textures only if we have a GL context.

ui/gl/gl_image_shm.cc

Index: ui/gl/gl_image_shm.cc
diff --git a/ui/gl/gl_image_shm.cc b/ui/gl/gl_image_shm.cc
index 75ef453eade794fe846ed1548dbe90d7e56f6f3f..148d16405c8d1faceea93fffce9fc22f3a2ed271 100644
--- a/ui/gl/gl_image_shm.cc
+++ b/ui/gl/gl_image_shm.cc
@@ -6,6 +6,7 @@
 
 #include "base/debug/trace_event.h"
 #include "base/process/process_handle.h"
+#include "ui/gl/gl_context.h"
 #include "ui/gl/scoped_binders.h"
 
 #if defined(OS_WIN) || defined(USE_X11) || defined(OS_ANDROID) || \
@@ -112,8 +113,9 @@ void GLImageShm::Destroy() {
     eglDestroyImageKHR(GLSurfaceEGL::GetHardwareDisplay(), egl_image_);
     egl_image_ = EGL_NO_IMAGE_KHR;
   }
-
-  if (egl_texture_id_) {
+  GLContext* current_context = GLContext::GetCurrent();
+  if (egl_texture_id_ &&
+      (current_context && current_context->IsCurrent(NULL))) {
     glDeleteTextures(1, &egl_texture_id_);
     egl_texture_id_ = 0u;
   }

[sohanjg, 2014/05/28 10:16:00]

Wouldnt we end up leaking the texture if current_context is NULL ?
I was wondering if we tackled this in ::BindTexImage, where we can use glTex,
and create eglImage and texture for each bind (and delete the old ones).

[no sievers, 2014/05/28 18:55:32]

I think we should rather pass through the flag from the decoder/context group.
The IsCurrent() check is a bit flaky because we don't know if it's the correct
context that is current (one that is in the correct share group).

We could even explicitly destroy the textures with a current context, similar to
what TextureManager does when the decoder or context group get destroyed.

[reveman, 2014/05/28 20:23:33]

I think the problem is that GLImages are ref counted and possibly used by
multiple contexts. Maybe it would be fine to explicitly destroy images when
destroying the context they were created with. The instance would still exist,
binding or using it would just fail after it has been destroyed. We need to make
sure all GLImage implementations do the right thing when used after being
destroyed in this case though.
 
Alternatively, we could just delete the texture when ::ReleaseTexImage is
called.

[no sievers, 2014/05/28 21:02:39]

Should a gpu::gles2::Texture also call ReleaseTexImage() when it gets deleted?
Or does it already somehow (doesn't look like it)?

That would allow us to sort of piggy-back on the existing Texture refcounting
(which is a bit more complicated because of the explicit destruction).

[reveman, 2014/05/28 21:14:56]

I don't think it does but it seems like a good idea to do that so a GLImage
implementation can assume that a BindTexImage call is always followed by a
ReleaseTexImage.