Implement shareResources==true in TestWebGraphicsContext3D

cc/test/test_web_graphics_context_3d.cc

Index: cc/test/test_web_graphics_context_3d.cc
diff --git a/cc/test/test_web_graphics_context_3d.cc b/cc/test/test_web_graphics_context_3d.cc
index f5ebb6b65c4b0d7b769c75fe69206463593e8951..161e9c8d31fd3791c2cddf6ca04cf46401422b53 100644
--- a/cc/test/test_web_graphics_context_3d.cc
+++ b/cc/test/test_web_graphics_context_3d.cc
@@ -35,12 +35,23 @@ static unsigned s_context_id = 1;
 
 const WebGLId TestWebGraphicsContext3D::kExternalTextureId = 1337;
 
+TestWebGraphicsContext3D::Namespace*
+    TestWebGraphicsContext3D::shared_namespace_ = NULL;

[danakj, 2013/07/16 02:57:02]

This will be a leak. Should we refcount and clean it up?

[piman, 2013/07/16 03:06:39]

It is refcounted, and it cleans itself up :)
See TestWebGraphicsContext3D::Namespace::~Namespace

[danakj, 2013/07/16 03:26:11]

Oh right, cool.

+
+TestWebGraphicsContext3D::Namespace::Namespace()
+    : next_buffer_id(1),
+      next_image_id(1),
+      next_texture_id(1) {
+}
+
+TestWebGraphicsContext3D::Namespace::~Namespace() {
+  if (shared_namespace_ == this)

[danakj, 2013/07/16 03:26:11]

Do we need a lock for accessing the shared_namespace_ ?

[piman, 2013/07/16 04:50:55]

Yes, we do! Thanks!

+    shared_namespace_ = NULL;
+}
+
 TestWebGraphicsContext3D::TestWebGraphicsContext3D()
     : FakeWebGraphicsContext3D(),
       context_id_(s_context_id++),
-      next_buffer_id_(1),
-      next_image_id_(1),
-      next_texture_id_(1),
       support_swapbuffers_complete_callback_(true),
       have_extension_io_surface_(false),
       have_extension_egl_image_(false),
@@ -57,16 +68,20 @@ TestWebGraphicsContext3D::TestWebGraphicsContext3D()
       width_(0),
       height_(0),
       bound_buffer_(0),
+      namespace_(new Namespace),
       weak_ptr_factory_(this) {
+  if (shared_namespace_) {

[danakj, 2013/07/16 03:26:11]

can you DCHECK(attributes.shareResources) here so we dont break this in the
future somehow? or have this code also be conditional (maybe shared method).

[piman, 2013/07/16 04:50:55]

Created helper function (made conditional).

+    namespace_ = shared_namespace_;
+  } else {
+    namespace_ = new Namespace;
+    shared_namespace_ = namespace_;

[danakj, 2013/07/16 03:26:11]

.get()

[piman, 2013/07/16 04:50:55]

Done.

+  }
 }
 
 TestWebGraphicsContext3D::TestWebGraphicsContext3D(
     const WebGraphicsContext3D::Attributes& attributes)
     : FakeWebGraphicsContext3D(),
       context_id_(s_context_id++),
-      next_buffer_id_(1),
-      next_image_id_(1),
-      next_texture_id_(1),
       attributes_(attributes),
       support_swapbuffers_complete_callback_(true),
       have_extension_io_surface_(false),
