content: Cleanup GpuMemoryBufferImpl destruction.

content/common/gpu/client/gpu_memory_buffer_impl_io_surface.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/common/gpu/client/gpu_memory_buffer_impl_io_surface.h"
 
 #include "base/logging.h"
 #include "ui/gl/gl_bindings.h"
 
 namespace content {
 
 GpuMemoryBufferImplIOSurface::GpuMemoryBufferImplIOSurface(
     const gfx::Size& size,
-    unsigned internalformat)
-    : GpuMemoryBufferImpl(size, internalformat) {}
+    const DestructionCallback& callback)
+    : GpuMemoryBufferImpl(size, internalformat), callback_(callback) {
+}
 
-GpuMemoryBufferImplIOSurface::~GpuMemoryBufferImplIOSurface() {}
+GpuMemoryBufferImplIOSurface::~GpuMemoryBufferImplIOSurface() {
+  if (!io_surface_)
+    return;
+
+  // 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) {
   switch (internalformat) {
     case GL_BGRA8_EXT:
       return true;
     default:
       return false;
   }
 }
@@ skipping 56 matching lines @@
 }
 
 gfx::GpuMemoryBufferHandle GpuMemoryBufferImplIOSurface::GetHandle() const {
   gfx::GpuMemoryBufferHandle handle;
   handle.type = gfx::IO_SURFACE_BUFFER;
   handle.io_surface_id = IOSurfaceGetID(io_surface_);
   return handle;
 }
 
 }  // namespace content