content: Cleanup GpuMemoryBufferImpl destruction.

content/common/gpu/client/gpu_memory_buffer_impl_io_surface.cc

Index: content/common/gpu/client/gpu_memory_buffer_impl_io_surface.cc
diff --git a/content/common/gpu/client/gpu_memory_buffer_impl_io_surface.cc b/content/common/gpu/client/gpu_memory_buffer_impl_io_surface.cc
index 029ea4f7c5b0c79172bc76d4c092e760a3fb278a..75c8edc08425e7392a92ef8b80cbc1d5b7962bfb 100644
--- a/content/common/gpu/client/gpu_memory_buffer_impl_io_surface.cc
+++ b/content/common/gpu/client/gpu_memory_buffer_impl_io_surface.cc
@@ -11,10 +11,17 @@ namespace content {
 
 GpuMemoryBufferImplIOSurface::GpuMemoryBufferImplIOSurface(
     const gfx::Size& size,
-    unsigned internalformat)
-    : GpuMemoryBufferImpl(size, internalformat) {}
+    const DestructionCallback& callback)
+    : GpuMemoryBufferImpl(size, internalformat), callback_(callback) {
+}
+
+GpuMemoryBufferImplIOSurface::~GpuMemoryBufferImplIOSurface() {
+  if (!io_surface_)
+    return;
 
-GpuMemoryBufferImplIOSurface::~GpuMemoryBufferImplIOSurface() {}
+  // Call destruction callback if instance was successfully initialized.
+  callback_.Run();

[piman, 2014/09/24 03:19:10]

I think it would be nice if the callback_ was managed by the base
GpuMemoryBufferImpl.

The way I would do it is by replacing the
GpuMemoryBufferImpl*::InitializeFromHandle pattern by a static
GpuMemoryBufferImpl*::CreateForHandle, that would validate/map/lookup/anything,
fail if needed, and if successful, pass the looked up data and the callback to
the constructor (that would then delegate to the base GpuMemoryBufferImpl
constructor).

That way we know that callback_ is called consistently, we avoid doing
allocations in case of failure, and we don't have to worry about invalid states
in the GpuMemoryBufferImpl* implementation.

WDYT?

[reveman, 2014/09/24 06:03:38]

Yes, that's much better. Thanks for the suggestion!

+}
 
 // static
 bool GpuMemoryBufferImplIOSurface::IsFormatSupported(unsigned internalformat) {