Add GL_TEXTURE_EXTERNAL_OES as supported texture target for CHROMIUM_map_image.

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 40ad67a5dc363d480579e42e34b999ce8d293e38..4b089e347f3fcde65677c55ff1b2f87f25b81284 100644
--- a/ui/gl/gl_image_shm.cc
+++ b/ui/gl/gl_image_shm.cc
@@ -6,7 +6,7 @@
 
 #include "base/debug/trace_event.h"
 #include "base/process/process_handle.h"
-#include "ui/gl/gl_bindings.h"
+#include "ui/gl/scoped_binders.h"
 
 namespace gfx {
 
@@ -64,8 +64,9 @@ GLenum BytesPerPixel(unsigned internalformat) {
 
 GLImageShm::GLImageShm(gfx::Size size, unsigned internalformat)
     : size_(size),
-      internalformat_(internalformat) {
-}
+      internalformat_(internalformat),
+      egl_texture_id_(0u),
+      egl_image_(EGL_NO_IMAGE_KHR) {}
 
 GLImageShm::~GLImageShm() {
   Destroy();
@@ -96,6 +97,14 @@ bool GLImageShm::Initialize(gfx::GpuMemoryBufferHandle buffer) {
 }
 
 void GLImageShm::Destroy() {
+  if (egl_image_ != EGL_NO_IMAGE_KHR) {
+    eglDestroyImageKHR(eglGetCurrentDisplay(), egl_image_);

[reveman, 2014/03/22 12:38:57]

GLSurfaceEGL::GetHardwareDisplay()

+    egl_image_ = EGL_NO_IMAGE_KHR;
+  }
+  if (egl_texture_id_) {
+    glDeleteTextures(1, &egl_texture_id_);
+    egl_texture_id_ = 0;

[reveman, 2014/03/22 12:38:57]

nit: s/0/0u/ to match the assign in ctor

+  }
 }
 
 gfx::Size GLImageShm::GetSize() {
@@ -115,16 +124,49 @@ bool GLImageShm::BindTexImage(unsigned target) {
   }
 
   DCHECK(shared_memory_->memory());
-  glTexImage2D(target,
-               0,  // mip level
-               TextureFormat(internalformat_),
-               size_.width(),
-               size_.height(),
-               0,  // border
-               DataFormat(internalformat_),
-               DataType(internalformat_),
-               shared_memory_->memory());
-
+  if (target != GL_TEXTURE_EXTERNAL_OES) {

[reveman, 2014/03/22 12:38:57]

nit: reversing these if-clauses might improve readability a bit. so having this
be "if (target == GL_TEXTURE_EXTERNAL_OES)" instead. your call though, I don't
feel that strongly about it.

+    glTexImage2D(target,
+                 0,  // mip level
+                 TextureFormat(internalformat_),
+                 size_.width(),
+                 size_.height(),
+                 0,  // border
+                 DataFormat(internalformat_),
+                 DataType(internalformat_),
+                 shared_memory_->memory());
+  } else {
+    glGenTextures(1, &egl_texture_id_);

[reveman, 2014/03/22 12:38:57]

Why did you change this? Do you really need to generate a new texture id
everytime?

+
+    {
+      ScopedTextureBinder texture_binder(GL_TEXTURE_2D, egl_texture_id_);
+      glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR);
+      glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE);
+      glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE);
+
+      glTexImage2D(GL_TEXTURE_2D,
+                   0,  // mip level
+                   TextureFormat(internalformat_),
+                   size_.width(),
+                   size_.height(),
+                   0,  // border
+                   DataFormat(internalformat_),
+                   DataType(internalformat_),
+                   shared_memory_->memory());
+    }

[reveman, 2014/03/22 12:38:57]

thanks for making the intended scope of the binder explicit. please do the same
in gl_image_egl.cc

+
+    EGLint egl_attrib_list[] = {EGL_GL_TEXTURE_LEVEL_KHR, 0,  // mip-level.

[reveman, 2014/03/22 12:38:57]

nit: maybe rename to "attrs" to be consistent with gl_image_egl.cc

+                                EGL_IMAGE_PRESERVED_KHR, EGL_TRUE, EGL_NONE};
+    egl_image_ =
+        eglCreateImageKHR(eglGetCurrentDisplay(),

[reveman, 2014/03/22 12:38:57]

GLSurfaceEGL::GetHardwareDisplay()

+                          eglGetCurrentContext(),

[reveman, 2014/03/22 12:38:57]

and NO_CONTEXT if possible. if that doesn't work, please add a short comment
explaining why eglGetCurrentContext() is needed here.

+                          EGL_GL_TEXTURE_2D_KHR,
+                          reinterpret_cast<EGLClientBuffer>(egl_texture_id_),
+                          egl_attrib_list);
+    DCHECK_NE(EGL_NO_IMAGE_KHR, egl_image_)
+        << "Error creating EGLImage: " << eglGetError();
+
+    glEGLImageTargetTexture2DOES(target, egl_image_);
+  }
   shared_memory_->Unmap();
   return true;
 }
@@ -135,8 +177,7 @@ void GLImageShm::ReleaseTexImage(unsigned target) {
 void GLImageShm::WillUseTexImage() {
 }
 
-void GLImageShm::DidUseTexImage() {
-}
+void GLImageShm::DidUseTexImage() { Destroy(); }

[reveman, 2014/03/22 12:38:57]

Please don't use Destroy here. That's confusing and also not correct as
Will/DidUseTexImage can be called multiple times after BindTexImage.

Please delete the old egl_image_ in GLImageShm::BindTexImage instead.

 
 void GLImageShm::WillModifyTexImage() {
 }