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

ui/gl/gl_image_shm.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "ui/gl/gl_image_shm.h"
 
 #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) || \
     defined(USE_OZONE)
 #include "ui/gl/gl_surface_egl.h"
 #endif
 
 namespace gfx {
 
 namespace {
@@ skipping 86 matching lines @@
   return true;
 }
 
 void GLImageShm::Destroy() {
 #if defined(OS_WIN) || defined(USE_X11) || defined(OS_ANDROID) || \
     defined(USE_OZONE)
   if (egl_image_ != EGL_NO_IMAGE_KHR) {
     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.

 #endif
 }
 
 gfx::Size GLImageShm::GetSize() { return size_; }
 
 bool GLImageShm::BindTexImage(unsigned target) {
   TRACE_EVENT0("gpu", "GLImageShm::BindTexImage");
   DCHECK(shared_memory_);
   DCHECK(ValidFormat(internalformat_));
 
@@ skipping 72 matching lines @@
                0,  // border
                DataFormat(internalformat_),
                DataType(internalformat_),
                shared_memory_->memory());
 
   shared_memory_->Unmap();
   return true;
 }
 
 }  // namespace gfx