@@ -85,6 +100,16 @@ TestWebGraphicsContext3D::TestWebGraphicsContext3D(
       height_(0),
       bound_buffer_(0),
       weak_ptr_factory_(this) {
+  if (attributes.shareResources) {
+    if (shared_namespace_) {
+      namespace_ = shared_namespace_;

[danakj, 2013/07/16 03:26:11]

cast from raw pointer to scoped_refptr is kinda icky, but i guess that will
remain valid for the forseeable future?

[piman, 2013/07/16 04:50:55]

It's not a cast but an assignment.. What bothers you? It's a somewhat common
pattern (e.g. refcounted field, accessor returns a raw pointer, passed to
another method that takes a pointer, assigned to a field that's a refptr).

In this case, shared_namespace_ is really a weakptr, i.e. it's either NULL or
someone has a ref on it, so I don't know how it could go wrong? refs are
taken/released under the lock.

[danakj, 2013/07/16 05:39:00]

Yeh, I've seen this used in chromium before as well. It seems to me that a raw
pointer should signify no ownership changing abilties, and should be able to be
assumed it is just a raw pointer forever. Like, with scoped_ptr you would never
take a raw pointer and turn it into a scoped_ptr. I feel like things would be
more clear if you couldn't do the same with a refptr either, that going to a raw
pointer would be a one-way change. I guess if this were to change though, it
would require some kind of patterned adjustment across the codebase.

+    } else {
+      namespace_ = new Namespace;
+      shared_namespace_ = namespace_;

[danakj, 2013/07/16 03:26:11]

.get()

[piman, 2013/07/16 04:50:55]

Done.

+    }
+  } else {
+    namespace_ = new Namespace;
+  }
 }
 
 TestWebGraphicsContext3D::~TestWebGraphicsContext3D() {
@@ -207,9 +232,10 @@ WebGLId TestWebGraphicsContext3D::createBuffer() {
 }
 
 void TestWebGraphicsContext3D::deleteBuffer(WebGLId id) {
+  base::AutoLock lock(namespace_->lock);
   unsigned context_id = id >> 17;
   unsigned buffer_id = id & 0x1ffff;
-  DCHECK(buffer_id && buffer_id < next_buffer_id_);
+  DCHECK(buffer_id && buffer_id < namespace_->next_buffer_id);
   DCHECK_EQ(context_id, context_id_);
 }
 
@@ -248,14 +274,17 @@ void TestWebGraphicsContext3D::deleteShader(WebGLId id) {
 WebGLId TestWebGraphicsContext3D::createTexture() {
   WebGLId texture_id = NextTextureId();
   DCHECK_NE(texture_id, kExternalTextureId);
-  textures_.push_back(texture_id);
+  base::AutoLock lock(namespace_->lock);
+  namespace_->textures.push_back(texture_id);
   return texture_id;
 }
 
 void TestWebGraphicsContext3D::deleteTexture(WebGLId texture_id) {
-  DCHECK(std::find(textures_.begin(), textures_.end(), texture_id) !=
-         textures_.end());
-  textures_.erase(std::find(textures_.begin(), textures_.end(), texture_id));
+  base::AutoLock lock(namespace_->lock);
+  std::vector<WebKit::WebGLId>& textures = namespace_->textures;
+  DCHECK(std::find(textures.begin(), textures.end(), texture_id) !=
+         textures.end());
+  textures.erase(std::find(textures.begin(), textures.end(), texture_id));
 }
 
 void TestWebGraphicsContext3D::attachShader(WebGLId program, WebGLId shader) {
@@ -297,8 +326,10 @@ void TestWebGraphicsContext3D::bindTexture(
     return;
   if (texture_id == kExternalTextureId)
     return;
-  DCHECK(std::find(textures_.begin(), textures_.end(), texture_id) !=
-         textures_.end());
+  base::AutoLock lock(namespace_->lock);
+  std::vector<WebKit::WebGLId>& textures = namespace_->textures;
+  DCHECK(std::find(textures.begin(), textures.end(), texture_id) !=
+         textures.end());
   used_textures_.insert(texture_id);
 }
 
@@ -422,55 +453,64 @@ void TestWebGraphicsContext3D::bindBuffer(WebKit::WGC3Denum target,
     return;
   unsigned context_id = buffer >> 17;
   unsigned buffer_id = buffer & 0x1ffff;
-  DCHECK(buffer_id && buffer_id < next_buffer_id_);
+  base::AutoLock lock(namespace_->lock);
+  DCHECK(buffer_id && buffer_id < namespace_->next_buffer_id);
   DCHECK_EQ(context_id, context_id_);
 
-  if (buffers_.count(bound_buffer_) == 0)
-    buffers_.set(bound_buffer_, make_scoped_ptr(new Buffer).Pass());
+  ScopedPtrHashMap<unsigned, Buffer>& buffers = namespace_->buffers;
+  if (buffers.count(bound_buffer_) == 0)
+    buffers.set(bound_buffer_, make_scoped_ptr(new Buffer).Pass());
 
-  buffers_.get(bound_buffer_)->target = target;
+  buffers.get(bound_buffer_)->target = target;
 }
 
 void TestWebGraphicsContext3D::bufferData(WebKit::WGC3Denum target,
                                           WebKit::WGC3Dsizeiptr size,
                                           const void* data,
                                           WebKit::WGC3Denum usage) {
-  DCHECK_GT(buffers_.count(bound_buffer_), 0u);
-  DCHECK_EQ(target, buffers_.get(bound_buffer_)->target);
+  base::AutoLock lock(namespace_->lock);
+  ScopedPtrHashMap<unsigned, Buffer>& buffers = namespace_->buffers;
+  DCHECK_GT(buffers.count(bound_buffer_), 0u);
+  DCHECK_EQ(target, buffers.get(bound_buffer_)->target);
   if (context_lost_) {
-    buffers_.get(bound_buffer_)->pixels.reset();
+    buffers.get(bound_buffer_)->pixels.reset();
     return;
   }
-  buffers_.get(bound_buffer_)->pixels.reset(new uint8[size]);
+  buffers.get(bound_buffer_)->pixels.reset(new uint8[size]);
   if (data != NULL)
-    memcpy(buffers_.get(bound_buffer_)->pixels.get(), data, size);
+    memcpy(buffers.get(bound_buffer_)->pixels.get(), data, size);
 }
 
 void* TestWebGraphicsContext3D::mapBufferCHROMIUM(WebKit::WGC3Denum target,
                                                   WebKit::WGC3Denum access) {
-  DCHECK_GT(buffers_.count(bound_buffer_), 0u);
-  DCHECK_EQ(target, buffers_.get(bound_buffer_)->target);
+  base::AutoLock lock(namespace_->lock);
+  ScopedPtrHashMap<unsigned, Buffer>& buffers = namespace_->buffers;
+  DCHECK_GT(buffers.count(bound_buffer_), 0u);
+  DCHECK_EQ(target, buffers.get(bound_buffer_)->target);
   if (times_map_buffer_chromium_succeeds_ >= 0) {
     if (!times_map_buffer_chromium_succeeds_) {
       return NULL;
     }
     --times_map_buffer_chromium_succeeds_;
   }
-  return buffers_.get(bound_buffer_)->pixels.get();
+  return buffers.get(bound_buffer_)->pixels.get();
 }
 
 WebKit::WGC3Dboolean TestWebGraphicsContext3D::unmapBufferCHROMIUM(
     WebKit::WGC3Denum target) {
-  DCHECK_GT(buffers_.count(bound_buffer_), 0u);
-  DCHECK_EQ(target, buffers_.get(bound_buffer_)->target);
-  buffers_.get(bound_buffer_)->pixels.reset();
+  base::AutoLock lock(namespace_->lock);
+  ScopedPtrHashMap<unsigned, Buffer>& buffers = namespace_->buffers;
+  DCHECK_GT(buffers.count(bound_buffer_), 0u);
+  DCHECK_EQ(target, buffers.get(bound_buffer_)->target);
+  buffers.get(bound_buffer_)->pixels.reset();
   return true;
 }
 
 void TestWebGraphicsContext3D::bindTexImage2DCHROMIUM(
     WebKit::WGC3Denum target,
     WebKit::WGC3Dint image_id) {
-  DCHECK_GT(images_.count(image_id), 0u);
+  base::AutoLock lock(namespace_->lock);
+  DCHECK_GT(namespace_->images.count(image_id), 0u);
 }
 
 WebKit::WGC3Duint TestWebGraphicsContext3D::createImageCHROMIUM(
@@ -478,16 +518,19 @@ WebKit::WGC3Duint TestWebGraphicsContext3D::createImageCHROMIUM(
       WebKit::WGC3Denum internalformat) {
   DCHECK_EQ(GL_RGBA8_OES, static_cast<int>(internalformat));
   WebKit::WGC3Duint image_id = NextImageId();
-  images_.set(image_id, make_scoped_ptr(new Image).Pass());
-  images_.get(image_id)->pixels.reset(new uint8[width * height * 4]);
+  base::AutoLock lock(namespace_->lock);
+  ScopedPtrHashMap<unsigned, Image> &images = namespace_->images;

[danakj, 2013/07/16 03:26:11]

& images

(cpplint fail)

[piman, 2013/07/16 04:50:55]

Done.

+  images.set(image_id, make_scoped_ptr(new Image).Pass());
+  images.get(image_id)->pixels.reset(new uint8[width * height * 4]);
   return image_id;
 }
 
 void TestWebGraphicsContext3D::destroyImageCHROMIUM(
     WebKit::WGC3Duint id) {
+  base::AutoLock lock(namespace_->lock);
   unsigned context_id = id >> 17;
   unsigned image_id = id & 0x1ffff;
-  DCHECK(image_id && image_id < next_image_id_);
+  DCHECK(image_id && image_id < namespace_->next_image_id);
   DCHECK_EQ(context_id, context_id_);
 }
 
@@ -495,44 +538,61 @@ void TestWebGraphicsContext3D::getImageParameterivCHROMIUM(
     WebKit::WGC3Duint image_id,
     WebKit::WGC3Denum pname,
     WebKit::WGC3Dint* params) {
-  DCHECK_GT(images_.count(image_id), 0u);
+  base::AutoLock lock(namespace_->lock);
+  DCHECK_GT(namespace_->images.count(image_id), 0u);
   DCHECK_EQ(GL_IMAGE_ROWBYTES_CHROMIUM, static_cast<int>(pname));
   *params = 0;
 }
 
 void* TestWebGraphicsContext3D::mapImageCHROMIUM(WebKit::WGC3Duint image_id,
                                                  WebKit::WGC3Denum access) {
-  DCHECK_GT(images_.count(image_id), 0u);
+  base::AutoLock lock(namespace_->lock);
+  ScopedPtrHashMap<unsigned, Image> &images = namespace_->images;

[danakj, 2013/07/16 03:26:11]

& images

[piman, 2013/07/16 04:50:55]

Done.

+  DCHECK_GT(images.count(image_id), 0u);
   if (times_map_image_chromium_succeeds_ >= 0) {
     if (!times_map_image_chromium_succeeds_) {
       return NULL;
     }
     --times_map_image_chromium_succeeds_;
   }
-  return images_.get(image_id)->pixels.get();
+  return images.get(image_id)->pixels.get();
 }
 
 void TestWebGraphicsContext3D::unmapImageCHROMIUM(
     WebKit::WGC3Duint image_id) {
-  DCHECK_GT(images_.count(image_id), 0u);
+  base::AutoLock lock(namespace_->lock);
+  DCHECK_GT(namespace_->images.count(image_id), 0u);
+}
+
+size_t TestWebGraphicsContext3D::NumTextures() const {
+  base::AutoLock lock(namespace_->lock);
+  return namespace_->textures.size();
+}
+
+WebKit::WebGLId TestWebGraphicsContext3D::TextureAt(int i) const {
+  base::AutoLock lock(namespace_->lock);
+  return namespace_->textures[i];
 }
 
 WebGLId TestWebGraphicsContext3D::NextTextureId() {
-  WebGLId texture_id = next_texture_id_++;
+  base::AutoLock lock(namespace_->lock);
+  WebGLId texture_id = namespace_->next_texture_id++;
   DCHECK(texture_id < (1 << 16));
   texture_id |= context_id_ << 16;
   return texture_id;
 }
 
 WebGLId TestWebGraphicsContext3D::NextBufferId() {
-  WebGLId buffer_id = next_buffer_id_++;
+  base::AutoLock lock(namespace_->lock);
+  WebGLId buffer_id = namespace_->next_buffer_id++;
   DCHECK(buffer_id < (1 << 17));
   buffer_id |= context_id_ << 17;
   return buffer_id;
 }
 
 WebKit::WGC3Duint TestWebGraphicsContext3D::NextImageId() {
-  WebKit::WGC3Duint image_id = next_image_id_++;
+  base::AutoLock lock(namespace_->lock);
+  WGC3Duint image_id = namespace_->next_image_id++;
   DCHECK(image_id < (1 << 17));
   image_id |= context_id_ << 17;
   return image_id;