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

gpu/command_buffer/service/image_manager.cc

Index: gpu/command_buffer/service/image_manager.cc
diff --git a/gpu/command_buffer/service/image_manager.cc b/gpu/command_buffer/service/image_manager.cc
index 953e3bcb5731b3b98bfaeef8d6b7bae11c96d376..c485f1d85f546b84ef576214fd02f97fba9b75cb 100644
--- a/gpu/command_buffer/service/image_manager.cc
+++ b/gpu/command_buffer/service/image_manager.cc
@@ -4,6 +4,7 @@
 
 #include "gpu/command_buffer/service/image_manager.h"
 
+#include "base/logging.h"
 #include "ui/gl/gl_image.h"
 
 namespace gpu {
@@ -15,12 +16,32 @@ ImageManager::ImageManager() {
 ImageManager::~ImageManager() {
 }
 
-void ImageManager::AddImage(gfx::GLImage* image, int32 service_id) {
+void ImageManager::Destroy(bool have_context) {
+  for (GLImageMap::const_iterator iter = images_.begin(); iter != images_.end();
+       ++iter)
+    iter->second.get()->Destroy(have_context);
+  images_.clear();
+}
+
+bool ImageManager::AddImage(gfx::GLImage* image, int32 service_id) {
+  GLImageMap::const_iterator iter = images_.find(service_id);
+  if (iter != images_.end()) {
+    LOG(ERROR) << "Image already exists with same ID.";
+    return false;
+  }
   images_[service_id] = image;

[no sievers, 2014/07/30 12:34:08]

ok now that you have moved the code, let's actually iterate only once :)

std::pair<GLImageMap::iterator,bool> ret = images_.insert(service_id, image);

if (!ret->second) {
  LOG(ERROR) << "Image already exists with same ID.";
  return false; 
}

return true;

[no sievers, 2014/07/30 12:40:41]

Scrap that, my bad. This doesn't work, since it will only return |false| I think
if both key and value match for the existing entry.

[sohanjg, 2014/07/30 13:08:02]

Acknowledged.

[no sievers, 2014/07/30 13:23:01]

Actually, I do think it works, and I just got confused by the MSDN example which
is crafted in a way that's a bit unfortunate.

You could try changing it but I'd add a unittest to
gles2_cmd_decoder_unittest.cc to make sure it works:

TEST_P(GLES2DecoderTest, AddImageWithExistingIdFails) {
  scoped_refptr<gfx::GLImage> old_image(new gfx::GLImageStub);
  scoped_refptr<gfx::GLImage> new_image(new gfx::GLImageStub);
  EXPECT_TRUE(GetImageManager()->AddImage(old_image.get(), 1));
  EXPECT_FALSE(GetImageManager()->AddImage(new_image.get(), 1));
}

+  return true;
 }
 
-void ImageManager::RemoveImage(int32 service_id) {
-  images_.erase(service_id);
+bool ImageManager::RemoveImage(int32 service_id) {
+  GLImageMap::iterator iter = images_.find(service_id);
+  if (iter == images_.end()) {
+    LOG(ERROR) << "Image with ID doesn't exist.";

[no sievers, 2014/07/30 12:34:08]

You can just do LOG_IF(ERROR, iter == images_.end()) and don't have to return a
value form this method.

[sohanjg, 2014/07/30 13:08:02]

Done.

+    return false;
+  }
+  iter->second.get()->Destroy(true);
+  images_.erase(iter);
+  return true;
 }
 
 gfx::GLImage* ImageManager::LookupImage(int32 service_id) {