cc: Move GpuMemoryBuffer allocation to worker threads.

cc/resources/resource_provider.cc

Index: cc/resources/resource_provider.cc
diff --git a/cc/resources/resource_provider.cc b/cc/resources/resource_provider.cc
index ded347763485a32184d0aa950f329e685ce204c4..a618cb65ce125e2385c097219c3104e318174a11 100644
--- a/cc/resources/resource_provider.cc
+++ b/cc/resources/resource_provider.cc
@@ -924,11 +924,9 @@ void ResourceProvider::UnlockForRead(ResourceId id) {
   }
 }
 
-const ResourceProvider::Resource* ResourceProvider::LockForWrite(
-    ResourceId id) {
+ResourceProvider::Resource* ResourceProvider::LockForWrite(ResourceId id) {
   Resource* resource = GetResource(id);
   DCHECK(CanLockForWrite(id));
-  LazyAllocate(resource);
 
   resource->locked_for_write = true;
   return resource;
@@ -941,103 +939,15 @@ bool ResourceProvider::CanLockForWrite(ResourceId id) {
          !resource->lost && ReadLockFenceHasPassed(resource);
 }
 
-void ResourceProvider::UnlockForWrite(ResourceId id) {
+ResourceProvider::Resource* ResourceProvider::UnlockForWrite(ResourceId id) {

[piman, 2014/10/10 21:52:08]

nit: since you keep the resource_id in the locks, maybe you could just instead
keep the Resource* and have this be a void
ResourceProvider::UnlockForWrite(Resource* resource), and save a map lookup?

[reveman, 2014/10/26 22:30:48]

Done.

   Resource* resource = GetResource(id);
   DCHECK(resource->locked_for_write);
   DCHECK_EQ(resource->exported_count, 0);
   DCHECK(resource->origin == Resource::Internal);
   resource->locked_for_write = false;
-}
-
-const ResourceProvider::Resource*
-ResourceProvider::LockForWriteToGpuMemoryBuffer(ResourceId id) {
-  Resource* resource = GetResource(id);
-  DCHECK_EQ(GLTexture, resource->type);
-  DCHECK(CanLockForWrite(id));
-
-  if (!resource->gpu_memory_buffer) {
-    scoped_ptr<gfx::GpuMemoryBuffer> gpu_memory_buffer =
-        gpu_memory_buffer_manager_->AllocateGpuMemoryBuffer(
-            resource->size,
-            ToGpuMemoryBufferFormat(resource->format),
-            gfx::GpuMemoryBuffer::MAP);
-    resource->gpu_memory_buffer = gpu_memory_buffer.release();
-    resource->allocated = true;
-  }
-
-  resource->locked_for_write = true;
-  return resource;
-}
-
-void ResourceProvider::UnlockForWriteToGpuMemoryBuffer(ResourceId id) {
-  Resource* resource = GetResource(id);
-  DCHECK(resource->locked_for_write);
-  DCHECK_EQ(resource->exported_count, 0);
-  DCHECK(resource->origin == Resource::Internal);
-  DCHECK_EQ(GLTexture, resource->type);
-
-  if (!resource->image_id) {
-    GLES2Interface* gl = ContextGL();
-    DCHECK(gl);
-    resource->image_id =
-        gl->CreateImageCHROMIUM(resource->gpu_memory_buffer->AsClientBuffer(),
-                                resource->size.width(),
-                                resource->size.height(),
-                                GL_RGBA);
-  }
-
-  resource->locked_for_write = false;
-  resource->dirty_image = true;
-
-  // GpuMemoryBuffer provides direct access to the memory used by the GPU.
-  // Read lock fences are required to ensure that we're not trying to map a
-  // buffer that is currently in-use by the GPU.
-  resource->read_lock_fences_enabled = true;
-}
-
-const ResourceProvider::Resource* ResourceProvider::LockForWriteToSkSurface(
-    ResourceId id) {
-  Resource* resource = GetResource(id);
-  DCHECK_EQ(GLTexture, resource->type);
-  DCHECK(CanLockForWrite(id));
-
-  resource->locked_for_write = true;
-  if (!resource->sk_surface) {
-    class GrContext* gr_context = GrContext();
-    // TODO(alokp): Implement TestContextProvider::GrContext().
-    if (!gr_context)
-      return resource;
-
-    LazyAllocate(resource);
-
-    GrBackendTextureDesc desc;
-    desc.fFlags = kRenderTarget_GrBackendTextureFlag;
-    desc.fWidth = resource->size.width();
-    desc.fHeight = resource->size.height();
-    desc.fConfig = ToGrPixelConfig(resource->format);
-    desc.fOrigin = kTopLeft_GrSurfaceOrigin;
-    desc.fTextureHandle = resource->gl_id;
-    skia::RefPtr<GrTexture> gr_texture =
-        skia::AdoptRef(gr_context->wrapBackendTexture(desc));
-    SkSurface::TextRenderMode text_render_mode =
-        use_distance_field_text_ ? SkSurface::kDistanceField_TextRenderMode
-                                 : SkSurface::kStandard_TextRenderMode;
-    resource->sk_surface = skia::AdoptRef(SkSurface::NewRenderTargetDirect(
-        gr_texture->asRenderTarget(), text_render_mode));
-  }
-
   return resource;
 }
 
-void ResourceProvider::UnlockForWriteToSkSurface(ResourceId id) {
-  Resource* resource = GetResource(id);
-  DCHECK(resource->locked_for_write);
-  DCHECK_EQ(resource->exported_count, 0);
-  DCHECK(resource->origin == Resource::Internal);
-  DCHECK_EQ(GLTexture, resource->type);
-  resource->locked_for_write = false;
-}
-
 ResourceProvider::ScopedReadLockGL::ScopedReadLockGL(
     ResourceProvider* resource_provider,
     ResourceProvider::ResourceId resource_id)
@@ -1076,9 +986,10 @@ ResourceProvider::ScopedSamplerGL::~ScopedSamplerGL() {
 ResourceProvider::ScopedWriteLockGL::ScopedWriteLockGL(
     ResourceProvider* resource_provider,
     ResourceProvider::ResourceId resource_id)
-    : resource_provider_(resource_provider),
-      resource_id_(resource_id),
-      texture_id_(resource_provider->LockForWrite(resource_id)->gl_id) {
+    : resource_provider_(resource_provider), resource_id_(resource_id) {
+  Resource* resource = resource_provider->LockForWrite(resource_id);
+  resource_provider_->LazyAllocate(resource);
+  texture_id_ = resource->gl_id;
   DCHECK(texture_id_);
 }
 
@@ -1128,27 +1039,91 @@ ResourceProvider::ScopedWriteLockGpuMemoryBuffer::
                                    ResourceProvider::ResourceId resource_id)
     : resource_provider_(resource_provider),
       resource_id_(resource_id),
-      gpu_memory_buffer_(
-          resource_provider->LockForWriteToGpuMemoryBuffer(resource_id)
-              ->gpu_memory_buffer) {
+      gpu_memory_buffer_(nullptr) {
+  Resource* resource = resource_provider->LockForWrite(resource_id);
+  DCHECK_EQ(GLTexture, resource->type);
+  format_ = resource->format;
+  size_ = resource->size;
+  std::swap(gpu_memory_buffer_, resource->gpu_memory_buffer);
 }
 
 ResourceProvider::ScopedWriteLockGpuMemoryBuffer::
     ~ScopedWriteLockGpuMemoryBuffer() {
-  resource_provider_->UnlockForWriteToGpuMemoryBuffer(resource_id_);
+  Resource* resource = resource_provider_->UnlockForWrite(resource_id_);
+  if (!gpu_memory_buffer_)
+    return;
+
+  if (!resource->image_id) {
+    GLES2Interface* gl = resource_provider_->ContextGL();
+    DCHECK(gl);
+
+    resource->image_id =
+        gl->CreateImageCHROMIUM(gpu_memory_buffer_->AsClientBuffer(),
+                                size_.width(),
+                                size_.height(),
+                                GL_RGBA);
+    DCHECK(resource->image_id);

[piman, 2014/10/10 21:52:08]

nit: remove DCHECK as with previous patch?

[reveman, 2014/10/26 22:30:48]

Done.

+  }
+
+  std::swap(resource->gpu_memory_buffer, gpu_memory_buffer_);
+  resource->allocated = true;
+  resource->dirty_image = true;
+
+  // GpuMemoryBuffer provides direct access to the memory used by the GPU.
+  // Read lock fences are required to ensure that we're not trying to map a
+  // buffer that is currently in-use by the GPU.
+  resource->read_lock_fences_enabled = true;
+}
+
+gfx::GpuMemoryBuffer*
+ResourceProvider::ScopedWriteLockGpuMemoryBuffer::GetGpuMemoryBuffer() {
+  if (!gpu_memory_buffer_) {
+    scoped_ptr<gfx::GpuMemoryBuffer> gpu_memory_buffer =
+        resource_provider_->gpu_memory_buffer_manager_->AllocateGpuMemoryBuffer(

[piman, 2014/10/10 21:52:08]

IIUC, this is called from the raster threads. ResourceProvider isn't really
thread safe (but GpuMemoryBufferManager is meant to be), so the
resource_provider_->gpu_memory_buffer_manager_ part of this feels iffy.
Should we save the GpuMemoryBufferManager pointer in the constructor?
Or can we make it an enforced rule that
resource_provider_->gpu_memory_buffer_manager_ can't change (e.g. making it a
const field - pointer to a non-const GpuMemoryBufferManager of course).

[reveman, 2014/10/26 22:30:48]

Done. Saving a GpuMemoryBufferManager pointer in the ctor.

+            size_, ToGpuMemoryBufferFormat(format_), gfx::GpuMemoryBuffer::MAP);
+    gpu_memory_buffer_ = gpu_memory_buffer.release();
+  }
+
+  return gpu_memory_buffer_;
 }
 
 ResourceProvider::ScopedWriteLockGr::ScopedWriteLockGr(
     ResourceProvider* resource_provider,
     ResourceProvider::ResourceId resource_id)
-    : resource_provider_(resource_provider),
-      resource_id_(resource_id),
-      sk_surface_(resource_provider->LockForWriteToSkSurface(resource_id)
-                      ->sk_surface.get()) {
+    : resource_provider_(resource_provider), resource_id_(resource_id) {
+  Resource* resource = resource_provider->LockForWrite(resource_id);
+  DCHECK_EQ(GLTexture, resource->type);
+
+  sk_surface_ = resource->sk_surface;
+  if (!sk_surface_.get()) {
+    class GrContext* gr_context = resource_provider->GrContext();
+    // TODO(alokp): Implement TestContextProvider::GrContext().
+    if (!gr_context)
+      return;
+
+    resource_provider->LazyAllocate(resource);
+
+    GrBackendTextureDesc desc;
+    desc.fFlags = kRenderTarget_GrBackendTextureFlag;
+    desc.fWidth = resource->size.width();
+    desc.fHeight = resource->size.height();
+    desc.fConfig = ToGrPixelConfig(resource->format);
+    desc.fOrigin = kTopLeft_GrSurfaceOrigin;
+    desc.fTextureHandle = resource->gl_id;
+    skia::RefPtr<GrTexture> gr_texture =
+        skia::AdoptRef(gr_context->wrapBackendTexture(desc));
+    SkSurface::TextRenderMode text_render_mode =
+        resource_provider_->use_distance_field_text_
+            ? SkSurface::kDistanceField_TextRenderMode
+            : SkSurface::kStandard_TextRenderMode;
+    sk_surface_ = skia::AdoptRef(SkSurface::NewRenderTargetDirect(
+        gr_texture->asRenderTarget(), text_render_mode));
+  }
 }
 
 ResourceProvider::ScopedWriteLockGr::~ScopedWriteLockGr() {
-  resource_provider_->UnlockForWriteToSkSurface(resource_id_);
+  Resource* resource = resource_provider_->UnlockForWrite(resource_id_);
+  resource->sk_surface = sk_surface_;
 }
 
 ResourceProvider::ResourceProvider(